Skip to content

Commit d6d49b0

Browse files
jonsequiturjonsequitur
authored
Allow multiple occurrences of option token to overwrite value when max arity is 1 (#1350)
* R: consolidate MultipleArgumentsPerToken tests into a new file * fix #1083
1 parent a2ea4b1 commit d6d49b0

File tree

8 files changed

+157
-178
lines changed

8 files changed

+157
-178
lines changed

‎src/System.CommandLine.Tests/Binding/TypeConversionTests.cs‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -611,13 +611,13 @@ public void Enum_values_that_cannot_be_parsed_result_in_an_informative_error()
611611
}
612612

613613
[Fact]
614-
public void When_getting_values_and_specifying_a_conversion_type_that_is_not_supported_then_it_throws()
614+
public void When_getting_a_single_value_and_specifying_a_conversion_type_that_is_not_supported_then_it_throws()
615615
{
616-
var option = new Option("-x", arity: ArgumentArity.ZeroOrOne);
616+
var option = new Option<int>("-x");
617617

618618
var result = option.Parse("-x not-an-int");
619619

620-
Action getValue = () => result.ValueForOption<int>("-x");
620+
Action getValue = () => result.ValueForOption(option);
621621

622622
getValue.Should()
623623
.Throw<InvalidOperationException>()
@@ -630,18 +630,18 @@ public void When_getting_values_and_specifying_a_conversion_type_that_is_not_sup
630630
[Fact]
631631
public void When_getting_an_array_of_values_and_specifying_a_conversion_type_that_is_not_supported_then_it_throws()
632632
{
633-
var option = new Option("-x", arity: ArgumentArity.ZeroOrOne);
633+
var option = new Option<int[]>("-x");
634634

635635
var result = option.Parse("-x not-an-int -x 2");
636636

637-
Action getValue = () => result.ValueForOption<int[]>("-x");
637+
Action getValue = () => result.ValueForOption(option);
638638

639639
getValue.Should()
640640
.Throw<InvalidOperationException>()
641641
.Which
642642
.Message
643643
.Should()
644-
.Be("Option '-x' expects a single argument but 2 were provided.");
644+
.Be("Cannot parse argument 'not-an-int' for option '-x' as expected type System.Int32.");
645645
}
646646
}
647647
}

‎src/System.CommandLine.Tests/OptionTests.MultipleArgumentsPerToken.cs‎

Lines changed: 141 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,141 @@
1+
// Copyright (c) .NET Foundation and contributors. All rights reserved.
2+
// Licensed under the MIT license. See LICENSE file in the project root for full license information.
3+
4+
using FluentAssertions;
5+
using System.Linq;
6+
using Xunit;
7+
8+
namespace System.CommandLine.Tests
9+
{
10+
public partial class OptionTests : SymbolTests
11+
{
12+
public class MultipleArgumentsPerToken
13+
{
14+
public class Allowed
15+
{
16+
[Fact]
17+
public void When_option_is_not_respecified_but_limit_is_not_reached_then_the_following_token_is_used_as_value()
18+
{
19+
var animalsOption = new Option(new[] { "-a", "--animals" })
20+
{
21+
AllowMultipleArgumentsPerToken = true,
22+
Arity = ArgumentArity.ZeroOrMore
23+
};
24+
var vegetablesOption = new Option(new[] { "-v", "--vegetables" }) { Arity = ArgumentArity.ZeroOrMore };
25+
26+
var command = new RootCommand
27+
{
28+
animalsOption,
29+
vegetablesOption
30+
};
31+
32+
var result = command.Parse("-a cat dog -v carrot");
33+
34+
result
35+
.FindResultFor(animalsOption)
36+
.Tokens
37+
.Select(t => t.Value)
38+
.Should()
39+
.BeEquivalentTo(new[] { "cat", "dog" });
40+
41+
result
42+
.FindResultFor(vegetablesOption)
43+
.Tokens
44+
.Select(t => t.Value)
45+
.Should()
46+
.BeEquivalentTo("carrot");
47+
48+
result
49+
.UnmatchedTokens
50+
.Should()
51+
.BeNullOrEmpty();
52+
}
53+
54+
[Fact]
55+
public void When_option_is_not_respecified_and_limit_is_reached_then_the_following_token_is_unmatched()
56+
{
57+
var animalsOption = new Option(new[] { "-a", "--animals" })
58+
{
59+
AllowMultipleArgumentsPerToken = true,
60+
Arity = ArgumentArity.ZeroOrOne
61+
};
62+
var vegetablesOption = new Option(new[] { "-v", "--vegetables" }) { Arity = ArgumentArity.ZeroOrMore };
63+
64+
var command = new RootCommand
65+
{
66+
animalsOption,
67+
vegetablesOption
68+
};
69+
70+
var result = command.Parse("-a cat some-arg -v carrot");
71+
72+
result.FindResultFor(animalsOption)
73+
.Tokens
74+
.Select(t => t.Value)
75+
.Should()
76+
.BeEquivalentTo("cat");
77+
78+
result.FindResultFor(vegetablesOption)
79+
.Tokens
80+
.Select(t => t.Value)
81+
.Should()
82+
.BeEquivalentTo("carrot");
83+
84+
result
85+
.UnmatchedTokens
86+
.Should()
87+
.BeEquivalentTo("some-arg");
88+
}
89+
}
90+
91+
public class Disallowed
92+
{
93+
[Fact]
94+
public void Single_option_arg_is_matched()
95+
{
96+
var option = new Option<string[]>("--option") { AllowMultipleArgumentsPerToken = false };
97+
var command = new Command("the-command") { option };
98+
99+
var result = command.Parse("--option 1 2");
100+
101+
var value = result.ValueForOption(option);
102+
103+
value.Should().BeEquivalentTo(new[] { "1" });
104+
result.UnmatchedTokens.Should().BeEquivalentTo(new[] { "2" });
105+
}
106+
107+
[Fact]
108+
public void When_max_arity_is_greater_than_1_then_multiple_option_args_are_matched()
109+
{
110+
var option = new Option<string[]>("--option") { AllowMultipleArgumentsPerToken = false };
111+
var command = new Command("the-command") { option };
112+
113+
var result = command.Parse("--option 1 --option 2");
114+
115+
var value = result.ValueForOption(option);
116+
117+
value.Should().BeEquivalentTo(new[] { "1", "2" });
118+
}
119+
120+
[Theory]
121+
[InlineData("--option 1 --option 2")]
122+
[InlineData("xyz --option 1 --option 2")]
123+
[InlineData("--option 1 xyz --option 2")]
124+
public void When_max_arity_is_1_then_subsequent_option_args_overwrite_its_value(string commandLine)
125+
{
126+
var option = new Option<string>("--option") { AllowMultipleArgumentsPerToken = false };
127+
var command = new Command("the-command") {
128+
option,
129+
new Argument<string>()
130+
};
131+
132+
var result = command.Parse(commandLine);
133+
134+
var value = result.ValueForOption(option);
135+
136+
value.Should().Be("2");
137+
}
138+
}
139+
}
140+
}
141+
}

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

Lines changed: 1 addition & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@
99

1010
namespace System.CommandLine.Tests
1111
{
12-
public class OptionTests : SymbolTests
12+
public partial class OptionTests : SymbolTests
1313
{
1414
[Fact]
1515
public void When_an_option_has_only_one_alias_then_that_alias_is_its_name()
@@ -443,33 +443,6 @@ public void Option_of_ICollection_of_T_defaults_to_empty_when_not_specified()
443443
.BeEmpty();
444444
}
445445

446-
[Fact]
447-
public void Single_option_arg_is_matched_when_disallowing_multiple_args_per_option_token()
448-
{
449-
var option = new Option<string[]>("--option") { AllowMultipleArgumentsPerToken = false };
450-
var command = new Command("the-command") { option };
451-
452-
var result = command.Parse("--option 1 2");
453-
454-
var optionResult = result.ValueForOption(option);
455-
456-
optionResult.Should().BeEquivalentTo(new[] { "1" });
457-
result.UnmatchedTokens.Should().BeEquivalentTo(new[] { "2" });
458-
}
459-
460-
[Fact]
461-
public void Multiple_option_args_are_matched_with_multiple_option_tokens_when_disallowing_multiple_args_per_option_token()
462-
{
463-
var option = new Option<string[]>("--option") { AllowMultipleArgumentsPerToken = false };
464-
var command = new Command("the-command") { option };
465-
466-
var result = command.Parse("--option 1 --option 2");
467-
468-
var optionResult = result.ValueForOption(option);
469-
470-
optionResult.Should().BeEquivalentTo(new[] { "1", "2" });
471-
}
472-
473446
[Fact]
474447
public void When_Name_is_set_to_its_current_value_then_it_is_not_removed_from_aliases()
475448
{

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

Lines changed: 0 additions & 110 deletions
Original file line numberDiff line numberDiff line change
@@ -450,116 +450,6 @@ public void Options_can_be_specified_multiple_times_and_their_arguments_are_coll
450450
.BeEquivalentTo("carrot");
451451
}
452452

453-
[Fact]
454-
public void When_a_Parser_root_option_is_not_respecified_but_limit_is_not_reached_then_the_following_token_is_used_as_value()
455-
{
456-
var animalsOption = new Option(new[] { "-a", "--animals" })
457-
{
458-
AllowMultipleArgumentsPerToken = true,
459-
Arity = ArgumentArity.ZeroOrMore
460-
};
461-
var vegetablesOption = new Option(new[] { "-v", "--vegetables" }) { Arity = ArgumentArity.ZeroOrMore };
462-
463-
var parser = new Parser(
464-
animalsOption,
465-
vegetablesOption);
466-
467-
var result = parser.Parse("-a cat dog -v carrot");
468-
469-
result
470-
.FindResultFor(animalsOption)
471-
.Tokens
472-
.Select(t => t.Value)
473-
.Should()
474-
.BeEquivalentTo(new[] { "cat", "dog" });
475-
476-
result
477-
.FindResultFor(vegetablesOption)
478-
.Tokens
479-
.Select(t => t.Value)
480-
.Should()
481-
.BeEquivalentTo("carrot");
482-
483-
result
484-
.UnmatchedTokens
485-
.Should()
486-
.BeNullOrEmpty();
487-
}
488-
489-
[Fact]
490-
public void When_a_Parser_root_option_is_not_respecified_and_limit_is_reached_then_the_following_token_is_unmatched()
491-
{
492-
var animalsOption = new Option(new[] { "-a", "--animals" })
493-
{
494-
AllowMultipleArgumentsPerToken = true,
495-
Arity = ArgumentArity.ZeroOrOne
496-
};
497-
var vegetablesOption = new Option(new[] { "-v", "--vegetables" }) { Arity = ArgumentArity.ZeroOrMore };
498-
499-
var parser = new Parser(
500-
animalsOption,
501-
vegetablesOption);
502-
503-
ParseResult result = parser.Parse("-a cat some-arg -v carrot");
504-
505-
result.FindResultFor(animalsOption)
506-
.Tokens
507-
.Select(t => t.Value)
508-
.Should()
509-
.BeEquivalentTo("cat");
510-
511-
result.FindResultFor(vegetablesOption)
512-
.Tokens
513-
.Select(t => t.Value)
514-
.Should()
515-
.BeEquivalentTo("carrot");
516-
517-
result
518-
.UnmatchedTokens
519-
.Should()
520-
.BeEquivalentTo("some-arg");
521-
}
522-
523-
[Fact]
524-
public void When_an_option_is_not_respecified_but_limit_is_not_reached_then_the_following_token_is_considered_as_value()
525-
{
526-
var animalsOption = new Option(new[] { "-a", "--animals" })
527-
{
528-
AllowMultipleArgumentsPerToken = true,
529-
Arity = ArgumentArity.ZeroOrMore
530-
};
531-
var vegetablesOption = new Option(new[] { "-v", "--vegetables" }) { Arity = ArgumentArity.ZeroOrMore };
532-
var parser = new Parser(
533-
new Command("the-command")
534-
{
535-
animalsOption,
536-
vegetablesOption
537-
},
538-
new Argument
539-
{
540-
Arity = ArgumentArity.ZeroOrMore
541-
});
542-
543-
var result = parser.Parse("the-command -a cat dog -v carrot");
544-
545-
result.FindResultFor(animalsOption)
546-
.Tokens
547-
.Select(t => t.Value)
548-
.Should()
549-
.BeEquivalentTo("cat", "dog");
550-
551-
result.FindResultFor(vegetablesOption)
552-
.Tokens
553-
.Select(t => t.Value)
554-
.Should()
555-
.BeEquivalentTo("carrot");
556-
557-
result.CommandResult
558-
.Tokens
559-
.Should()
560-
.BeNullOrEmpty();
561-
}
562-
563453
[Fact]
564454
public void When_an_option_is_not_respecified_but_limit_is_reached_then_the_following_token_is_considered_an_argument_to_the_parent_command()
565455
{

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

Lines changed: 0 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -891,34 +891,6 @@ public void A_command_with_subcommands_is_valid_to_invoke_if_it_has_a_handler()
891891
result.CommandResult.Command.Should().Be(inner);
892892
}
893893

894-
[Fact]
895-
public void When_an_option_is_specified_more_than_once_but_only_allowed_once_then_an_informative_error_is_returned()
896-
{
897-
var parser = new Parser(
898-
new Option("-x", arity: ArgumentArity.ExactlyOne));
899-
900-
var result = parser.Parse("-x 1 -x 2");
901-
902-
result.Errors
903-
.Select(e => e.Message)
904-
.Should()
905-
.Contain("Option '-x' expects a single argument but 2 were provided.");
906-
}
907-
908-
[Fact]
909-
public void When_arity_is_ExactlyOne_it_validates_against_extra_arguments()
910-
{
911-
var parser = new Parser(
912-
new Option<int>("-x"));
913-
914-
var result = parser.Parse("-x 1 -x 2");
915-
916-
result.Errors
917-
.Select(e => e.Message)
918-
.Should()
919-
.Contain("Option '-x' expects a single argument but 2 were provided.");
920-
}
921-
922894
[Fact]
923895
public void When_an_option_has_a_default_value_it_is_not_valid_to_specify_the_option_without_an_argument()
924896
{

‎src/System.CommandLine/ArgumentArity.cs‎

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -76,9 +76,12 @@ public ArgumentArity(int minimumNumberOfValues, int maximumNumberOfValues)
7676

7777
if (tokenCount > maximumNumberOfValues)
7878
{
79-
return new TooManyArgumentsConversionResult(
80-
argument,
81-
symbolResult!.Resources.ExpectsOneArgument(symbolResult));
79+
if (symbolResult is not OptionResult { Option: { AllowMultipleArgumentsPerToken: false } })
80+
{
81+
return new TooManyArgumentsConversionResult(
82+
argument,
83+
symbolResult!.Resources.ExpectsOneArgument(symbolResult));
84+
}
8285
}
8386

8487
return null;

0 commit comments

Comments
 (0)