Auto merge of #8808 - weihanglo:fix/8591, r=ehuss

List available packages if providing `--package` with an empty value

May resolves #8591

## How

First we need to take the responsibility of check command line arguments from claps. I've examine all 10 build commands and all of them call [`ArgMatchesExt::compile_options`](2f115a76e5/src/cargo/util/command_prelude.rs (L389-L395)) directly or indirectly. And `compile_options` [calls `check_optional_opts`](2f115a76e5/src/cargo/util/command_prelude.rs (L499-L501)) to check if target selection options given an empty value. So we can do the same logic there.

I've also add a error message for an edge case though that one would never trigger at this moment.

src/bin/cargo/commands/clean.rs

@@ -1,6 +1,7 @@
 use crate::command_prelude::*;
 
 use cargo::ops::{self, CleanOptions};
+use cargo::util::print_available_packages;
 
 pub fn cli() -> App {
     subcommand("clean")
@@ -18,6 +19,11 @@ pub fn cli() -> App {
 
 pub fn exec(config: &mut Config, args: &ArgMatches<'_>) -> CliResult {
     let ws = args.workspace(config)?;
+
+    if args.is_present_with_zero_values("package") {
+        print_available_packages(&ws)?;
+    }
+
     let opts = CleanOptions {
         config,
         spec: values(args, "package"),

src/bin/cargo/commands/pkgid.rs

@@ -1,6 +1,7 @@
 use crate::command_prelude::*;
 
 use cargo::ops;
+use cargo::util::print_available_packages;
 
 pub fn cli() -> App {
     subcommand("pkgid")
@@ -14,6 +15,9 @@ pub fn cli() -> App {
 
 pub fn exec(config: &mut Config, args: &ArgMatches<'_>) -> CliResult {
     let ws = args.workspace(config)?;
+    if args.is_present_with_zero_values("package") {
+        print_available_packages(&ws)?
+    }
     let spec = args.value_of("spec").or_else(|| args.value_of("package"));
     let spec = ops::pkgid(&ws, spec)?;
     cargo::drop_println!(config, "{}", spec);

src/bin/cargo/commands/tree.rs

@@ -4,6 +4,7 @@ use anyhow::{bail, format_err};
 use cargo::core::dependency::DepKind;
 use cargo::ops::tree::{self, EdgeKind};
 use cargo::ops::Packages;
+use cargo::util::print_available_packages;
 use cargo::util::CargoResult;
 use std::collections::HashSet;
 use std::str::FromStr;
@@ -176,6 +177,11 @@ subtree of the package given to -p.\n\
     }
 
     let ws = args.workspace(config)?;
+
+    if args.is_present_with_zero_values("package") {
+        print_available_packages(&ws)?;
+    }
+
     let charset = tree::Charset::from_str(args.value_of("charset").unwrap())
         .map_err(|e| anyhow::anyhow!("{}", e))?;
     let opts = tree::TreeOptions {

src/bin/cargo/commands/uninstall.rs

@@ -15,6 +15,15 @@ pub fn cli() -> App {
 
 pub fn exec(config: &mut Config, args: &ArgMatches<'_>) -> CliResult {
     let root = args.value_of("root");
+
+    if args.is_present_with_zero_values("package") {
+        return Err(anyhow::anyhow!(
+            "\"--package <SPEC>\" requires a SPEC format value.\n\
+            Run `cargo help pkgid` for more information about SPEC format."
+        )
+        .into());
+    }
+
     let specs = args
         .values_of("spec")
         .unwrap_or_else(|| args.values_of("package").unwrap_or_default())

src/bin/cargo/commands/update.rs

@@ -1,6 +1,7 @@
 use crate::command_prelude::*;
 
 use cargo::ops::{self, UpdateOptions};
+use cargo::util::print_available_packages;
 
 pub fn cli() -> App {
     subcommand("update")
@@ -20,6 +21,10 @@ pub fn cli() -> App {
 pub fn exec(config: &mut Config, args: &ArgMatches<'_>) -> CliResult {
     let ws = args.workspace(config)?;
 
+    if args.is_present_with_zero_values("package") {
+        print_available_packages(&ws)?;
+    }
+
     let update_opts = UpdateOptions {
         aggressive: args.is_present("aggressive"),
         precise: args.value_of("precise"),

src/cargo/util/command_prelude.rs

@@ -8,7 +8,7 @@ use crate::util::restricted_names::is_glob_pattern;
 use crate::util::{paths, toml::TomlProfile, validate_package_name};
 use crate::util::{
     print_available_benches, print_available_binaries, print_available_examples,
-    print_available_tests,
+    print_available_packages, print_available_tests,
 };
 use crate::CargoResult;
 use anyhow::bail;
@@ -52,11 +52,15 @@ pub trait AppExt: Sized {
     }
 
     fn arg_package_spec_simple(self, package: &'static str) -> Self {
-        self._arg(multi_opt("package", "SPEC", package).short("p"))
+        self._arg(optional_multi_opt("package", "SPEC", package).short("p"))
     }
 
     fn arg_package(self, package: &'static str) -> Self {
-        self._arg(opt("package", package).short("p").value_name("SPEC"))
+        self._arg(
+            optinal_opt("package", package)
+                .short("p")
+                .value_name("SPEC"),
+        )
     }
 
     fn arg_jobs(self) -> Self {
@@ -220,6 +224,10 @@ pub fn opt(name: &'static str, help: &'static str) -> Arg<'static, 'static> {
     Arg::with_name(name).long(name).help(help)
 }
 
+pub fn optinal_opt(name: &'static str, help: &'static str) -> Arg<'static, 'static> {
+    opt(name, help).min_values(0)
+}
+
 pub fn optional_multi_opt(
     name: &'static str,
     value_name: &'static str,
@@ -498,6 +506,14 @@ pub trait ArgMatchesExt {
 
         if let Some(ws) = workspace {
             self.check_optional_opts(ws, &opts)?;
+        } else if self.is_present_with_zero_values("package") {
+            // As for cargo 0.50.0, this won't occur but if someone sneaks in
+            // we can still provide this informative message for them.
+            anyhow::bail!(
+                "\"--package <SPEC>\" requires a SPEC format value, \
+                which can be any package ID specifier in the dependency graph.\n\
+                Run `cargo help pkgid` for more information about SPEC format."
+            )
         }
 
         Ok(opts)
@@ -588,6 +604,10 @@ about this warning.";
         workspace: &Workspace<'_>,
         compile_opts: &CompileOptions,
     ) -> CargoResult<()> {
+        if self.is_present_with_zero_values("package") {
+            print_available_packages(workspace)?
+        }
+
         if self.is_present_with_zero_values("example") {
             print_available_examples(workspace, compile_opts)?;
         }

src/cargo/util/mod.rs

@@ -28,7 +28,7 @@ pub use self::to_semver::ToSemver;
 pub use self::vcs::{existing_vcs_repo, FossilRepo, GitRepo, HgRepo, PijulRepo};
 pub use self::workspace::{
     print_available_benches, print_available_binaries, print_available_examples,
-    print_available_tests,
+    print_available_packages, print_available_tests,
 };
 
 mod canonical_url;

src/cargo/util/workspace.rs

@@ -8,7 +8,7 @@ fn get_available_targets<'a>(
     filter_fn: fn(&Target) -> bool,
     ws: &'a Workspace<'_>,
     options: &'a CompileOptions,
-) -> CargoResult<Vec<&'a Target>> {
+) -> CargoResult<Vec<&'a str>> {
     let packages = options.spec.get_packages(ws)?;
 
     let mut targets: Vec<_> = packages
@@ -19,6 +19,7 @@ fn get_available_targets<'a>(
                 .iter()
                 .filter(|target| filter_fn(target))
         })
+        .map(Target::name)
         .collect();
 
     targets.sort();
@@ -26,7 +27,7 @@ fn get_available_targets<'a>(
     Ok(targets)
 }
 
-fn print_available(
+fn print_available_targets(
     filter_fn: fn(&Target) -> bool,
     ws: &Workspace<'_>,
     options: &CompileOptions,
@@ -43,24 +44,48 @@ fn print_available(
     } else {
         writeln!(output, "Available {}:", plural_name)?;
         for target in targets {
-            writeln!(output, "    {}", target.name())?;
+            writeln!(output, "    {}", target)?;
+        }
+    }
+    bail!("{}", output)
+}
+
+pub fn print_available_packages(ws: &Workspace<'_>) -> CargoResult<()> {
+    let packages = ws
+        .members()
+        .map(|pkg| pkg.name().as_str())
+        .collect::<Vec<_>>();
+
+    let mut output = "\"--package <SPEC>\" requires a SPEC format value, \
+        which can be any package ID specifier in the dependency graph.\n\
+        Run `cargo help pkgid` for more information about SPEC format.\n\n"
+        .to_string();
+
+    if packages.is_empty() {
+        // This would never happen.
+        // Just in case something regresses we covers it here.
+        writeln!(output, "No packages available.")?;
+    } else {
+        writeln!(output, "Possible packages/workspace members:")?;
+        for package in packages {
+            writeln!(output, "    {}", package)?;
         }
     }
     bail!("{}", output)
 }
 
 pub fn print_available_examples(ws: &Workspace<'_>, options: &CompileOptions) -> CargoResult<()> {
-    print_available(Target::is_example, ws, options, "--example", "examples")
+    print_available_targets(Target::is_example, ws, options, "--example", "examples")
 }
 
 pub fn print_available_binaries(ws: &Workspace<'_>, options: &CompileOptions) -> CargoResult<()> {
-    print_available(Target::is_bin, ws, options, "--bin", "binaries")
+    print_available_targets(Target::is_bin, ws, options, "--bin", "binaries")
 }
 
 pub fn print_available_benches(ws: &Workspace<'_>, options: &CompileOptions) -> CargoResult<()> {
-    print_available(Target::is_bench, ws, options, "--bench", "benches")
+    print_available_targets(Target::is_bench, ws, options, "--bench", "benches")
 }
 
 pub fn print_available_tests(ws: &Workspace<'_>, options: &CompileOptions) -> CargoResult<()> {
-    print_available(Target::is_test, ws, options, "--test", "tests")
+    print_available_targets(Target::is_test, ws, options, "--test", "tests")
 }

tests/testsuite/install.rs

@@ -1181,6 +1181,19 @@ fn uninstall_multiple_and_specifying_bin() {
         .run();
 }
 
+#[cargo_test]
+fn uninstall_with_empty_pakcage_option() {
+    cargo_process("uninstall -p")
+        .with_status(101)
+        .with_stderr(
+            "\
+[ERROR] \"--package <SPEC>\" requires a SPEC format value.
+Run `cargo help pkgid` for more information about SPEC format.
+",
+        )
+        .run();
+}
+
 #[cargo_test]
 fn uninstall_multiple_and_some_pkg_does_not_exist() {
     pkg("foo", "0.0.1");

tests/testsuite/list_availables.rs

@@ -1,13 +1,15 @@
-//! Tests for target filter flags giving suggestions on which targets are available.
+//! Tests for packages/target filter flags giving suggestions on which
+//! packages/targets are available.
 
 use cargo_test_support::project;
 
-const EXAMPLE: u8 = 0x1;
+const EXAMPLE: u8 = 1 << 0;
-const BIN: u8 = 0x2;
+const BIN: u8 = 1 << 1;
-const TEST: u8 = 0x4;
+const TEST: u8 = 1 << 2;
-const BENCH: u8 = 0x8;
+const BENCH: u8 = 1 << 3;
+const PACKAGE: u8 = 1 << 4;
 
-fn list_targets_test(command: &str, targets: u8) {
+fn list_availables_test(command: &str, targets: u8) {
     let full_project = project()
         .file("examples/a.rs", "fn main() { }")
         .file("examples/b.rs", "fn main() { }")
@@ -16,6 +18,7 @@ fn list_targets_test(command: &str, targets: u8) {
         .file("tests/test1.rs", "")
         .file("tests/test2.rs", "")
         .file("src/main.rs", "fn main() { }")
+        .file("Cargo.lock", "") // for `cargo pkgid`
         .build();
 
     if targets & EXAMPLE != 0 {
@@ -75,6 +78,24 @@ Available tests:
     test1
     test2
 
+",
+            )
+            .with_status(101)
+            .run();
+    }
+
+    if targets & PACKAGE != 0 {
+        full_project
+            .cargo(&format!("{} -p", command))
+            .with_stderr(
+                "\
+[ERROR] \"--package <SPEC>\" requires a SPEC format value, \
+which can be any package ID specifier in the dependency graph.
+Run `cargo help pkgid` for more information about SPEC format.
+
+Possible packages/workspace members:
+    foo
+
 ",
             )
             .with_status(101)
@@ -141,51 +162,71 @@ No tests available.
 }
 
 #[cargo_test]
-fn build_list_targets() {
+fn build_list_availables() {
-    list_targets_test("build", EXAMPLE | BIN | TEST | BENCH);
+    list_availables_test("build", EXAMPLE | BIN | TEST | BENCH | PACKAGE);
 }
 
 #[cargo_test]
-fn check_list_targets() {
+fn check_list_availables() {
-    list_targets_test("check", EXAMPLE | BIN | TEST | BENCH);
+    list_availables_test("check", EXAMPLE | BIN | TEST | BENCH | PACKAGE);
 }
 
 #[cargo_test]
-fn doc_list_targets() {
+fn doc_list_availables() {
-    list_targets_test("doc", BIN);
+    list_availables_test("doc", BIN | PACKAGE);
 }
 
 #[cargo_test]
-fn fix_list_targets() {
+fn fix_list_availables() {
-    list_targets_test("fix", EXAMPLE | BIN | TEST | BENCH);
+    list_availables_test("fix", EXAMPLE | BIN | TEST | BENCH | PACKAGE);
 }
 
 #[cargo_test]
-fn run_list_targets() {
+fn run_list_availables() {
-    list_targets_test("run", EXAMPLE | BIN);
+    list_availables_test("run", EXAMPLE | BIN | PACKAGE);
 }
 
 #[cargo_test]
-fn test_list_targets() {
+fn test_list_availables() {
-    list_targets_test("test", EXAMPLE | BIN | TEST | BENCH);
+    list_availables_test("test", EXAMPLE | BIN | TEST | BENCH | PACKAGE);
 }
 
 #[cargo_test]
-fn bench_list_targets() {
+fn bench_list_availables() {
-    list_targets_test("bench", EXAMPLE | BIN | TEST | BENCH);
+    list_availables_test("bench", EXAMPLE | BIN | TEST | BENCH | PACKAGE);
 }
 
 #[cargo_test]
-fn install_list_targets() {
+fn install_list_availables() {
-    list_targets_test("install", EXAMPLE | BIN);
+    list_availables_test("install", EXAMPLE | BIN);
 }
 
 #[cargo_test]
-fn rustdoc_list_targets() {
+fn rustdoc_list_availables() {
-    list_targets_test("rustdoc", EXAMPLE | BIN | TEST | BENCH);
+    list_availables_test("rustdoc", EXAMPLE | BIN | TEST | BENCH | PACKAGE);
 }
 
 #[cargo_test]
-fn rustc_list_targets() {
+fn rustc_list_availables() {
-    list_targets_test("rustc", EXAMPLE | BIN | TEST | BENCH);
+    list_availables_test("rustc", EXAMPLE | BIN | TEST | BENCH | PACKAGE);
+}
+
+#[cargo_test]
+fn pkgid_list_availables() {
+    list_availables_test("pkgid", PACKAGE);
+}
+
+#[cargo_test]
+fn tree_list_availables() {
+    list_availables_test("tree", PACKAGE);
+}
+
+#[cargo_test]
+fn clean_list_availables() {
+    list_availables_test("clean", PACKAGE);
+}
+
+#[cargo_test]
+fn update_list_availables() {
+    list_availables_test("update", PACKAGE);
 }

tests/testsuite/main.rs

@@ -59,7 +59,7 @@ mod init;
 mod install;
 mod install_upgrade;
 mod jobserver;
-mod list_targets;
+mod list_availables;
 mod local_registry;
 mod locate_project;
 mod lockfile_compat;