Command validators don't seem to use the default value factory of an Option
Nobody has claimed this yet.
Assessment
- Difficulty
- 3/5
- Estimated time
- 1-2 days
- Newbie friendliness
- 52/100
Research direction
Start by tracing how command-level Validators use result.GetValue(_multiplier) and how an Option's DefaultValueFactory is applied. Compare that behavior with option-level validators and GetValueOrDefault(); done means the intended handling of omitted options is established and the observed discrepancy is covered by the relevant validation behavior.
Written by the indexing model from the issue text.
Description
I've been on 2.0.0-beta4 for a while, and only recently gotten around to update to 2.0.10. One of the more interresting changes has been how validations work. It bugged me that I only had a single error message return and had to join multiple ones myself if there was more than one issue; but new API with result.AddError makes this a lot nicer.
However, I noticed that my old command-level validator didn't work anymore:
internal sealed class MySubCommand : Command
{
private readonly Option<int> _multiplier = new("-m", "--multiplier") { Description = "Value multiplier, must be a positive non-zero value", DefaultValueFactory = _ => 1);
public MySubCommand() : base("mysub", "Babies first subcommand")
{
Add(_multiplier);
Validators.Add(result =>
{
if (result.GetValue(_multiplier) <= 0)
result.AddError("Multiplier must be greater than 0.");
});
SetAction(Handle);
}
public int Handle(ParseResult parseResult) { /* ... */ }
}
As it turns out, that would return 0 (the default value for int) rather than what DefaultValueFactory would give me.
The -m argument is generally optional; but when it's specified I need it to be positive/non-zero.
In my case though, the fix is simple: Put the validation on the option itself (which wasn't a thing before; or I just overlooked it):
- Validators.Add(result =>
+ _multiplier.Validators.Add(result =>
{
if (result.GetValue(_multiplier) <= 0)
result.AddError("Multiplier must be greater than 0.");
});
(Which could even go and use result.GetValueOrDefault<int>() instead, since it's specific to the option that way.)
In 2.0.0-beta4, this was simply:
AddValidator(result =>
{
if (result.GetValueForOption(_multiplier) <= 0)
result.ErrorMessage = "Multiplier must be greater than 0.";
});
(Which felt straight-forward to migrate over, since there was no real mention of this behavior in the 2.0.0-beta5 migration guide. And the fact that I had to keep my own error list if I did more than one validation in there; but I omitted that for brevity.)
And that made me wonder: Since command-level validations are intended for cross-argument checks (like, if related arguments like ranges or from/to etc. are passed; whether they work in combination), wouldn't this potentially cause subtle bugs if someone expected the result to be as produced by the DefaultValueFactory? Is this the intended behavior of the command-level validator?
- Dominant language
- C#
- Stars
- 3.7k
- Forks
- 432
- PR merge metrics
- No merged PRs in 30d
Getting set up
- No Dockerfile or Docker Compose file
- No pull request template
- Read the contributing guide
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
More from dotnet/command-line-api
-
German localization is incompletePossibly taken @b-v-d-e-v claimed this 3 days ago. Open
Difficulty 2/5 1-3 hours Newbie friendliness 65/100
dotnet/command-line-api#2852 ·
-
Incomplete French (fr) translation: RequiredOptionWasNotProvided not translatedPossibly taken @JPBlanc claimed this 101 days ago. Open
Difficulty 2/5 1-3 hours Newbie friendliness 65/100
dotnet/command-line-api#2822 · 1 comment ·
-
Difficulty 2/5 1-3 hours Newbie friendliness 62/100
dotnet/command-line-api#2792 · 2 comments · 15 reactions ·
-
Difficulty 2/5 1-3 hours Newbie friendliness 62/100
dotnet/command-line-api#2704 ·
-
GetCompletions should check exit code of invoked applicationPossibly taken @baradgur claimed this 1272 days ago. OpenArea-Completions bug help wanted
Difficulty 2/5 1-3 hours Newbie friendliness 72/100
dotnet/command-line-api#2137 · 1 comment · 3 reactions ·
All issues in dotnet/command-line-api
Similar issues
-
area:frontend bug FE P3
Difficulty 2/5 1-3 hours Newbie friendliness 68/100
klasolsson81/jobbliggaren#2010 ·
Maintainers usually reply within 1 day
-
agentic-workflows untriaged
Difficulty 1/5 Under an hour Newbie friendliness 65/100
Maintainers usually reply within 1 day
-
area: homeblaze type: bug
Difficulty 2/5 1-3 hours Newbie friendliness 74/100
RicoSuter/Namotion.Interceptor#630 ·
Maintainers usually reply within 1 day
-
Akka.Hosting enhancement
Difficulty 2/5 1-3 hours Newbie friendliness 65/100
-
bug needs-triage
Difficulty 2/5 1-3 hours Newbie friendliness 70/100
microsoft/microsoft-ui-xaml#12158 ·
Maintainers usually reply within 1 day