Skip to content

Commit 579174c

Browse files
borsbors
committed
Auto merge of #7593 - LukasKalbertodt:master, r=Eh2406
Document private items for binary crates by default I suggested this change in default behavior [a long time ago](#1520 (comment)) and [a year ago again](#1520 (comment)). Both time, everyone seemed to agree that this is a good idea, so I thought I could just implement it. This PR already changed the default behavior, but there are a few things we should talk about/I should do before merging: - [x] ~~Do we *really* want this changed default behavior? If not, *why* not?~~ -> [yip](#7593 (comment)) - [x] Is changing this default behavior OK regarding backwards compatibility? -> [apparently](#7593 (comment)) - [x] ~~We should also probably add a `--document-only-public-items` flag or something like that if we change the default behavior. Otherwise users have no way to not document private items for binary crates. Right?~~ -> [We can do it later](#7593 (comment)) - [x] I should probably add some tests for this new behavior -> I did - [ ] I don't like that I added `rustdoc_document_private_items` to `CompileOptions`, but this was the sanest way I could think of without rewriting a whole lot. Is this OK or how else would one do it? The flag would belong to `DocOptions`. But `compile` does not have access to that. Any ideas? Btw: If we also add `--document-only-private-items`, I would change the type from `bool` to `Option<bool>`.
2 parents 5252abb + 00b21c8 commit 579174c

File tree

5 files changed

+128
-8
lines changed

5 files changed

+128
-8
lines changed

‎src/bin/cargo/commands/doc.rs

+2-5
Original file line numberDiff line numberDiff line change
@@ -55,11 +55,8 @@ pub fn exec(config: &mut Config, args: &ArgMatches<'_>) -> CliResult {
5555
};
5656
let mut compile_opts =
5757
args.compile_options(config, mode, Some(&ws), ProfileChecking::Checked)?;
58-
compile_opts.local_rustdoc_args = if args.is_present("document-private-items") {
59-
Some(vec!["--document-private-items".to_string()])
60-
} else {
61-
None
62-
};
58+
compile_opts.rustdoc_document_private_items = args.is_present("document-private-items");
59+
6360
let doc_opts = DocOptions {
6461
open_result: args.is_present("open"),
6562
compile_opts,

‎src/cargo/ops/cargo_compile.rs

+19-3
Original file line numberDiff line numberDiff line change
@@ -66,6 +66,9 @@ pub struct CompileOptions<'a> {
6666
pub target_rustc_args: Option<Vec<String>>,
6767
/// Extra arguments passed to all selected targets for rustdoc.
6868
pub local_rustdoc_args: Option<Vec<String>>,
69+
/// Whether the `--document-private-items` flags was specified and should
70+
/// be forwarded to `rustdoc`.
71+
pub rustdoc_document_private_items: bool,
6972
/// The directory to copy final artifacts to. Note that even if `out_dir` is
7073
/// set, a copy of artifacts still could be found a `target/(debug\release)`
7174
/// as usual.
@@ -89,6 +92,7 @@ impl<'a> CompileOptions<'a> {
8992
target_rustdoc_args: None,
9093
target_rustc_args: None,
9194
local_rustdoc_args: None,
95+
rustdoc_document_private_items: false,
9296
export_dir: None,
9397
})
9498
}
@@ -271,6 +275,7 @@ pub fn compile_ws<'a>(
271275
ref target_rustdoc_args,
272276
ref target_rustc_args,
273277
ref local_rustdoc_args,
278+
rustdoc_document_private_items,
274279
ref export_dir,
275280
} = *options;
276281

@@ -434,9 +439,20 @@ pub fn compile_ws<'a>(
434439
}
435440
bcx.extra_compiler_args.insert(units[0], args);
436441
}
437-
if let Some(args) = local_rustdoc_args {
438-
for unit in &units {
439-
if unit.mode.is_doc() || unit.mode.is_doc_test() {
442+
for unit in &units {
443+
if unit.mode.is_doc() || unit.mode.is_doc_test() {
444+
let mut extra_args = local_rustdoc_args.clone();
445+
446+
// Add `--document-private-items` rustdoc flag if requested or if
447+
// the target is a binary. Binary crates get their private items
448+
// documented by default.
449+
if rustdoc_document_private_items || unit.target.is_bin() {
450+
let mut args = extra_args.take().unwrap_or(vec![]);
451+
args.push("--document-private-items".into());
452+
extra_args = Some(args);
453+
}
454+
455+
if let Some(args) = extra_args {
440456
bcx.extra_compiler_args.insert(*unit, args.clone());
441457
}
442458
}

‎src/cargo/ops/cargo_package.rs

+1
Original file line numberDiff line numberDiff line change
@@ -661,6 +661,7 @@ fn run_verify(ws: &Workspace<'_>, tar: &FileLock, opts: &PackageOpts<'_>) -> Car
661661
target_rustdoc_args: None,
662662
target_rustc_args: rustc_args,
663663
local_rustdoc_args: None,
664+
rustdoc_document_private_items: false,
664665
export_dir: None,
665666
},
666667
&exec,

‎src/cargo/util/command_prelude.rs

+1
Original file line numberDiff line numberDiff line change
@@ -463,6 +463,7 @@ pub trait ArgMatchesExt {
463463
target_rustdoc_args: None,
464464
target_rustc_args: None,
465465
local_rustdoc_args: None,
466+
rustdoc_document_private_items: false,
466467
export_dir: None,
467468
};
468469

‎tests/testsuite/doc.rs

+105
Original file line numberDiff line numberDiff line change
@@ -1401,3 +1401,108 @@ fn doc_example() {
14011401
.join("fn.x.html")
14021402
.exists());
14031403
}
1404+
1405+
#[cargo_test]
1406+
fn bin_private_items() {
1407+
let p = project()
1408+
.file(
1409+
"Cargo.toml",
1410+
r#"
1411+
[package]
1412+
name = "foo"
1413+
version = "0.0.1"
1414+
authors = []
1415+
"#,
1416+
)
1417+
.file(
1418+
"src/main.rs",
1419+
"
1420+
pub fn foo_pub() {}
1421+
fn foo_priv() {}
1422+
struct FooStruct;
1423+
enum FooEnum {}
1424+
trait FooTrait {}
1425+
type FooType = u32;
1426+
mod foo_mod {}
1427+
1428+
",
1429+
)
1430+
.build();
1431+
1432+
p.cargo("doc")
1433+
.with_stderr(
1434+
"\
1435+
[DOCUMENTING] foo v0.0.1 ([CWD])
1436+
[FINISHED] dev [unoptimized + debuginfo] target(s) in [..]
1437+
",
1438+
)
1439+
.run();
1440+
1441+
assert!(p.root().join("target/doc/foo/index.html").is_file());
1442+
assert!(p.root().join("target/doc/foo/fn.foo_pub.html").is_file());
1443+
assert!(p.root().join("target/doc/foo/fn.foo_priv.html").is_file());
1444+
assert!(p
1445+
.root()
1446+
.join("target/doc/foo/struct.FooStruct.html")
1447+
.is_file());
1448+
assert!(p.root().join("target/doc/foo/enum.FooEnum.html").is_file());
1449+
assert!(p
1450+
.root()
1451+
.join("target/doc/foo/trait.FooTrait.html")
1452+
.is_file());
1453+
assert!(p.root().join("target/doc/foo/type.FooType.html").is_file());
1454+
assert!(p.root().join("target/doc/foo/foo_mod/index.html").is_file());
1455+
}
1456+
1457+
#[cargo_test]
1458+
fn bin_private_items_deps() {
1459+
let p = project()
1460+
.file(
1461+
"Cargo.toml",
1462+
r#"
1463+
[package]
1464+
name = "foo"
1465+
version = "0.0.1"
1466+
authors = []
1467+
1468+
[dependencies.bar]
1469+
path = "bar"
1470+
"#,
1471+
)
1472+
.file(
1473+
"src/main.rs",
1474+
"
1475+
fn foo_priv() {}
1476+
pub fn foo_pub() {}
1477+
",
1478+
)
1479+
.file("bar/Cargo.toml", &basic_manifest("bar", "0.0.1"))
1480+
.file(
1481+
"bar/src/lib.rs",
1482+
"
1483+
#[allow(dead_code)]
1484+
fn bar_priv() {}
1485+
pub fn bar_pub() {}
1486+
",
1487+
)
1488+
.build();
1489+
1490+
p.cargo("doc")
1491+
.with_stderr_unordered(
1492+
"\
1493+
[DOCUMENTING] bar v0.0.1 ([..])
1494+
[CHECKING] bar v0.0.1 ([..])
1495+
[DOCUMENTING] foo v0.0.1 ([CWD])
1496+
[FINISHED] dev [unoptimized + debuginfo] target(s) in [..]
1497+
",
1498+
)
1499+
.run();
1500+
1501+
assert!(p.root().join("target/doc/foo/index.html").is_file());
1502+
assert!(p.root().join("target/doc/foo/fn.foo_pub.html").is_file());
1503+
assert!(p.root().join("target/doc/foo/fn.foo_priv.html").is_file());
1504+
1505+
assert!(p.root().join("target/doc/bar/index.html").is_file());
1506+
assert!(p.root().join("target/doc/bar/fn.bar_pub.html").is_file());
1507+
assert!(!p.root().join("target/doc/bar/fn.bar_priv.html").exists());
1508+
}

0 commit comments

Comments
 (0)