Upgrade to Clap 3

- 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`.
- The parser now panics when accessing an undefined attribute 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.

BREAKING CHANGE: API now uses clap3

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"

src/bin/cargo/cli.rs

@@ -35,8 +35,8 @@ pub fn main(config: &mut Config) -> CliResult {
         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)?,
@@ -292,7 +292,7 @@ For more information, see issue #10049 <https://github.com/rust-lang/cargo/issue
                     .setting(AppSettings::NoBinaryName)
                     .get_matches_from_safe(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"),
@@ -411,9 +411,12 @@ fn cli() -> App {
         .settings(&[
             AppSettings::UnifiedHelpMessage,
             AppSettings::DeriveDisplayOrder,
-            AppSettings::VersionlessSubcommands,
             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)
         .usage(usage)
         .template(
             "\
@@ -423,7 +426,7 @@ USAGE:
     {usage}
 
 OPTIONS:
-{unified}
+{options}
 
 Some common cargo commands are (see all commands with --list):
     build, b    Compile the current package
@@ -443,16 +446,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()
@@ -475,11 +478,16 @@ See 'cargo help <command>' for more information on a specific command.\n",
         .arg(
             Arg::with_name("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)
                 .global(true),
         )
         .subcommands(commands::builtin())
 }
+
+#[test]
+fn verify_cli() {
+    cli().debug_assert();
+}

src/bin/cargo/commands/bench.rs

@@ -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,

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,

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"));

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") {

src/bin/cargo/commands/config.rs

@@ -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(())

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"),

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 {

src/bin/cargo/commands/fix.rs

@@ -65,7 +65,7 @@ pub fn cli() -> App {
         .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"));

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(())

src/bin/cargo/commands/git_checkout.rs

@@ -9,6 +9,6 @@ pub fn cli() -> App {
         .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())
 }

src/bin/cargo/commands/init.rs

@@ -12,7 +12,7 @@ pub fn cli() -> App {
         .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

src/bin/cargo/commands/install.rs

@@ -45,7 +45,7 @@ pub fn cli() -> App {
             "list all installed packages and their versions",
         ))
         .arg_jobs()
-        .arg(opt("force", "Force overwriting existing crates or binaries").short("f"))
+        .arg(opt("force", "Force overwriting existing crates or binaries").short('f'))
         .arg(opt("no-track", "Do not save tracking information"))
         .arg_features()
         .arg_profile("Install artifacts with the specified profile")
@@ -75,7 +75,7 @@ pub fn cli() -> App {
         .after_help("Run `cargo help install` for more detailed information.\n")
 }
 
-pub fn exec(config: &mut Config, args: &ArgMatches<'_>) -> CliResult {
+pub fn exec(config: &mut Config, args: &ArgMatches) -> CliResult {
     if let Some(path) = args.value_of_path("path", config) {
         config.reload_rooted_at(path)?;
     } else {

src/bin/cargo/commands/locate_project.rs

@@ -24,7 +24,7 @@ pub struct ProjectLocation<'a> {
     root: &'a str,
 }
 
-pub fn exec(config: &mut Config, args: &ArgMatches<'_>) -> CliResult {
+pub fn exec(config: &mut Config, args: &ArgMatches) -> CliResult {
     let root_manifest;
     let workspace;
     let root = match WhatToFind::parse(args) {
@@ -64,7 +64,7 @@ enum WhatToFind {
 }
 
 impl WhatToFind {
-    fn parse(args: &ArgMatches<'_>) -> Self {
+    fn parse(args: &ArgMatches) -> Self {
         if args.is_present("workspace") {
             WhatToFind::Workspace
         } else {
@@ -79,7 +79,7 @@ enum MessageFormat {
 }
 
 impl MessageFormat {
-    fn parse(args: &ArgMatches<'_>) -> CargoResult<Self> {
+    fn parse(args: &ArgMatches) -> CargoResult<Self> {
         let fmt = match args.value_of("message-format") {
             Some(fmt) => fmt,
             None => return Ok(MessageFormat::Json),

src/bin/cargo/commands/login.rs

@@ -14,7 +14,7 @@ pub fn cli() -> App {
         .after_help("Run `cargo help login` for more detailed information.\n")
 }
 
-pub fn exec(config: &mut Config, args: &ArgMatches<'_>) -> CliResult {
+pub fn exec(config: &mut Config, args: &ArgMatches) -> CliResult {
     ops::registry_login(
         config,
         args.value_of("token").map(String::from),

src/bin/cargo/commands/logout.rs

@@ -9,7 +9,7 @@ pub fn cli() -> App {
         .after_help("Run `cargo help logout` for more detailed information.\n")
 }
 
-pub fn exec(config: &mut Config, args: &ArgMatches<'_>) -> CliResult {
+pub fn exec(config: &mut Config, args: &ArgMatches) -> CliResult {
     if !config.cli_unstable().credential_process {
         config
             .cli_unstable()

src/bin/cargo/commands/metadata.rs

@@ -29,7 +29,7 @@ pub fn cli() -> App {
         .after_help("Run `cargo help metadata` 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 version = match args.value_of("format-version") {

src/bin/cargo/commands/mod.rs

@@ -40,7 +40,7 @@ pub fn builtin() -> Vec<App> {
     ]
 }
 
-pub fn builtin_exec(cmd: &str) -> Option<fn(&mut Config, &ArgMatches<'_>) -> CliResult> {
+pub fn builtin_exec(cmd: &str) -> Option<fn(&mut Config, &ArgMatches) -> CliResult> {
     let f = match cmd {
         "bench" => bench::exec,
         "build" => build::exec,

src/bin/cargo/commands/new.rs

@@ -12,7 +12,7 @@ pub fn cli() -> App {
         .after_help("Run `cargo help new` 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)?;
 
     ops::new(&opts, config)?;

src/bin/cargo/commands/owner.rs

@@ -13,24 +13,24 @@ pub fn cli() -> App {
                 "LOGIN",
                 "Name of a user or team to invite as an owner",
             )
-            .short("a"),
+            .short('a'),
         )
         .arg(
             multi_opt(
                 "remove",
                 "LOGIN",
                 "Name of a user or team to remove as an owner",
             )
-            .short("r"),
+            .short('r'),
         )
-        .arg(opt("list", "List owners of a crate").short("l"))
+        .arg(opt("list", "List owners of a crate").short('l'))
         .arg(opt("index", "Registry index to modify owners for").value_name("INDEX"))
         .arg(opt("token", "API token to use when authenticating").value_name("TOKEN"))
         .arg(opt("registry", "Registry to use").value_name("REGISTRY"))
         .after_help("Run `cargo help owner` for more detailed information.\n")
 }
 
-pub fn exec(config: &mut Config, args: &ArgMatches<'_>) -> CliResult {
+pub fn exec(config: &mut Config, args: &ArgMatches) -> CliResult {
     config.load_credentials()?;
 
     let registry = args.registry(config)?;