Skip to content

Commit 01aee74

Browse files
adamsitnikadamsitnik
authored
SymbolResults improvements (#2024)
* LocalizationResources should be readonly and same for entire symbol tree * remove Symbol from SymbolResult, as each derived type defines more specific property anyway * not every SymbolResult can have children, move the property to derived types as well * do not store the list of children per symbol result, use Parent property to find children using symbol dictionary
1 parent 9c48247 commit 01aee74

18 files changed

+204
-225
lines changed

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

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -393,6 +393,7 @@ System.CommandLine.Parsing
393393
public class CommandLineStringSplitter
394394
public System.Collections.Generic.IEnumerable<System.String> Split(System.String commandLine)
395395
public class CommandResult : SymbolResult
396+
public System.Collections.Generic.IEnumerable<SymbolResult> Children { get; }
396397
public System.CommandLine.Command Command { get; }
397398
public Token Token { get; }
398399
public class OptionResult : SymbolResult
@@ -422,11 +423,9 @@ System.CommandLine.Parsing
422423
public static System.Threading.Tasks.Task<System.Int32> InvokeAsync(this Parser parser, System.String[] args, System.CommandLine.IConsole console = null, System.Threading.CancellationToken cancellationToken = null)
423424
public static System.CommandLine.ParseResult Parse(this Parser parser, System.String commandLine)
424425
public abstract class SymbolResult
425-
public System.Collections.Generic.IReadOnlyList<SymbolResult> Children { get; }
426426
public System.String ErrorMessage { get; set; }
427-
public System.CommandLine.LocalizationResources LocalizationResources { get; set; }
427+
public System.CommandLine.LocalizationResources LocalizationResources { get; }
428428
public SymbolResult Parent { get; }
429-
public System.CommandLine.Symbol Symbol { get; }
430429
public System.Collections.Generic.IReadOnlyList<Token> Tokens { get; }
431430
public ArgumentResult FindResultFor(System.CommandLine.Argument argument)
432431
public CommandResult FindResultFor(System.CommandLine.Command command)

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

Lines changed: 16 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -131,7 +131,7 @@ public void Validation_failure_message_can_be_specified_when_parsing_tokens()
131131
argument.Parse("x")
132132
.Errors
133133
.Should()
134-
.ContainSingle(e => e.SymbolResult.Symbol == argument)
134+
.ContainSingle(e => ((ArgumentResult)e.SymbolResult).Argument == argument)
135135
.Which
136136
.Message
137137
.Should()
@@ -150,7 +150,7 @@ public void Validation_failure_message_can_be_specified_when_evaluating_default_
150150
argument.Parse("")
151151
.Errors
152152
.Should()
153-
.ContainSingle(e => e.SymbolResult.Symbol == argument)
153+
.ContainSingle(e => ((ArgumentResult)e.SymbolResult).Argument == argument)
154154
.Which
155155
.Message
156156
.Should()
@@ -248,7 +248,10 @@ public void Option_ArgumentResult_Parent_is_set_correctly_when_token_is_implicit
248248

249249
argumentResult
250250
.Parent
251-
.Symbol
251+
.Should()
252+
.BeOfType<OptionResult>()
253+
.Which
254+
.Option
252255
.Should()
253256
.Be(command.Options.Single());
254257
}
@@ -274,9 +277,12 @@ public void Option_ArgumentResult_parentage_to_root_symbol_is_set_correctly_when
274277
argumentResult
275278
.Parent
276279
.Parent
277-
.Symbol
278280
.Should()
279-
.Be(command);
281+
.BeAssignableTo<CommandResult>()
282+
.Which
283+
.Command
284+
.Should()
285+
.BeSameAs(command);
280286
}
281287

282288
[Theory]
@@ -333,9 +339,12 @@ public void Command_ArgumentResult_Parent_is_set_correctly_when_token_is_implici
333339

334340
argumentResult
335341
.Parent
336-
.Symbol
337342
.Should()
338-
.Be(command);
343+
.BeAssignableTo<CommandResult>()
344+
.Which
345+
.Command
346+
.Should()
347+
.BeSameAs(command);
339348
}
340349

341350
[Fact]

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

Lines changed: 14 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,7 @@ public void Outer_command_is_identified_correctly_by_RootCommand()
3131

3232
result
3333
.RootCommandResult
34-
.Symbol
34+
.Command
3535
.Name
3636
.Should()
3737
.Be("outer");
@@ -45,7 +45,10 @@ public void Outer_command_is_identified_correctly_by_Parent_property()
4545
result
4646
.CommandResult
4747
.Parent
48-
.Symbol
48+
.Should()
49+
.BeAssignableTo<CommandResult>()
50+
.Which
51+
.Command
4952
.Name
5053
.Should()
5154
.Be("outer");
@@ -57,7 +60,10 @@ public void Inner_command_is_identified_correctly()
5760
var result = _parser.Parse("outer inner --option argument1");
5861

5962
result.CommandResult
60-
.Symbol
63+
.Should()
64+
.BeOfType<CommandResult>()
65+
.Which
66+
.Command
6167
.Name
6268
.Should()
6369
.Be("inner");
@@ -71,7 +77,10 @@ public void Inner_command_option_is_identified_correctly()
7177
result.CommandResult
7278
.Children
7379
.ElementAt(0)
74-
.Symbol
80+
.Should()
81+
.BeOfType<OptionResult>()
82+
.Which
83+
.Option
7584
.Name
7685
.Should()
7786
.Be("option");
@@ -195,7 +204,7 @@ public void ParseResult_Command_identifies_innermost_command(string input, strin
195204

196205
var result = outer.Parse(input);
197206

198-
result.CommandResult.Symbol.Name.Should().Be(expectedCommand);
207+
result.CommandResult.Command.Name.Should().Be(expectedCommand);
199208
}
200209

201210
[Fact]

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -75,12 +75,12 @@ public void Command_will_not_accept_a_command_if_a_sibling_command_has_already_b
7575

7676
var result = new Parser(command).Parse("outer inner-one inner-two");
7777

78-
result.CommandResult.Symbol.Name.Should().Be("inner-one");
78+
result.CommandResult.Command.Name.Should().Be("inner-one");
7979
result.Errors.Count.Should().Be(1);
8080

8181
var result2 = new Parser(command).Parse("outer inner-two inner-one");
8282

83-
result2.CommandResult.Symbol.Name.Should().Be("inner-two");
83+
result2.CommandResult.Command.Name.Should().Be("inner-two");
8484
result2.Errors.Count.Should().Be(1);
8585
}
8686
}

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

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
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.CommandLine.Parsing;
45
using System.Linq;
56
using FluentAssertions;
67
using Xunit;
@@ -141,7 +142,15 @@ public void A_command_can_be_specified_in_more_than_one_position(
141142
var result = outer.Parse(commandLine);
142143

143144
result.Errors.Should().BeEmpty();
144-
result.CommandResult.Parent.Symbol.Name.Should().Be(expectedParent);
145+
result.CommandResult
146+
.Parent
147+
.Should()
148+
.BeOfType<CommandResult>()
149+
.Which
150+
.Command
151+
.Name
152+
.Should()
153+
.Be(expectedParent);
145154
}
146155

147156
[Fact]

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

Lines changed: 19 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -129,7 +129,7 @@ public void Option_short_forms_can_be_bundled()
129129

130130
result.CommandResult
131131
.Children
132-
.Select(o => o.Symbol.Name)
132+
.Select(o => ((OptionResult)o).Option.Name)
133133
.Should()
134134
.BeEquivalentTo("x", "y", "z");
135135
}
@@ -172,7 +172,7 @@ public void Option_long_forms_do_not_get_unbundled()
172172

173173
result.CommandResult
174174
.Children
175-
.Select(o => o.Symbol.Name)
175+
.Select(o => ((OptionResult)o).Option.Name)
176176
.Should()
177177
.BeEquivalentTo("xyz");
178178
}
@@ -427,13 +427,13 @@ public void Command_with_multiple_options_is_parsed_correctly()
427427
.Children
428428
.Should()
429429
.ContainSingle(o =>
430-
o.Symbol.Name == "inner1" &&
430+
((OptionResult)o).Option.Name == "inner1" &&
431431
o.Tokens.Single().Value == "argument1");
432432
result.CommandResult
433433
.Children
434434
.Should()
435435
.ContainSingle(o =>
436-
o.Symbol.Name == "inner2" &&
436+
((OptionResult)o).Option.Name == "inner2" &&
437437
o.Tokens.Single().Value == "argument2");
438438
}
439439

@@ -667,13 +667,16 @@ public void When_options_with_the_same_name_are_defined_on_parent_and_child_comm
667667

668668
result.CommandResult
669669
.Parent
670+
.Should()
671+
.BeAssignableTo<CommandResult>()
672+
.Which
670673
.Children
671674
.Should()
672-
.NotContain(o => o.Symbol.Name == "x");
675+
.AllBeAssignableTo<CommandResult>();
673676
result.CommandResult
674677
.Children
675678
.Should()
676-
.ContainSingle(o => o.Symbol.Name == "x");
679+
.ContainSingle(o => ((OptionResult)o).Option.Name == "x");
677680
}
678681

679682
[Fact]
@@ -693,9 +696,12 @@ public void When_options_with_the_same_name_are_defined_on_parent_and_child_comm
693696
.BeEmpty();
694697
result.CommandResult
695698
.Parent
699+
.Should()
700+
.BeAssignableTo<CommandResult>()
701+
.Which
696702
.Children
697703
.Should()
698-
.ContainSingle(o => o.Symbol.Name == "x");
704+
.ContainSingle(o => o is OptionResult && ((OptionResult)o).Option.Name == "x");
699705
}
700706

701707
[Fact]
@@ -1001,9 +1007,12 @@ public void Option_and_Command_can_have_the_same_alias()
10011007
parser.Parse("outer --inner inner")
10021008
.CommandResult
10031009
.Parent
1010+
.Should()
1011+
.BeAssignableTo<CommandResult>()
1012+
.Which
10041013
.Children
10051014
.Should()
1006-
.Contain(c => c.Symbol == option);
1015+
.Contain(o => ((OptionResult)o).Option == option);
10071016
}
10081017

10091018
[Fact]
@@ -1020,12 +1029,12 @@ public void Options_can_have_the_same_alias_differentiated_only_by_prefix()
10201029

10211030
parser.Parse("-a").CommandResult
10221031
.Children
1023-
.Select(s => s.Symbol)
1032+
.Select(s => ((OptionResult)s).Option)
10241033
.Should()
10251034
.BeEquivalentTo(option1);
10261035
parser.Parse("--a").CommandResult
10271036
.Children
1028-
.Select(s => s.Symbol)
1037+
.Select(s => ((OptionResult)s).Option)
10291038
.Should()
10301039
.BeEquivalentTo(option2);
10311040
}

0 commit comments

Comments
 (0)