Auto merge of #10265 - epage:clap-port, r=joshtriplett

Port cargo to clap3 ### What does this PR try to resolve? This moves cargo to the latest major version of clap. This supersedes #10259 and #10262 ### How should we test and review this PR? For testing, I mostly relied on existing tests. I did manually validate that `cargo run <non-escaped command args>` behaved the same between both ```console $ cargo run release --help Finished dev [unoptimized + debuginfo] target(s) in 0.22s Running `target/debug/cargo-release release --help` cargo-release 0.18.8 ... $ cargo run --manifest-path ../cargo/Cargo.toml -- run release --help Finished dev [unoptimized + debuginfo] target(s) in 0.05s Running `/home/epage/src/personal/cargo/target/debug/cargo run release --help` Finished dev [unoptimized + debuginfo] target(s) in 1.31s Running `target/debug/cargo-release release --help` cargo-release 0.18.8 ... ``` For reviewing, I split out each deprecation resolution into a separate commit so its easy to focus on more mechanical changes (most of the deprecation fixes) from interesting ones (the port, the `Arg::multiple` deprecation) ### Additional information - One parser change found by `cargo_config::includes` is that clap 2 would ignore any values after a `=` for flags. `cargo config --show-origin` is a flag but the test passed `--show-origin=yes` which happens to give the desired result for that test but is the same as `--show-origin=no` or `--show-origin=alien-invasion`. - `ArgMatches` now panics when accessing an undefined argument but clap takes advantage of that for sharing code across commands that have different subsets of arguments defined. I've extended clap so we can "look before you leap" and put the checks at the argument calls to start off with so its very clear what is tenuously shared. This allows us to go in either direction in the future, either addressing how we are sharing between commands or by moving this down into the extension methods and pretending this clap feature doesn't exist - On that topic, a test found clap-rs/clap#3263. For now, there is a hack in clap. Depending on how we fix that in clap for clap 4.0, we might need to re-address things in cargo. - `value_of_os` now requires setting `allow_invalid_utf8`, otherwise it asserts. To help catch this, I updated the argument definitions associated with lookups reported by: - `rg 'values?_os' src/` - `rg 'values?_of_os' src/` - clap now reports `2` for usage errors, so we had to bypass clap's `exit` call to keep the same exit code. - `cargo vendor --sync` did not use `multi_opt` and so it has both multiple occurrences **and** multiple values. If we want to deprecate this, we'll need `unstable-grouped` to be stablized (or pin our clap version) and ensure each group has only 1 value.
rust-lang · Jan 11, 2022 · 06b9d31 · 06b9d31
2 parents 0655dd2 + 92fa72d
commit 06b9d31
Show file tree

Hide file tree

Showing 47 changed files with 261 additions and 236 deletions.
diff --git a/Cargo.toml b/Cargo.toml
@@ -61,7 +61,7 @@ toml = "0.5.7"
 unicode-xid = "0.2.0"
 url = "2.2.2"
 walkdir = "2.2"
-clap = "2.34.0"
+clap = "3.0.5"
 unicode-width = "0.1.5"
 openssl = { version = '0.10.11', optional = true }
 im-rc = "15.0.0"

diff --git a/src/bin/cargo/cli.rs b/src/bin/cargo/cli.rs
@@ -30,13 +30,13 @@ pub fn main(config: &mut Config) -> CliResult {
         return Ok(());
     }
 
-    let args = match cli().get_matches_safe() {
+    let args = match cli().try_get_matches() {
         Ok(args) => args,
         Err(e) => {
             if e.kind == clap::ErrorKind::UnrecognizedSubcommand {
                 // An unrecognized subcommand might be an external subcommand.
-                let cmd = &e.info.as_ref().unwrap()[0].to_owned();
-                return super::execute_external_subcommand(config, cmd, &[cmd, "--help"])
+                let cmd = e.info[0].clone();
+                return super::execute_external_subcommand(config, &cmd, &[&cmd, "--help"])
                     .map_err(|_| e.into());
             } else {
                 return Err(e.into());
@@ -152,7 +152,7 @@ Run with 'cargo -Z [FLAG] [SUBCOMMAND]'",
     }
 
     let (cmd, subcommand_args) = match expanded_args.subcommand() {
-        (cmd, Some(args)) => (cmd, args),
+        Some((cmd, args)) => (cmd, args),
         _ => {
             // No subcommand provided.
             cli().print_help()?;
@@ -236,10 +236,10 @@ fn add_ssl(version_string: &mut String) {
 
 fn expand_aliases(
     config: &mut Config,
-    args: ArgMatches<'static>,
+    args: ArgMatches,
     mut already_expanded: Vec<String>,
-) -> Result<(ArgMatches<'static>, GlobalArgs), CliError> {
-    if let (cmd, Some(args)) = args.subcommand() {
+) -> Result<(ArgMatches, GlobalArgs), CliError> {
+    if let Some((cmd, args)) = args.subcommand() {
         match (
             commands::builtin_exec(cmd),
             super::aliased_command(config, cmd)?,
@@ -290,9 +290,9 @@ For more information, see issue #10049 <https://github.com/rust-lang/cargo/issue
                 let global_args = GlobalArgs::new(args);
                 let new_args = cli()
                     .setting(AppSettings::NoBinaryName)
-                    .get_matches_from_safe(alias)?;
+                    .try_get_matches_from(alias)?;
 
-                let (new_cmd, _) = new_args.subcommand();
+                let new_cmd = new_args.subcommand_name().expect("subcommand is required");
                 already_expanded.push(cmd.to_string());
                 if already_expanded.contains(&new_cmd.to_string()) {
                     // Crash if the aliases are corecursive / unresolvable
@@ -316,16 +316,20 @@ For more information, see issue #10049 <https://github.com/rust-lang/cargo/issue
 
 fn config_configure(
     config: &mut Config,
-    args: &ArgMatches<'_>,
-    subcommand_args: &ArgMatches<'_>,
+    args: &ArgMatches,
+    subcommand_args: &ArgMatches,
     global_args: GlobalArgs,
 ) -> CliResult {
-    let arg_target_dir = &subcommand_args.value_of_path("target-dir", config);
+    let arg_target_dir = &subcommand_args
+        ._is_valid_arg("target-dir")
+        .then(|| subcommand_args.value_of_path("target-dir", config))
+        .flatten();
     let verbose = global_args.verbose + args.occurrences_of("verbose") as u32;
     // quiet is unusual because it is redefined in some subcommands in order
     // to provide custom help text.
-    let quiet =
-        args.is_present("quiet") || subcommand_args.is_present("quiet") || global_args.quiet;
+    let quiet = args.is_present("quiet")
+        || subcommand_args.is_valid_and_present("quiet")
+        || global_args.quiet;
     let global_color = global_args.color; // Extract so it can take reference.
     let color = args.value_of("color").or_else(|| global_color.as_deref());
     let frozen = args.is_present("frozen") || global_args.frozen;
@@ -353,11 +357,7 @@ fn config_configure(
     Ok(())
 }
 
-fn execute_subcommand(
-    config: &mut Config,
-    cmd: &str,
-    subcommand_args: &ArgMatches<'_>,
-) -> CliResult {
+fn execute_subcommand(config: &mut Config, cmd: &str, subcommand_args: &ArgMatches) -> CliResult {
     if let Some(exec) = commands::builtin_exec(cmd) {
         return exec(config, subcommand_args);
     }
@@ -380,7 +380,7 @@ struct GlobalArgs {
 }
 
 impl GlobalArgs {
-    fn new(args: &ArgMatches<'_>) -> GlobalArgs {
+    fn new(args: &ArgMatches) -> GlobalArgs {
         GlobalArgs {
             verbose: args.occurrences_of("verbose") as u32,
             quiet: args.is_present("quiet"),
@@ -408,22 +408,24 @@ fn cli() -> App {
         "cargo [OPTIONS] [SUBCOMMAND]"
     };
     App::new("cargo")
-        .settings(&[
-            AppSettings::UnifiedHelpMessage,
-            AppSettings::DeriveDisplayOrder,
-            AppSettings::VersionlessSubcommands,
-            AppSettings::AllowExternalSubcommands,
-        ])
-        .usage(usage)
-        .template(
+        .setting(
+            AppSettings::DeriveDisplayOrder
+                | AppSettings::AllowExternalSubcommands
+                | AppSettings::NoAutoVersion,
+        )
+        // Doesn't mix well with our list of common cargo commands.  See clap-rs/clap#3108 for
+        // opening clap up to allow us to style our help template
+        .global_setting(AppSettings::DisableColoredHelp)
+        .override_usage(usage)
+        .help_template(
             "\
 Rust's package manager
 
 USAGE:
     {usage}
 
 OPTIONS:
-{unified}
+{options}
 
 Some common cargo commands are (see all commands with --list):
     build, b    Compile the current package
@@ -443,16 +445,16 @@ Some common cargo commands are (see all commands with --list):
 
 See 'cargo help <command>' for more information on a specific command.\n",
         )
-        .arg(opt("version", "Print version info and exit").short("V"))
+        .arg(opt("version", "Print version info and exit").short('V'))
         .arg(opt("list", "List installed commands"))
         .arg(opt("explain", "Run `rustc --explain CODE`").value_name("CODE"))
         .arg(
             opt(
                 "verbose",
                 "Use verbose output (-vv very verbose/build.rs output)",
             )
-            .short("v")
-            .multiple(true)
+            .short('v')
+            .multiple_occurrences(true)
             .global(true),
         )
         .arg_quiet()
@@ -473,13 +475,17 @@ See 'cargo help <command>' for more information on a specific command.\n",
             .global(true),
         )
         .arg(
-            Arg::with_name("unstable-features")
+            Arg::new("unstable-features")
                 .help("Unstable (nightly-only) flags to Cargo, see 'cargo -Z help' for details")
-                .short("Z")
+                .short('Z')
                 .value_name("FLAG")
-                .multiple(true)
-                .number_of_values(1)
+                .multiple_occurrences(true)
                 .global(true),
         )
         .subcommands(commands::builtin())
 }
+
+#[test]
+fn verify_cli() {
+    cli().debug_assert();
+}
diff --git a/src/bin/cargo/commands/bench.rs b/src/bin/cargo/commands/bench.rs
@@ -7,13 +7,13 @@ pub fn cli() -> App {
         .about("Execute all benchmarks of a local package")
         .arg_quiet()
         .arg(
-            Arg::with_name("BENCHNAME")
+            Arg::new("BENCHNAME")
                 .help("If specified, only run benches containing this string in their names"),
         )
         .arg(
-            Arg::with_name("args")
+            Arg::new("args")
                 .help("Arguments for the bench binary")
-                .multiple(true)
+                .multiple_values(true)
                 .last(true),
         )
         .arg_targets_all(
@@ -50,7 +50,7 @@ pub fn cli() -> App {
         .after_help("Run `cargo help bench` for more detailed information.\n")
 }
 
-pub fn exec(config: &mut Config, args: &ArgMatches<'_>) -> CliResult {
+pub fn exec(config: &mut Config, args: &ArgMatches) -> CliResult {
     let ws = args.workspace(config)?;
     let mut compile_opts = args.compile_options(
         config,

diff --git a/src/bin/cargo/commands/build.rs b/src/bin/cargo/commands/build.rs
@@ -47,7 +47,7 @@ pub fn cli() -> App {
         .after_help("Run `cargo help build` for more detailed information.\n")
 }
 
-pub fn exec(config: &mut Config, args: &ArgMatches<'_>) -> CliResult {
+pub fn exec(config: &mut Config, args: &ArgMatches) -> CliResult {
     let ws = args.workspace(config)?;
     let mut compile_opts = args.compile_options(
         config,

diff --git a/src/bin/cargo/commands/check.rs b/src/bin/cargo/commands/check.rs
@@ -39,7 +39,7 @@ pub fn cli() -> App {
         .after_help("Run `cargo help check` for more detailed information.\n")
 }
 
-pub fn exec(config: &mut Config, args: &ArgMatches<'_>) -> CliResult {
+pub fn exec(config: &mut Config, args: &ArgMatches) -> CliResult {
     let ws = args.workspace(config)?;
     // This is a legacy behavior that causes `cargo check` to pass `--test`.
     let test = matches!(args.value_of("profile"), Some("test"));

diff --git a/src/bin/cargo/commands/clean.rs b/src/bin/cargo/commands/clean.rs
@@ -17,7 +17,7 @@ pub fn cli() -> App {
         .after_help("Run `cargo help clean` for more detailed information.\n")
 }
 
-pub fn exec(config: &mut Config, args: &ArgMatches<'_>) -> CliResult {
+pub fn exec(config: &mut Config, args: &ArgMatches) -> CliResult {
     let ws = args.workspace(config)?;
 
     if args.is_present_with_zero_values("package") {

diff --git a/src/bin/cargo/commands/config.rs b/src/bin/cargo/commands/config.rs
@@ -8,7 +8,7 @@ pub fn cli() -> App {
         .setting(clap::AppSettings::SubcommandRequiredElseHelp)
         .subcommand(
             subcommand("get")
-                .arg(Arg::with_name("key").help("The config key to display"))
+                .arg(Arg::new("key").help("The config key to display"))
                 .arg(
                     opt("format", "Display format")
                         .possible_values(cargo_config::ConfigFormat::POSSIBLE_VALUES)
@@ -26,12 +26,12 @@ pub fn cli() -> App {
         )
 }
 
-pub fn exec(config: &mut Config, args: &ArgMatches<'_>) -> CliResult {
+pub fn exec(config: &mut Config, args: &ArgMatches) -> CliResult {
     config
         .cli_unstable()
         .fail_if_stable_command(config, "config", 9301)?;
     match args.subcommand() {
-        ("get", Some(args)) => {
+        Some(("get", args)) => {
             let opts = cargo_config::GetOptions {
                 key: args.value_of("key"),
                 format: args.value_of("format").unwrap().parse()?,
@@ -40,8 +40,11 @@ pub fn exec(config: &mut Config, args: &ArgMatches<'_>) -> CliResult {
             };
             cargo_config::get(config, &opts)?;
         }
-        (cmd, _) => {
-            panic!("unexpected command `{}`", cmd)
+        Some((cmd, _)) => {
+            unreachable!("unexpected command {}", cmd)
+        }
+        None => {
+            unreachable!("unexpected command")
         }
     }
     Ok(())

diff --git a/src/bin/cargo/commands/doc.rs b/src/bin/cargo/commands/doc.rs
@@ -39,7 +39,7 @@ pub fn cli() -> App {
         .after_help("Run `cargo help doc` for more detailed information.\n")
 }
 
-pub fn exec(config: &mut Config, args: &ArgMatches<'_>) -> CliResult {
+pub fn exec(config: &mut Config, args: &ArgMatches) -> CliResult {
     let ws = args.workspace(config)?;
     let mode = CompileMode::Doc {
         deps: !args.is_present("no-deps"),

diff --git a/src/bin/cargo/commands/fetch.rs b/src/bin/cargo/commands/fetch.rs
@@ -12,7 +12,7 @@ pub fn cli() -> App {
         .after_help("Run `cargo help fetch` for more detailed information.\n")
 }
 
-pub fn exec(config: &mut Config, args: &ArgMatches<'_>) -> CliResult {
+pub fn exec(config: &mut Config, args: &ArgMatches) -> CliResult {
     let ws = args.workspace(config)?;
 
     let opts = FetchOptions {

diff --git a/src/bin/cargo/commands/fix.rs b/src/bin/cargo/commands/fix.rs
@@ -32,40 +32,40 @@ pub fn cli() -> App {
         .arg_manifest_path()
         .arg_message_format()
         .arg(
-            Arg::with_name("broken-code")
+            Arg::new("broken-code")
                 .long("broken-code")
                 .help("Fix code even if it already has compiler errors"),
         )
         .arg(
-            Arg::with_name("edition")
+            Arg::new("edition")
                 .long("edition")
                 .help("Fix in preparation for the next edition"),
         )
         .arg(
-            Arg::with_name("idioms")
+            Arg::new("idioms")
                 .long("edition-idioms")
                 .help("Fix warnings to migrate to the idioms of an edition"),
         )
         .arg(
-            Arg::with_name("allow-no-vcs")
+            Arg::new("allow-no-vcs")
                 .long("allow-no-vcs")
                 .help("Fix code even if a VCS was not detected"),
         )
         .arg(
-            Arg::with_name("allow-dirty")
+            Arg::new("allow-dirty")
                 .long("allow-dirty")
                 .help("Fix code even if the working directory is dirty"),
         )
         .arg(
-            Arg::with_name("allow-staged")
+            Arg::new("allow-staged")
                 .long("allow-staged")
                 .help("Fix code even if the working directory has staged changes"),
         )
         .arg_ignore_rust_version()
         .after_help("Run `cargo help fix` for more detailed information.\n")
 }
 
-pub fn exec(config: &mut Config, args: &ArgMatches<'_>) -> CliResult {
+pub fn exec(config: &mut Config, args: &ArgMatches) -> CliResult {
     let ws = args.workspace(config)?;
     // This is a legacy behavior that causes `cargo fix` to pass `--test`.
     let test = matches!(args.value_of("profile"), Some("test"));

diff --git a/src/bin/cargo/commands/generate_lockfile.rs b/src/bin/cargo/commands/generate_lockfile.rs
@@ -10,7 +10,7 @@ pub fn cli() -> App {
         .after_help("Run `cargo help generate-lockfile` for more detailed information.\n")
 }
 
-pub fn exec(config: &mut Config, args: &ArgMatches<'_>) -> CliResult {
+pub fn exec(config: &mut Config, args: &ArgMatches) -> CliResult {
     let ws = args.workspace(config)?;
     ops::generate_lockfile(&ws)?;
     Ok(())

diff --git a/src/bin/cargo/commands/git_checkout.rs b/src/bin/cargo/commands/git_checkout.rs
@@ -5,10 +5,10 @@ const REMOVED: &str = "The `git-checkout` subcommand has been removed.";
 pub fn cli() -> App {
     subcommand("git-checkout")
         .about("This subcommand has been removed")
-        .settings(&[AppSettings::Hidden])
-        .help(REMOVED)
+        .setting(AppSettings::Hidden)
+        .override_help(REMOVED)
 }
 
-pub fn exec(_config: &mut Config, _args: &ArgMatches<'_>) -> CliResult {
+pub fn exec(_config: &mut Config, _args: &ArgMatches) -> CliResult {
     Err(anyhow::format_err!(REMOVED).into())
 }
diff --git a/src/bin/cargo/commands/init.rs b/src/bin/cargo/commands/init.rs
@@ -6,13 +6,13 @@ pub fn cli() -> App {
     subcommand("init")
         .about("Create a new cargo package in an existing directory")
         .arg_quiet()
-        .arg(Arg::with_name("path").default_value("."))
+        .arg(Arg::new("path").default_value("."))
         .arg(opt("registry", "Registry to use").value_name("REGISTRY"))
         .arg_new_opts()
         .after_help("Run `cargo help init` for more detailed information.\n")
 }
 
-pub fn exec(config: &mut Config, args: &ArgMatches<'_>) -> CliResult {
+pub fn exec(config: &mut Config, args: &ArgMatches) -> CliResult {
     let opts = args.new_options(config)?;
     let project_kind = ops::init(&opts, config)?;
     config