chore: Upgrade to clap 3.2

[epage]

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.

[rust-highfive]

r? @ehuss

(rust-highfive has picked a reviewer for you, use r? to override)

[Muscraft]

It looks like this might still be needed for the CI to pass.

#10755 and #10754 both fail on i686-unknown-linux-gnu on the below test.

 ---- cargo_add::invalid_target_empty::invalid_target_empty stdout ----
thread 'cargo_add::invalid_target_empty::invalid_target_empty' panicked at '
---- expected: tests/testsuite/cargo_add/invalid_target_empty/stderr.log
++++ actual:   stderr
   1    1 | error: The argument '--target <TARGET>' requires a value but none was supplied
   2    2 | 
   3      - USAGE:
   4      -     cargo add [OPTIONS] <DEP>[@<VERSION>] ...
   5      -     cargo add [OPTIONS] --path <PATH> ...
   6      -     cargo add [OPTIONS] --git <URL> ...
   7      - 
   8    3 | For more information try --help

[weihanglo]

Looks good!

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.

Sorry, I don't get it. How does this help?

[epage]

Looks good!

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.

Sorry, I don't get it. How does this help?

Instead of is_valid_arg calls being made before accessing an arg, a lot of lower level calls do a strip out the MatchesError::UnknownArgument case.

We need to do something because any given cargo command may look up values for an arg that isn't defined because of how the code sharing works.

Having a lot of is_valid_arg checks at the call sites was annoying to manage though it is nicer for catching bugs though they are likely to be rare and caught through tests.

[weihanglo]

Make sense. Thanks for the explanation. Going to merge this.

@bors r+

[bors]

📌 Commit fc0ca1e has been approved by weihanglo

[bors]

⌛ Testing commit fc0ca1e with merge 17f8088...

[bors]

☀️ Test successful - checks-actions
Approved by: weihanglo
Pushing 17f8088 to master...