Skip to content

Commit 34a2df4

Browse files
jonsequiturjonsequitur
authored
Differentiate symbol types (#1044)
* extract IIdentifierSymbol from ISymbol so that arguments don't have alias methods * rename a few tests, add test: argument name not among suggestions
1 parent 9ed8edc commit 34a2df4

22 files changed

+200
-134
lines changed

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

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,24 @@
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+
5+
using FluentAssertions;
6+
using Xunit;
7+
8+
namespace System.CommandLine.Tests
9+
{
10+
public abstract class NamedSymbolTests : SymbolTests
11+
{
12+
[Fact]
13+
public void When_Name_is_changed_then_old_name_is_not_among_aliases()
14+
{
15+
var symbol = (NamedSymbol) CreateSymbol("original");
16+
17+
symbol.Name = "changed";
18+
19+
symbol.HasAlias("original").Should().BeFalse();
20+
symbol.Aliases.Should().NotContain("original");
21+
symbol.Aliases.Should().NotContain("original");
22+
}
23+
}
24+
}

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

Lines changed: 29 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@ public SuggestionTests(ITestOutputHelper output)
2222
}
2323

2424
[Fact]
25-
public void Option_Suggest_returns_argument_suggestions_if_configured()
25+
public void Option_GetSuggestions_returns_argument_suggestions_if_configured()
2626
{
2727
var option = new Option("--hello")
2828
{
@@ -39,7 +39,7 @@ public void Option_Suggest_returns_argument_suggestions_if_configured()
3939
}
4040

4141
[Fact]
42-
public void Command_Suggest_returns_available_option_aliases()
42+
public void Command_GetSuggestions_returns_available_option_aliases()
4343
{
4444
IReadOnlyCollection<Symbol> symbols = new[] {
4545
new Option("--one", "option one"),
@@ -64,7 +64,7 @@ public void Command_Suggest_returns_available_option_aliases()
6464
}
6565

6666
[Fact]
67-
public void Command_Suggest_returns_available_subcommands()
67+
public void Command_GetSuggestions_returns_available_subcommands()
6868
{
6969
var command = new Command("command")
7070
{
@@ -79,7 +79,7 @@ public void Command_Suggest_returns_available_subcommands()
7979
}
8080

8181
[Fact]
82-
public void Command_Suggest_returns_available_subcommands_and_option_aliases()
82+
public void Command_GetSuggestions_returns_available_subcommands_and_option_aliases()
8383
{
8484
var command = new Command("command")
8585
{
@@ -93,17 +93,17 @@ public void Command_Suggest_returns_available_subcommands_and_option_aliases()
9393
}
9494

9595
[Fact]
96-
public void Command_Suggest_returns_available_subcommands_and_option_aliases_and_configured_arguments()
96+
public void Command_GetSuggestions_returns_available_subcommands_and_option_aliases_and_configured_arguments()
9797
{
9898
var command = new Command("command")
9999
{
100100
new Command("subcommand", "subcommand"),
101101
new Option("--option", "option"),
102102
new Argument
103-
{
104-
Arity = ArgumentArity.OneOrMore,
105-
Suggestions = { "command-argument" }
106-
}
103+
{
104+
Arity = ArgumentArity.OneOrMore,
105+
Suggestions = { "command-argument" }
106+
}
107107
};
108108

109109
var suggestions = command.GetSuggestions();
@@ -113,7 +113,7 @@ public void Command_Suggest_returns_available_subcommands_and_option_aliases_and
113113
}
114114

115115
[Fact]
116-
public void Command_Suggest_without_text_to_match_orders_alphabetically()
116+
public void Command_GetSuggestions_without_text_to_match_orders_alphabetically()
117117
{
118118
var command = new Command("command")
119119
{
@@ -128,7 +128,20 @@ public void Command_Suggest_without_text_to_match_orders_alphabetically()
128128
}
129129

130130
[Fact]
131-
public void Command_Suggest_with_text_to_match_orders_by_match_position_then_alphabetically()
131+
public void Command_GetSuggestions_does_not_return_argument_names()
132+
{
133+
var command = new Command("command")
134+
{
135+
new Argument("the-argument")
136+
};
137+
138+
var suggestions = command.GetSuggestions();
139+
140+
suggestions.Should().NotContain("the-argument");
141+
}
142+
143+
[Fact]
144+
public void Command_GetSuggestions_with_text_to_match_orders_by_match_position_then_alphabetically()
132145
{
133146
var command = new Command("command")
134147
{
@@ -171,7 +184,7 @@ public void When_an_option_has_a_default_value_it_will_still_be_suggested()
171184
}
172185

173186
[Fact]
174-
public void Command_suggest_can_access_ParseResult()
187+
public void Command_Getsuggestions_can_access_ParseResult()
175188
{
176189
var parser = new Parser(
177190
new Option("--origin")
@@ -203,7 +216,7 @@ public void Command_suggest_can_access_ParseResult()
203216
}
204217

205218
[Fact]
206-
public void Command_suggest_can_access_ParseResult_reverse_order()
219+
public void Command_Getsuggestions_can_access_ParseResult_reverse_order()
207220
{
208221
var parser = new Parser(
209222
new Option("--origin")
@@ -510,7 +523,7 @@ public void Both_subcommands_and_options_are_available_as_suggestions()
510523
[Theory(Skip = "Needs discussion, Issue #19")]
511524
[InlineData("outer ")]
512525
[InlineData("outer -")]
513-
public void Option_suggestions_are_not_provided_without_matching_prefix(string input)
526+
public void Option_Getsuggestionsions_are_not_provided_without_matching_prefix(string input)
514527
{
515528
var command = new Command("outer")
516529
{
@@ -526,7 +539,7 @@ public void Option_suggestions_are_not_provided_without_matching_prefix(string i
526539
}
527540

528541
[Fact]
529-
public void Option_suggestions_can_be_based_on_the_proximate_option()
542+
public void Option_Getsuggestionsions_can_be_based_on_the_proximate_option()
530543
{
531544
var parser = new Parser(
532545
new Command("outer")
@@ -573,7 +586,7 @@ public void Argument_suggestions_can_be_based_on_the_proximate_option()
573586
}
574587

575588
[Fact]
576-
public void Option_suggestions_can_be_based_on_the_proximate_option_and_partial_input()
589+
public void Option_Getsuggestionsions_can_be_based_on_the_proximate_option_and_partial_input()
577590
{
578591
var parser = new Parser(
579592
new Command("outer")

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

Lines changed: 0 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -18,18 +18,6 @@ public void When_Name_is_explicitly_set_then_adding_aliases_does_not_change_it()
1818
symbol.Name.Should().Be("changed");
1919
}
2020

21-
[Fact]
22-
public void When_Name_is_changed_then_old_name_is_not_among_aliases()
23-
{
24-
var symbol = CreateSymbol("original");
25-
26-
symbol.Name = "changed";
27-
28-
symbol.HasAlias("original").Should().BeFalse();
29-
symbol.Aliases.Should().NotContain("original");
30-
symbol.Aliases.Should().NotContain("original");
31-
}
32-
3321
protected abstract Symbol CreateSymbol(string name);
3422
}
3523
}

‎src/System.CommandLine/Argument.cs

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -27,8 +27,6 @@ public Argument(string name)
2727
{
2828
Name = name!;
2929
}
30-
31-
AddAliasInner(name);
3230
}
3331

3432
internal HashSet<string>? AllowedValues { get; private set; }

‎src/System.CommandLine/Binding/Binder.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@ internal static bool IsMatch(this string parameterName, string alias) =>
1313
parameterName,
1414
StringComparison.OrdinalIgnoreCase);
1515

16-
internal static bool IsMatch(this string parameterName, ISymbol symbol) =>
16+
internal static bool IsMatch(this string parameterName, IOption symbol) =>
1717
parameterName.IsMatch(symbol.Name) ||
1818
symbol.HasAlias(parameterName);
1919

‎src/System.CommandLine/Binding/FailedArgumentTypeConversionResult.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@ private static string FormatErrorMessage(
2525
{
2626
// TODO: (FailedArgumentTypeConversionResult) localize
2727

28-
var firstParent = a.Parents[0];
28+
var firstParent = (IIdentifierSymbol) a.Parents[0];
2929

3030
var symbolType =
3131
firstParent switch {

‎src/System.CommandLine/Collections/SymbolSet.cs

Lines changed: 17 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -44,30 +44,29 @@ internal bool IsAnyAliasInUse(
4444
{
4545
EnsureAliasIndexIsCurrent();
4646

47-
if (item is IArgument)
47+
if (item is IIdentifierSymbol identifier)
4848
{
49-
// arguments don't have aliases so match based on Name
50-
for (var i = 0; i < Items.Count; i++)
49+
var aliases = identifier.Aliases.ToArray();
50+
51+
for (var i = 0; i < aliases.Length; i++)
5152
{
52-
var existing = Items[i];
53-
if (string.Equals(item.Name, existing.Name, StringComparison.Ordinal))
53+
var alias = aliases[i];
54+
55+
if (ItemsByAlias.ContainsKey(alias))
5456
{
55-
aliasAlreadyInUse = existing.Name;
57+
aliasAlreadyInUse = alias;
5658
return true;
5759
}
5860
}
5961
}
6062
else
6163
{
62-
var itemRawAliases = item.Aliases.ToArray();
63-
64-
for (var i = 0; i < itemRawAliases.Length; i++)
64+
for (var i = 0; i < Items.Count; i++)
6565
{
66-
var alias = itemRawAliases[i];
67-
68-
if (ItemsByAlias.ContainsKey(alias))
66+
var existing = Items[i];
67+
if (string.Equals(item.Name, existing.Name, StringComparison.Ordinal))
6968
{
70-
aliasAlreadyInUse = alias;
69+
aliasAlreadyInUse = existing.Name;
7170
return true;
7271
}
7372
}
@@ -87,6 +86,10 @@ internal void ThrowIfAnyAliasIsInUse(ISymbol item)
8786
}
8887

8988
protected override IReadOnlyCollection<string> GetAliases(ISymbol item) =>
90-
item.Aliases;
89+
item switch
90+
{
91+
IIdentifierSymbol named => named.Aliases,
92+
_ => new[] { item.Name }
93+
};
9194
}
9295
}

‎src/System.CommandLine/Command.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@
1111
namespace System.CommandLine
1212
{
1313
public class Command :
14-
Symbol,
14+
NamedSymbol,
1515
ICommand,
1616
IEnumerable<Symbol>
1717
{

‎src/System.CommandLine/CommandLineConfiguration.cs

Lines changed: 14 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@ public class CommandLineConfiguration
1919

2020
public CommandLineConfiguration(
2121
IReadOnlyCollection<Symbol> symbols,
22-
IReadOnlyCollection<char>? argumentDelimiters = null,
22+
IReadOnlyList<char>? argumentDelimiters = null,
2323
bool enablePosixBundling = true,
2424
bool enableDirectives = true,
2525
ValidationMessages? validationMessages = null,
@@ -39,26 +39,30 @@ public CommandLineConfiguration(
3939

4040
if (argumentDelimiters is null)
4141
{
42-
ArgumentDelimitersInternal = new HashSet<char>
42+
ArgumentDelimitersInternal = new []
4343
{
44-
':',
44+
':',
4545
'='
4646
};
4747
}
4848
else
4949
{
50-
ArgumentDelimitersInternal = new HashSet<char>(argumentDelimiters);
50+
ArgumentDelimitersInternal = argumentDelimiters.Distinct().ToArray();
5151
}
5252

5353
foreach (var symbol in symbols)
5454
{
55-
foreach (var alias in symbol.Aliases)
55+
if (symbol is IIdentifierSymbol identifier)
5656
{
57-
foreach (var delimiter in ArgumentDelimiters)
57+
foreach (var alias in identifier.Aliases)
5858
{
59-
if (alias.Contains(delimiter))
59+
for (var i = 0; i < ArgumentDelimiters.Count; i++)
6060
{
61-
throw new ArgumentException($"{symbol.GetType().Name} \"{alias}\" is not allowed to contain a delimiter but it contains \"{delimiter}\"");
61+
var delimiter = ArgumentDelimiters[i];
62+
if (alias.Contains(delimiter))
63+
{
64+
throw new ArgumentException($"{symbol.GetType().Name} \"{alias}\" is not allowed to contain a delimiter but it contains \"{delimiter}\"");
65+
}
6266
}
6367
}
6468
}
@@ -123,9 +127,9 @@ private void AddGlobalOptionsToChildren(Command parentCommand)
123127

124128
public ISymbolSet Symbols => _symbols;
125129

126-
public IReadOnlyCollection<char> ArgumentDelimiters => ArgumentDelimitersInternal;
130+
public IReadOnlyList<char> ArgumentDelimiters => ArgumentDelimitersInternal;
127131

128-
internal HashSet<char> ArgumentDelimitersInternal { get; }
132+
internal IReadOnlyList<char> ArgumentDelimitersInternal { get; }
129133

130134
public bool EnableDirectives { get; }
131135

‎src/System.CommandLine/Help/HelpBuilder.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -478,7 +478,7 @@ protected virtual string DefaultValueHint(IArgument argument, bool isSingleArgum
478478
_ => ""
479479
};
480480

481-
private IEnumerable<HelpItem> GetOptionHelpItems(ISymbol symbol)
481+
private IEnumerable<HelpItem> GetOptionHelpItems(IIdentifierSymbol symbol)
482482
{
483483
var rawAliases = symbol
484484
.Aliases

‎src/System.CommandLine/ICommand.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@
55

66
namespace System.CommandLine
77
{
8-
public interface ICommand : ISymbol
8+
public interface ICommand : IIdentifierSymbol
99
{
1010
bool TreatUnmatchedTokensAsErrors { get; }
1111

‎src/System.CommandLine/IIdentifierSymbol.cs

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
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 System.Collections.Generic;
5+
6+
namespace System.CommandLine
7+
{
8+
public interface IIdentifierSymbol : ISymbol
9+
{
10+
IReadOnlyCollection<string> Aliases { get; }
11+
12+
bool HasAlias(string alias);
13+
}
14+
}

‎src/System.CommandLine/IOption.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@
55

66
namespace System.CommandLine
77
{
8-
public interface IOption : ISymbol, IValueDescriptor
8+
public interface IOption : IIdentifierSymbol, IValueDescriptor
99
{
1010
IArgument Argument { get; }
1111

‎src/System.CommandLine/ISymbol.cs

Lines changed: 1 addition & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,6 @@
11
// Copyright (c) .NET Foundation and contributors. All rights reserved.
22
// Licensed under the MIT license. See LICENSE file in the project root for full license information.
33

4-
using System.Collections.Generic;
54
using System.CommandLine.Collections;
65
using System.CommandLine.Suggestions;
76

@@ -13,14 +12,10 @@ public interface ISymbol : ISuggestionSource
1312

1413
string? Description { get; }
1514

16-
IReadOnlyCollection<string> Aliases { get; }
17-
18-
bool HasAlias(string alias);
19-
2015
bool IsHidden { get; }
2116

2217
ISymbolSet Children { get; }
2318

2419
ISymbolSet Parents { get; }
2520
}
26-
}
21+
}

0 commit comments

Comments
 (0)