Parsing gives incorrect/unexpected/inconsistent error results

[smaillet]

Version: 2.0.0-beta7.25380.108 (Latest NuGet as of this writing)

[TestMethod]
public void RawApi_Version_Error_tests( )
{
    var rootCommand = new RootCommand("Test Root")
    {
        new Option<string>("--option1")
        {
            Description = "Test option `",
            Required = true,
        },
    };

    var result = rootCommand.Parse(["--FooBar", "--version"]);
    Assert.HasCount( 3, result.Errors, "Errors should account for, bogus arg (`--FooBar`), missing required arg (`--option1`), AND that `--version` should be solo" );
}

This test won't pass, the error count is only 2, the error regarding the --version is completely missing. Errors are: [0] = "Unrecognized command or argument '--FooBar'." [1] = "Option '--option1' is required."

For help it's even worse...

[TestMethod]
public void RawApi_Help_Error_tests( )
{
    var rootCommand = new RootCommand("Test Root")
    {
        new Option<string>("--option1")
        {
            Description = "Test option `",
            Required = true,
        },
    };

    var result = rootCommand.Parse(["--FooBar", "--help"]);
    Assert.HasCount( 3, result.Errors, "Errors should account for, bogus arg (`--FooBar`), missing required arg (`--option1`), AND that `--version` should be solo" );
}

This results in NO errors. But the expectation is that an error for the bogus param AND the missing required argument is present. Basically, the problem is one of co-mingling parsing (which is really lex+parse) with validation which is semantic analysis and is VERY application dependent. Ideally, there is a way to disable all validations and move that to a distinct operation the application is in control of.

[jonsequitur]

It has always been the intended behavior (whether or not it's the correct behavior) that --help requests don't report parse errors. As far as I know, this has also been the actual behavior for a long time.

For --version, this looks like a bug.

[smaillet]

Additional details (and maybe another bug?)

[TestMethod]
public void RawApi_Version_with_required_option_has_errors( )
{
    var rootCommand = new RootCommand("Test Root")
    {
        new Option<string>("--option1")
        {
            Description = "Test option `",
            Required = true,
        },
    };

    var result = rootCommand.Parse(["--version", "--option1"]);

    // This assert will fail - result.Errors.Count == 3!
    // result.Errors:
    //    [0]{--version option cannot be combined with other arguments.}
    //    [1]{Required argument missing for option: '--option1'.}
    //    [2]{Required argument missing for option: '--option1'.} // Why is this occurring twice?
    //Assert.HasCount( 2, result.Errors, "Should be two errors (version not used solo, missing arg)" );

    // try with arguments in reversed order (--version is later)
    result = rootCommand.Parse(["--option1", "--version"]);

    // result.Action == null! [BUG]
    // result.Errors.Count == 0! [BUG]
    Assert.HasCount( 2, result.Errors, "Should be two errors (version not used solo, missing arg)" );

    result = rootCommand.Parse("--option1 --version");

    // result.Action == null! [BUG]
    // result.Errors.Count == 0! [BUG]
    Assert.HasCount( 2, result.Errors, "Should be two errors (version not used solo, missing arg)" );
}

From what I can tell here, in the last case, '--version' is considered the Argument for --Option1, and therefore no error exists. But this is non-sensical from the perspective of an end user. The leading -- should mark it as an option and NOT an argument, if I want an argument that includes such things I'd need to include quotes. Thus, I'm confused if this is a bug in the implementation or a bug in the intended design.

[jonsequitur]

This is by design. There's no special treatment for option prefixes, and since --option1 is expecting an argument, --version satisfies that. Quotes are not treated as a universal escaping mechanism, in part due to the fact that many shells remove them so that the command line parser never even sees them.

[smaillet]

Ok... That's problematic for anyone trying to test things as there is no way to differentiate between a missing argument and an invalid option. Even if the parser doesn't account for any "prefix" it is included in the VersionOption so that should have a higher priority on the interpretation.

How else is an app supposed to report the error on input to a user in a way that makes sense? "--version isn't a valid argument for --option1" isn't all that helpful. A user would expect something along the lines of "argument for --option1 is missing" as that's what the actual error is (or at least one of them anyway)

Also what's the deal with multiple occurrences of the same error message in the inverse?:

var result = rootCommand.Parse(["--version", "--option1"]);

// This assert will fail - result.Errors.Count == 3!
// result.Errors:
//    [0]{--version option cannot be combined with other arguments.}
//    [1]{Required argument missing for option: '--option1'.}
//    [2]{Required argument missing for option: '--option1'.} // Why is this occurring twice?
Assert.HasCount( 2, result.Errors, "Should be two errors (version not used solo, missing arg)" );

[jonsequitur]

Just to clarify, the following is valid given the above configuration, as --version will be treated as an argument to --option1.

> myapp --option1 --version

Even if the parser doesn't account for any "prefix" it is included in the VersionOption so that should have a higher priority on the interpretation.

In this case there's no way to pass the string "--version" to the --option1 option. This is why the option argument is greedy.

[smaillet]

Well, it's my argument and expectation as a user, and from users is that

> myapp --option1 --version

results in TWO options

1. '--option' without any argument (error)
2. '--version' Option that should only appear solo (error)

So, that's two errors, not none the app should report to the user and NOT move forward. It's confusing to the end user if the error invalidates '--version' as a value for option1. And that's ONLY if such a value is in fact invalid! In such a case there's no way to even detect the problem and report it to the user.

[KalleOlaviNiemitalo]

Related issue https://github.com/dotnet/command-line-api/issues/2072 — if that were implemented, and --option1 were defined with an optional argument, then --option1 --version would be parsed as two options (which a validator could then reject), and --option1=--version would be parsed as an option that has an argument.

[jeffhandley]

We aren't making any changes here for 2.0.0, but we will leave issues of this sort open in the Backlog for the time being to see if more input comes in that gives a compelling case for changes in the future. Another related issue that influenced this discussion.

• #2591