search

clap-rs / clap

A full featured, fast Command Line Argument Parser for Rust
docs.rs/clap
Apache License 2.0
13.64k stars 1.02k forks source link

`num_args = 2..` does not error with 0 args and error has incorrect wording with 1 arg #5526

Closed qtfkwk closed 3 weeks ago

qtfkwk commented 3 weeks ago

Please complete the following tasks

Rust Version

rustc 1.78.0 (9b00956e5 2024-04-29)

Clap Version

4.5.6

Minimal reproducible code

use clap::Parser;

#[derive(Parser)]
struct Cli {
    /// Args
    #[arg(value_name = "ARG", num_args = 2..)]
    args: Vec<String>,
}

fn main() {
    let cli = Cli::parse();
    println!("{:#?}", cli.args);
}

Steps to reproduce the bug with the above code

cargo new clap-num-args-issue
cd clap-num-args-issue
cargo add clap -F derive
cat <<EOF >src/main.rs
use clap::Parser;

#[derive(Parser)]
struct Cli {
    /// Args
    #[arg(value_name = "ARG", num_args = 2..)]
    args: Vec<String>,
}

fn main() {
    let cli = Cli::parse();
    println!("{:#?}", cli.args);
}
EOF
cargo run # false negative: prints "[]\n" instead of an error because len 0 < minimum of 2
cargo run -- asdf # error is not grammatically correct; see below
cargo run -- asdf blah # good: prints "[\n    \"asdf\",\n    \"blah\",\n]\n"
cargo run -- asdf blah spam # good: prints "[\n    \"asdf\",\n    \"blah\",\n    \"spam\",\n]\n"

Error when running cargo run -- asdf:

error: 2 more values required by '[ARG] [ARG]...'; only 1 was provided

Shouldn't it say either of the following?

  1. error: 1 more value required by '[ARG] [ARG]...'; only 1 was provided
  2. error: 2 values required by '[ARG] [ARG]...'; only 1 was provided (my personal preference is for this one)

Actual Behaviour

There are 2 issues:

  1. With num_args set to 2.. and user gives 0, it does not produce an error as it should because the minimum number of args is 2
  2. If the user gives 1 arg, it errors, but the message says it needs "2 more args" which is wrong; it needs a minimum of 2 args and 1 was given, so it needs 1 more arg not 2

Expected Behaviour

  1. With num_args set to 2.. and user gives 0, it should produce an error saying that it requires 2 args and 0 were given
  2. If the user gives 1 arg, it should say it requires 2 args and 1 was given
    • (or alternatively it could say it needs 1 more arg and 1 was given; but I feel this is less precise and requires calculating the diff)

Additional Context

  1. Unsure root cause on this one... a quick search did not find it... but I imagine that somewhere it tests the len against the num_args range (?)

  2. https://github.com/clap-rs/clap/blob/2f645d3e81c783a4e76ad17f1ccf283a58b75660/clap_builder/src/error/format.rs#L340:

    should be:

    "{}{min_values}{} values required by '{}{invalid_arg}{}'; only {}{actual_num_values}{}{were_provided}",

    remove the word "more"

Debug Output

$ cargo run -- asdf
    Finished `dev` profile [unoptimized + debuginfo] target(s) in 0.02s
     Running `target/debug/clap-num-args-issue asdf`
[clap_builder::builder::command]Command::_do_parse
[clap_builder::builder::command]Command::_build: name="clap-num-args-issue"
[clap_builder::builder::command]Command::_propagate:clap-num-args-issue
[clap_builder::builder::command]Command::_check_help_and_version:clap-num-args-issue expand_help_tree=false
[clap_builder::builder::command]Command::long_help_exists
[clap_builder::builder::command]Command::_check_help_and_version: Building default --help
[clap_builder::builder::command]Command::_propagate_global_args:clap-num-args-issue
[clap_builder::builder::debug_asserts]Command::_debug_asserts
[clap_builder::builder::debug_asserts]Arg::_debug_asserts:args
[clap_builder::builder::debug_asserts]Arg::_debug_asserts:help
[clap_builder::builder::debug_asserts]Command::_verify_positionals
[clap_builder::parser::parser]Parser::get_matches_with
[clap_builder::parser::parser]Parser::get_matches_with: Begin parsing '"asdf"'
[clap_builder::parser::parser]Parser::possible_subcommand: arg=Ok("asdf")
[clap_builder::parser::parser]Parser::get_matches_with: sc=None
[clap_builder::parser::parser]Parser::get_matches_with: Positional counter...1
[clap_builder::parser::parser]Parser::get_matches_with: Low index multiples...false
[clap_builder::parser::parser]Parser::resolve_pending: id="args"
[clap_builder::parser::parser]Parser::react action=Append, identifier=Some(Index), source=CommandLine
[ clap_builder::output::usage]Usage::create_usage_with_title
[ clap_builder::output::usage]Usage::create_usage_no_title
[ clap_builder::output::usage]Usage::write_help_usage
[ clap_builder::output::usage]Usage::write_arg_usage; incl_reqs=true
[ clap_builder::output::usage]Usage::needs_options_tag
[ clap_builder::output::usage]Usage::needs_options_tag:iter: f=help
[ clap_builder::output::usage]Usage::needs_options_tag:iter Option is built-in
[ clap_builder::output::usage]Usage::needs_options_tag: [OPTIONS] not required
[ clap_builder::output::usage]Usage::write_args: incls=[]
[ clap_builder::output::usage]Usage::get_args: unrolled_reqs=[]
[ clap_builder::output::usage]Usage::write_subcommand_usage
[ clap_builder::output::usage]Usage::create_usage_with_title: usage=Usage: clap-num-args-issue [ARG] [ARG]...
[clap_builder::builder::command]Command::color: Color setting...
[clap_builder::builder::command]Auto
[clap_builder::builder::command]Command::color: Color setting...
[clap_builder::builder::command]Auto
error: 2 more values required by '[ARG] [ARG]...'; only 1 was provided

Usage: clap-num-args-issue [ARG] [ARG]...

For more information, try '--help'.
epage commented 3 weeks ago

There are 2 issues:

In the future, please open an issue per concern.

With num_args set to 2.. and user gives 0, it does not produce an error as it should because the minimum number of args is 2

The behavior is correct. From the docs:

Specifies the number of arguments parsed per occurrence

This isn't as obvious for positionals but for options, the behavior is more clear.

use clap::Parser;

#[derive(Parser)]
struct Cli {
    /// Args
    #[arg(long, value_name = "ARG", num_args = 2..)]
    args: Vec<String>,
}

fn main() {
    let cli = Cli::parse();
    println!("{:#?}", cli.args);
}
$ # good: 2+ values
$ cmd --args asdf blah
$ # good: 2+ values
$ cmd --args asdf blah spam
$ # good: 2+ values per `--arg`
$ cmd --args asdf blah --arg spam pizza
$ # bad: too few values
$ cmd --args asdf
$ # bad: too few values for last `--arg`
$ cmd --args asdf blah --arg spam
$ # good:  no occurrences
$ cmd

What you are looking for is required = true. You also likely want to set action = ArgAction::Set because a Vec is automatically an Append, see https://docs.rs/clap/latest/clap/_derive/index.html#arg-types

use clap::Parser;
use clap::builder::ArgAction;

#[derive(Parser)]
struct Cli {
    /// Args
    #[arg(long, value_name = "ARG", num_args = 2.., required = true, action = ArgAction::Set)]
    args: Vec<String>,
}

fn main() {
    let cli = Cli::parse();
    println!("{:#?}", cli.args);
}
qtfkwk commented 2 weeks ago

Thank you for the detailed response, @epage! Will try your suggested fix.