Skip to content

Commit

Permalink
fix #1573
Browse files Browse the repository at this point in the history
  • Loading branch information
@jonsequitur
jonsequitur committed Jan 12, 2022
1 parent d00241c commit 7e5307c
Show file tree
Hide file tree
Showing 4 changed files with 121 additions and 53 deletions.
160 changes: 112 additions & 48 deletions src/System.CommandLine.Tests/ParsingValidationTests.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -31,8 +31,9 @@ public void When_an_option_accepts_only_specific_arguments_but_a_wrong_one_is_su

result.Errors
.Select(e => e.Message)
.Single()
.Should()
.HaveCount(1)
.And
.Contain("Argument 'none-of-those' not recognized. Must be one of:\n\t'this'\n\t'that'\n\t'the-other-thing'");
}

Expand Down Expand Up @@ -109,7 +110,9 @@ public void When_FromAmong_is_used_for_multiple_arguments_and_invalid_input_is_p

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

result.Errors.Should().ContainSingle()
result.Errors
.Should()
.ContainSingle()
.Which
.Message
.Should()
Expand All @@ -127,7 +130,9 @@ public void When_FromAmong_is_used_for_multiple_arguments_and_invalid_input_is_p

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

result.Errors.Should().ContainSingle()
result.Errors
.Should()
.ContainSingle()
.Which
.Message
.Should()
Expand All @@ -143,6 +148,8 @@ public void When_a_required_argument_is_not_supplied_then_an_error_is_returned()

result.Errors
.Should()
.HaveCount(1)
.And
.Contain(e => e.Message == "Required argument missing for option: '-x'.");
}

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

result.Errors
.Should()
.ContainSingle(e => e.SymbolResult.Symbol == command)
.HaveCount(1)
.And
.Contain(e => e.SymbolResult.Symbol == command)
.Which
.Message
.Should()
Expand Down Expand Up @@ -225,7 +234,9 @@ public void When_no_option_accepts_arguments_but_one_is_supplied_then_an_error_i
result.Errors
.Select(e => e.Message)
.Should()
.ContainSingle(e => e == "Unrecognized command or argument 'some-arg'.");
.HaveCount(1)
.And
.Contain(e => e == "Unrecognized command or argument 'some-arg'.");
}

[Fact]
Expand All @@ -252,7 +263,9 @@ public void A_custom_validator_can_be_added_to_a_command()
.Errors
.Select(e => e.Message)
.Should()
.ContainSingle("Options '--one' and '--two' cannot be used together.");
.HaveCount(1)
.And
.Contain("Options '--one' and '--two' cannot be used together.");
}

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

result.Errors
.Should()
.ContainSingle(e => e.SymbolResult.Symbol == option)
.HaveCount(1)
.And
.Contain(e => e.SymbolResult.Symbol == option)
.Which
.Message
.Should()
Expand All @@ -298,7 +313,9 @@ public void A_custom_validator_can_be_added_to_an_argument()

result.Errors
.Should()
.ContainSingle(e => e.SymbolResult.Symbol == argument)
.HaveCount(1)
.And
.Contain(e => e.SymbolResult.Symbol == argument)
.Which
.Message
.Should()
Expand Down Expand Up @@ -365,7 +382,9 @@ public void Validators_on_global_options_are_executed_when_invoking_a_subcommand

result.Errors
.Should()
.ContainSingle(e => e.SymbolResult.Symbol == option)
.HaveCount(1)
.And
.Contain(e => e.SymbolResult.Symbol == option)
.Which
.Message
.Should()
Expand Down Expand Up @@ -421,11 +440,13 @@ public void Custom_validator_error_messages_are_not_repeated()
argument
};

var result = cmd.Parse("get something");
var result = cmd.Parse("get something");

result.Errors
.Should()
.ContainSingle(errorMessage);
.HaveCount(1)
.And
.Contain(e => e.Message == errorMessage);
}

public class PathValidity
Expand All @@ -444,6 +465,8 @@ public void LegalFilePathsOnly_rejects_command_arguments_containing_invalid_path

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

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

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

result.Errors
.Should()
.HaveCount(1)
.And
.Contain(e => e.SymbolResult.Symbol.Name == "x" &&
e.Message == $"Character not allowed in a file name: '{invalidCharacter}'.");
}
Expand Down Expand Up @@ -739,8 +768,8 @@ public void A_command_argument_with_multiple_directories_can_be_invalid_based_on
.Should()
.HaveCount(1)
.And
.Contain(e => e.SymbolResult.Symbol.Name == "to" &&
e.Message == $"Directory does not exist: '{path}'.");
.ContainSingle(e => e.SymbolResult.Symbol.Name == "to" &&
e.Message == $"Directory does not exist: '{path}'.");
}

[Fact]
Expand All @@ -758,8 +787,8 @@ public void An_option_argument_with_multiple_directories_can_be_invalid_based_on
.Should()
.HaveCount(1)
.And
.Contain(e => e.SymbolResult.Symbol.Name == "to" &&
e.Message == $"Directory does not exist: '{path}'.");
.ContainSingle(e => e.SymbolResult.Symbol.Name == "to" &&
e.Message == $"Directory does not exist: '{path}'.");
}

[Fact]
Expand All @@ -779,8 +808,8 @@ public void A_command_argument_with_multiple_FileSystemInfos_can_be_invalid_base
result.Errors
.Should()
.Contain(e => e.SymbolResult.Symbol.Name == "to" &&
e.Message == $"File or directory does not exist: '{path}'.");
.ContainSingle(e => e.SymbolResult.Symbol.Name == "to" &&
e.Message == $"File or directory does not exist: '{path}'.");
}

[Fact]
Expand All @@ -798,8 +827,8 @@ public void An_option_argument_with_multiple_FileSystemInfos_can_be_invalid_base
result.Errors
.Should()
.Contain(e => e.SymbolResult.Symbol.Name == "to" &&
e.Message == $"File or directory does not exist: '{path}'.");
.ContainSingle(e => e.SymbolResult.Symbol.Name == "to" &&
e.Message == $"File or directory does not exist: '{path}'.");
}

[Fact]
Expand All @@ -817,8 +846,8 @@ public void A_command_argument_with_multiple_FileSystemInfos_can_be_invalid_base
.Should()
.HaveCount(1)
.And
.Contain(e => e.SymbolResult.Symbol.Name == "to" &&
e.Message == $"File or directory does not exist: '{path}'.");
.ContainSingle(e => e.SymbolResult.Symbol.Name == "to" &&
e.Message == $"File or directory does not exist: '{path}'.");
}

[Fact]
Expand All @@ -836,8 +865,8 @@ public void An_option_argument_with_multiple_FileSystemInfos_can_be_invalid_base
.Should()
.HaveCount(1)
.And
.Contain(e => e.SymbolResult.Symbol.Name == "to" &&
e.Message == $"File or directory does not exist: '{path}'.");
.ContainSingle(e => e.SymbolResult.Symbol.Name == "to" &&
e.Message == $"File or directory does not exist: '{path}'.");
}

[Fact]
Expand Down Expand Up @@ -999,7 +1028,7 @@ public void Arity_failures_are_not_reported_for_both_an_argument_and_its_parent_
{
var newCommand = new Command("test")
{
new Option<string>("--opt", ParseMe)
new Option<string>("--opt")
};

var parseResult = newCommand.Parse("test --opt");
Expand All @@ -1010,36 +1039,71 @@ public void Arity_failures_are_not_reported_for_both_an_argument_and_its_parent_
.Which
.Message
.Should()
.Be(
"Required argument missing for option: '--opt'.");
.Be("Required argument missing for option: '--opt'.");
}

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

if (argumentResult.Tokens.Count == 0)
{
if (or.IsImplicit)
{
return "default";
}
//no arg is not allowed
argumentResult.ErrorMessage = $"(CUSTOM) Required argument missing for option: {or.Token.Value}.";
return default!;
}
else if (argumentResult.Tokens.Count == 1)
var parseResult = command.Parse("");

parseResult.Errors
.Should()
.ContainSingle()
.Which
.Message
.Should()
.Be("Wrong");
}

[Fact] // https://github.com/dotnet/command-line-api/issues/1573
public void Multiple_validators_on_the_same_option_do_not_report_duplicate_errors()
{
var option = new Option<string>("-x");
option.AddValidator(result => result.ErrorMessage = "Wrong");
option.AddValidator(_ => { });

var command = new RootCommand
{
return "value";
}
else
option
};

var parseResult = command.Parse("-x b");

parseResult.Errors
.Should()
.ContainSingle()
.Which
.Message
.Should()
.Be("Wrong");
}

[Fact] // https://github.com/dotnet/command-line-api/issues/1573
public void Multiple_validators_on_the_same_argument_do_not_report_duplicate_errors()
{
var argument = new Argument<string>();
argument.AddValidator(result => result.ErrorMessage = "Wrong");
argument.AddValidator(_ => { });

var command = new RootCommand
{
argumentResult.ErrorMessage = $"Using more than 1 argument is not allowed for '{or.Token.Value}', used: {argumentResult.Tokens.Count}.";
return default!;
}
argument
};

var parseResult = command.Parse("b");

parseResult.Errors
.Should()
.ContainSingle()
.Which
.Message
.Should()
.Be("Wrong");
}
}
}
2 changes: 1 addition & 1 deletion src/System.CommandLine/Parsing/ArgumentResult.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -114,7 +114,7 @@ Parent is { } &&
return failedResult;
}

if (argument is Argument arg)
if (argument is { } arg)
{
if (Parent!.UseDefaultValueFor(argument))
{
Expand Down
4 changes: 1 addition & 3 deletions src/System.CommandLine/Parsing/OptionResult.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -92,9 +92,7 @@ internal ArgumentConversionResult ArgumentConversionResult
return _argumentConversionResult;
}
}

internal bool IsMinimumArgumentAritySatisfied => Tokens.Count >= Option.Argument.Arity.MinimumNumberOfValues;


internal override bool UseDefaultValueFor(Argument argument) => IsImplicit;
}
}
8 changes: 7 additions & 1 deletion src/System.CommandLine/Parsing/ParseResultVisitor.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -330,7 +330,11 @@ private void ValidateCommandResult()

if (!string.IsNullOrWhiteSpace(_innermostCommandResult.ErrorMessage))
{
AddErrorToResult(_innermostCommandResult, new ParseError(_innermostCommandResult.ErrorMessage!, _innermostCommandResult));
AddErrorToResult(
_innermostCommandResult,
new ParseError(_innermostCommandResult.ErrorMessage!, _innermostCommandResult));

return;
}
}

Expand Down Expand Up @@ -445,6 +449,8 @@ private void ValidateAndConvertOptionResult(OptionResult optionResult)
if (!string.IsNullOrWhiteSpace(optionResult.ErrorMessage))
{
AddErrorToResult(optionResult, new ParseError(optionResult.ErrorMessage!, optionResult));

return;
}
}
}
Expand Down

0 comments on commit 7e5307c

Please sign in to comment.