-
Notifications
You must be signed in to change notification settings - Fork 386
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Type mismatch between string and string array to error out #787
Comments
We don't currently validate the binding, so things like type mismatches and argument name mismatches (the most common gotcha with the current API) will only be discovered at runtime. (This is consistent with ASP.NET's model binding behavior, for what it's worth.) It's unfortunate that these kinds of things are a little hard to discover. But part of why we haven't addressed it is that it's not clear the performance hit would be justified, since it's pretty easily sorted out in unit testing. Also, app models layered on top (like DragonFruit) make this kind of validation unnecessary, since they'll typically derive the parser configuration from the handler, so the parameter types and names are always aligned. All that said, if you have any suggestions for how to better validate this, we're happy to try things out. |
Thanks @jonsequitur. In this particular case ( I was thinking could this be detected at binding time (still during the runtime) and thrown to the consumer (so we get a chance to add forgotten |
I'm not sure I fully understand the issue. It sounds like Can you provide an example in the form of a unit test that displays the unexpected behavior? |
If there is a mismatch is between argument description (using var rootCommand = new RootCommand
{
new Option(new[] { "--include"})
{
Argument = new Argument<string>(() => null)
}
};
rootCommand.Handler = CommandHandler.Create(GetType().GetMethod(nameof(Run))); and the command handler (using public static async Task<int> Run(string[] include) there is no run-time error. As a consumer I would expect that binder will throw error or warn somehow, as |
Noticed at dotnet/format#551 (comment), if
option.Argument = new Argument<string>(() => null)
is used and the target delegate's signature hasstring[]
, it can be an error so the user knows that API is not going to work as expected.will only function properly at runtime with
option.Argument = new Argument<string[]>(() => null)
.The text was updated successfully, but these errors were encountered: