Auto merge of #10753 - epage:clap32-deprecated, r=weihanglo

chore: Upgrade to clap 3.2

I decided to use cargo as a test case for upgrading to clap 3.2 prior to release.  From the builder API's perspective, a lot has change.  While the changelog summarizes it, the release announcement is still pending.  In short, the API is now typed.  You declare what type an `Arg` is and access it by that type.  flags (both `is_present` and `occurrences_of`) are also now specified through `ArgAction` and the result gets stored like any other arg value.  I made a `ArgMatchesExt::flag` and `command_prelude::flag` functions to make working with these easier.

Now that clap exposes non-panicking variants of its functions, I switched from a "look before you leap" approach with `is_arg_valid` to a "better to ask forgiveness than permission" with checking the error variant.  I decided to just make a convenience to swallow that error type.  Not a fan of how loose things are but I think this will just be something we iterate on over time.

Author: bors

Cargo.toml

@@ -63,7 +63,7 @@ toml_edit =  { version = "0.14.3", features = ["serde", "easy", "perf"] }
 unicode-xid = "0.2.0"
 url = "2.2.2"
 walkdir = "2.2"
-clap = "3.1.0"
+clap = "3.2.1"
 unicode-width = "0.1.5"
 openssl = { version = '0.10.11', optional = true }
 im-rc = "15.0.0"

src/bin/cargo/cli.rs

@@ -60,7 +60,11 @@ pub fn main(config: &mut Config) -> CliResult {
     // (appearing before the subcommand).
     let (expanded_args, global_args) = expand_aliases(config, args, vec![])?;
 
-    if expanded_args.value_of("unstable-features") == Some("help") {
+    if expanded_args
+        .get_one::<String>("unstable-features")
+        .map(String::as_str)
+        == Some("help")
+    {
         let options = CliUnstable::help();
         let non_hidden_options: Vec<(String, String)> = options
             .iter()
@@ -112,20 +116,20 @@ Run with 'cargo -Z [FLAG] [SUBCOMMAND]'",
         return Ok(());
     }
 
-    let is_verbose = expanded_args.occurrences_of("verbose") > 0;
-    if expanded_args.is_present("version") {
+    let is_verbose = expanded_args.verbose() > 0;
+    if expanded_args.flag("version") {
         let version = get_version_string(is_verbose);
         drop_print!(config, "{}", version);
         return Ok(());
     }
 
-    if let Some(code) = expanded_args.value_of("explain") {
+    if let Some(code) = expanded_args.get_one::<String>("explain") {
         let mut procss = config.load_global_rustc(None)?.process();
         procss.arg("--explain").arg(code).exec()?;
         return Ok(());
     }
 
-    if expanded_args.is_present("list") {
+    if expanded_args.flag("list") {
         drop_println!(config, "Installed Commands:");
         for (name, command) in list_commands(config) {
             let known_external_desc = KNOWN_EXTERNAL_COMMAND_DESCRIPTIONS.get(name.as_str());
@@ -262,7 +266,7 @@ fn expand_aliases(
             }
             (Some(_), None) => {
                 // Command is built-in and is not conflicting with alias, but contains ignored values.
-                if let Some(mut values) = args.values_of("") {
+                if let Some(mut values) = args.get_many::<String>("") {
                     config.shell().warn(format!(
                         "trailing arguments after built-in command `{}` are ignored: `{}`",
                         cmd,
@@ -287,11 +291,7 @@ For more information, see issue #10049 <https://github.com/rust-lang/cargo/issue
                     ))?;
                 }
 
-                alias.extend(
-                    args.values_of("")
-                        .unwrap_or_default()
-                        .map(|s| s.to_string()),
-                );
+                alias.extend(args.get_many::<String>("").unwrap_or_default().cloned());
                 // new_args strips out everything before the subcommand, so
                 // capture those global options now.
                 // Note that an alias to an external command will not receive
@@ -327,28 +327,26 @@ fn config_configure(
     subcommand_args: &ArgMatches,
     global_args: GlobalArgs,
 ) -> CliResult {
-    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;
+    let arg_target_dir = &subcommand_args.value_of_path("target-dir", config);
+    let verbose = global_args.verbose + args.verbose();
     // 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_valid_and_present("quiet")
-        || global_args.quiet;
+    let quiet = args.flag("quiet") || subcommand_args.flag("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;
-    let locked = args.is_present("locked") || global_args.locked;
-    let offline = args.is_present("offline") || global_args.offline;
+    let color = args
+        .get_one::<String>("color")
+        .map(String::as_str)
+        .or_else(|| global_color.as_deref());
+    let frozen = args.flag("frozen") || global_args.frozen;
+    let locked = args.flag("locked") || global_args.locked;
+    let offline = args.flag("offline") || global_args.offline;
     let mut unstable_flags = global_args.unstable_flags;
-    if let Some(values) = args.values_of("unstable-features") {
-        unstable_flags.extend(values.map(|s| s.to_string()));
+    if let Some(values) = args.get_many::<String>("unstable-features") {
+        unstable_flags.extend(values.cloned());
     }
     let mut config_args = global_args.config_args;
-    if let Some(values) = args.values_of("config") {
-        config_args.extend(values.map(|s| s.to_string()));
+    if let Some(values) = args.get_many::<String>("config") {
+        config_args.extend(values.cloned());
     }
     config.configure(
         verbose,
@@ -370,7 +368,12 @@ fn execute_subcommand(config: &mut Config, cmd: &str, subcommand_args: &ArgMatch
     }
 
     let mut ext_args: Vec<&str> = vec![cmd];
-    ext_args.extend(subcommand_args.values_of("").unwrap_or_default());
+    ext_args.extend(
+        subcommand_args
+            .get_many::<String>("")
+            .unwrap_or_default()
+            .map(String::as_str),
+    );
     super::execute_external_subcommand(config, cmd, &ext_args)
 }
 
@@ -389,19 +392,21 @@ struct GlobalArgs {
 impl GlobalArgs {
     fn new(args: &ArgMatches) -> GlobalArgs {
         GlobalArgs {
-            verbose: args.occurrences_of("verbose") as u32,
-            quiet: args.is_present("quiet"),
-            color: args.value_of("color").map(|s| s.to_string()),
-            frozen: args.is_present("frozen"),
-            locked: args.is_present("locked"),
-            offline: args.is_present("offline"),
+            verbose: args.verbose(),
+            quiet: args.flag("quiet"),
+            color: args.get_one::<String>("color").cloned(),
+            frozen: args.flag("frozen"),
+            locked: args.flag("locked"),
+            offline: args.flag("offline"),
             unstable_flags: args
-                .values_of_lossy("unstable-features")
-                .unwrap_or_default(),
+                .get_many::<String>("unstable-features")
+                .unwrap_or_default()
+                .cloned()
+                .collect(),
             config_args: args
-                .values_of("config")
+                .get_many::<String>("config")
                 .unwrap_or_default()
-                .map(|s| s.to_string())
+                .cloned()
                 .collect(),
         }
     }
@@ -416,7 +421,7 @@ fn cli() -> App {
     };
     App::new("cargo")
         .allow_external_subcommands(true)
-        .setting(AppSettings::DeriveDisplayOrder | AppSettings::NoAutoVersion)
+        .setting(AppSettings::DeriveDisplayOrder)
         // 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
         .disable_colored_help(true)
@@ -450,16 +455,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("list", "List installed commands"))
+        .arg(flag("version", "Print version info and exit").short('V'))
+        .arg(flag("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_occurrences(true)
+            .action(ArgAction::Count)
             .global(true),
         )
         .arg_quiet()
@@ -468,9 +473,9 @@ See 'cargo help <command>' for more information on a specific command.\n",
                 .value_name("WHEN")
                 .global(true),
         )
-        .arg(opt("frozen", "Require Cargo.lock and cache are up to date").global(true))
-        .arg(opt("locked", "Require Cargo.lock is up to date").global(true))
-        .arg(opt("offline", "Run without accessing the network").global(true))
+        .arg(flag("frozen", "Require Cargo.lock and cache are up to date").global(true))
+        .arg(flag("locked", "Require Cargo.lock is up to date").global(true))
+        .arg(flag("offline", "Run without accessing the network").global(true))
         .arg(
             multi_opt(
                 "config",
@@ -484,7 +489,7 @@ See 'cargo help <command>' for more information on a specific command.\n",
                 .help("Unstable (nightly-only) flags to Cargo, see 'cargo -Z help' for details")
                 .short('Z')
                 .value_name("FLAG")
-                .multiple_occurrences(true)
+                .action(ArgAction::Append)
                 .global(true),
         )
         .subcommands(commands::builtin())

src/bin/cargo/commands/add.rs

@@ -27,7 +27,7 @@ pub fn cli() -> clap::Command<'static> {
             clap::Arg::new("crates")
                 .takes_value(true)
                 .value_name("DEP_ID")
-                .multiple_occurrences(true)
+                .multiple_values(true)
                 .help("Reference to a package to add as a dependency")
                 .long_help(
                 "Reference to a package to add as a dependency
@@ -37,30 +37,26 @@ You can reference a package by:
 - `<name>@<version-req>`, like `cargo add serde@1` or `cargo add serde@=1.0.38`"
             )
                 .group("selected"),
-            clap::Arg::new("no-default-features")
-                .long("no-default-features")
-                .help("Disable the default features"),
-            clap::Arg::new("default-features")
-                .long("default-features")
-                .help("Re-enable the default features")
+            flag("no-default-features",
+                "Disable the default features"),
+            flag("default-features",
+                "Re-enable the default features")
                 .overrides_with("no-default-features"),
             clap::Arg::new("features")
                 .short('F')
                 .long("features")
                 .takes_value(true)
                 .value_name("FEATURES")
-                .multiple_occurrences(true)
+                .action(ArgAction::Append)
                 .help("Space or comma separated list of features to activate"),
-            clap::Arg::new("optional")
-                .long("optional")
-                .help("Mark the dependency as optional")
+            flag("optional",
+                "Mark the dependency as optional")
                 .long_help("Mark the dependency as optional
 
 The package name will be exposed as feature of your crate.")
                 .conflicts_with("dev"),
-            clap::Arg::new("no-optional")
-                .long("no-optional")
-                .help("Mark the dependency as required")
+            flag("no-optional",
+                "Mark the dependency as required")
                 .long_help("Mark the dependency as required
 
 The package will be removed from your features.")
@@ -141,18 +137,16 @@ This is the catch all, handling hashes to named references in remote repositorie
         ])
         .next_help_heading("SECTION")
         .args([
-            clap::Arg::new("dev")
-                .long("dev")
-                .help("Add as development dependency")
+            flag("dev",
+                "Add as development dependency")
                 .long_help("Add as development dependency
 
 Dev-dependencies are not used when compiling a package for building, but are used for compiling tests, examples, and benchmarks.
 
 These dependencies are not propagated to other packages which depend on this package.")
                 .group("section"),
-            clap::Arg::new("build")
-                .long("build")
-                .help("Add as build dependency")
+            flag("build",
+                "Add as build dependency")
                 .long_help("Add as build dependency
 
 Build-dependencies are the only dependencies available for use by build scripts (`build.rs` files).")
@@ -161,13 +155,13 @@ Build-dependencies are the only dependencies available for use by build scripts
                 .long("target")
                 .takes_value(true)
                 .value_name("TARGET")
-                .forbid_empty_values(true)
+                .value_parser(clap::builder::NonEmptyStringValueParser::new())
                 .help("Add as dependency to the given target platform")
         ])
 }
 
 pub fn exec(config: &mut Config, args: &ArgMatches) -> CliResult {
-    let dry_run = args.is_present("dry-run");
+    let dry_run = args.dry_run();
     let section = parse_section(args);
 
     let ws = args.workspace(config)?;
@@ -206,21 +200,21 @@ pub fn exec(config: &mut Config, args: &ArgMatches) -> CliResult {
 }
 
 fn parse_dependencies(config: &Config, matches: &ArgMatches) -> CargoResult<Vec<DepOp>> {
-    let path = matches.value_of("path");
-    let git = matches.value_of("git");
-    let branch = matches.value_of("branch");
-    let rev = matches.value_of("rev");
-    let tag = matches.value_of("tag");
-    let rename = matches.value_of("rename");
+    let path = matches.get_one::<String>("path");
+    let git = matches.get_one::<String>("git");
+    let branch = matches.get_one::<String>("branch");
+    let rev = matches.get_one::<String>("rev");
+    let tag = matches.get_one::<String>("tag");
+    let rename = matches.get_one::<String>("rename");
     let registry = matches.registry(config)?;
     let default_features = default_features(matches);
     let optional = optional(matches);
 
     let mut crates = matches
-        .values_of("crates")
+        .get_many::<String>("crates")
         .into_iter()
         .flatten()
-        .map(|c| (Some(String::from(c)), None))
+        .map(|c| (Some(c.clone()), None))
         .collect::<IndexMap<_, _>>();
     let mut infer_crate_name = false;
     if crates.is_empty() {
@@ -232,9 +226,10 @@ fn parse_dependencies(config: &Config, matches: &ArgMatches) -> CargoResult<Vec<
         }
     }
     for feature in matches
-        .values_of("features")
+        .get_many::<String>("features")
         .into_iter()
         .flatten()
+        .map(String::as_str)
         .flat_map(parse_feature)
     {
         let parsed_value = FeatureValue::new(InternedString::new(feature));
@@ -310,16 +305,13 @@ fn parse_dependencies(config: &Config, matches: &ArgMatches) -> CargoResult<Vec<
 
 fn default_features(matches: &ArgMatches) -> Option<bool> {
     resolve_bool_arg(
-        matches.is_present("default-features"),
-        matches.is_present("no-default-features"),
+        matches.flag("default-features"),
+        matches.flag("no-default-features"),
     )
 }
 
 fn optional(matches: &ArgMatches) -> Option<bool> {
-    resolve_bool_arg(
-        matches.is_present("optional"),
-        matches.is_present("no-optional"),
-    )
+    resolve_bool_arg(matches.flag("optional"), matches.flag("no-optional"))
 }
 
 fn resolve_bool_arg(yes: bool, no: bool) -> Option<bool> {
@@ -332,17 +324,17 @@ fn resolve_bool_arg(yes: bool, no: bool) -> Option<bool> {
 }
 
 fn parse_section(matches: &ArgMatches) -> DepTable {
-    let kind = if matches.is_present("dev") {
+    let kind = if matches.flag("dev") {
         DepKind::Development
-    } else if matches.is_present("build") {
+    } else if matches.flag("build") {
         DepKind::Build
     } else {
         DepKind::Normal
     };
 
     let mut table = DepTable::new().set_kind(kind);
 
-    if let Some(target) = matches.value_of("target") {
+    if let Some(target) = matches.get_one::<String>("target") {
         assert!(!target.is_empty(), "Target specification may not be empty");
         table = table.set_target(target);
     }