Skip to content

SymbolResults improvements #2024

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
adamsitnik merged 5 commits into from
Jan 19, 2023
Merged

SymbolResults improvements #2024

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
e6cc70e
LocalizationResources should be readonly and same for entire symbol tree
adamsitnik Jan 18, 2023
4d93e7a
remove Symbol from SymbolResult, as each derived type defines more sp…
adamsitnik Jan 18, 2023
5b57add
not every SymbolResult can have children, move the property to derive…
adamsitnik Jan 18, 2023
ebd0395
do not store the list of children per symbol result, use Parent prope…
adamsitnik Jan 18, 2023
a10c271
fix the build ;)
adamsitnik Jan 18, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 2 additions & 3 deletions ...ty.Tests/ApiCompatibilityApprovalTests.System_CommandLine_api_is_not_changed.approved.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -393,6 +393,7 @@ System.CommandLine.Parsing
public class CommandLineStringSplitter
public System.Collections.Generic.IEnumerable<System.String> Split(System.String commandLine)
public class CommandResult : SymbolResult
public System.Collections.Generic.IEnumerable<SymbolResult> Children { get; }
public System.CommandLine.Command Command { get; }
public Token Token { get; }
public class OptionResult : SymbolResult
Expand Down Expand Up @@ -422,11 +423,9 @@ System.CommandLine.Parsing
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)
public static System.CommandLine.ParseResult Parse(this Parser parser, System.String commandLine)
public abstract class SymbolResult
public System.Collections.Generic.IReadOnlyList<SymbolResult> Children { get; }
public System.String ErrorMessage { get; set; }
public System.CommandLine.LocalizationResources LocalizationResources { get; set; }
public System.CommandLine.LocalizationResources LocalizationResources { get; }
public SymbolResult Parent { get; }
public System.CommandLine.Symbol Symbol { get; }
public System.Collections.Generic.IReadOnlyList<Token> Tokens { get; }
public ArgumentResult FindResultFor(System.CommandLine.Argument argument)
public CommandResult FindResultFor(System.CommandLine.Command command)
Expand Down
23 changes: 16 additions & 7 deletions src/System.CommandLine.Tests/ArgumentTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -131,7 +131,7 @@ public void Validation_failure_message_can_be_specified_when_parsing_tokens()
argument.Parse("x")
.Errors
.Should()
.ContainSingle(e => e.SymbolResult.Symbol == argument)
.ContainSingle(e => ((ArgumentResult)e.SymbolResult).Argument == argument)
.Which
.Message
.Should()
Expand All @@ -150,7 +150,7 @@ public void Validation_failure_message_can_be_specified_when_evaluating_default_
argument.Parse("")
.Errors
.Should()
.ContainSingle(e => e.SymbolResult.Symbol == argument)
.ContainSingle(e => ((ArgumentResult)e.SymbolResult).Argument == argument)
.Which
.Message
.Should()
Expand Down Expand Up @@ -248,7 +248,10 @@ public void Option_ArgumentResult_Parent_is_set_correctly_when_token_is_implicit

argumentResult
.Parent
.Symbol
.Should()
.BeOfType<OptionResult>()
.Which
.Option
.Should()
.Be(command.Options.Single());
}
Expand All @@ -274,9 +277,12 @@ public void Option_ArgumentResult_parentage_to_root_symbol_is_set_correctly_when
argumentResult
.Parent
.Parent
.Symbol
.Should()
.Be(command);
.BeAssignableTo<CommandResult>()
.Which
.Command
.Should()
.BeSameAs(command);
}

[Theory]
Expand Down Expand Up @@ -333,9 +339,12 @@ public void Command_ArgumentResult_Parent_is_set_correctly_when_token_is_implici

argumentResult
.Parent
.Symbol
.Should()
.Be(command);
.BeAssignableTo<CommandResult>()

@jonsequitur jonsequitur Jan 18, 2023

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think you can use BeOfType here, no?

@adamsitnik adamsitnik Jan 19, 2023 •
edited
Loading

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

iirc it's RootCommandResult here (which is internal)

edit:

[xUnit.net 00:00:00.93]     System.CommandLine.Tests.ArgumentTests+CustomParsing.Command_ArgumentResult_Parent_is_set_correctly_when_token_is_implicit [FAIL]
  Failed System.CommandLine.Tests.ArgumentTests+CustomParsing.Command_ArgumentResult_Parent_is_set_correctly_when_token_is_implicit [34 ms]
  Error Message:
   Expected type to be System.CommandLine.Parsing.CommandResult, but found System.CommandLine.Parsing.RootCommandResult.
  Stack Trace:
     at FluentAssertions.Execution.XUnit2TestFramework.Throw(String message)
   at FluentAssertions.Execution.TestFrameworkProvider.Throw(String message)
   at FluentAssertions.Execution.DefaultAssertionStrategy.HandleFailure(String message)
   at FluentAssertions.Execution.AssertionScope.FailWith(Func`1 failReasonFunc)
   at FluentAssertions.Execution.AssertionScope.FailWith(Func`1 failReasonFunc)
   at FluentAssertions.Execution.AssertionScope.FailWith(String message, Object[] args)
   at FluentAssertions.Types.TypeAssertions.Be(Type expected, String because, Object[] becauseArgs)
   at FluentAssertions.Primitives.ReferenceTypeAssertions`2.BeOfType(Type expectedType, String because, Object[] becauseArgs)
   at FluentAssertions.Primitives.ReferenceTypeAssertions`2.BeOfType[T](String because, Object[] becauseArgs)
   at System.CommandLine.Tests.ArgumentTests.CustomParsing.Command_ArgumentResult_Parent_is_set_correctly_when_token_is_implicit() in D:\projects\command-line-api\src\System.CommandLine.Tests\ArgumentTests.cs:line 340
   at System.RuntimeMethodHandle.InvokeMethod(Object target, Void** arguments, Signature sig, Boolean isConstructor)
   at System.Reflection.MethodInvoker.Invoke(Object obj, IntPtr* args, BindingFlags invokeAttr)

.Which
.Command
.Should()
.BeSameAs(command);
}

[Fact]
Expand Down
19 changes: 14 additions & 5 deletions src/System.CommandLine.Tests/CommandTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ public void Outer_command_is_identified_correctly_by_RootCommand()

result
.RootCommandResult
.Symbol
.Command
.Name
.Should()
.Be("outer");
Expand All @@ -45,7 +45,10 @@ public void Outer_command_is_identified_correctly_by_Parent_property()
result
.CommandResult
.Parent
.Symbol
.Should()
.BeAssignableTo<CommandResult>()
.Which
.Command
.Name
.Should()
.Be("outer");
Expand All @@ -57,7 +60,10 @@ public void Inner_command_is_identified_correctly()
var result = _parser.Parse("outer inner --option argument1");

result.CommandResult
.Symbol
.Should()
.BeOfType<CommandResult>()
.Which
.Command
.Name
.Should()
.Be("inner");
Expand All @@ -71,7 +77,10 @@ public void Inner_command_option_is_identified_correctly()
result.CommandResult
.Children
.ElementAt(0)
.Symbol
.Should()
.BeOfType<OptionResult>()
.Which
.Option
.Name
.Should()
.Be("option");
Expand Down Expand Up @@ -195,7 +204,7 @@ public void ParseResult_Command_identifies_innermost_command(string input, strin

var result = outer.Parse(input);

result.CommandResult.Symbol.Name.Should().Be(expectedCommand);
result.CommandResult.Command.Name.Should().Be(expectedCommand);
}

[Fact]
Expand Down
4 changes: 2 additions & 2 deletions src/System.CommandLine.Tests/ParseResultTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -75,12 +75,12 @@ public void Command_will_not_accept_a_command_if_a_sibling_command_has_already_b

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

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

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

result2.CommandResult.Symbol.Name.Should().Be("inner-two");
result2.CommandResult.Command.Name.Should().Be("inner-two");
result2.Errors.Count.Should().Be(1);
}
}
Expand Down
11 changes: 10 additions & 1 deletion src/System.CommandLine.Tests/ParserTests.MultiplePositions.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
// Copyright (c) .NET Foundation and contributors. All rights reserved.
// Licensed under the MIT license. See LICENSE file in the project root for full license information.

using System.CommandLine.Parsing;
using System.Linq;
using FluentAssertions;
using Xunit;
Expand Down Expand Up @@ -141,7 +142,15 @@ public void A_command_can_be_specified_in_more_than_one_position(
var result = outer.Parse(commandLine);

result.Errors.Should().BeEmpty();
result.CommandResult.Parent.Symbol.Name.Should().Be(expectedParent);
result.CommandResult
.Parent
.Should()
.BeOfType<CommandResult>()
.Which
.Command
.Name
.Should()
.Be(expectedParent);
}

[Fact]
Expand Down
29 changes: 19 additions & 10 deletions src/System.CommandLine.Tests/ParserTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -129,7 +129,7 @@ public void Option_short_forms_can_be_bundled()

result.CommandResult
.Children
.Select(o => o.Symbol.Name)
.Select(o => ((OptionResult)o).Option.Name)
.Should()
.BeEquivalentTo("x", "y", "z");
}
Expand Down Expand Up @@ -172,7 +172,7 @@ public void Option_long_forms_do_not_get_unbundled()

result.CommandResult
.Children
.Select(o => o.Symbol.Name)
.Select(o => ((OptionResult)o).Option.Name)
.Should()
.BeEquivalentTo("xyz");
}
Expand Down Expand Up @@ -427,13 +427,13 @@ public void Command_with_multiple_options_is_parsed_correctly()
.Children
.Should()
.ContainSingle(o =>
o.Symbol.Name == "inner1" &&
((OptionResult)o).Option.Name == "inner1" &&
o.Tokens.Single().Value == "argument1");
result.CommandResult
.Children
.Should()
.ContainSingle(o =>
o.Symbol.Name == "inner2" &&
((OptionResult)o).Option.Name == "inner2" &&
o.Tokens.Single().Value == "argument2");
}

Expand Down Expand Up @@ -667,13 +667,16 @@ public void When_options_with_the_same_name_are_defined_on_parent_and_child_comm

result.CommandResult
.Parent
.Should()
.BeAssignableTo<CommandResult>()
.Which
.Children
.Should()
.NotContain(o => o.Symbol.Name == "x");
.AllBeAssignableTo<CommandResult>();
result.CommandResult
.Children
.Should()
.ContainSingle(o => o.Symbol.Name == "x");
.ContainSingle(o => ((OptionResult)o).Option.Name == "x");
}

[Fact]
Expand All @@ -693,9 +696,12 @@ public void When_options_with_the_same_name_are_defined_on_parent_and_child_comm
.BeEmpty();
result.CommandResult
.Parent
.Should()
.BeAssignableTo<CommandResult>()
.Which
.Children
.Should()
.ContainSingle(o => o.Symbol.Name == "x");
.ContainSingle(o => o is OptionResult && ((OptionResult)o).Option.Name == "x");
}

[Fact]
Expand Down Expand Up @@ -1001,9 +1007,12 @@ public void Option_and_Command_can_have_the_same_alias()
parser.Parse("outer --inner inner")
.CommandResult
.Parent
.Should()
.BeAssignableTo<CommandResult>()
.Which
.Children
.Should()
.Contain(c => c.Symbol == option);
.Contain(o => ((OptionResult)o).Option == option);
}

[Fact]
Expand All @@ -1020,12 +1029,12 @@ public void Options_can_have_the_same_alias_differentiated_only_by_prefix()

parser.Parse("-a").CommandResult
.Children
.Select(s => s.Symbol)
.Select(s => ((OptionResult)s).Option)
.Should()
.BeEquivalentTo(option1);
parser.Parse("--a").CommandResult
.Children
.Select(s => s.Symbol)
.Select(s => ((OptionResult)s).Option)
.Should()
.BeEquivalentTo(option2);
}
Expand Down
Loading