Command Validators are still run when a parser fails

[Choosechee]

If the default or custom parser for an Option adds an error, its Validators are still run. This is a problem because an exception will be thrown when a validator tries to get the value. Here is an example:

using System.CommandLine;

Option<int> intOption = new("--int")
{
    Validators =
    {
        result =>
        {
            int value = result.GetValueOrDefault<int>();
            if (value < 1)
                result.AddError("Value must be at least 1.");
        }
    }
};

RootCommand command = new()
{
    intOption
};
command.SetAction(result => Console.WriteLine("Command ran successfully"));

command.Parse(["--int", "nonsense"]).Invoke();

Running the code on my system gives this hostile error message:

Unhandled exception. System.InvalidOperationException: Cannot parse argument 'nonsense' for option '--int' as expected type 'System.Int32'.
   at System.CommandLine.Binding.ArgumentConverter.GetValueOrDefault[T](ArgumentConversionResult result)
   at System.CommandLine.Parsing.OptionResult.GetValueOrDefault[T]()
   at Program.<>c.<<Main>$>b__0_1(OptionResult result) in /home/choosechee/RiderProjects/CommandLineIssue/CommandLineIssue/Program.cs:line 9
   at System.CommandLine.Parsing.CommandResult.ValidateOptions(Boolean completeValidation)
   at System.CommandLine.Parsing.CommandResult.Validate(Boolean completeValidation)
   at System.CommandLine.Parsing.ParseOperation.Validate()
   at System.CommandLine.Parsing.ParseOperation.Parse()
   at System.CommandLine.Parsing.CommandLineParser.Parse(Command command, IReadOnlyList`1 arguments, String rawInput, CommandLineConfiguration configuration)
   at System.CommandLine.Parsing.CommandLineParser.Parse(Command command, IReadOnlyList`1 args, CommandLineConfiguration configuration)
   at System.CommandLine.Command.Parse(IReadOnlyList`1 args, CommandLineConfiguration configuration)
   at Program.<Main>$(String[] args) in /home/choosechee/RiderProjects/CommandLineIssue/CommandLineIssue/Program.cs:line 22

Process finished with exit code 134.

If I remove the validator, it gives the expected friendlier error message:

Cannot parse argument 'nonsense' for option '--int' as expected type 'System.Int32'.

Description:

Usage:
  CommandLineIssue [options]

Options:
  -?, -h, --help  Show help and usage information
  --version       Show version information
  --int


Process finished with exit code 0.

It happens with a CustomParser as well:

using System.CommandLine;

Option<int> intOption = new("--int")
{
    CustomParser = result =>
    {
        if (int.TryParse(result.Tokens.Single().Value, out int value))
            return value;
        
        result.AddError("Value must be an integer."); // cause of exception
        return -1;
    },
    Validators =
    {
        result =>
        {
            int value = result.GetValueOrDefault<int>();
            if (value < 1)
                result.AddError("Value must be at least 1.");
        }
    }
};

RootCommand command = new()
{
    intOption
};
command.SetAction(result => Console.WriteLine("Command ran successfully"));

command.Parse(["--int", "nonsense"]).Invoke();

Unhandled exception. System.InvalidOperationException: Value must be an integer.
   at System.CommandLine.Binding.ArgumentConverter.GetValueOrDefault[T](ArgumentConversionResult result)
   at System.CommandLine.Parsing.OptionResult.GetValueOrDefault[T]()
  ...yadda yadda yadda

Process finished with exit code 134.

I can put the validator code that gets the value in a try-catch block to fix this, but I think this is unintuitive to have to do this. I think Validators should not be run when parsing fails, or at least run in a try-catch block to handle these exceptions.

[jonsequitur]

The issue here is that GetValueOrDefault<T> is intended to throw when there is a parsing error, including custom parsing errors defined by validators. After all, there's no way to convert nonsense to an int. A safer approach is to check the Tokens property on the OptionResult or ArgumentResult passed to the validator.

[xIceFox]

How would you implement validators on the command then, that check if combinations of given options are valid?

For example in pseudocode:
if(option1 is given && option2 is not given)
AddError("You need to provide option2 if option 1 is given!")

Get the OptionResult with the option as parameter and check if the option was provided, by checking the tokens?

[jonsequitur]

You can call SymbolResult.GetResult. If it returns null or it returns a result where Implicit is true, then the option or argument was not provided on the command line. If it returns a result where Implicit is true, it came from a DefaultValueFactory.

[Choosechee]

The issue here is that GetValueOrDefault<T> is intended to throw when there is a parsing error, including custom parsing errors defined by validators. After all, there's no way to convert nonsense to an int. A safer approach is to check the Tokens property on the OptionResult or ArgumentResult passed to the validator.

I was just not understanding why a validator would still be run if the parser failed. If you didn't have a parsed value, how could you validate it?

How would you implement validators on the command then, that check if combinations of given options are valid?

For example in pseudocode:
if(option1 is given && option2 is not given)
AddError("You need to provide option2 if option 1 is given!")

Get the OptionResult with the option as parameter and check if the option was provided, by checking the tokens?

However, this is an example of validation that wouldn't need a parsed value, so I guess it should still be run. Maybe still run it in a try-catch block, as I said as an alternative?

[jonsequitur]

I was just not understanding why a validator would still be run if the parser failed.

Validators are the mechanism by which many parse errors are created in the first place including internally within System.CommandLine, and all available errors are reported.

If you didn't have a parsed value, how could you validate it?

The DefaultValueFactory can return a value from a source other than the command line (e.g. an environment variable) which might still be invalid. Trying to convert "nonsense" to an int is a good example of why this validation must still take place for values coming from a DefaultValueFactory.

[xIceFox]

You can call SymbolResult.GetResult. If it returns null or it returns a result where Implicit is true, then the option or argument was not provided on the command line. If it returns a result where Implicit is true, it came from a DefaultValueFactory.

Thank you very much, that helps me a lot, is this something that should go into documentation?

[jeffhandley]

Related to #2607, we need to capture the expectations of this behavior and look for any remaining inconsistencies.