Skip to content

Commit 29c4d22

Browse files
jonsequiturjonsequitur
authored
fix #1556 (#1560)
* fix #1556 * improve tests
1 parent e3e3aa6 commit 29c4d22

File tree

9 files changed

+141
-63
lines changed

9 files changed

+141
-63
lines changed

‎src/System.CommandLine.ApiCompatibility.Tests/ApiCompatibilityApprovalTests.System_CommandLine_api_is_not_changed.approved.txt

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -514,7 +514,6 @@ System.CommandLine.Parsing
514514
public static System.Boolean op_Equality(Token left, Token right)
515515
public static System.Boolean op_Inequality(Token left, Token right)
516516
.ctor(System.String value, TokenType type, System.CommandLine.Symbol symbol)
517-
public System.CommandLine.Symbol Symbol { get; }
518517
public TokenType Type { get; }
519518
public System.String Value { get; }
520519
public System.Boolean Equals(System.Object obj)

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

Lines changed: 21 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -1419,53 +1419,55 @@ public void Parse_can_be_called_with_null_args()
14191419
[Fact]
14201420
public void Command_argument_arity_can_be_a_fixed_value_greater_than_1()
14211421
{
1422+
var argument = new Argument
1423+
{
1424+
Arity = new ArgumentArity(3, 3)
1425+
};
14221426
var command = new Command("the-command")
14231427
{
1424-
new Argument
1425-
{
1426-
Arity = new ArgumentArity(3, 3)
1427-
}
1428+
argument
14281429
};
14291430

14301431
command.Parse("1 2 3")
14311432
.CommandResult
14321433
.Tokens
14331434
.Should()
14341435
.BeEquivalentTo(
1435-
new Token("1", TokenType.Argument, default),
1436-
new Token("2", TokenType.Argument, default),
1437-
new Token("3", TokenType.Argument, default));
1436+
new Token("1", TokenType.Argument, argument),
1437+
new Token("2", TokenType.Argument, argument),
1438+
new Token("3", TokenType.Argument, argument));
14381439
}
14391440

14401441
[Fact]
14411442
public void Command_argument_arity_can_be_a_range_with_a_lower_bound_greater_than_1()
14421443
{
1444+
var argument = new Argument
1445+
{
1446+
Arity = new ArgumentArity(3, 5)
1447+
};
14431448
var command = new Command("the-command")
14441449
{
1445-
new Argument
1446-
{
1447-
Arity = new ArgumentArity(3, 5)
1448-
}
1450+
argument
14491451
};
14501452

14511453
command.Parse("1 2 3")
14521454
.CommandResult
14531455
.Tokens
14541456
.Should()
14551457
.BeEquivalentTo(
1456-
new Token("1", TokenType.Argument, default),
1457-
new Token("2", TokenType.Argument, default),
1458-
new Token("3", TokenType.Argument, default));
1458+
new Token("1", TokenType.Argument, argument),
1459+
new Token("2", TokenType.Argument, argument),
1460+
new Token("3", TokenType.Argument, argument));
14591461
command.Parse("1 2 3 4 5")
14601462
.CommandResult
14611463
.Tokens
14621464
.Should()
14631465
.BeEquivalentTo(
1464-
new Token("1", TokenType.Argument, default),
1465-
new Token("2", TokenType.Argument, default),
1466-
new Token("3", TokenType.Argument, default),
1467-
new Token("4", TokenType.Argument, default),
1468-
new Token("5", TokenType.Argument, default));
1466+
new Token("1", TokenType.Argument, argument),
1467+
new Token("2", TokenType.Argument, argument),
1468+
new Token("3", TokenType.Argument, argument),
1469+
new Token("4", TokenType.Argument, argument),
1470+
new Token("5", TokenType.Argument, argument));
14691471
}
14701472

14711473
[Fact]

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

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -84,6 +84,56 @@ public void When_FromAmong_is_used_then_the_ArgumentResult_ErrorMessage_is_set()
8484
.NotBeNull();
8585
}
8686

87+
[Fact] // https://github.com/dotnet/command-line-api/issues/1556
88+
public void When_FromAmong_is_used_for_multiple_arguments_and_valid_input_is_provided_then_there_are_no_errors()
89+
{
90+
var command = new Command("set")
91+
{
92+
new Argument<string>("key").FromAmong("key1", "key2"),
93+
new Argument<string>("value").FromAmong("value1", "value2")
94+
};
95+
96+
var result = command.Parse("set key1 value1");
97+
98+
result.Errors.Should().BeEmpty();
99+
}
100+
101+
[Fact]
102+
public void When_FromAmong_is_used_for_multiple_arguments_and_invalid_input_is_provided_for_the_first_one_then_the_error_is_informative()
103+
{
104+
var command = new Command("set")
105+
{
106+
new Argument<string>("key").FromAmong("key1", "key2"),
107+
new Argument<string>("value").FromAmong("value1", "value2")
108+
};
109+
110+
var result = command.Parse("set not-key1 value1");
111+
112+
result.Errors.Should().ContainSingle()
113+
.Which
114+
.Message
115+
.Should()
116+
.Be(LocalizationResources.Instance.UnrecognizedArgument("not-key1", new[] { "key1", "key2" }));
117+
}
118+
119+
[Fact]
120+
public void When_FromAmong_is_used_for_multiple_arguments_and_invalid_input_is_provided_for_the_second_one_then_the_error_is_informative()
121+
{
122+
var command = new Command("set")
123+
{
124+
new Argument<string>("key").FromAmong("key1", "key2"),
125+
new Argument<string>("value").FromAmong("value1", "value2")
126+
};
127+
128+
var result = command.Parse("set key1 not-value1");
129+
130+
result.Errors.Should().ContainSingle()
131+
.Which
132+
.Message
133+
.Should()
134+
.Be(LocalizationResources.Instance.UnrecognizedArgument("not-value1", new[] { "value1", "value2" }));
135+
}
136+
87137
[Fact]
88138
public void When_a_required_argument_is_not_supplied_then_an_error_is_returned()
89139
{

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

Lines changed: 10 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -169,8 +169,13 @@ private void VisitCommandArgumentNode(CommandArgumentNode argumentNode)
169169
AddToResult(argumentResult);
170170
}
171171

172-
argumentResult.AddToken(argumentNode.Token);
173-
_innermostCommandResult?.AddToken(argumentNode.Token);
172+
var token = argumentNode.Token.Symbol is null
173+
? new Token(argumentNode.Token.Value, TokenType.Argument, argumentResult.Argument)
174+
: argumentNode.Token;
175+
176+
argumentResult.AddToken(token);
177+
178+
_innermostCommandResult?.AddToken(token);
174179
}
175180

176181
private void VisitOptionNode(OptionNode optionNode)
@@ -446,9 +451,9 @@ private void ValidateAndConvertOptionResult(OptionResult optionResult)
446451

447452
if (optionResult.Children.Count == 0)
448453
{
449-
if (optionResult.Option.Argument is Argument { HasCustomParser: true })
454+
if (optionResult.Option.Argument is { HasCustomParser: true })
450455
{
451-
if (optionResult.Option is Option opt)
456+
if (optionResult.Option is { } opt)
452457
{
453458
var argResult = optionResult.GetOrCreateDefaultArgumentResult(opt.Argument);
454459
optionResult.AddChild(argResult);
@@ -471,7 +476,7 @@ private void ValidateAndConvertOptionResult(OptionResult optionResult)
471476

472477
private void ValidateAndConvertArgumentResult(ArgumentResult argumentResult)
473478
{
474-
if (argumentResult.Argument is Argument argument)
479+
if (argumentResult.Argument is { } argument)
475480
{
476481
var parseError =
477482
argumentResult.Parent?.UnrecognizedArgumentError(argument) ??

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

Lines changed: 37 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -29,15 +29,21 @@ internal static int IndexOfCaseInsensitive(
2929
internal static string RemovePrefix(this string alias)
3030
{
3131
int prefixLength = GetPrefixLength(alias);
32-
return prefixLength > 0 ? alias.Substring(prefixLength) : alias;
32+
return prefixLength > 0
33+
? alias.Substring(prefixLength)
34+
: alias;
3335
}
3436

3537
internal static int GetPrefixLength(this string alias)
3638
{
3739
if (alias[0] == '-')
40+
{
3841
return alias.Length > 1 && alias[1] == '-' ? 2 : 1;
42+
}
3943
if (alias[0] == '/')
44+
{
4045
return 1;
46+
}
4147

4248
return 0;
4349
}
@@ -62,7 +68,7 @@ internal static TokenizeResult Tokenize(
6268
{
6369
var errorList = new List<TokenizeError>();
6470

65-
Command? currentCommand = null;
71+
Command currentCommand = configuration.RootCommand;
6672
var foundDoubleDash = false;
6773
var foundEndOfDirectives = !configuration.EnableDirectives;
6874
var argList = NormalizeRootCommand(configuration, args);
@@ -82,7 +88,7 @@ internal static TokenizeResult Tokenize(
8288
}
8389
else
8490
{
85-
tokenList.Add(Argument(arg));
91+
tokenList.Add(CommandArgument(arg, currentCommand!));
8692
}
8793
continue;
8894
}
@@ -122,9 +128,9 @@ internal static TokenizeResult Tokenize(
122128

123129
if (knownTokens.TryGetValue(arg, out var token))
124130
{
125-
if (PreviousTokenIsAnOptionExpectingAnArgument())
131+
if (PreviousTokenIsAnOptionExpectingAnArgument(out var option))
126132
{
127-
tokenList.Add(Argument(arg));
133+
tokenList.Add(OptionArgument(arg, option!));
128134
}
129135
else
130136
{
@@ -138,7 +144,7 @@ internal static TokenizeResult Tokenize(
138144
Command cmd = (Command)token.Symbol!;
139145
if (cmd != currentCommand)
140146
{
141-
if (!(currentCommand is null && cmd == configuration.RootCommand))
147+
if (cmd != configuration.RootCommand)
142148
{
143149
knownTokens = cmd.ValidTokens();
144150
}
@@ -154,19 +160,21 @@ internal static TokenizeResult Tokenize(
154160
}
155161
}
156162
}
157-
else if (arg.TrySplitIntoSubtokens(out var first, out var rest)
158-
&& knownTokens.TryGetValue(first, out var subtoken) && subtoken.Type == TokenType.Option)
163+
else if (arg.TrySplitIntoSubtokens(out var first, out var rest) &&
164+
knownTokens.TryGetValue(first, out var subtoken) &&
165+
subtoken.Type == TokenType.Option)
159166
{
160167
tokenList.Add(Option(first, (Option)subtoken.Symbol!));
161168

162-
if (rest is { })
169+
if (rest is not null)
163170
{
164171
tokenList.Add(Argument(rest));
165172
}
166173
}
167-
else if (configuration.EnablePosixBundling && CanBeUnbundled(arg) && TryUnbundle(arg.AsSpan(1), i))
174+
else if (configuration.EnablePosixBundling &&
175+
CanBeUnbundled(arg) &&
176+
TryUnbundle(arg.AsSpan(1), i))
168177
{
169-
continue;
170178
}
171179
else if (arg.Length > 0)
172180
{
@@ -175,6 +183,10 @@ internal static TokenizeResult Tokenize(
175183

176184
Token Argument(string value) => new(value, TokenType.Argument, default, i);
177185

186+
Token CommandArgument(string value, Command command) => new(value, TokenType.Argument, command, i);
187+
188+
Token OptionArgument(string value, Option option) => new(value, TokenType.Argument, option, i);
189+
178190
Token Command(string value, Command cmd) => new(value, TokenType.Command, cmd, i);
179191

180192
Token Option(string value, Option option) => new(value, TokenType.Option, option, i);
@@ -189,11 +201,11 @@ internal static TokenizeResult Tokenize(
189201
return new TokenizeResult(tokenList, errorList);
190202

191203
bool CanBeUnbundled(string arg)
192-
=> arg.Length > 2
204+
=> arg.Length > 2
193205
&& arg[0] == '-'
194206
&& char.IsLetter(arg[1]) // don't check for "--" prefixed args
195207
&& arg[2] != ':' && arg[2] != '=' // handled by TrySplitIntoSubtokens
196-
&& !PreviousTokenIsAnOptionExpectingAnArgument();
208+
&& !PreviousTokenIsAnOptionExpectingAnArgument(out _);
197209

198210
bool TryUnbundle(ReadOnlySpan<char> alias, int argumentIndex)
199211
{
@@ -258,25 +270,23 @@ void RevertTokens(int lastValidIndex)
258270
}
259271
}
260272

261-
bool PreviousTokenIsAnOptionExpectingAnArgument()
273+
bool PreviousTokenIsAnOptionExpectingAnArgument(out Option? option)
262274
{
263-
if (tokenList.Count <= 1)
264-
{
265-
return false;
266-
}
267-
268-
var token = tokenList[tokenList.Count - 1];
269-
270-
if (token.Type != TokenType.Option)
275+
if (tokenList.Count > 1)
271276
{
272-
return false;
273-
}
277+
var token = tokenList[tokenList.Count - 1];
274278

275-
if (((Option)token.Symbol!).IsGreedy)
276-
{
277-
return true;
279+
if (token.Type == TokenType.Option)
280+
{
281+
if (token.Symbol is Option { IsGreedy: true } opt)
282+
{
283+
option = opt;
284+
return true;
285+
}
286+
}
278287
}
279288

289+
option = null;
280290
return false;
281291
}
282292

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

Lines changed: 10 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -2,8 +2,6 @@
22
// Licensed under the MIT license. See LICENSE file in the project root for full license information.
33

44
using System.Collections.Generic;
5-
using System.CommandLine.Collections;
6-
using System.CommandLine.Completions;
75
using System.Linq;
86

97
namespace System.CommandLine.Parsing
@@ -13,7 +11,7 @@ namespace System.CommandLine.Parsing
1311
/// </summary>
1412
public abstract class SymbolResult
1513
{
16-
private readonly List<SymbolResult> _children = new List<SymbolResult>();
14+
private readonly List<SymbolResult> _children = new();
1715
private protected readonly List<Token> _tokens = new();
1816
private LocalizationResources? _resources;
1917
private readonly Dictionary<Argument, ArgumentResult> _defaultArgumentValues = new();
@@ -25,7 +23,7 @@ private protected SymbolResult(
2523
Symbol = symbol ?? throw new ArgumentNullException(nameof(symbol));
2624

2725
Parent = parent;
28-
26+
2927
Root = parent?.Root;
3028
}
3129

@@ -147,12 +145,15 @@ internal ArgumentResult GetOrCreateDefaultArgumentResult(Argument argument) =>
147145
for (var i = 0; i < Tokens.Count; i++)
148146
{
149147
var token = Tokens[i];
150-
if (!argument.AllowedValues.Contains(token.Value))
148+
149+
if (token.Symbol is null || token.Symbol == argument)
151150
{
152-
return new ParseError(
153-
LocalizationResources
154-
.UnrecognizedArgument(token.Value, argument.AllowedValues),
155-
this);
151+
if (!argument.AllowedValues.Contains(token.Value))
152+
{
153+
return new ParseError(
154+
LocalizationResources.UnrecognizedArgument(token.Value, argument.AllowedValues),
155+
this);
156+
}
156157
}
157158
}
158159
}

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,7 @@ internal Token(string? value, TokenType type, Symbol? symbol, int position)
4848
/// <summary>
4949
/// The Symbol represented by the token (if any).
5050
/// </summary>
51-
public Symbol? Symbol { get; }
51+
internal Symbol? Symbol { get; }
5252

5353
/// <inheritdoc />
5454
public override bool Equals(object obj) => obj is Token other && Equals(other);

‎src/System.CommandLine/System.CommandLine.csproj

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@
66
<TargetFramework>netstandard2.0</TargetFramework>
77
<Nullable>enable</Nullable>
88
<AllowUnsafeBlocks>true</AllowUnsafeBlocks>
9-
9+
<LangVersion>10</LangVersion>
1010
<Description>This package includes a powerful command line parser and other tools for building command line applications, including:
1111

1212
* Shell-agnostic support for command line completions

0 commit comments

Comments
 (0)