From: Lukas Lueg Date: Fri, 29 Sep 2017 11:46:31 +0000 (+0200) Subject: Improve error ico doc-targets with same name X-Git-Url: https://git.proxmox.com/?a=commitdiff_plain;h=9c635636b0770fae9f7d050f6ee1871d1cbed885;p=cargo.git 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 2d79cf063..d4d562036 100644 --- 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::>>()?; - 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 2c322a917..97a8a613f 100644 --- 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 08d09424d..ad4bc826e 100644 --- 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[..]")); }