Improve error ico doc-targets with same name

We can currently only document one doc-target per name.
Replace an assert! with a clear error message in that case.
Fixes #4539.

diff --git a/src/cargo/ops/cargo_doc.rs b/src/cargo/ops/cargo_doc.rs
index 2d79cf0630d1c4ccf9e6c513fe42cba7dcfe8040..d4d5620367b9dcc25401102bb7b4cfb9820791f9 100644 (file)
--- a/src/cargo/ops/cargo_doc.rs
+++ b/src/cargo/ops/cargo_doc.rs
@@ -1,4 +1,4 @@
-use std::collections::HashSet;
+use std::collections::HashMap;
 use std::fs;
 use std::path::Path;
 use std::process::Command;
@@ -32,21 +32,38 @@ pub fn doc(ws: &Workspace, options: &DocOptions) -> CargoResult<()> {
         packages.get(pkgid)
     }).collect::<CargoResult<Vec<_>>>()?;
 
-    let mut lib_names = HashSet::new();
-    let mut bin_names = HashSet::new();
+    let mut lib_names = HashMap::new();
+    let mut bin_names = HashMap::new();
     for package in &pkgs {
         for target in package.targets().iter().filter(|t| t.documented()) {
             if target.is_lib() {
-                assert!(lib_names.insert(target.crate_name()));
+                if let Some(prev) = lib_names.insert(target.crate_name(), package) {
+                    bail!("The library `{}` is specified by packages `{}` and \
+                          `{}` but can only be documented once. Consider renaming \
+                          or marking one of the targets as `doc = false`.",
+                          target.crate_name(), prev, package);
+                }
             } else {
-                assert!(bin_names.insert(target.crate_name()));
+                if let Some(prev) = bin_names.insert(target.crate_name(), package) {
+                    bail!("The binary `{}` is specified by packages `{}` and \
+                          `{}` but can be documented only once. Consider renaming \
+                          or marking one of the targets as `doc = false`.",
+                          target.crate_name(), prev, package);
+                }
             }
         }
-        for bin in bin_names.iter() {
-            if lib_names.contains(bin) {
-                bail!("cannot document a package where a library and a binary \
-                       have the same name. Consider renaming one or marking \
-                       the target as `doc = false`")
+        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)
+                       });
             }
         }
     }
@@ -59,7 +76,7 @@ pub fn doc(ws: &Workspace, options: &DocOptions) -> CargoResult<()> {
         } else if pkgs.len() == 1 {
             pkgs[0].name().replace("-", "_")
         } else {
-            match lib_names.iter().chain(bin_names.iter()).nth(0) {
+            match lib_names.keys().chain(bin_names.keys()).nth(0) {
                 Some(s) => s.to_string(),
                 None => return Ok(()),
             }

diff --git a/tests/doc.rs b/tests/doc.rs
index 2c322a91709953c34b380a9e5fdd46d49b15ff4a..97a8a613f4e7e29e903bba7c43e102b29bf18905 100644 (file)
--- a/tests/doc.rs
+++ b/tests/doc.rs
@@ -203,6 +203,131 @@ fn doc_only_bin() {
     assert_that(&p.root().join("target/doc/foo/index.html"), existing_file());
 }
 
+#[test]
+fn doc_multiple_targets_same_name_lib() {
+    let p = project("foo")
+        .file("Cargo.toml", r#"
+            [workspace]
+            members = ["foo", "bar"]
+        "#)
+        .file("foo/Cargo.toml", r#"
+            [package]
+            name = "foo"
+            version = "0.1.0"
+            [lib]
+            name = "foo_lib"
+        "#)
+        .file("foo/src/lib.rs", "")
+        .file("bar/Cargo.toml", r#"
+            [package]
+            name = "bar"
+            version = "0.1.0"
+            [lib]
+            name = "foo_lib"
+        "#)
+        .file("bar/src/lib.rs", "");
+
+        assert_that(p.cargo_process("doc").arg("--all"),
+                    execs()
+                    .with_status(101)
+                    .with_stderr_contains("[..] library `foo_lib` is specified [..]")
+                    .with_stderr_contains("[..] `foo v0.1.0[..]` [..]")
+                    .with_stderr_contains("[..] `bar v0.1.0[..]` [..]"));
+}
+
+#[test]
+fn doc_multiple_targets_same_name() {
+    let p = project("foo")
+        .file("Cargo.toml", r#"
+            [workspace]
+            members = ["foo", "bar"]
+        "#)
+        .file("foo/Cargo.toml", r#"
+            [package]
+            name = "foo"
+            version = "0.1.0"
+            [[bin]]
+            name = "foo_lib"
+        "#)
+        .file("foo/src/foo_lib.rs", "")
+        .file("bar/Cargo.toml", r#"
+            [package]
+            name = "bar"
+            version = "0.1.0"
+            [lib]
+            name = "foo_lib"
+        "#)
+        .file("bar/src/lib.rs", "");
+
+        assert_that(p.cargo_process("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[..]` [..]"));
+}
+
+#[test]
+fn doc_multiple_targets_same_name_bin() {
+    let p = project("foo")
+        .file("Cargo.toml", r#"
+            [workspace]
+            members = ["foo", "bar"]
+        "#)
+        .file("foo/Cargo.toml", r#"
+            [package]
+            name = "foo"
+            version = "0.1.0"
+            [[bin]]
+            name = "foo-cli"
+        "#)
+        .file("foo/src/foo-cli.rs", "")
+        .file("bar/Cargo.toml", r#"
+            [package]
+            name = "bar"
+            version = "0.1.0"
+            [[bin]]
+            name = "foo-cli"
+        "#)
+        .file("bar/src/foo-cli.rs", "");
+
+        assert_that(p.cargo_process("doc").arg("--all"),
+                    execs()
+                    .with_status(101)
+                    .with_stderr_contains("[..] binary `foo_cli` is specified [..]")
+                    .with_stderr_contains("[..] `foo v0.1.0[..]` [..]")
+                    .with_stderr_contains("[..] `bar v0.1.0[..]` [..]"));
+}
+
+#[test]
+fn doc_multiple_targets_same_name_undoced() {
+    let p = project("foo")
+        .file("Cargo.toml", r#"
+            [workspace]
+            members = ["foo", "bar"]
+        "#)
+        .file("foo/Cargo.toml", r#"
+            [package]
+            name = "foo"
+            version = "0.1.0"
+            [[bin]]
+            name = "foo-cli"
+        "#)
+        .file("foo/src/foo-cli.rs", "")
+        .file("bar/Cargo.toml", r#"
+            [package]
+            name = "bar"
+            version = "0.1.0"
+            [[bin]]
+            name = "foo-cli"
+            doc = false
+        "#)
+        .file("bar/src/foo-cli.rs", "");
+
+        assert_that(p.cargo_process("doc").arg("--all"),
+                    execs().with_status(0));
+}
+
 #[test]
 fn doc_lib_bin_same_name() {
     let p = project("foo")
@@ -218,9 +343,8 @@ fn doc_lib_bin_same_name() {
     assert_that(p.cargo_process("doc"),
                 execs().with_status(101)
                        .with_stderr("\
-[ERROR] cannot document a package where a library and a binary have the same name. \
-Consider renaming one or marking the target as `doc = false`
-"));
+[ERROR] The target `foo` is specified as a library and as a binary by package \
+`foo [..]`. It can be documented[..]"));
 }
 
 #[test]

diff --git a/tests/rustdoc.rs b/tests/rustdoc.rs
index 08d09424d0ef4ce5c9caa1d62ab09ccb85a4e358..ad4bc826e87090bad23758c1c19327c0c1310f4a 100644 (file)
--- a/tests/rustdoc.rs
+++ b/tests/rustdoc.rs
@@ -160,7 +160,6 @@ fn rustdoc_same_name_err() {
                  .arg("--").arg("--cfg=foo"),
                 execs()
                 .with_status(101)
-                .with_stderr("[ERROR] cannot document a package where a library and a \
-                              binary have the same name. Consider renaming one \
-                              or marking the target as `doc = false`"));
+                .with_stderr("[ERROR] The target `foo` is specified as a \
+library and as a binary by package `foo [..]`. It can be documented[..]"));
 }