Is it possible to control the order of an option validations?

[TPIvan]

Let's have a root command with one int16 option

    var root = new RootCommand("Root command description");
    var tstopt = new Option<Int16>("-t", "testing int16 option");
    root.AddOption(tstopt);

If I use not numeric value I am getting an error as expected

Cannot parse argument 'hh' for option '-t' as expected type 'System.Int16'.

But when I add a validator

   root.AddValidator((result) =>
   {
       if (string.IsNullOrEmpty(result.ErrorMessage) && result.GetValueForOption(tstopt) == 3)
       { result.ErrorMessage = "cannot be 3"; }
   });

I am getting an exception

Unhandled exception. System.InvalidOperationException: Cannot parse argument 'hh' for option '-t' as expected type 'System.Int16'.
   at System.CommandLine.Binding.ArgumentConverter.GetValueOrDefault[T](ArgumentConversionResult result)
   at System.CommandLine.Parsing.OptionResult.GetValueOrDefault[T]()
   at System.CommandLine.Parsing.SymbolResult.GetValueForOption[T](Option`1 option)
   at Program.<>c__DisplayClass0_0.<<Main>$>b__0(CommandResult result) in C:\MyDiskArea\LiveProgProjects\IVAN_TEST\TechnologyTesting\System.CommandLine\ConsoleApp73\Program.cs:line 10
   at System.CommandLine.Parsing.ParseResultVisitor.UseValidators(Command command, CommandResult innermostCommandResult)
   at System.CommandLine.Parsing.ParseResultVisitor.ValidateCommandResult()
   at System.CommandLine.Parsing.ParseResultVisitor.Stop()
   at System.CommandLine.Parsing.ParseResultVisitor.Visit(SyntaxNode node)
   at System.CommandLine.Parsing.Parser.Parse(IReadOnlyList`1 arguments, String rawInput)
   at System.CommandLine.Parsing.ParserExtensions.InvokeAsync(Parser parser, String[] args, IConsole console)
   at Program.<Main>$(String[] args) in C:\MyDiskArea\LiveProgProjects\IVAN_TEST\TechnologyTesting\System.CommandLine\ConsoleApp73\Program.cs:line 19
   at Program.<Main>(String[] args)

So it seems that the custom validatio is runnin before built it ones.
Is it possible to change the behavior?

[jonsequitur]

At first glance this looks to me like a bug. I'm not sure if the ability to specify the order in which these things happen is generally useful, but it does seem like the child (e.g. option) validation and parsing should be completed before the parent command's validator is checked.

[TPIvan]

One thing I forget to mention: the version is 2.0.0-beta4.22272.1 (last nuget).

[3geek14]

I'm on a slightly older version (2.0.0-beta2.21617.1), and I've had a very similar issue. I have an Option<int>:

Option<int> fakeOption = new("--fake-flag", "this is fake");

When passed a value that isn't an int (such as --fake-flag asdf), the built-in type-checking validator will print a nice error message:

Cannot parse argument 'asdf' for option '--fake-flag' as expected type 'System.Int32'.

Now I want to add a custom validator to ensure the value is positive:

fakeOption.AddValidator(
    result => result.GetValueOrDefault<int>() <= 0
        ? "fake-flag must be a positive integer"
        : null);

Now I get a stack trace:

Unhandled Exception: System.InvalidOperationException: Cannot parse argument 'asdf' for option '--fake-flag' as expected type 'System.Int32'.
   at System.CommandLine.Binding.ArgumentConverter.GetValueOrDefault[T](ArgumentConversionResult result)
   at …

I've also tried result.GetValueOrDefault() is <= 0, hoping that GetValueOrDefault() will return an object without attempting to cast to an int (letting the pattern matching gracefully return false if the cast fails), but it appears that the generic version of OptionResult.GetValueOrDefault still performs a cast when the corresponding Option is actually an Option<T>.

My current workaround is to wrap GetValueOrDefault() in a try block. In the catch block, I return null, because I know the built-in validator will print a useful error message, and I don't want to duplicate that message. (I think it's semantically wrong to observe an error during validation and return as if there are no issues, but it seems fine as a temporary workaround for a known bug.)

I think that the built-in type validation should run before any other validators, and custom validators should be skipped when the type checking fails. At the very least, if the type checking for fakeOption fails, custom validators added to fakeOption should be skipped, letting us safely access the value. As a bonus, I think it would be nice to be able to specify for a specific validator other Options that need to pass type checking in order to run the validator (so we can do things like "if the value of this option is foo, then ensure the value of a different option satisfies condition bar).

[adamsitnik]

With our recent changes (#2033, #1987), it's possible.

@3geek14 example:

private static int Main(string[] args)
{
    Option<int> fakeOption = new("--fake-flag");
    fakeOption.Description = "this is fake";
    fakeOption.Validators.Add(result =>
    {
        if (result.GetValueOrDefault<int>() <= 0)
        {
            result.AddError("fake-flag must be a positive integer");
        }
    });

    RootCommand rootCommand = new()
    {
        fakeOption
    };

    return rootCommand.Parse(args).Invoke();
}

Moreover, Option is now exposing a list of validators, so it's possible to remove the built-in validators, place your own before them etc.

[3geek14]

Thanks for the update @adamsitnik! Do you know if there's also been any way added to ensure that certain options are validated (or at least type-checked) before other options?

Also, do you have any predictions as to when the next version will be published to NuGet?

[adamsitnik]

Do you know if there's also been any way added to ensure that certain options are validated (or at least type-checked) before other options?

No such changes were made, I am afraid it would increase the complexity of S.CL to the point it would not be worth it. Could you please provide an example of what you want to achieve and why?

Also, do you have any predictions as to when the next version will be published to NuGet?

Optimistic: 2 weeks
Pessimistic: 4-5 weeks

[3geek14]

Example (using the old API, as I don't have the new version on my machine yet):

I have an option that expects an enum. Depending on which value is passed, more information might be needed from the user. So I have a second option to accept that extra information. If the user supplies the extra information without the corresponding enum value, they're likely confused, so I want to print an error message.

Enum:

public enum Strategy {
  X, Y, Z,
}

Options:

Option<Strategy> strategy = new("--strategy", "pick the strategy") {
  Arity = ArgumentArity.ExactlyOne,
};

Option<int> paramForZ = new("--z-parameter", "parameter for Z") {
  Arity = ArgumentArity.ZeroOrOne,
};

paramForZ.AddValidator(
    result => {
      OptionResult strategyResult = result.FindResultFor(strategy);
      Strategy strat = strategyResult.GetValueOrDefault<Strategy>(); // this line is risky
      return strat == Strategy.Z ? null : "--z-parameter can only be used with --strategy=Z";
    });

If the user writes --strategy=Zz --z-parameter=1 (with a typo that doesn't type check as one of the enum values), this validator will print a stack trace if it runs. I'd prefer to skip this validator if the type check for --strategy fails, but that requires validating --strategy before validating --z-parameter.

Does this example make sense? Is there a better way to have options depend on each other?

[jonsequitur]

Also, do you have any predictions as to when the next version will be published to NuGet?

@3geek14, you can try out the latest API at any time from this feed: https://pkgs.dev.azure.com/dnceng/public/_packaging/dotnet-libraries/nuget/v3/index.json

[3geek14]

@adamsitnik would you like me to file a new issue with a feature request for options that depend on each other?