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>`.

src/bin/cargo/commands/doc.rs

@@ -55,11 +55,8 @@ pub fn exec(config: &mut Config, args: &ArgMatches<'_>) -> CliResult {
     };
     let mut compile_opts =
         args.compile_options(config, mode, Some(&ws), ProfileChecking::Checked)?;
-    compile_opts.local_rustdoc_args = if args.is_present("document-private-items") {
-        Some(vec!["--document-private-items".to_string()])
-    } else {
-        None
-    };
+    compile_opts.rustdoc_document_private_items = args.is_present("document-private-items");
+
     let doc_opts = DocOptions {
         open_result: args.is_present("open"),
         compile_opts,

src/cargo/ops/cargo_compile.rs

@@ -66,6 +66,9 @@ pub struct CompileOptions<'a> {
     pub target_rustc_args: Option<Vec<String>>,
     /// Extra arguments passed to all selected targets for rustdoc.
     pub local_rustdoc_args: Option<Vec<String>>,
+    /// Whether the `--document-private-items` flags was specified and should
+    /// be forwarded to `rustdoc`.
+    pub rustdoc_document_private_items: bool,
     /// The directory to copy final artifacts to. Note that even if `out_dir` is
     /// set, a copy of artifacts still could be found a `target/(debug\release)`
     /// as usual.
@@ -89,6 +92,7 @@ impl<'a> CompileOptions<'a> {
             target_rustdoc_args: None,
             target_rustc_args: None,
             local_rustdoc_args: None,
+            rustdoc_document_private_items: false,
             export_dir: None,
         })
     }
@@ -271,6 +275,7 @@ pub fn compile_ws<'a>(
         ref target_rustdoc_args,
         ref target_rustc_args,
         ref local_rustdoc_args,
+        rustdoc_document_private_items,
         ref export_dir,
     } = *options;
 
@@ -434,9 +439,20 @@ pub fn compile_ws<'a>(
         }
         bcx.extra_compiler_args.insert(units[0], args);
     }
-    if let Some(args) = local_rustdoc_args {
-        for unit in &units {
-            if unit.mode.is_doc() || unit.mode.is_doc_test() {
+    for unit in &units {
+        if unit.mode.is_doc() || unit.mode.is_doc_test() {
+            let mut extra_args = local_rustdoc_args.clone();
+
+            // Add `--document-private-items` rustdoc flag if requested or if
+            // the target is a binary. Binary crates get their private items
+            // documented by default.
+            if rustdoc_document_private_items || unit.target.is_bin() {
+                let mut args = extra_args.take().unwrap_or(vec![]);
+                args.push("--document-private-items".into());
+                extra_args = Some(args);
+            }
+
+            if let Some(args) = extra_args {
                 bcx.extra_compiler_args.insert(*unit, args.clone());
             }
         }

src/cargo/ops/cargo_package.rs

@@ -661,6 +661,7 @@ fn run_verify(ws: &Workspace<'_>, tar: &FileLock, opts: &PackageOpts<'_>) -> Car
             target_rustdoc_args: None,
             target_rustc_args: rustc_args,
             local_rustdoc_args: None,
+            rustdoc_document_private_items: false,
             export_dir: None,
         },
         &exec,

src/cargo/util/command_prelude.rs

@@ -463,6 +463,7 @@ pub trait ArgMatchesExt {
             target_rustdoc_args: None,
             target_rustc_args: None,
             local_rustdoc_args: None,
+            rustdoc_document_private_items: false,
             export_dir: None,
         };
 

tests/testsuite/doc.rs

@@ -1401,3 +1401,108 @@ fn doc_example() {
         .join("fn.x.html")
         .exists());
 }
+
+#[cargo_test]
+fn bin_private_items() {
+    let p = project()
+        .file(
+            "Cargo.toml",
+            r#"
+            [package]
+            name = "foo"
+            version = "0.0.1"
+            authors = []
+        "#,
+        )
+        .file(
+            "src/main.rs",
+            "
+            pub fn foo_pub() {}
+            fn foo_priv() {}
+            struct FooStruct;
+            enum FooEnum {}
+            trait FooTrait {}
+            type FooType = u32;
+            mod foo_mod {}
+
+        ",
+        )
+        .build();
+
+    p.cargo("doc")
+        .with_stderr(
+            "\
+[DOCUMENTING] foo v0.0.1 ([CWD])
+[FINISHED] dev [unoptimized + debuginfo] target(s) in [..]
+",
+        )
+        .run();
+
+    assert!(p.root().join("target/doc/foo/index.html").is_file());
+    assert!(p.root().join("target/doc/foo/fn.foo_pub.html").is_file());
+    assert!(p.root().join("target/doc/foo/fn.foo_priv.html").is_file());
+    assert!(p
+        .root()
+        .join("target/doc/foo/struct.FooStruct.html")
+        .is_file());
+    assert!(p.root().join("target/doc/foo/enum.FooEnum.html").is_file());
+    assert!(p
+        .root()
+        .join("target/doc/foo/trait.FooTrait.html")
+        .is_file());
+    assert!(p.root().join("target/doc/foo/type.FooType.html").is_file());
+    assert!(p.root().join("target/doc/foo/foo_mod/index.html").is_file());
+}
+
+#[cargo_test]
+fn bin_private_items_deps() {
+    let p = project()
+        .file(
+            "Cargo.toml",
+            r#"
+            [package]
+            name = "foo"
+            version = "0.0.1"
+            authors = []
+
+            [dependencies.bar]
+            path = "bar"
+        "#,
+        )
+        .file(
+            "src/main.rs",
+            "
+            fn foo_priv() {}
+            pub fn foo_pub() {}
+        ",
+        )
+        .file("bar/Cargo.toml", &basic_manifest("bar", "0.0.1"))
+        .file(
+            "bar/src/lib.rs",
+            "
+            #[allow(dead_code)]
+            fn bar_priv() {}
+            pub fn bar_pub() {}
+        ",
+        )
+        .build();
+
+    p.cargo("doc")
+        .with_stderr_unordered(
+            "\
+[DOCUMENTING] bar v0.0.1 ([..])
+[CHECKING] bar v0.0.1 ([..])
+[DOCUMENTING] foo v0.0.1 ([CWD])
+[FINISHED] dev [unoptimized + debuginfo] target(s) in [..]
+",
+        )
+        .run();
+
+    assert!(p.root().join("target/doc/foo/index.html").is_file());
+    assert!(p.root().join("target/doc/foo/fn.foo_pub.html").is_file());
+    assert!(p.root().join("target/doc/foo/fn.foo_priv.html").is_file());
+
+    assert!(p.root().join("target/doc/bar/index.html").is_file());
+    assert!(p.root().join("target/doc/bar/fn.bar_pub.html").is_file());
+    assert!(!p.root().join("target/doc/bar/fn.bar_priv.html").exists());
+}