fix(run): Disambiguate bins from different packages that share a name (#15298)

### What does this PR try to resolve?

This builds on the work done in #15199 to improve target selection
errors to also disambiguate when the same binary name is used in
multiple packages.

This also makes the errors from #15199 more consistent with the rustc
style guide and reduces code duplication.

Fixes #13312

### How should we test and review this PR?

This is a first pass and does not do the full `--package foo --bin bar`
syntax. I wanted to focus on the basic functionality before we iterated
on it, e.g. investigating how well we can predict the CLI flags, how
much noise they might add, etc.

### Additional information

Author: weihanglo

src/cargo/ops/cargo_compile/unit_generator.rs

@@ -259,37 +259,40 @@ impl<'a> UnitGenerator<'a, '_> {
         };
         let proposals = self.filter_targets(filter, true, mode);
         if proposals.is_empty() {
-            let targets = self
-                .packages
-                .iter()
-                .flat_map(|pkg| {
-                    pkg.targets()
-                        .iter()
-                        .filter(|target| is_expected_kind(target))
-                })
-                .collect::<Vec<_>>();
-            let suggestion = closest_msg(target_name, targets.iter(), |t| t.name(), "target");
+            let mut targets = std::collections::BTreeMap::new();
+            for (pkg, target) in self.packages.iter().flat_map(|pkg| {
+                pkg.targets()
+                    .iter()
+                    .filter(|target| is_expected_kind(target))
+                    .map(move |t| (pkg, t))
+            }) {
+                targets
+                    .entry(target.name())
+                    .or_insert_with(Vec::new)
+                    .push((pkg, target));
+            }
+
+            let suggestion = closest_msg(target_name, targets.keys(), |t| t, "target");
             let targets_elsewhere = self.get_targets_from_other_packages(filter)?;
-            let need_append_targets_elsewhere = !targets_elsewhere.is_empty();
-            let append_targets_elsewhere = |msg: &mut String, prefix: &str| {
+            let append_targets_elsewhere = |msg: &mut String| {
                 let mut available_msg = Vec::new();
-                for (package, targets) in targets_elsewhere {
+                for (package, targets) in &targets_elsewhere {
                     if !targets.is_empty() {
                         available_msg.push(format!(
-                            "help: Available {target_desc} in `{package}` package:"
+                            "help: available {target_desc} in `{package}` package:"
                         ));
                         for target in targets {
                             available_msg.push(format!("    {target}"));
                         }
                     }
                 }
                 if !available_msg.is_empty() {
-                    write!(msg, "{prefix}{}", available_msg.join("\n"))?;
+                    write!(msg, "\n{}", available_msg.join("\n"))?;
                 }
                 CargoResult::Ok(())
             };
 
-            let unmatched_packages = || match self.spec {
+            let unmatched_packages = match self.spec {
                 Packages::Default | Packages::OptOut(_) | Packages::All(_) => {
                     "default-run packages".to_owned()
                 }
@@ -305,33 +308,25 @@ impl<'a> UnitGenerator<'a, '_> {
                 }
             };
 
+            let named = if is_glob { "matches pattern" } else { "named" };
+
             let mut msg = String::new();
-            if !suggestion.is_empty() {
-                write!(
-                    msg,
-                    "no {} target {} `{}` in {}{}",
-                    target_desc,
-                    if is_glob { "matches pattern" } else { "named" },
-                    target_name,
-                    unmatched_packages(),
-                    suggestion,
-                )?;
-                append_targets_elsewhere(&mut msg, "\n")?;
-            } else {
-                writeln!(
-                    msg,
-                    "no {} target {} `{}` in {}.",
-                    target_desc,
-                    if is_glob { "matches pattern" } else { "named" },
-                    target_name,
-                    unmatched_packages()
-                )?;
-
-                append_targets_elsewhere(&mut msg, "")?;
-                if !targets.is_empty() && !need_append_targets_elsewhere {
-                    writeln!(msg, "Available {} targets:", target_desc)?;
-                    for target in targets {
-                        writeln!(msg, "    {}", target.name())?;
+            write!(
+                msg,
+                "no {target_desc} target {named} `{target_name}` in {unmatched_packages}{suggestion}",
+            )?;
+            if !targets_elsewhere.is_empty() {
+                append_targets_elsewhere(&mut msg)?;
+            } else if suggestion.is_empty() && !targets.is_empty() {
+                write!(msg, "\nhelp: available {} targets:", target_desc)?;
+                for (target_name, pkgs) in targets {
+                    if pkgs.len() == 1 {
+                        write!(msg, "\n    {target_name}")?;
+                    } else {
+                        for (pkg, _) in pkgs {
+                            let pkg_name = pkg.name();
+                            write!(msg, "\n    {target_name} in package {pkg_name}")?;
+                        }
                     }
                 }
             }

src/cargo/ops/cargo_run.rs

@@ -1,4 +1,5 @@
 use std::ffi::OsString;
+use std::fmt::Write as _;
 use std::iter;
 use std::path::Path;
 
@@ -69,10 +70,20 @@ pub fn run(
                 names.join(", ")
             )
         } else {
-            anyhow::bail!(
-                "`cargo run` can run at most one executable, but \
+            let mut message = "`cargo run` can run at most one executable, but \
                  multiple were specified"
-            )
+                .to_owned();
+            write!(&mut message, "\nhelp: available targets:")?;
+            for (pkg, bin) in &bins {
+                write!(
+                    &mut message,
+                    "\n    {} `{}` in package `{}`",
+                    bin.kind().description(),
+                    bin.name(),
+                    pkg.name()
+                )?;
+            }
+            anyhow::bail!(message)
         }
     }
 

tests/testsuite/build.rs

@@ -1337,11 +1337,10 @@ fn cargo_compile_with_filename() {
     p.cargo("build --bin bin.rs")
         .with_status(101)
         .with_stderr_data(str![[r#"
-[ERROR] no bin target named `bin.rs` in default-run packages.
-Available bin targets:
+[ERROR] no bin target named `bin.rs` in default-run packages
+[HELP] available bin targets:
     a
 
-
 "#]])
         .run();
 
@@ -1358,11 +1357,10 @@ Available bin targets:
     p.cargo("build --example example.rs")
         .with_status(101)
         .with_stderr_data(str![[r#"
-[ERROR] no example target named `example.rs` in default-run packages.
-Available example targets:
+[ERROR] no example target named `example.rs` in default-run packages
+[HELP] available example targets:
     a
 
-
 "#]])
         .run();