Skip to content

Commit 7e5307c

Browse files
jonsequiturjonsequitur
committed
fix #1573 
1 parent d00241c commit 7e5307c

File tree

4 files changed

+121
-53
lines changed

4 files changed

+121
-53
lines changed

‎src/System.CommandLine.Tests/ParsingValidationTests.cs

Lines changed: 112 additions & 48 deletions
Original file line numberDiff line numberDiff line change
@@ -31,8 +31,9 @@ public void When_an_option_accepts_only_specific_arguments_but_a_wrong_one_is_su
3131

3232
result.Errors
3333
.Select(e => e.Message)
34-
.Single()
3534
.Should()
35+
.HaveCount(1)
36+
.And
3637
.Contain("Argument 'none-of-those' not recognized. Must be one of:\n\t'this'\n\t'that'\n\t'the-other-thing'");
3738
}
3839

@@ -109,7 +110,9 @@ public void When_FromAmong_is_used_for_multiple_arguments_and_invalid_input_is_p
109110

110111
var result = command.Parse("set not-key1 value1");
111112

112-
result.Errors.Should().ContainSingle()
113+
result.Errors
114+
.Should()
115+
.ContainSingle()
113116
.Which
114117
.Message
115118
.Should()
@@ -127,7 +130,9 @@ public void When_FromAmong_is_used_for_multiple_arguments_and_invalid_input_is_p
127130

128131
var result = command.Parse("set key1 not-value1");
129132

130-
result.Errors.Should().ContainSingle()
133+
result.Errors
134+
.Should()
135+
.ContainSingle()
131136
.Which
132137
.Message
133138
.Should()
@@ -143,6 +148,8 @@ public void When_a_required_argument_is_not_supplied_then_an_error_is_returned()
143148

144149
result.Errors
145150
.Should()
151+
.HaveCount(1)
152+
.And
146153
.Contain(e => e.Message == "Required argument missing for option: '-x'.");
147154
}
148155

@@ -161,7 +168,9 @@ public void When_a_required_option_is_not_supplied_then_an_error_is_returned()
161168

162169
result.Errors
163170
.Should()
164-
.ContainSingle(e => e.SymbolResult.Symbol == command)
171+
.HaveCount(1)
172+
.And
173+
.Contain(e => e.SymbolResult.Symbol == command)
165174
.Which
166175
.Message
167176
.Should()
@@ -225,7 +234,9 @@ public void When_no_option_accepts_arguments_but_one_is_supplied_then_an_error_i
225234
result.Errors
226235
.Select(e => e.Message)
227236
.Should()
228-
.ContainSingle(e => e == "Unrecognized command or argument 'some-arg'.");
237+
.HaveCount(1)
238+
.And
239+
.Contain(e => e == "Unrecognized command or argument 'some-arg'.");
229240
}
230241

231242
[Fact]
@@ -252,7 +263,9 @@ public void A_custom_validator_can_be_added_to_a_command()
252263
.Errors
253264
.Select(e => e.Message)
254265
.Should()
255-
.ContainSingle("Options '--one' and '--two' cannot be used together.");
266+
.HaveCount(1)
267+
.And
268+
.Contain("Options '--one' and '--two' cannot be used together.");
256269
}
257270

258271
[Fact]
@@ -273,7 +286,9 @@ public void A_custom_validator_can_be_added_to_an_option()
273286

274287
result.Errors
275288
.Should()
276-
.ContainSingle(e => e.SymbolResult.Symbol == option)
289+
.HaveCount(1)
290+
.And
291+
.Contain(e => e.SymbolResult.Symbol == option)
277292
.Which
278293
.Message
279294
.Should()
@@ -298,7 +313,9 @@ public void A_custom_validator_can_be_added_to_an_argument()
298313

299314
result.Errors
300315
.Should()
301-
.ContainSingle(e => e.SymbolResult.Symbol == argument)
316+
.HaveCount(1)
317+
.And
318+
.Contain(e => e.SymbolResult.Symbol == argument)
302319
.Which
303320
.Message
304321
.Should()
@@ -365,7 +382,9 @@ public void Validators_on_global_options_are_executed_when_invoking_a_subcommand
365382

366383
result.Errors
367384
.Should()
368-
.ContainSingle(e => e.SymbolResult.Symbol == option)
385+
.HaveCount(1)
386+
.And
387+
.Contain(e => e.SymbolResult.Symbol == option)
369388
.Which
370389
.Message
371390
.Should()
@@ -421,11 +440,13 @@ public void Custom_validator_error_messages_are_not_repeated()
421440
argument
422441
};
423442

424-
var result = cmd.Parse("get something");
443+
var result = cmd.Parse("get something");
425444

426445
result.Errors
427446
.Should()
428-
.ContainSingle(errorMessage);
447+
.HaveCount(1)
448+
.And
449+
.Contain(e => e.Message == errorMessage);
429450
}
430451

431452
public class PathValidity
@@ -444,6 +465,8 @@ public void LegalFilePathsOnly_rejects_command_arguments_containing_invalid_path
444465

445466
result.Errors
446467
.Should()
468+
.HaveCount(1)
469+
.And
447470
.Contain(e => e.SymbolResult.Symbol == command.Arguments.First() &&
448471
e.Message == $"Character not allowed in a path: '{invalidCharacter}'.");
449472
}
@@ -462,6 +485,8 @@ public void LegalFilePathsOnly_rejects_option_arguments_containing_invalid_path_
462485

463486
result.Errors
464487
.Should()
488+
.HaveCount(1)
489+
.And
465490
.Contain(e => e.SymbolResult.Symbol.Name == "x" &&
466491
e.Message == $"Character not allowed in a path: '{invalidCharacter}'.");
467492
}
@@ -515,6 +540,8 @@ public void LegalFileNamesOnly_rejects_command_arguments_containing_invalid_file
515540

516541
result.Errors
517542
.Should()
543+
.HaveCount(1)
544+
.And
518545
.Contain(e => e.SymbolResult.Symbol == command.Arguments.First() &&
519546
e.Message == $"Character not allowed in a file name: '{invalidCharacter}'.");
520547
}
@@ -533,6 +560,8 @@ public void LegalFileNamesOnly_rejects_option_arguments_containing_invalid_file_
533560

534561
result.Errors
535562
.Should()
563+
.HaveCount(1)
564+
.And
536565
.Contain(e => e.SymbolResult.Symbol.Name == "x" &&
537566
e.Message == $"Character not allowed in a file name: '{invalidCharacter}'.");
538567
}
@@ -739,8 +768,8 @@ public void A_command_argument_with_multiple_directories_can_be_invalid_based_on
739768
.Should()
740769
.HaveCount(1)
741770
.And
742-
.Contain(e => e.SymbolResult.Symbol.Name == "to" &&
743-
e.Message == $"Directory does not exist: '{path}'.");
771+
.ContainSingle(e => e.SymbolResult.Symbol.Name == "to" &&
772+
e.Message == $"Directory does not exist: '{path}'.");
744773
}
745774

746775
[Fact]
@@ -758,8 +787,8 @@ public void An_option_argument_with_multiple_directories_can_be_invalid_based_on
758787
.Should()
759788
.HaveCount(1)
760789
.And
761-
.Contain(e => e.SymbolResult.Symbol.Name == "to" &&
762-
e.Message == $"Directory does not exist: '{path}'.");
790+
.ContainSingle(e => e.SymbolResult.Symbol.Name == "to" &&
791+
e.Message == $"Directory does not exist: '{path}'.");
763792
}
764793

765794
[Fact]
@@ -779,8 +808,8 @@ public void A_command_argument_with_multiple_FileSystemInfos_can_be_invalid_base
779808
780809
result.Errors
781810
.Should()
782-
.Contain(e => e.SymbolResult.Symbol.Name == "to" &&
783-
e.Message == $"File or directory does not exist: '{path}'.");
811+
.ContainSingle(e => e.SymbolResult.Symbol.Name == "to" &&
812+
e.Message == $"File or directory does not exist: '{path}'.");
784813
}
785814

786815
[Fact]
@@ -798,8 +827,8 @@ public void An_option_argument_with_multiple_FileSystemInfos_can_be_invalid_base
798827
799828
result.Errors
800829
.Should()
801-
.Contain(e => e.SymbolResult.Symbol.Name == "to" &&
802-
e.Message == $"File or directory does not exist: '{path}'.");
830+
.ContainSingle(e => e.SymbolResult.Symbol.Name == "to" &&
831+
e.Message == $"File or directory does not exist: '{path}'.");
803832
}
804833

805834
[Fact]
@@ -817,8 +846,8 @@ public void A_command_argument_with_multiple_FileSystemInfos_can_be_invalid_base
817846
.Should()
818847
.HaveCount(1)
819848
.And
820-
.Contain(e => e.SymbolResult.Symbol.Name == "to" &&
821-
e.Message == $"File or directory does not exist: '{path}'.");
849+
.ContainSingle(e => e.SymbolResult.Symbol.Name == "to" &&
850+
e.Message == $"File or directory does not exist: '{path}'.");
822851
}
823852

824853
[Fact]
@@ -836,8 +865,8 @@ public void An_option_argument_with_multiple_FileSystemInfos_can_be_invalid_base
836865
.Should()
837866
.HaveCount(1)
838867
.And
839-
.Contain(e => e.SymbolResult.Symbol.Name == "to" &&
840-
e.Message == $"File or directory does not exist: '{path}'.");
868+
.ContainSingle(e => e.SymbolResult.Symbol.Name == "to" &&
869+
e.Message == $"File or directory does not exist: '{path}'.");
841870
}
842871

843872
[Fact]
@@ -999,7 +1028,7 @@ public void Arity_failures_are_not_reported_for_both_an_argument_and_its_parent_
9991028
{
10001029
var newCommand = new Command("test")
10011030
{
1002-
new Option<string>("--opt", ParseMe)
1031+
new Option<string>("--opt")
10031032
};
10041033

10051034
var parseResult = newCommand.Parse("test --opt");
@@ -1010,36 +1039,71 @@ public void Arity_failures_are_not_reported_for_both_an_argument_and_its_parent_
10101039
.Which
10111040
.Message
10121041
.Should()
1013-
.Be(
1014-
"Required argument missing for option: '--opt'.");
1042+
.Be("Required argument missing for option: '--opt'.");
10151043
}
10161044

1017-
private static string ParseMe(ArgumentResult argumentResult)
1045+
[Fact] // https://github.com/dotnet/command-line-api/issues/1573
1046+
public void Multiple_validators_on_the_same_command_do_not_report_duplicate_errors()
10181047
{
1019-
if (argumentResult.Parent is not OptionResult or)
1020-
{
1021-
throw new NotSupportedException("The method should be only used with option.");
1022-
}
1048+
var command = new RootCommand();
1049+
command.AddValidator(result => result.ErrorMessage = "Wrong");
1050+
command.AddValidator(_ => { });
10231051

1024-
if (argumentResult.Tokens.Count == 0)
1025-
{
1026-
if (or.IsImplicit)
1027-
{
1028-
return "default";
1029-
}
1030-
//no arg is not allowed
1031-
argumentResult.ErrorMessage = $"(CUSTOM) Required argument missing for option: {or.Token.Value}.";
1032-
return default!;
1033-
}
1034-
else if (argumentResult.Tokens.Count == 1)
1052+
var parseResult = command.Parse("");
1053+
1054+
parseResult.Errors
1055+
.Should()
1056+
.ContainSingle()
1057+
.Which
1058+
.Message
1059+
.Should()
1060+
.Be("Wrong");
1061+
}
1062+
1063+
[Fact] // https://github.com/dotnet/command-line-api/issues/1573
1064+
public void Multiple_validators_on_the_same_option_do_not_report_duplicate_errors()
1065+
{
1066+
var option = new Option<string>("-x");
1067+
option.AddValidator(result => result.ErrorMessage = "Wrong");
1068+
option.AddValidator(_ => { });
1069+
1070+
var command = new RootCommand
10351071
{
1036-
return "value";
1037-
}
1038-
else
1072+
option
1073+
};
1074+
1075+
var parseResult = command.Parse("-x b");
1076+
1077+
parseResult.Errors
1078+
.Should()
1079+
.ContainSingle()
1080+
.Which
1081+
.Message
1082+
.Should()
1083+
.Be("Wrong");
1084+
}
1085+
1086+
[Fact] // https://github.com/dotnet/command-line-api/issues/1573
1087+
public void Multiple_validators_on_the_same_argument_do_not_report_duplicate_errors()
1088+
{
1089+
var argument = new Argument<string>();
1090+
argument.AddValidator(result => result.ErrorMessage = "Wrong");
1091+
argument.AddValidator(_ => { });
1092+
1093+
var command = new RootCommand
10391094
{
1040-
argumentResult.ErrorMessage = $"Using more than 1 argument is not allowed for '{or.Token.Value}', used: {argumentResult.Tokens.Count}.";
1041-
return default!;
1042-
}
1095+
argument
1096+
};
1097+
1098+
var parseResult = command.Parse("b");
1099+
1100+
parseResult.Errors
1101+
.Should()
1102+
.ContainSingle()
1103+
.Which
1104+
.Message
1105+
.Should()
1106+
.Be("Wrong");
10431107
}
10441108
}
10451109
}

‎src/System.CommandLine/Parsing/ArgumentResult.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -114,7 +114,7 @@ Parent is { } &&
114114
return failedResult;
115115
}
116116

117-
if (argument is Argument arg)
117+
if (argument is { } arg)
118118
{
119119
if (Parent!.UseDefaultValueFor(argument))
120120
{

‎src/System.CommandLine/Parsing/OptionResult.cs

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -92,9 +92,7 @@ internal ArgumentConversionResult ArgumentConversionResult
9292
return _argumentConversionResult;
9393
}
9494
}
95-
96-
internal bool IsMinimumArgumentAritySatisfied => Tokens.Count >= Option.Argument.Arity.MinimumNumberOfValues;
97-
95+
9896
internal override bool UseDefaultValueFor(Argument argument) => IsImplicit;
9997
}
10098
}

‎src/System.CommandLine/Parsing/ParseResultVisitor.cs

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -330,7 +330,11 @@ private void ValidateCommandResult()
330330

331331
if (!string.IsNullOrWhiteSpace(_innermostCommandResult.ErrorMessage))
332332
{
333-
AddErrorToResult(_innermostCommandResult, new ParseError(_innermostCommandResult.ErrorMessage!, _innermostCommandResult));
333+
AddErrorToResult(
334+
_innermostCommandResult,
335+
new ParseError(_innermostCommandResult.ErrorMessage!, _innermostCommandResult));
336+
337+
return;
334338
}
335339
}
336340

@@ -445,6 +449,8 @@ private void ValidateAndConvertOptionResult(OptionResult optionResult)
445449
if (!string.IsNullOrWhiteSpace(optionResult.ErrorMessage))
446450
{
447451
AddErrorToResult(optionResult, new ParseError(optionResult.ErrorMessage!, optionResult));
452+
453+
return;
448454
}
449455
}
450456
}

0 commit comments

Comments
 (0)