From 7340170331423573c6bd5d56326c359941be4550 Mon Sep 17 00:00:00 2001 From: Adam Sitnik Date: Wed, 11 Jan 2023 13:40:53 +0100 Subject: [PATCH 01/12] remove Parent property from SyntaxNode as it's used only in one place --- .../Parsing/CommandArgumentNode.cs | 2 +- src/System.CommandLine/Parsing/CommandNode.cs | 3 +- .../Parsing/DirectiveNode.cs | 3 +- .../Parsing/NonterminalSyntaxNode.cs | 2 +- .../Parsing/OptionArgumentNode.cs | 2 +- src/System.CommandLine/Parsing/OptionNode.cs | 3 +- .../Parsing/ParseOperation.cs | 12 +++---- .../Parsing/ParseResultVisitor.cs | 33 +++++++++---------- src/System.CommandLine/Parsing/SyntaxNode.cs | 7 +--- 9 files changed, 27 insertions(+), 40 deletions(-) diff --git a/src/System.CommandLine/Parsing/CommandArgumentNode.cs b/src/System.CommandLine/Parsing/CommandArgumentNode.cs index d9d776a1fe..b819c0e913 100644 --- a/src/System.CommandLine/Parsing/CommandArgumentNode.cs +++ b/src/System.CommandLine/Parsing/CommandArgumentNode.cs @@ -10,7 +10,7 @@ internal class CommandArgumentNode : SyntaxNode public CommandArgumentNode( Token token, Argument argument, - CommandNode parent) : base(token, parent) + CommandNode parent) : base(token) { Debug.Assert(token.Type == TokenType.Argument, $"Incorrect token type: {token}"); diff --git a/src/System.CommandLine/Parsing/CommandNode.cs b/src/System.CommandLine/Parsing/CommandNode.cs index b62875b22c..93ac4ad352 100644 --- a/src/System.CommandLine/Parsing/CommandNode.cs +++ b/src/System.CommandLine/Parsing/CommandNode.cs @@ -7,8 +7,7 @@ internal class CommandNode : NonterminalSyntaxNode { public CommandNode( Token token, - Command command, - CommandNode? parent) : base(token, parent) + Command command) : base(token) { Command = command; } diff --git a/src/System.CommandLine/Parsing/DirectiveNode.cs b/src/System.CommandLine/Parsing/DirectiveNode.cs index e4bebe1897..3766c417f3 100644 --- a/src/System.CommandLine/Parsing/DirectiveNode.cs +++ b/src/System.CommandLine/Parsing/DirectiveNode.cs @@ -9,9 +9,8 @@ internal class DirectiveNode : SyntaxNode { public DirectiveNode( Token token, - CommandNode parent, string name, - string? value) : base(token, parent) + string? value) : base(token) { Debug.Assert(token.Type == TokenType.Directive, $"Incorrect token type: {token}"); diff --git a/src/System.CommandLine/Parsing/NonterminalSyntaxNode.cs b/src/System.CommandLine/Parsing/NonterminalSyntaxNode.cs index 175f892232..e2dd57d4df 100644 --- a/src/System.CommandLine/Parsing/NonterminalSyntaxNode.cs +++ b/src/System.CommandLine/Parsing/NonterminalSyntaxNode.cs @@ -9,7 +9,7 @@ internal abstract class NonterminalSyntaxNode : SyntaxNode { private List? _children; - protected NonterminalSyntaxNode(Token token, SyntaxNode? parent) : base(token, parent) + protected NonterminalSyntaxNode(Token token) : base(token) { } diff --git a/src/System.CommandLine/Parsing/OptionArgumentNode.cs b/src/System.CommandLine/Parsing/OptionArgumentNode.cs index 36cd51006d..0ccb37e333 100644 --- a/src/System.CommandLine/Parsing/OptionArgumentNode.cs +++ b/src/System.CommandLine/Parsing/OptionArgumentNode.cs @@ -10,7 +10,7 @@ internal class OptionArgumentNode : SyntaxNode public OptionArgumentNode( Token token, Argument argument, - OptionNode parent) : base(token, parent) + OptionNode parent) : base(token) { Debug.Assert(token.Type == TokenType.Argument, $"Incorrect token type: {token}"); diff --git a/src/System.CommandLine/Parsing/OptionNode.cs b/src/System.CommandLine/Parsing/OptionNode.cs index bd4f30e800..bc1cc800fd 100644 --- a/src/System.CommandLine/Parsing/OptionNode.cs +++ b/src/System.CommandLine/Parsing/OptionNode.cs @@ -7,8 +7,7 @@ internal class OptionNode : NonterminalSyntaxNode { public OptionNode( Token token, - Option option, - CommandNode parent) : base(token, parent) + Option option) : base(token) { Option = option; } diff --git a/src/System.CommandLine/Parsing/ParseOperation.cs b/src/System.CommandLine/Parsing/ParseOperation.cs index aa36d10f9c..34997725bb 100644 --- a/src/System.CommandLine/Parsing/ParseOperation.cs +++ b/src/System.CommandLine/Parsing/ParseOperation.cs @@ -5,7 +5,7 @@ namespace System.CommandLine.Parsing { - internal class ParseOperation + internal sealed class ParseOperation { private readonly List _tokens; private readonly CommandLineConfiguration _configuration; @@ -43,8 +43,7 @@ private CommandNode ParseRootCommand() { var rootCommandNode = new CommandNode( CurrentToken, - _configuration.RootCommand, - null); + _configuration.RootCommand); Advance(); @@ -57,7 +56,7 @@ private CommandNode ParseRootCommand() private void ParseSubcommand(CommandNode parentNode) { - var commandNode = new CommandNode(CurrentToken, (Command)CurrentToken.Symbol!, parentNode); + var commandNode = new CommandNode(CurrentToken, (Command)CurrentToken.Symbol!); Advance(); @@ -135,8 +134,7 @@ private void ParseOption(CommandNode parent) { OptionNode optionNode = new( CurrentToken, - (Option)CurrentToken.Symbol!, - parent); + (Option)CurrentToken.Symbol!); Advance(); @@ -209,7 +207,7 @@ void ParseDirective(CommandNode parent) ? withoutBrackets.Slice(indexOfColon + 1).ToString() : null; - var directiveNode = new DirectiveNode(token, parent, key, value); + var directiveNode = new DirectiveNode(token, key, value); parent.AddChildNode(directiveNode); diff --git a/src/System.CommandLine/Parsing/ParseResultVisitor.cs b/src/System.CommandLine/Parsing/ParseResultVisitor.cs index cdc04ac739..5b97a0e5c2 100644 --- a/src/System.CommandLine/Parsing/ParseResultVisitor.cs +++ b/src/System.CommandLine/Parsing/ParseResultVisitor.cs @@ -52,9 +52,11 @@ internal ParseResultVisitor( } } - public void Visit(SyntaxNode node) + internal void Visit(CommandNode rootCommandNode) { - VisitInternal(node); + VisitRootCommandNode(rootCommandNode); + + VisitChildren(rootCommandNode); Stop(); } @@ -69,29 +71,16 @@ private void VisitInternal(SyntaxNode node) break; case CommandNode commandNode: - if (commandNode.Parent is null) - { - VisitRootCommandNode(commandNode); - } - else - { - VisitCommandNode(commandNode); - } + VisitCommandNode(commandNode); - for (var i = 0; i < commandNode.Children.Count; i++) - { - VisitInternal(commandNode.Children[i]); - } + VisitChildren(commandNode); break; case OptionNode optionNode: VisitOptionNode(optionNode); - for (var i = 0; i < optionNode.Children.Count; i++) - { - VisitInternal(optionNode.Children[i]); - } + VisitChildren(optionNode); break; @@ -241,6 +230,14 @@ private void VisitDirectiveNode(DirectiveNode directiveNode) } } + private void VisitChildren(NonterminalSyntaxNode parentNode) + { + for (var i = 0; i < parentNode.Children.Count; i++) + { + VisitInternal(parentNode.Children[i]); + } + } + private void Stop() { if (_isHelpRequested) diff --git a/src/System.CommandLine/Parsing/SyntaxNode.cs b/src/System.CommandLine/Parsing/SyntaxNode.cs index 48105dec91..5ad049f415 100644 --- a/src/System.CommandLine/Parsing/SyntaxNode.cs +++ b/src/System.CommandLine/Parsing/SyntaxNode.cs @@ -5,16 +5,11 @@ namespace System.CommandLine.Parsing { internal abstract class SyntaxNode { - protected SyntaxNode( - Token token, - SyntaxNode? parent) + protected SyntaxNode(Token token) { Token = token; - Parent = parent; } - public SyntaxNode? Parent { get; } - public Token Token { get; } public override string ToString() => Token.Value; From 23b324a2b814b131aac5c3c626cffa8e94cd53ad Mon Sep 17 00:00:00 2001 From: Adam Sitnik Date: Wed, 11 Jan 2023 15:28:47 +0100 Subject: [PATCH 02/12] make NonterminalSyntaxNode.Children nullable to reduce number of JIT compilations --- src/System.CommandLine/Parsing/NonterminalSyntaxNode.cs | 2 +- src/System.CommandLine/Parsing/ParseResultVisitor.cs | 7 +++++-- 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/src/System.CommandLine/Parsing/NonterminalSyntaxNode.cs b/src/System.CommandLine/Parsing/NonterminalSyntaxNode.cs index e2dd57d4df..dcf9dd833d 100644 --- a/src/System.CommandLine/Parsing/NonterminalSyntaxNode.cs +++ b/src/System.CommandLine/Parsing/NonterminalSyntaxNode.cs @@ -13,7 +13,7 @@ protected NonterminalSyntaxNode(Token token) : base(token) { } - public IReadOnlyList Children => _children is not null ? _children : Array.Empty(); + public IReadOnlyList? Children => _children; internal void AddChildNode(SyntaxNode node) => (_children ??= new()).Add(node); } diff --git a/src/System.CommandLine/Parsing/ParseResultVisitor.cs b/src/System.CommandLine/Parsing/ParseResultVisitor.cs index 5b97a0e5c2..f64d98ed23 100644 --- a/src/System.CommandLine/Parsing/ParseResultVisitor.cs +++ b/src/System.CommandLine/Parsing/ParseResultVisitor.cs @@ -232,9 +232,12 @@ private void VisitDirectiveNode(DirectiveNode directiveNode) private void VisitChildren(NonterminalSyntaxNode parentNode) { - for (var i = 0; i < parentNode.Children.Count; i++) + if (parentNode.Children is not null) { - VisitInternal(parentNode.Children[i]); + for (var i = 0; i < parentNode.Children.Count; i++) + { + VisitInternal(parentNode.Children[i]); + } } } From 0c43b203f06abd5e3d7923bc20b8b13819a857bc Mon Sep 17 00:00:00 2001 From: Adam Sitnik Date: Wed, 11 Jan 2023 16:15:53 +0100 Subject: [PATCH 03/12] don't store a dedicated list of OptionResults, re-use data present in symbolResults dictionary --- .../Parsing/ParseResultVisitor.cs | 15 ++++----------- 1 file changed, 4 insertions(+), 11 deletions(-) diff --git a/src/System.CommandLine/Parsing/ParseResultVisitor.cs b/src/System.CommandLine/Parsing/ParseResultVisitor.cs index f64d98ed23..9e2a569652 100644 --- a/src/System.CommandLine/Parsing/ParseResultVisitor.cs +++ b/src/System.CommandLine/Parsing/ParseResultVisitor.cs @@ -21,7 +21,6 @@ internal sealed class ParseResultVisitor private readonly Dictionary _symbolResults = new(); - private List? _optionResults; private List? _argumentResults; private RootCommandResult? _rootCommandResult; @@ -105,10 +104,7 @@ private void AddToResult(CommandResult result) private void AddToResult(OptionResult result) { _innermostCommandResult?.AddChild(result); - if (_symbolResults.TryAdd(result.Option, result)) - { - (_optionResults ??= new()).Add(result); - } + _symbolResults.TryAdd(result.Option, result); } private void AddToResult(ArgumentResult result) @@ -254,9 +250,9 @@ private void Stop() ValidateCommandResult(); - if (_optionResults is not null) + foreach (var symbolPair in _symbolResults) { - foreach (var optionResult in _optionResults) + if (symbolPair.Value is OptionResult optionResult) { ValidateAndConvertOptionResult(optionResult); } @@ -608,10 +604,7 @@ void Handle(SymbolResult? symbolResult, Symbol symbol) optionResult.AddChild(childArgumentResult); commandResult.AddChild(optionResult); - if (_symbolResults.TryAdd(optionResult.Symbol, optionResult)) - { - (_optionResults ??= new()).Add(optionResult); - } + _symbolResults.TryAdd(optionResult.Symbol, optionResult); break; From eea506bf41681191d86205881443ee5011068661 Mon Sep 17 00:00:00 2001 From: Adam Sitnik Date: Thu, 12 Jan 2023 13:08:34 +0100 Subject: [PATCH 04/12] avoid unnecessary casting: * Option -> Symbol -> Option * Argument -> Symbol -> Argument --- .../Parsing/ParseResultVisitor.cs | 8 ++++---- .../Parsing/RootCommandResult.cs | 15 --------------- 2 files changed, 4 insertions(+), 19 deletions(-) diff --git a/src/System.CommandLine/Parsing/ParseResultVisitor.cs b/src/System.CommandLine/Parsing/ParseResultVisitor.cs index 9e2a569652..5775d8d55e 100644 --- a/src/System.CommandLine/Parsing/ParseResultVisitor.cs +++ b/src/System.CommandLine/Parsing/ParseResultVisitor.cs @@ -556,8 +556,8 @@ private void PopulateDefaultValues() var options = commandResult.Command.Options; for (var i = 0; i < options.Count; i++) { - Symbol symbol = options[i]; - Handle(_rootCommandResult!.FindResultForSymbol(symbol), symbol); + Option option = options[i]; + Handle(_rootCommandResult!.FindResultFor(option), option); } } @@ -566,8 +566,8 @@ private void PopulateDefaultValues() var arguments = commandResult.Command.Arguments; for (var i = 0; i < arguments.Count; i++) { - Symbol symbol = arguments[i]; - Handle(_rootCommandResult!.FindResultForSymbol(symbol), symbol); + Argument argument = arguments[i]; + Handle(_rootCommandResult!.FindResultFor(argument), argument); } } diff --git a/src/System.CommandLine/Parsing/RootCommandResult.cs b/src/System.CommandLine/Parsing/RootCommandResult.cs index c5f0ae28ba..939002f4f9 100644 --- a/src/System.CommandLine/Parsing/RootCommandResult.cs +++ b/src/System.CommandLine/Parsing/RootCommandResult.cs @@ -51,20 +51,5 @@ public RootCommandResult( return default; } - - internal SymbolResult? FindResultForSymbol(Symbol symbol) - { - switch (symbol) - { - case Argument argument: - return FindResultFor(argument); - case Command command: - return FindResultFor(command); - case Option option: - return FindResultFor(option); - default: - throw new ArgumentException($"Unsupported symbol type: {symbol.GetType()}"); - } - } } } From 675396f64d43c06dd6b7c644ced6e271593fa36f Mon Sep 17 00:00:00 2001 From: Adam Sitnik Date: Thu, 12 Jan 2023 13:15:23 +0100 Subject: [PATCH 05/12] We own Symbol->SymbolResult dictionary and we know that for each Argument key the value can be only ArgumentResult etc --- .../Parsing/RootCommandResult.cs | 30 ++----------------- 1 file changed, 3 insertions(+), 27 deletions(-) diff --git a/src/System.CommandLine/Parsing/RootCommandResult.cs b/src/System.CommandLine/Parsing/RootCommandResult.cs index 939002f4f9..49a051a3fd 100644 --- a/src/System.CommandLine/Parsing/RootCommandResult.cs +++ b/src/System.CommandLine/Parsing/RootCommandResult.cs @@ -20,36 +20,12 @@ public RootCommandResult( internal override RootCommandResult Root => this; public override ArgumentResult? FindResultFor(Argument argument) - { - if (_symbolResults.TryGetValue(argument, out var result) && - result is ArgumentResult argumentResult) - { - return argumentResult; - } - - return default; - } + => _symbolResults.TryGetValue(argument, out SymbolResult? result) ? (ArgumentResult)result : default; public override CommandResult? FindResultFor(Command command) - { - if (_symbolResults.TryGetValue(command, out var result) && - result is CommandResult commandResult) - { - return commandResult; - } - - return default; - } + => _symbolResults.TryGetValue(command, out SymbolResult? result) ? (CommandResult)result : default; public override OptionResult? FindResultFor(Option option) - { - if (_symbolResults.TryGetValue(option, out var result) && - result is OptionResult optionResult) - { - return optionResult; - } - - return default; - } + => _symbolResults.TryGetValue(option, out SymbolResult? result) ? (OptionResult)result : default; } } From 6d4c8982740d6b67c1403d85317810a49e6dbd99 Mon Sep 17 00:00:00 2001 From: Adam Sitnik Date: Thu, 12 Jan 2023 13:26:04 +0100 Subject: [PATCH 06/12] seal ArgumentResult, OptionResult and RootCommandResult --- src/System.CommandLine/Parsing/ArgumentResult.cs | 2 +- src/System.CommandLine/Parsing/OptionResult.cs | 2 +- src/System.CommandLine/Parsing/RootCommandResult.cs | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/System.CommandLine/Parsing/ArgumentResult.cs b/src/System.CommandLine/Parsing/ArgumentResult.cs index 50b975acc1..0b8450eaf7 100644 --- a/src/System.CommandLine/Parsing/ArgumentResult.cs +++ b/src/System.CommandLine/Parsing/ArgumentResult.cs @@ -10,7 +10,7 @@ namespace System.CommandLine.Parsing /// /// A result produced when parsing an . /// - public class ArgumentResult : SymbolResult + public sealed class ArgumentResult : SymbolResult { private ArgumentConversionResult? _conversionResult; diff --git a/src/System.CommandLine/Parsing/OptionResult.cs b/src/System.CommandLine/Parsing/OptionResult.cs index 934f77dcfb..38419b1b0c 100644 --- a/src/System.CommandLine/Parsing/OptionResult.cs +++ b/src/System.CommandLine/Parsing/OptionResult.cs @@ -10,7 +10,7 @@ namespace System.CommandLine.Parsing /// /// A result produced when parsing an . /// - public class OptionResult : SymbolResult + public sealed class OptionResult : SymbolResult { private ArgumentConversionResult? _argumentConversionResult; private Dictionary? _defaultArgumentValues; diff --git a/src/System.CommandLine/Parsing/RootCommandResult.cs b/src/System.CommandLine/Parsing/RootCommandResult.cs index 49a051a3fd..2e2390f176 100644 --- a/src/System.CommandLine/Parsing/RootCommandResult.cs +++ b/src/System.CommandLine/Parsing/RootCommandResult.cs @@ -5,7 +5,7 @@ namespace System.CommandLine.Parsing { - internal class RootCommandResult : CommandResult + internal sealed class RootCommandResult : CommandResult { private readonly Dictionary _symbolResults; From 13ae0675a294679175e43ad2bc06d5c1a8f82f98 Mon Sep 17 00:00:00 2001 From: Adam Sitnik Date: Thu, 12 Jan 2023 13:28:14 +0100 Subject: [PATCH 07/12] remove unused internal type (SymbolResultVisitor) --- .../Parsing/SymbolResultVisitor.cs | 82 ------------------- 1 file changed, 82 deletions(-) delete mode 100644 src/System.CommandLine/Parsing/SymbolResultVisitor.cs diff --git a/src/System.CommandLine/Parsing/SymbolResultVisitor.cs b/src/System.CommandLine/Parsing/SymbolResultVisitor.cs deleted file mode 100644 index 443e32cca1..0000000000 --- a/src/System.CommandLine/Parsing/SymbolResultVisitor.cs +++ /dev/null @@ -1,82 +0,0 @@ -// 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. - -namespace System.CommandLine.Parsing -{ - internal abstract class SymbolResultVisitor - { - public void Visit(SymbolResult symbolResult) - { - Start(symbolResult); - - VisitInternal(symbolResult); - - Stop(symbolResult); - } - - private void VisitInternal(SymbolResult node) - { - switch (node) - { - case ArgumentResult argumentResult: - VisitArgumentResult(argumentResult); - - break; - - case RootCommandResult rootCommandResult: - VisitRootCommandResult(rootCommandResult); - - for (var i = 0; i < rootCommandResult.Children.Count; i++) - { - VisitInternal(rootCommandResult.Children[i]); - } - - break; - - case CommandResult commandResult: - VisitCommandResult(commandResult); - - for (var i = 0; i < commandResult.Children.Count; i++) - { - VisitInternal(commandResult.Children[i]); - } - - break; - - case OptionResult optionResult: - VisitOptionResult(optionResult); - - for (var i = 0; i < optionResult.Children.Count; i++) - { - VisitInternal(optionResult.Children[i]); - } - - break; - } - } - - protected virtual void VisitOptionResult(OptionResult optionResult) - { - } - - protected virtual void VisitCommandResult(CommandResult commandResult) - { - } - - protected virtual void VisitArgumentResult(ArgumentResult argumentResult) - { - } - - protected virtual void VisitRootCommandResult(RootCommandResult rootCommandResult) - { - } - - protected virtual void Start(SymbolResult node) - { - } - - protected virtual void Stop(SymbolResult node) - { - } - } -} \ No newline at end of file From e728796b37f1bb89dc9f2003c557293c0b65d980 Mon Sep 17 00:00:00 2001 From: Adam Sitnik Date: Thu, 12 Jan 2023 13:54:06 +0100 Subject: [PATCH 08/12] remove Root property, we already have Parent property and can get the Root very quickly --- src/System.CommandLine/ArgumentArity.cs | 2 +- .../Parsing/RootCommandResult.cs | 2 -- .../Parsing/SymbolResult.cs | 27 ++++++++++++------- 3 files changed, 18 insertions(+), 13 deletions(-) diff --git a/src/System.CommandLine/ArgumentArity.cs b/src/System.CommandLine/ArgumentArity.cs index c0153fc86e..6e3d496af5 100644 --- a/src/System.CommandLine/ArgumentArity.cs +++ b/src/System.CommandLine/ArgumentArity.cs @@ -81,7 +81,7 @@ public override int GetHashCode() var argumentResult = symbolResult switch { ArgumentResult a => a, - _ => symbolResult.Root!.FindResultFor(argument) + _ => symbolResult.FindResultFor(argument) }; var tokenCount = argumentResult?.Tokens.Count ?? 0; diff --git a/src/System.CommandLine/Parsing/RootCommandResult.cs b/src/System.CommandLine/Parsing/RootCommandResult.cs index 2e2390f176..2f7888005e 100644 --- a/src/System.CommandLine/Parsing/RootCommandResult.cs +++ b/src/System.CommandLine/Parsing/RootCommandResult.cs @@ -17,8 +17,6 @@ public RootCommandResult( _symbolResults = symbolResults; } - internal override RootCommandResult Root => this; - public override ArgumentResult? FindResultFor(Argument argument) => _symbolResults.TryGetValue(argument, out SymbolResult? result) ? (ArgumentResult)result : default; diff --git a/src/System.CommandLine/Parsing/SymbolResult.cs b/src/System.CommandLine/Parsing/SymbolResult.cs index 5415ef2458..433477ece7 100644 --- a/src/System.CommandLine/Parsing/SymbolResult.cs +++ b/src/System.CommandLine/Parsing/SymbolResult.cs @@ -3,6 +3,7 @@ using System.Collections.Generic; using System.CommandLine.Binding; +using System.Diagnostics; using System.Linq; namespace System.CommandLine.Parsing @@ -23,8 +24,6 @@ private protected SymbolResult( Symbol = symbol ?? throw new ArgumentNullException(nameof(symbol)); Parent = parent; - - Root = parent?.Root; } /// @@ -45,8 +44,6 @@ private protected SymbolResult( /// public SymbolResult? Parent { get; } - internal virtual RootCommandResult? Root { get; } - /// /// The symbol to which the result applies. /// @@ -111,24 +108,34 @@ public LocalizationResources LocalizationResources /// /// The argument for which to find a result. /// An argument result if the argument was matched by the parser or has a default value; otherwise, null. - public virtual ArgumentResult? FindResultFor(Argument argument) => - Root?.FindResultFor(argument); + public virtual ArgumentResult? FindResultFor(Argument argument) => GetRoot().FindResultFor(argument); /// /// Finds a result for the specific command anywhere in the parse tree, including parent and child symbol results. /// /// The command for which to find a result. /// An command result if the command was matched by the parser; otherwise, null. - public virtual CommandResult? FindResultFor(Command command) => - Root?.FindResultFor(command); + public virtual CommandResult? FindResultFor(Command command) => GetRoot().FindResultFor(command); /// /// Finds a result for the specific option anywhere in the parse tree, including parent and child symbol results. /// /// The option for which to find a result. /// An option result if the option was matched by the parser or has a default value; otherwise, null. - public virtual OptionResult? FindResultFor(Option option) => - Root?.FindResultFor(option); + public virtual OptionResult? FindResultFor(Option option) => GetRoot().FindResultFor(option); + + private SymbolResult GetRoot() + { + SymbolResult result = this; + while (result.Parent is not null) + { + result = result.Parent; + } + + Debug.Assert(result is RootCommandResult); + + return result; + } /// public T GetValue(Argument argument) From 271bc394b0954e78cf1bebd681c5c58d75f89bce Mon Sep 17 00:00:00 2001 From: Adam Sitnik Date: Thu, 12 Jan 2023 14:49:45 +0100 Subject: [PATCH 09/12] don't use SymbolResult.Symbol property when more specific information is available --- src/System.CommandLine/ParseResult.cs | 3 +-- src/System.CommandLine/Parsing/ParseResultVisitor.cs | 4 ++-- 2 files changed, 3 insertions(+), 4 deletions(-) diff --git a/src/System.CommandLine/ParseResult.cs b/src/System.CommandLine/ParseResult.cs index b4573b78b0..0cd1a2044e 100644 --- a/src/System.CommandLine/ParseResult.cs +++ b/src/System.CommandLine/ParseResult.cs @@ -234,8 +234,7 @@ static IEnumerable OptionsWithArgumentLimitReached(SymbolResult symbolRe .Children .Where(c => c.IsArgumentLimitReached) .OfType() - .Select(o => o.Symbol) - .OfType() + .Select(o => o.Option) .SelectMany(c => c.Aliases); } diff --git a/src/System.CommandLine/Parsing/ParseResultVisitor.cs b/src/System.CommandLine/Parsing/ParseResultVisitor.cs index 5775d8d55e..53d7d5b48b 100644 --- a/src/System.CommandLine/Parsing/ParseResultVisitor.cs +++ b/src/System.CommandLine/Parsing/ParseResultVisitor.cs @@ -303,7 +303,7 @@ private void ValidateAndConvertArgumentResults(IList arguments, List= argumentResults.Count) @@ -604,7 +604,7 @@ void Handle(SymbolResult? symbolResult, Symbol symbol) optionResult.AddChild(childArgumentResult); commandResult.AddChild(optionResult); - _symbolResults.TryAdd(optionResult.Symbol, optionResult); + _symbolResults.TryAdd(optionResult.Option, optionResult); break; From 26396dc4a380f4438a2bff49b1de5d291d01601f Mon Sep 17 00:00:00 2001 From: Adam Sitnik Date: Thu, 12 Jan 2023 15:05:01 +0100 Subject: [PATCH 10/12] seal more internal derived types --- src/System.CommandLine/Parsing/CommandArgumentNode.cs | 2 +- src/System.CommandLine/Parsing/CommandNode.cs | 2 +- src/System.CommandLine/Parsing/DirectiveNode.cs | 2 +- src/System.CommandLine/Parsing/OptionArgumentNode.cs | 2 +- src/System.CommandLine/Parsing/OptionNode.cs | 2 +- 5 files changed, 5 insertions(+), 5 deletions(-) diff --git a/src/System.CommandLine/Parsing/CommandArgumentNode.cs b/src/System.CommandLine/Parsing/CommandArgumentNode.cs index b819c0e913..7c21746ff5 100644 --- a/src/System.CommandLine/Parsing/CommandArgumentNode.cs +++ b/src/System.CommandLine/Parsing/CommandArgumentNode.cs @@ -5,7 +5,7 @@ namespace System.CommandLine.Parsing { - internal class CommandArgumentNode : SyntaxNode + internal sealed class CommandArgumentNode : SyntaxNode { public CommandArgumentNode( Token token, diff --git a/src/System.CommandLine/Parsing/CommandNode.cs b/src/System.CommandLine/Parsing/CommandNode.cs index 93ac4ad352..63eeb9b589 100644 --- a/src/System.CommandLine/Parsing/CommandNode.cs +++ b/src/System.CommandLine/Parsing/CommandNode.cs @@ -3,7 +3,7 @@ namespace System.CommandLine.Parsing { - internal class CommandNode : NonterminalSyntaxNode + internal sealed class CommandNode : NonterminalSyntaxNode { public CommandNode( Token token, diff --git a/src/System.CommandLine/Parsing/DirectiveNode.cs b/src/System.CommandLine/Parsing/DirectiveNode.cs index 3766c417f3..c747212378 100644 --- a/src/System.CommandLine/Parsing/DirectiveNode.cs +++ b/src/System.CommandLine/Parsing/DirectiveNode.cs @@ -5,7 +5,7 @@ namespace System.CommandLine.Parsing { - internal class DirectiveNode : SyntaxNode + internal sealed class DirectiveNode : SyntaxNode { public DirectiveNode( Token token, diff --git a/src/System.CommandLine/Parsing/OptionArgumentNode.cs b/src/System.CommandLine/Parsing/OptionArgumentNode.cs index 0ccb37e333..f9452cd514 100644 --- a/src/System.CommandLine/Parsing/OptionArgumentNode.cs +++ b/src/System.CommandLine/Parsing/OptionArgumentNode.cs @@ -5,7 +5,7 @@ namespace System.CommandLine.Parsing { - internal class OptionArgumentNode : SyntaxNode + internal sealed class OptionArgumentNode : SyntaxNode { public OptionArgumentNode( Token token, diff --git a/src/System.CommandLine/Parsing/OptionNode.cs b/src/System.CommandLine/Parsing/OptionNode.cs index bc1cc800fd..25ca29023b 100644 --- a/src/System.CommandLine/Parsing/OptionNode.cs +++ b/src/System.CommandLine/Parsing/OptionNode.cs @@ -3,7 +3,7 @@ namespace System.CommandLine.Parsing { - internal class OptionNode : NonterminalSyntaxNode + internal sealed class OptionNode : NonterminalSyntaxNode { public OptionNode( Token token, From 603ccf71b7605b74b798aad03769c54cdd833753 Mon Sep 17 00:00:00 2001 From: Adam Sitnik Date: Thu, 12 Jan 2023 15:21:31 +0100 Subject: [PATCH 11/12] only CommandResult can have Children of OptionResult type don't create the enumerable more than once --- src/System.CommandLine/ParseResult.cs | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/src/System.CommandLine/ParseResult.cs b/src/System.CommandLine/ParseResult.cs index 0cd1a2044e..23d0326fa1 100644 --- a/src/System.CommandLine/ParseResult.cs +++ b/src/System.CommandLine/ParseResult.cs @@ -224,18 +224,23 @@ currentSymbol is not null ? currentSymbol.GetCompletions(context) : Array.Empty(); + string[] optionsWithArgumentLimitReached = currentSymbolResult is CommandResult commandResult + ? OptionsWithArgumentLimitReached(commandResult) + : Array.Empty(); + completions = - completions.Where(item => OptionsWithArgumentLimitReached(currentSymbolResult).All(s => s != item.Label)); + completions.Where(item => optionsWithArgumentLimitReached.All(s => s != item.Label)); return completions; - static IEnumerable OptionsWithArgumentLimitReached(SymbolResult symbolResult) => - symbolResult + static string[] OptionsWithArgumentLimitReached(CommandResult commandResult) => + commandResult .Children .Where(c => c.IsArgumentLimitReached) .OfType() .Select(o => o.Option) - .SelectMany(c => c.Aliases); + .SelectMany(c => c.Aliases) + .ToArray(); } private SymbolResult SymbolToComplete(int? position = null) From 75989fd497364fad04b8345fd08149c22c0773b2 Mon Sep 17 00:00:00 2001 From: Adam Sitnik Date: Thu, 12 Jan 2023 15:39:01 +0100 Subject: [PATCH 12/12] don't allocate an array if all we care about is the last element --- src/System.CommandLine/ParseResult.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/System.CommandLine/ParseResult.cs b/src/System.CommandLine/ParseResult.cs index 23d0326fa1..b34f2ffb22 100644 --- a/src/System.CommandLine/ParseResult.cs +++ b/src/System.CommandLine/ParseResult.cs @@ -247,7 +247,7 @@ private SymbolResult SymbolToComplete(int? position = null) { var commandResult = CommandResult; - var allSymbolResultsForCompletion = AllSymbolResultsForCompletion().ToArray(); + var allSymbolResultsForCompletion = AllSymbolResultsForCompletion(); var currentSymbol = allSymbolResultsForCompletion.Last();