Hard to know how to respond to 3.2.0 deprecation messages in the derive API

[Alexhuszagh]

Maintainer's note:

• As of 3.2.3, deprecations are behind the deprecated feature which is off by default for now (see fix: Allow people to opt-in to deprecations #3830)
• Until we improve the messages (if someone steps up to do so), See the release announcement for more details on the deprecations and cargo-deny for an example of how to migrate to clap 3.2 (with a tl;dr).
  • One simple way of reducing the messages is opting into unstable-v4 feature in your Cargo.toml. See the changelog for what else this affects.
• Explanation of assumptions and circumstances that got us here

---

Please complete the following tasks

• I have searched the discussions
• I have searched the open and rejected issues

Rust Version

rustc 1.61.0 (fe5b13d68 2022-05-18)

Clap Version

3.2.1 (or 3.2.0)

Minimal reproducible code

main.rs

use clap::{Parser, Subcommand};

#[derive(Parser, Debug)]
#[clap(version, about, long_about = None)]
struct Cli {
    #[clap(subcommand)]
    command: Commands,
}

#[derive(Subcommand, Debug)]
enum Commands {
    /// Simple Command.
    GetArgs {
        /// Provide verbose diagnostic output.
        #[clap(short, long)]
        verbose: bool,
        /// Sample optional string.
        #[clap(long)]
        value: Option<String>,
        /// Just a list of valies
        list: Vec<String>
    },
}

fn main() {
    let cli = Cli::parse();
    match &cli.command {
        Commands::GetArgs { verbose, value, list } => {
            println!("verbose={:?}", verbose);
            println!("value={:?}", value);
            println!("list={:?}", list);
        }
    }
}

Cargo.toml

[package]
name = "test"
version = "0.1.0"
edition = "2021"

[dependencies]
clap = { version = "3.2.1", features = ["derive"] }

Steps to reproduce the bug with the above code

Use clap >= 3.2.0, this does not produce any warnings in 3.1.18.

cargo build

Actual Behaviour

warning: use of deprecated unit variant `clap::ArgAction::StoreValue`: Replaced with `ArgAction::Set` or `ArgAction::Append`
  --> src/main.rs:17:9
   |
17 | /         /// Sample optional string.
18 | |         #[clap(long)]
19 | |         value: Option<String>,
   | |_____________________________^
   |
   = note: `#[warn(deprecated)]` on by default

warning: use of deprecated unit variant `clap::ArgAction::StoreValue`: Replaced with `ArgAction::Set` or `ArgAction::Append`
  --> src/main.rs:20:9
   |
20 | /         /// Just a list of valies
21 | |         list: Vec<String>
   | |_________________________^

warning: use of deprecated associated function `clap::ArgMatches::is_present`: Replaced with either `ArgAction::SetTrue` or `ArgMatches::contains_id(...)`
  --> src/main.rs:16:18
   |
16 |         verbose: bool,
   |                  ^^^^

warning: use of deprecated associated function `clap::Arg::<'help>::validator`: Replaced with `Arg::value_parser(...)`
  --> src/main.rs:17:9
   |
17 | /         /// Sample optional string.
18 | |         #[clap(long)]
19 | |         value: Option<String>,
   | |_____________________________^

warning: use of deprecated associated function `clap::Arg::<'help>::multiple_occurrences`: Replaced with `Arg::action` (Issue #3772)
  --> src/main.rs:21:15
   |
21 |         list: Vec<String>
   |               ^^^^^^^^^^^

warning: use of deprecated associated function `clap::Arg::<'help>::validator`: Replaced with `Arg::value_parser(...)`
  --> src/main.rs:20:9
   |
20 | /         /// Just a list of valies
21 | |         list: Vec<String>
   | |_________________________^

warning: `clap` (bin "clap") generated 6 warnings
    Finished dev [unoptimized + debuginfo] target(s) in 0.04s
     Running `target/debug/clap get-args`

Expected Behaviour

None of these warnings should occur, I believe.

Additional Context

No response

Debug Output

The debug output is the same as the above output.

[epage]

These deprecation warnings are intentional though, due to the derive API wrapping the builder API, the fix might not be as obvious. We do expect to be adding deprecations every minor release and we recommend not blocking CI on this as the toil adds backpressure on crates to not evolve their API. See the 3.1 announcement for details on this.

See the release announcement for more details on the deprecations and cargo-deny for an example of how to migrate to clap 3.2 (with a tl;dr).

[Emilgardis]

So how does one fix the deprecation warnings with the above sample code?

nvm, follow the tldr in cargo-deny

[CraZySacX]

Shouldn't non-deprecated code be "selected" per the docs: When not present: will auto-select an implementation based on the field type

Based off of this I assumed value-parser is always used, but it seems to use deprecated code when I don't have it explicitly specified.

[Alexhuszagh]

So how does one fix the deprecation warnings with the above sample code?

Change everything to say, #[clap(value_parser), which clutters up everything. See this.

[epage]

Shouldn't non-deprecated code be "selected" per the docs: When not present: will auto-select an implementation based on the field type

The problem is the new behavior is a breaking change, so you need to opt-in to it. In clap 4.0, we'll make it the default.

Change everything to say, #[clap(value_parser), which clutters up everything.

Yes, it clutters things up but it also let's you start testing with the clap 4.0 behavior and provide feedback. If you would rather wait, you also always allow these deprecations.

[Alexhuszagh]

Shouldn't non-deprecated code be "selected" per the docs: When not present: will auto-select an implementation based on the field type

The problem is the new behavior is a breaking change, so you need to opt-in to it. In clap 4.0, we'll make it the default.

Maybe change the deprecation warnings then in the derive API? Use #[allow(deprecated)] and then change the deprecation warning so you mention the default behavior is changing, and mention that the default behavior is changing in 4.0, you probably want to add value_parser? These deprecation notices are relatively opaque since they reference a different API than the one currently used, and have no mention of how they arise.

I'm glad to at least see the default behavior will change.

[epage]

I'll go ahead and re-open this for tracking improving of the deprecations within the derive API. I'm assuming the easiest way is to wrap the existing calls in a function that allows deprecated in its body but adds its own deprecation message specific to the builder. The challenges will be to not allow too many deprecation messages and not overly contorting the code base for this short-lived, one off need.

I'm unsure if I'll get to this but if someone wants to tackle it, I'll help move it along to be merged.

[aswild]

I skimmed through the docs but couldn't quite figure this out - what's the difference between value_parser and action when used with defaults in the derive API?

As an example,

#[derive(Debug, Parser)]
struct Args {
    #[clap(short, long)]
    flag: bool,

    #[clap(short, long)]
    optional: Option<PathBuf>,

    required: String,
}

This is what I'm used to writing in 3.0 and 3.1, but gives deprecation warnings in 3.2. Adding either action or value_parser attributes to each of the fields stops the warnings and seems to produce the same behavior so it's unclear which one should be used (or is more idiomatic).

On the plus side, the PathBuf argument works with invalid UTF-8 in 3.2 without needing parse(from_os_str) so that's pretty slick.

[epage]

what's the difference between value_parser and action when used with defaults in the derive API?

From EmbarkStudios/cargo-deny#431

By specifying a default #[clap(action)] or #[clap(value_parser)] attribute, you opt-in to the new behavior that will be the default in clap 4.0.

Either attribute can be set to get the behavior slated for 4.0. You also can explicitly set your action, parser, or both.

Example of setting an action

use clap::Parser;

#[derive(Parser)]
#[clap(author, version, about, long_about = None)]
struct Cli {
    #[clap(short, long, action = clap::ArgAction::Count)]
    verbose: u8,
}

fn main() {
    let cli = Cli::parse();

    println!("verbose: {:?}", cli.verbose);
}

Example of setting the value_parser

use clap::Parser;

#[derive(Parser)]
#[clap(author, version, about, long_about = None)]
struct Cli {
    /// Network port to use
    #[clap(value_parser = clap::value_parser!(u16).range(1..))]
    port: u16,
}

fn main() {
    let cli = Cli::parse();

    println!("PORT = {}", cli.port);
}

[jplatte]

I really dislike having to add a clap attribute for what seems very close to a no-op in behavior of the resulting binary? Will it behave differently at all for bool or String arguments with no clap attributes? Anyways, I guess I'll silence the deprecated lint for now, luckily there's only a handful of structs the #[allow] needs to go on, because I wouldn't want to use #![allow(deprecated)] in a whole module / crate.

edit: Oh no, #[allow(deprecated)] on a struct that uses a proc-macro derive doesn't seem to apply to the code generated by that derive. I guess I'll update the clap dependency to ~3.1 then 😕

[epage]

I just released v3.2.4 which improves the deprecation messages.. You can preview some of the messages in #3832.

I didn't immediately close this issue as fixed by that PR as I wanted to get a feel for people if its sufficient or if people have feedback on further improvements.

[autumnontape]

@de-vri-es This is why I suggested a struct-level attribute. Adding and later removing one line per #[derive(Parser)] struct seems more reasonable to me than modifying many lines per struct and then changing them back later. I'm also not super comfortable with using a non-additive unstable feature for this. I do want to test the new behavior and have updated my projects to use #[clap(action)], though.

[epage]

@autumnontape if you want to resolve all Builder deprecation messages and not just the ones clap_derive exposes in 3.2.4 with the deprecated feature, you can enable the clap_derive/raw-deprecated feature.

I am unlikely to add a struct-level opt-in at this point

• I'm assuming most people won't care about clap_derive/raw-deprecated to take advantage of it
• A #[clap(action)] or #[clap(value_parser)] doesn't quite make sense at the struct level. At the field level, we already have the pattern of bald attributes providing a default behavior. A different name would need to be discussed
• We'd then have to plumb this new attribute in

In balance, the costs don't seem worth the benefit at this time. If there is enough demand, we can re-evaluate.

[Licenser]

👏 for the great handling of this! (got to be said :) )

[steveklabnik]

I didn't immediately close this issue as fixed by that PR as I wanted to get a feel for people if its sufficient or if people have feedback on further improvements.

I think this works pretty well! a simple clap = { version = "3.2.3", features = ["deprecated"] } and I'm off to the races. Errors like this:

warning: use of deprecated unit variant `clap::ArgAction::StoreValue`: Replaced with `ArgAction::Set` or `ArgAction::Append`
  --> cmd\rendmp\src\lib.rs:36:5
   |
36 |     bus: Option<String>,
   |     ^^^

This message is way better. It would be even nicer if there was a way to actually explain what's going on here; it's confusing at first because this sort of option has no clap annotations on it, so it's weird to get a message about a thing I'm not even using (directly, of course, but if I was new I wouldn't have known this). I don't think this is something you can improve, though :)

The only other things I'm unsure about is not related to those messages; I opened a discussion so as not to pollute this ticket with that stuff.

Thanks again, I have to second @Licenser, this is a great handling of this issue :)

[epage]

This message is way better. It would be even nicer if there was a way to actually explain what's going on here; it's confusing at first because this sort of option has no clap annotations on it, so it's weird to get a message about a thing I'm not even using (directly, of course, but if I was new I wouldn't have known this). I don't think this is something you can improve, though :)

It should be explaining what is going on and this is a bug. Could you post a reproduction case? The messages should either be about attributes you set or be in described in derive terms. Somehow, this one slipped through.

[steveklabnik]

You know what, it might be my error. I will do so, though I can't right this moment.

[steveklabnik]

Okay so, I'm kinda stumped at the moment. When i try and create a standalone reproduction, it doesn't warn. The original project is in a workspace, so I thought maybe I had two different versions of clap, but verified that that wasn't an issue either. Will work on reducing tomorrow.

[steveklabnik]

Ah! For some reason, I was still using 3.2.3, how embarrassing. Everything is great now that I'm on 3.2.5. And also, thanks for improving that one other message too:

warning: use of deprecated function `<QspiArgs as clap::Args>::augment_args::parse_try_from_str`: Replaced with `#[clap(value_parser = ...)]`
   --> cmd\qspi\src\lib.rs:110:30
    |
110 |         parse(try_from_str = parse_int::parse)
    |                              ^^^^^^^^^
    |
    = note: `#[warn(deprecated)]` on by default

mentioning the attribute syntax is great here!

[epage]

I'm going to consider this resolved at this point.

Feel free to open new issues for specific points of feedback on this process.

[tsal]

Sure would be nice if examples using is_present got removed from the clap_complete docs.

[epage]

Could you point to the example you are referring to?