Skip to content

Commit daec22f

Browse files
authored
More SymbolResult improvements (#2027)
* remove RootCommandResult (internal), introduce SymbolResultTree (internal) and make all virtual SymbolResult methods non-virtual and delegating to SymbolResultTree * remove internal SymbolResult.GetChildren, expose SymbolResult.SymbolResultTree as internal field, optimize SymbolResultTree.GetChildren for AgumentResult * make SymbolResultTree derive from Dictionary<Symbol, SymbolResult> * it makes SymbolResultTree more responsible and less artificial (subjective) * it removes one allocation (nit) * update the token with missing information during parsing, so later stages don't need to modify it * refactor the code so root command and inner most command are never nulls
1 parent 92bd70e commit daec22f

17 files changed

+126
-144
lines changed

src/System.CommandLine.Tests/ArgumentTests.cs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -278,7 +278,7 @@ public void Option_ArgumentResult_parentage_to_root_symbol_is_set_correctly_when
278278
.Parent
279279
.Parent
280280
.Should()
281-
.BeAssignableTo<CommandResult>()
281+
.BeOfType<CommandResult>()
282282
.Which
283283
.Command
284284
.Should()
@@ -340,7 +340,7 @@ public void Command_ArgumentResult_Parent_is_set_correctly_when_token_is_implici
340340
argumentResult
341341
.Parent
342342
.Should()
343-
.BeAssignableTo<CommandResult>()
343+
.BeOfType<CommandResult>()
344344
.Which
345345
.Command
346346
.Should()

src/System.CommandLine.Tests/CommandTests.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,7 @@ public void Outer_command_is_identified_correctly_by_Parent_property()
4646
.CommandResult
4747
.Parent
4848
.Should()
49-
.BeAssignableTo<CommandResult>()
49+
.BeOfType<CommandResult>()
5050
.Which
5151
.Command
5252
.Name

src/System.CommandLine.Tests/ParserTests.cs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -668,7 +668,7 @@ public void When_options_with_the_same_name_are_defined_on_parent_and_child_comm
668668
result.CommandResult
669669
.Parent
670670
.Should()
671-
.BeAssignableTo<CommandResult>()
671+
.BeOfType<CommandResult>()
672672
.Which
673673
.Children
674674
.Should()
@@ -697,7 +697,7 @@ public void When_options_with_the_same_name_are_defined_on_parent_and_child_comm
697697
result.CommandResult
698698
.Parent
699699
.Should()
700-
.BeAssignableTo<CommandResult>()
700+
.BeOfType<CommandResult>()
701701
.Which
702702
.Children
703703
.Should()
@@ -1008,7 +1008,7 @@ public void Option_and_Command_can_have_the_same_alias()
10081008
.CommandResult
10091009
.Parent
10101010
.Should()
1011-
.BeAssignableTo<CommandResult>()
1011+
.BeOfType<CommandResult>()
10121012
.Which
10131013
.Children
10141014
.Should()

src/System.CommandLine/Argument.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -113,7 +113,7 @@ private protected override string DefaultName
113113
/// <returns>Returns the default value for the argument, if defined. Null otherwise.</returns>
114114
public object? GetDefaultValue()
115115
{
116-
return GetDefaultValue(new ArgumentResult(this, null));
116+
return GetDefaultValue(new ArgumentResult(this, null!, null));
117117
}
118118

119119
internal abstract object? GetDefaultValue(ArgumentResult argumentResult);

src/System.CommandLine/ParseResult.cs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -15,14 +15,14 @@ namespace System.CommandLine
1515
public class ParseResult
1616
{
1717
private readonly IReadOnlyList<ParseError> _errors;
18-
private readonly RootCommandResult _rootCommandResult;
18+
private readonly CommandResult _rootCommandResult;
1919
private readonly IReadOnlyList<Token> _unmatchedTokens;
2020
private Dictionary<string, IReadOnlyList<string>>? _directives;
2121
private CompletionContext? _completionContext;
2222

2323
internal ParseResult(
2424
Parser parser,
25-
RootCommandResult rootCommandResult,
25+
CommandResult rootCommandResult,
2626
CommandResult commandResult,
2727
Dictionary<string, IReadOnlyList<string>>? directives,
2828
List<Token> tokens,

src/System.CommandLine/Parsing/ArgumentResult.cs

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,8 @@ public sealed class ArgumentResult : SymbolResult
1616

1717
internal ArgumentResult(
1818
Argument argument,
19-
SymbolResult? parent) : base(parent)
19+
SymbolResultTree symbolResultTree,
20+
SymbolResult? parent) : base(symbolResultTree, parent)
2021
{
2122
Argument = argument ?? throw new ArgumentNullException(nameof(argument));
2223
}
@@ -110,7 +111,7 @@ Parent is { } &&
110111

111112
if (Parent!.UseDefaultValueFor(argument))
112113
{
113-
var argumentResult = new ArgumentResult(argument, Parent);
114+
var argumentResult = new ArgumentResult(argument, SymbolResultTree, Parent);
114115

115116
var defaultValue = argument.GetDefaultValue(argumentResult);
116117

src/System.CommandLine/Parsing/CommandResult.cs

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -8,13 +8,14 @@ namespace System.CommandLine.Parsing
88
/// <summary>
99
/// A result produced when parsing a <see cref="Command" />.
1010
/// </summary>
11-
public class CommandResult : SymbolResult
11+
public sealed class CommandResult : SymbolResult
1212
{
1313
internal CommandResult(
1414
Command command,
1515
Token token,
16+
SymbolResultTree symbolResultTree,
1617
CommandResult? parent = null) :
17-
base(parent)
18+
base(symbolResultTree, parent)
1819
{
1920
Command = command ?? throw new ArgumentNullException(nameof(command));
2021
Token = token ?? throw new ArgumentNullException(nameof(token));
@@ -33,7 +34,7 @@ internal CommandResult(
3334
/// <summary>
3435
/// Child symbol results in the parse tree.
3536
/// </summary>
36-
public IEnumerable<SymbolResult> Children => GetChildren(this);
37+
public IEnumerable<SymbolResult> Children => SymbolResultTree.GetChildren(this);
3738

3839
internal sealed override int MaximumArgumentCapacity
3940
{

src/System.CommandLine/Parsing/OptionResult.cs

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,9 +15,10 @@ public sealed class OptionResult : SymbolResult
1515

1616
internal OptionResult(
1717
Option option,
18+
SymbolResultTree symbolResultTree,
1819
Token? token = null,
1920
CommandResult? parent = null) :
20-
base(parent)
21+
base(symbolResultTree, parent)
2122
{
2223
Option = option ?? throw new ArgumentNullException(nameof(option));
2324
Token = token;

src/System.CommandLine/Parsing/ParseOperation.cs

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -102,6 +102,12 @@ private void ParseCommandArguments(CommandNode commandNode, ref int currentArgum
102102

103103
if (currentArgumentCount < argument.Arity.MaximumNumberOfValues)
104104
{
105+
if (CurrentToken.Symbol is null)
106+
{
107+
// update the token with missing information now, so later stages don't need to modify it
108+
CurrentToken.Symbol = argument;
109+
}
110+
105111
var argumentNode = new CommandArgumentNode(
106112
CurrentToken,
107113
argument,

src/System.CommandLine/Parsing/ParseResultExtensions.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -159,7 +159,7 @@ private static void Diagram(
159159
builder.Append("[ ");
160160
builder.Append(symbolResult.Token().Value);
161161

162-
foreach (SymbolResult child in symbolResult.GetChildren(symbolResult))
162+
foreach (SymbolResult child in symbolResult.SymbolResultTree.GetChildren(symbolResult))
163163
{
164164
if (child is ArgumentResult arg &&
165165
(arg.Argument.ValueType == typeof(bool) ||

0 commit comments

Comments
 (0)