Fix version option validation #1270
Conversation
| @@ -20,9 +20,11 @@ public class ArgumentResult : SymbolResult | |||
|
|
|||
| public IArgument Argument { get; } | |||
|
|
|||
| public bool IsImplicit => PassedOnTokens == null; | |||
There was a problem hiding this comment.
I'm not sure this is correct. IsImplicit means something a bit different in the existing OptionResult.IsImplicit, which is that the token was generated by the parser as a sort of placeholder for a default value.
At the least, the PassedOnTokens == null case looks like it might only accidentally correlate with the presence of a default value. This collection is intended to capture the overflow when a custom parser doesn't want to consume all of the available tokens.
| @@ -526,7 +526,7 @@ public static CommandLineBuilder UseHelp(this CommandLineBuilder builder) | |||
| description: Resources.Instance.VersionOptionDescription(), | |||
| parseArgument: result => | |||
| { | |||
| if (result.FindResultFor(command)?.Children.Count > 1) | |||
| if (result.FindResultFor(command)?.Children.Where(IsNotImplicit).Count() > 1) | |||
There was a problem hiding this comment.
Can we do this without the LINQ? A bunch of LINQ removal was done for performance reasons.
I wonder if a simpler implementation could work based on just the token count?
| var versionOptionResult = result.Parent; | ||
| foreach (var symbolResult in result.FindResultFor(command)?.Children) | ||
| foreach (var symbolResult in commandChildren) |
There was a problem hiding this comment.
I think you can use a for loop here.
| @@ -526,8 +526,14 @@ public static CommandLineBuilder UseHelp(this CommandLineBuilder builder) | |||
| description: Resources.Instance.VersionOptionDescription(), | |||
| parseArgument: result => | |||
| { | |||
| var commandChildren = result.FindResultFor(command)?.Children; | |||
| if (commandChildren == null) | |||
There was a problem hiding this comment.
| if (commandChildren == null) | |
| if (commandChildren is null) |
As reported at dotnet/format#1136
The version option validation (introduced in #1124) treats default options and arguments as if they were passed by the user. This PR adds an IsImplicit property to ArgumentResult and filters implicit argument and option results when validating the version option.