Auto merge of #4627 - integer32llc:document-libs-and-bins, r=matklad

Document the lib if a lib and bin have the same name

Fixes #4341, as discussed in that issue.

- Removes the check that bailed if there was a  bin and lib with the
  same name
- Exclude bins with the same name as libs from the proposed targets to
  build when compiling docs
- Adjust tests to expect this behavior

src/cargo/ops/cargo_compile.rs

@@ -489,7 +489,10 @@ fn generate_auto_targets<'a>(mode: CompileMode, targets: &'a [Target],
         }
         CompileMode::Doc { .. } => {
             targets.iter().filter(|t| {
-                t.documented()
+                t.documented() && (
+                    !t.is_bin() ||
+                    !targets.iter().any(|l| l.is_lib() && l.name() == t.name())
+                )
             }).map(|t| BuildProposal {
                 target: t,
                 profile: profile,

src/cargo/ops/cargo_doc.rs

@@ -52,20 +52,6 @@ pub fn doc(ws: &Workspace, options: &DocOptions) -> CargoResult<()> {
                 }
             }
         }
-        for (bin, bin_package) in bin_names.iter() {
-            if let Some(lib_package) = lib_names.get(bin) {
-                bail!("The target `{}` is specified as a library {}. It can be \
-                       documented only once. Consider renaming or marking one \
-                       of the targets as `doc = false`.",
-                       bin,
-                       if lib_package == bin_package {
-                           format!("and as a binary by package `{}`", lib_package)
-                       } else {
-                           format!("by package `{}` and as a binary by \
-                                    package `{}`", lib_package, bin_package)
-                       });
-            }
-        }
     }
 
     ops::compile(ws, &options.compile_opts)?;

tests/doc.rs

@@ -3,7 +3,8 @@ extern crate hamcrest;
 extern crate cargo;
 
 use std::str;
-use std::fs;
+use std::fs::{self, File};
+use std::io::Read;
 
 use cargotest::rustc_host;
 use cargotest::support::{project, execs, path2url};
@@ -255,6 +256,7 @@ fn doc_multiple_targets_same_name() {
             version = "0.1.0"
             [[bin]]
             name = "foo_lib"
+            path = "src/foo_lib.rs"
         "#)
         .file("foo/src/foo_lib.rs", "")
         .file("bar/Cargo.toml", r#"
@@ -267,12 +269,17 @@ fn doc_multiple_targets_same_name() {
         .file("bar/src/lib.rs", "")
         .build();
 
+        let root = path2url(p.root());
+
         assert_that(p.cargo("doc").arg("--all"),
                     execs()
-                    .with_status(101)
-                    .with_stderr_contains("[..] target `foo_lib` [..]")
-                    .with_stderr_contains("[..] binary by package `foo v0.1.0[..]`[..]")
-                    .with_stderr_contains("[..] library by package `bar v0.1.0[..]` [..]"));
+                    .with_status(0)
+                    .with_stderr_contains(&format!("[DOCUMENTING] foo v0.1.0 ({}/foo)", root))
+                    .with_stderr_contains(&format!("[DOCUMENTING] bar v0.1.0 ({}/bar)", root))
+                    .with_stderr_contains("[FINISHED] dev [unoptimized + debuginfo] target(s) in [..]"));
+        assert_that(&p.root().join("target/doc"), existing_dir());
+        let doc_file = p.root().join("target/doc/foo_lib/index.html");
+        assert_that(&doc_file, existing_file());
 }
 
 #[test]
@@ -339,23 +346,149 @@ fn doc_multiple_targets_same_name_undoced() {
 }
 
 #[test]
-fn doc_lib_bin_same_name() {
+fn doc_lib_bin_same_name_documents_lib() {
     let p = project("foo")
         .file("Cargo.toml", r#"
             [package]
             name = "foo"
             version = "0.0.1"
             authors = []
         "#)
-        .file("src/main.rs", "fn main() {}")
-        .file("src/lib.rs", "fn foo() {}")
+        .file("src/main.rs", r#"
+            //! Binary documentation
+            extern crate foo;
+            fn main() {
+                foo::foo();
+            }
+        "#)
+        .file("src/lib.rs", r#"
+            //! Library documentation
+            pub fn foo() {}
+        "#)
         .build();
 
     assert_that(p.cargo("doc"),
-                execs().with_status(101)
-                       .with_stderr("\
-[ERROR] The target `foo` is specified as a library and as a binary by package \
-`foo [..]`. It can be documented[..]"));
+                execs().with_status(0).with_stderr(&format!("\
+[DOCUMENTING] foo v0.0.1 ({dir})
+[FINISHED] dev [unoptimized + debuginfo] target(s) in [..]
+", dir = path2url(p.root()))));
+    assert_that(&p.root().join("target/doc"), existing_dir());
+    let doc_file = p.root().join("target/doc/foo/index.html");
+    assert_that(&doc_file, existing_file());
+    let mut doc_html = String::new();
+    File::open(&doc_file).unwrap().read_to_string(&mut doc_html).unwrap();
+    assert!(doc_html.contains("Library"));
+    assert!(!doc_html.contains("Binary"));
+}
+
+#[test]
+fn doc_lib_bin_same_name_documents_lib_when_requested() {
+    let p = project("foo")
+        .file("Cargo.toml", r#"
+            [package]
+            name = "foo"
+            version = "0.0.1"
+            authors = []
+        "#)
+        .file("src/main.rs", r#"
+            //! Binary documentation
+            extern crate foo;
+            fn main() {
+                foo::foo();
+            }
+        "#)
+        .file("src/lib.rs", r#"
+            //! Library documentation
+            pub fn foo() {}
+        "#)
+        .build();
+
+    assert_that(p.cargo("doc").arg("--lib"),
+                execs().with_status(0).with_stderr(&format!("\
+[DOCUMENTING] foo v0.0.1 ({dir})
+[FINISHED] dev [unoptimized + debuginfo] target(s) in [..]
+", dir = path2url(p.root()))));
+    assert_that(&p.root().join("target/doc"), existing_dir());
+    let doc_file = p.root().join("target/doc/foo/index.html");
+    assert_that(&doc_file, existing_file());
+    let mut doc_html = String::new();
+    File::open(&doc_file).unwrap().read_to_string(&mut doc_html).unwrap();
+    assert!(doc_html.contains("Library"));
+    assert!(!doc_html.contains("Binary"));
+}
+
+#[test]
+fn doc_lib_bin_same_name_documents_named_bin_when_requested() {
+    let p = project("foo")
+        .file("Cargo.toml", r#"
+            [package]
+            name = "foo"
+            version = "0.0.1"
+            authors = []
+        "#)
+        .file("src/main.rs", r#"
+            //! Binary documentation
+            extern crate foo;
+            fn main() {
+                foo::foo();
+            }
+        "#)
+        .file("src/lib.rs", r#"
+            //! Library documentation
+            pub fn foo() {}
+        "#)
+        .build();
+
+    assert_that(p.cargo("doc").arg("--bin").arg("foo"),
+                execs().with_status(0).with_stderr(&format!("\
+[COMPILING] foo v0.0.1 ({dir})
+[DOCUMENTING] foo v0.0.1 ({dir})
+[FINISHED] dev [unoptimized + debuginfo] target(s) in [..]
+", dir = path2url(p.root()))));
+    assert_that(&p.root().join("target/doc"), existing_dir());
+    let doc_file = p.root().join("target/doc/foo/index.html");
+    assert_that(&doc_file, existing_file());
+    let mut doc_html = String::new();
+    File::open(&doc_file).unwrap().read_to_string(&mut doc_html).unwrap();
+    assert!(!doc_html.contains("Library"));
+    assert!(doc_html.contains("Binary"));
+}
+
+#[test]
+fn doc_lib_bin_same_name_documents_bins_when_requested() {
+    let p = project("foo")
+        .file("Cargo.toml", r#"
+            [package]
+            name = "foo"
+            version = "0.0.1"
+            authors = []
+        "#)
+        .file("src/main.rs", r#"
+            //! Binary documentation
+            extern crate foo;
+            fn main() {
+                foo::foo();
+            }
+        "#)
+        .file("src/lib.rs", r#"
+            //! Library documentation
+            pub fn foo() {}
+        "#)
+        .build();
+
+    assert_that(p.cargo("doc").arg("--bins"),
+                execs().with_status(0).with_stderr(&format!("\
+[COMPILING] foo v0.0.1 ({dir})
+[DOCUMENTING] foo v0.0.1 ({dir})
+[FINISHED] dev [unoptimized + debuginfo] target(s) in [..]
+", dir = path2url(p.root()))));
+    assert_that(&p.root().join("target/doc"), existing_dir());
+    let doc_file = p.root().join("target/doc/foo/index.html");
+    assert_that(&doc_file, existing_file());
+    let mut doc_html = String::new();
+    File::open(&doc_file).unwrap().read_to_string(&mut doc_html).unwrap();
+    assert!(!doc_html.contains("Library"));
+    assert!(doc_html.contains("Binary"));
 }
 
 #[test]
@@ -503,7 +636,7 @@ fn output_not_captured() {
     if let CargoError(CargoErrorKind::ProcessErrorKind(perr), ..) = error {
         let output = perr.output.unwrap();
         let stderr = str::from_utf8(&output.stderr).unwrap();
-    
+
         assert!(stderr.contains("☃"), "no snowman\n{}", stderr);
         assert!(stderr.contains("unknown start of token"), "no message{}", stderr);
     } else {

tests/rustdoc.rs

@@ -147,7 +147,7 @@ fn rustdoc_only_bar_dependency() {
 
 
 #[test]
-fn rustdoc_same_name_err() {
+fn rustdoc_same_name_documents_lib() {
     let p = project("foo")
         .file("Cargo.toml", r#"
             [package]
@@ -164,7 +164,13 @@ fn rustdoc_same_name_err() {
     assert_that(p.cargo("rustdoc").arg("-v")
                  .arg("--").arg("--cfg=foo"),
                 execs()
-                .with_status(101)
-                .with_stderr("[ERROR] The target `foo` is specified as a \
-library and as a binary by package `foo [..]`. It can be documented[..]"));
+                .with_status(0)
+                .with_stderr(format!("\
+[DOCUMENTING] foo v0.0.1 ([..])
+[RUNNING] `rustdoc --crate-name foo src[/]lib.rs \
+        -o {dir}[/]target[/]doc \
+        --cfg=foo \
+        -L dependency={dir}[/]target[/]debug[/]deps`
+[FINISHED] dev [unoptimized + debuginfo] target(s) in [..]
+", dir = p.root().display())));
 }