From f8cdc393c0a10355448f1b53bdfabe7ae7dfdb73 Mon Sep 17 00:00:00 2001 From: Jon Sequeira Date: Wed, 30 Jul 2025 10:02:49 -0700 Subject: [PATCH 1/6] fix #2591 --- .../ParsingValidationTests.cs | 2 +- .../VersionOptionTests.cs | 18 ++++ .../Completions/CompletionAction.cs | 2 + src/System.CommandLine/Help/HelpAction.cs | 2 + .../Invocation/CommandLineAction.cs | 2 + .../Parsing/ParseOperation.cs | 85 +++++++++++++------ src/System.CommandLine/VersionOption.cs | 3 +- 7 files changed, 86 insertions(+), 28 deletions(-) diff --git a/src/System.CommandLine.Tests/ParsingValidationTests.cs b/src/System.CommandLine.Tests/ParsingValidationTests.cs index 76a0c6c979..e151f94a54 100644 --- a/src/System.CommandLine.Tests/ParsingValidationTests.cs +++ b/src/System.CommandLine.Tests/ParsingValidationTests.cs @@ -1269,7 +1269,7 @@ public void Multiple_validators_on_the_same_argument_do_not_report_duplicate_err } [Fact] // https://github.com/dotnet/command-line-api/issues/1609 - internal void When_there_is_an_arity_error_then_further_errors_are_not_reported() + public void When_there_is_an_arity_error_then_further_errors_are_not_reported() { var option = new Option("-o"); option.Validators.Add(result => diff --git a/src/System.CommandLine.Tests/VersionOptionTests.cs b/src/System.CommandLine.Tests/VersionOptionTests.cs index 135f0a8b14..e4d4424478 100644 --- a/src/System.CommandLine.Tests/VersionOptionTests.cs +++ b/src/System.CommandLine.Tests/VersionOptionTests.cs @@ -61,6 +61,24 @@ public void When_the_version_option_is_specified_then_there_are_no_parse_errors_ parseResult.Errors.Should().BeEmpty(); } + [Fact] // https://github.com/dotnet/command-line-api/issues/2591 + public void When_the_version_option_is_specified_then_there_are_no_parse_errors_due_to_missing_required_option() + { + Option option = new("-x") + { + Required = true + }; + RootCommand root = new() + { + option + }; + root.SetAction(_ => 0); + + var parseResult = root.Parse("--version"); + + parseResult.Errors.Should().BeEmpty(); + } + [Fact] public async Task Version_option_appears_in_help() { diff --git a/src/System.CommandLine/Completions/CompletionAction.cs b/src/System.CommandLine/Completions/CompletionAction.cs index e6e8f36518..daaa8e5ff9 100644 --- a/src/System.CommandLine/Completions/CompletionAction.cs +++ b/src/System.CommandLine/Completions/CompletionAction.cs @@ -39,4 +39,6 @@ public override int Invoke(ParseResult parseResult) return 0; } + + internal override bool IgnoresParseErrors => true; } \ No newline at end of file diff --git a/src/System.CommandLine/Help/HelpAction.cs b/src/System.CommandLine/Help/HelpAction.cs index 8b6460631a..023c236dce 100644 --- a/src/System.CommandLine/Help/HelpAction.cs +++ b/src/System.CommandLine/Help/HelpAction.cs @@ -65,5 +65,7 @@ public override int Invoke(ParseResult parseResult) return 0; } + + internal override bool IgnoresParseErrors => true; } } \ No newline at end of file diff --git a/src/System.CommandLine/Invocation/CommandLineAction.cs b/src/System.CommandLine/Invocation/CommandLineAction.cs index e97c0827cf..c4c74b7a66 100644 --- a/src/System.CommandLine/Invocation/CommandLineAction.cs +++ b/src/System.CommandLine/Invocation/CommandLineAction.cs @@ -16,4 +16,6 @@ private protected CommandLineAction() /// Indicates that the action terminates a command line invocation, and later actions are skipped. /// public bool Terminating { get; protected init; } = true; + + internal virtual bool IgnoresParseErrors => false; } \ No newline at end of file diff --git a/src/System.CommandLine/Parsing/ParseOperation.cs b/src/System.CommandLine/Parsing/ParseOperation.cs index c5c7b9fdf8..e94aa59da9 100644 --- a/src/System.CommandLine/Parsing/ParseOperation.cs +++ b/src/System.CommandLine/Parsing/ParseOperation.cs @@ -2,8 +2,8 @@ // Licensed under the MIT license. See LICENSE file in the project root for full license information. using System.Collections.Generic; -using System.CommandLine.Help; using System.CommandLine.Invocation; +using System.Linq; namespace System.CommandLine.Parsing { @@ -17,7 +17,6 @@ internal sealed class ParseOperation private int _index; private CommandResult _innermostCommandResult; - private bool _isHelpRequested; private bool _isTerminatingDirectiveSpecified; private CommandLineAction? _primaryAction; private List? _preActions; @@ -63,21 +62,7 @@ internal ParseResult Parse() ValidateAndAddDefaultResults(); - - if (_isHelpRequested) - { - _symbolResultTree.Errors?.Clear(); - } - - if (_primaryAction is null) - { - if (_symbolResultTree.ErrorCount > 0) - { - _primaryAction = new ParseErrorAction(); - } - } - - return new ( + return new( _configuration, _rootCommandResult, _innermostCommandResult, @@ -197,11 +182,6 @@ private void ParseOption() // directives have a precedence over --help and --version if (!_isTerminatingDirectiveSpecified) { - if (option is HelpOption) - { - _isHelpRequested = true; - } - if (option.Action.Terminating) { _primaryAction = option.Action; @@ -386,11 +366,64 @@ private void ValidateAndAddDefaultResults() currentResult = currentResult.Parent as CommandResult; } - if (_primaryAction is null && - _innermostCommandResult is { Command: { Action: null, HasSubcommands: true } }) + if (_primaryAction is null) + { + if (_innermostCommandResult is { Command: { Action: null, HasSubcommands: true } }) + { + _symbolResultTree.InsertFirstError( + new ParseError(LocalizationResources.RequiredCommandWasNotProvided(), _innermostCommandResult)); + } + + if (_symbolResultTree.ErrorCount > 0) + { + _primaryAction = new ParseErrorAction(); + } + } + else { - _symbolResultTree.InsertFirstError( - new ParseError(LocalizationResources.RequiredCommandWasNotProvided(), _innermostCommandResult)); + if (_symbolResultTree.ErrorCount > 0 && + _primaryAction.IgnoresParseErrors && + _symbolResultTree.Errors is not null) + { + foreach (var kvp in _symbolResultTree) + { + var symbol = kvp.Key; + if (symbol is Option { Action: { } optionAction } option) + { + if (_primaryAction == optionAction) + { + var errorsForPrimarySymbol = _symbolResultTree + .Errors + .Where(e => e.SymbolResult is OptionResult r && r.Option == option) + .ToList(); + + _symbolResultTree.Errors = errorsForPrimarySymbol; + + return; + } + } + + if (symbol is Command { Action: { } commandAction } command) + { + if (_primaryAction == commandAction) + { + var errorsForPrimarySymbol = _symbolResultTree + .Errors + .Where(e => e.SymbolResult is CommandResult r && r.Command == command) + .ToList(); + + _symbolResultTree.Errors = errorsForPrimarySymbol; + + return; + } + } + } + + if (_symbolResultTree.ErrorCount > 0) + { + _symbolResultTree.Errors?.Clear(); + } + } } } } diff --git a/src/System.CommandLine/VersionOption.cs b/src/System.CommandLine/VersionOption.cs index 0fde75a764..5369c3c492 100644 --- a/src/System.CommandLine/VersionOption.cs +++ b/src/System.CommandLine/VersionOption.cs @@ -1,7 +1,6 @@ // 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.Help; using System.CommandLine.Invocation; using System.CommandLine.Parsing; using System.Linq; @@ -68,6 +67,8 @@ public override int Invoke(ParseResult parseResult) parseResult.InvocationConfiguration.Output.WriteLine(RootCommand.ExecutableVersion); return 0; } + + internal override bool IgnoresParseErrors => true; } } } \ No newline at end of file From ca03fd1c200bc6c85f20b3e1d39e89de32c4b6d2 Mon Sep 17 00:00:00 2001 From: Jon Sequeira Date: Sun, 3 Aug 2025 10:49:49 -0700 Subject: [PATCH 2/6] move LocalizationTests out of ApiCompatibility.Tests --- .../LocalizationTests.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) rename src/{System.CommandLine.ApiCompatibility.Tests => System.CommandLine.Tests}/LocalizationTests.cs (95%) diff --git a/src/System.CommandLine.ApiCompatibility.Tests/LocalizationTests.cs b/src/System.CommandLine.Tests/LocalizationTests.cs similarity index 95% rename from src/System.CommandLine.ApiCompatibility.Tests/LocalizationTests.cs rename to src/System.CommandLine.Tests/LocalizationTests.cs index 3d730759b1..e79811f683 100644 --- a/src/System.CommandLine.ApiCompatibility.Tests/LocalizationTests.cs +++ b/src/System.CommandLine.Tests/LocalizationTests.cs @@ -2,7 +2,7 @@ using System.Linq; using Xunit; -namespace System.CommandLine.ApiCompatibility.Tests +namespace System.CommandLine.Tests { public class LocalizationTests { From 2f6a452a17b8e806d649cfce716abffa694b779f Mon Sep 17 00:00:00 2001 From: Jon Sequeira Date: Sun, 3 Aug 2025 10:51:03 -0700 Subject: [PATCH 3/6] rename IgnoresParseErrors to ClearsParseErrors --- ...ovalTests.System_CommandLine_api_is_not_changed.approved.txt | 2 ++ src/System.CommandLine/Completions/CompletionAction.cs | 2 +- src/System.CommandLine/Help/HelpAction.cs | 2 +- src/System.CommandLine/Invocation/CommandLineAction.cs | 2 +- src/System.CommandLine/Parsing/ParseOperation.cs | 2 +- src/System.CommandLine/VersionOption.cs | 2 +- 6 files changed, 7 insertions(+), 5 deletions(-) diff --git a/src/System.CommandLine.ApiCompatibility.Tests/ApiCompatibilityApprovalTests.System_CommandLine_api_is_not_changed.approved.txt b/src/System.CommandLine.ApiCompatibility.Tests/ApiCompatibilityApprovalTests.System_CommandLine_api_is_not_changed.approved.txt index cd832ed67a..f19908599f 100644 --- a/src/System.CommandLine.ApiCompatibility.Tests/ApiCompatibilityApprovalTests.System_CommandLine_api_is_not_changed.approved.txt +++ b/src/System.CommandLine.ApiCompatibility.Tests/ApiCompatibilityApprovalTests.System_CommandLine_api_is_not_changed.approved.txt @@ -179,6 +179,7 @@ System.CommandLine.Completions System.CommandLine.Help public class HelpAction : System.CommandLine.Invocation.SynchronousCommandLineAction .ctor() + public System.Boolean ClearsParseErrors { get; } public System.Int32 MaxWidth { get; set; } public System.Int32 Invoke(System.CommandLine.ParseResult parseResult) public class HelpOption : System.CommandLine.Option @@ -190,6 +191,7 @@ System.CommandLine.Invocation public abstract class AsynchronousCommandLineAction : CommandLineAction public System.Threading.Tasks.Task InvokeAsync(System.CommandLine.ParseResult parseResult, System.Threading.CancellationToken cancellationToken = null) public abstract class CommandLineAction + public System.Boolean ClearsParseErrors { get; } public System.Boolean Terminating { get; } protected System.Void set_Terminating(System.Boolean value) public class ParseErrorAction : SynchronousCommandLineAction diff --git a/src/System.CommandLine/Completions/CompletionAction.cs b/src/System.CommandLine/Completions/CompletionAction.cs index daaa8e5ff9..44cee60f58 100644 --- a/src/System.CommandLine/Completions/CompletionAction.cs +++ b/src/System.CommandLine/Completions/CompletionAction.cs @@ -40,5 +40,5 @@ public override int Invoke(ParseResult parseResult) return 0; } - internal override bool IgnoresParseErrors => true; + public override bool ClearsParseErrors => true; } \ No newline at end of file diff --git a/src/System.CommandLine/Help/HelpAction.cs b/src/System.CommandLine/Help/HelpAction.cs index 023c236dce..5566380d7e 100644 --- a/src/System.CommandLine/Help/HelpAction.cs +++ b/src/System.CommandLine/Help/HelpAction.cs @@ -66,6 +66,6 @@ public override int Invoke(ParseResult parseResult) return 0; } - internal override bool IgnoresParseErrors => true; + public override bool ClearsParseErrors => true; } } \ No newline at end of file diff --git a/src/System.CommandLine/Invocation/CommandLineAction.cs b/src/System.CommandLine/Invocation/CommandLineAction.cs index c4c74b7a66..4980d44799 100644 --- a/src/System.CommandLine/Invocation/CommandLineAction.cs +++ b/src/System.CommandLine/Invocation/CommandLineAction.cs @@ -17,5 +17,5 @@ private protected CommandLineAction() /// public bool Terminating { get; protected init; } = true; - internal virtual bool IgnoresParseErrors => false; + public virtual bool ClearsParseErrors => false; } \ No newline at end of file diff --git a/src/System.CommandLine/Parsing/ParseOperation.cs b/src/System.CommandLine/Parsing/ParseOperation.cs index e94aa59da9..1907eda126 100644 --- a/src/System.CommandLine/Parsing/ParseOperation.cs +++ b/src/System.CommandLine/Parsing/ParseOperation.cs @@ -382,7 +382,7 @@ private void ValidateAndAddDefaultResults() else { if (_symbolResultTree.ErrorCount > 0 && - _primaryAction.IgnoresParseErrors && + _primaryAction.ClearsParseErrors && _symbolResultTree.Errors is not null) { foreach (var kvp in _symbolResultTree) diff --git a/src/System.CommandLine/VersionOption.cs b/src/System.CommandLine/VersionOption.cs index 5369c3c492..2aaef04e8a 100644 --- a/src/System.CommandLine/VersionOption.cs +++ b/src/System.CommandLine/VersionOption.cs @@ -68,7 +68,7 @@ public override int Invoke(ParseResult parseResult) return 0; } - internal override bool IgnoresParseErrors => true; + public override bool ClearsParseErrors => true; } } } \ No newline at end of file From fbe20433a9e4060e85124d16a488c8c188fdeb35 Mon Sep 17 00:00:00 2001 From: Jon Sequeira Date: Sun, 3 Aug 2025 11:54:22 -0700 Subject: [PATCH 4/6] add a few tests for ClearsParseErrors --- .../ParseErrorReportingTests.cs | 38 ++++++++++++++++++- .../{TestCliActions.cs => TestActions.cs} | 16 +++++++- .../Parsing/ParseOperation.cs | 14 ++++++- 3 files changed, 63 insertions(+), 5 deletions(-) rename src/System.CommandLine.Tests/{TestCliActions.cs => TestActions.cs} (67%) diff --git a/src/System.CommandLine.Tests/ParseErrorReportingTests.cs b/src/System.CommandLine.Tests/ParseErrorReportingTests.cs index 824029b5d3..53af52b33c 100644 --- a/src/System.CommandLine.Tests/ParseErrorReportingTests.cs +++ b/src/System.CommandLine.Tests/ParseErrorReportingTests.cs @@ -15,7 +15,7 @@ namespace System.CommandLine.Tests; public class ParseErrorReportingTests { [Fact] // https://github.com/dotnet/command-line-api/issues/817 - public void Parse_error_reporting_reports_error_when_help_is_used_and_required_subcommand_is_missing() + public void Help_is_shown_when_required_subcommand_is_missing() { var root = new RootCommand { @@ -112,4 +112,40 @@ public void When_no_help_option_is_present_then_help_is_not_shown_for_parse_erro output.ToString().Should().NotShowHelp(); } + + [Fact] + public void Custom_action_can_ignore_parse_errors_on_child_commands() + { + Command subcommand = new Command("subcommand"); + subcommand.Action = new SynchronousTestAction( + _ => { }, + true, + true); + var rootCommand = new RootCommand + { + Subcommands = { subcommand } + }; + + var result = rootCommand.Parse("subcommand --nonexistent option and other things"); + + result.Errors.Should().BeEmpty(); + } + + [Fact] + public void Custom_action_cannot_ignore_parse_errors_on_parent_commands() + { + Command subcommand = new Command("subcommand"); + subcommand.Action = new SynchronousTestAction( + _ => { }, + true, + true); + var rootCommand = new RootCommand + { + Subcommands = { subcommand } + }; + + var result = rootCommand.Parse("--what nope subcommand"); + + result.Errors.Should().NotBeEmpty(); + } } \ No newline at end of file diff --git a/src/System.CommandLine.Tests/TestCliActions.cs b/src/System.CommandLine.Tests/TestActions.cs similarity index 67% rename from src/System.CommandLine.Tests/TestCliActions.cs rename to src/System.CommandLine.Tests/TestActions.cs index 889d3b06ae..eb948b3cce 100644 --- a/src/System.CommandLine.Tests/TestCliActions.cs +++ b/src/System.CommandLine.Tests/TestActions.cs @@ -11,12 +11,18 @@ public class SynchronousTestAction : SynchronousCommandLineAction { private readonly Action _invoke; - public SynchronousTestAction(Action invoke, bool terminating = true) + public SynchronousTestAction( + Action invoke, + bool terminating = true, + bool clearsParseErrors = false) { + ClearsParseErrors = clearsParseErrors; _invoke = invoke; Terminating = terminating; } + public override bool ClearsParseErrors { get; } + public override int Invoke(ParseResult parseResult) { _invoke(parseResult); @@ -28,12 +34,18 @@ public class AsynchronousTestAction : AsynchronousCommandLineAction { private readonly Action _invoke; - public AsynchronousTestAction(Action invoke, bool terminating = true) + public AsynchronousTestAction( + Action invoke, + bool terminating = true, + bool clearsParseErrors = false) { + ClearsParseErrors = clearsParseErrors; _invoke = invoke; Terminating = terminating; } + public override bool ClearsParseErrors { get; } + public override Task InvokeAsync(ParseResult parseResult, CancellationToken cancellationToken = default) { _invoke(parseResult); diff --git a/src/System.CommandLine/Parsing/ParseOperation.cs b/src/System.CommandLine/Parsing/ParseOperation.cs index 1907eda126..0ae9c4bcf3 100644 --- a/src/System.CommandLine/Parsing/ParseOperation.cs +++ b/src/System.CommandLine/Parsing/ParseOperation.cs @@ -374,7 +374,17 @@ private void ValidateAndAddDefaultResults() new ParseError(LocalizationResources.RequiredCommandWasNotProvided(), _innermostCommandResult)); } - if (_symbolResultTree.ErrorCount > 0) + if (_innermostCommandResult is { Command.Action.ClearsParseErrors: true } && + _symbolResultTree.Errors is not null) + { + var errorsNotUnderInnermostCommand = _symbolResultTree + .Errors + .Where(e => e.SymbolResult != _innermostCommandResult) + .ToList(); + + _symbolResultTree.Errors = errorsNotUnderInnermostCommand; + } + else if (_symbolResultTree.ErrorCount > 0) { _primaryAction = new ParseErrorAction(); } @@ -382,7 +392,7 @@ private void ValidateAndAddDefaultResults() else { if (_symbolResultTree.ErrorCount > 0 && - _primaryAction.ClearsParseErrors && + _primaryAction.ClearsParseErrors && _symbolResultTree.Errors is not null) { foreach (var kvp in _symbolResultTree) From fca5b5db5b8c00f8fb14b6be6e00655b8b2e1881 Mon Sep 17 00:00:00 2001 From: Jon Sequeira Date: Mon, 4 Aug 2025 09:32:41 -0700 Subject: [PATCH 5/6] add test to verify preactions can't clear parse errors --- .../ParseErrorReportingTests.cs | 24 +++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/src/System.CommandLine.Tests/ParseErrorReportingTests.cs b/src/System.CommandLine.Tests/ParseErrorReportingTests.cs index 53af52b33c..6b59494ea8 100644 --- a/src/System.CommandLine.Tests/ParseErrorReportingTests.cs +++ b/src/System.CommandLine.Tests/ParseErrorReportingTests.cs @@ -148,4 +148,28 @@ public void Custom_action_cannot_ignore_parse_errors_on_parent_commands() result.Errors.Should().NotBeEmpty(); } + + [Fact] + public void Pre_actions_cannot_clear_parse_errors() + { + var rootCommand = new RootCommand + { + Directives = + { + new Directive("pre") + { + Action = new SynchronousTestAction( + _ => { }, + false, + true) + } + }, + }; + + rootCommand.SetAction(_ => { }); + + var result = rootCommand.Parse("[pre] --not valid"); + + result.Errors.Should().NotBeEmpty(); + } } \ No newline at end of file From 9b465fc4798e115bf69db736617346bfceb145c7 Mon Sep 17 00:00:00 2001 From: Jon Sequeira Date: Mon, 4 Aug 2025 10:01:50 -0700 Subject: [PATCH 6/6] add XML doc comment for ClearsParseErrors --- src/System.CommandLine/Invocation/CommandLineAction.cs | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/System.CommandLine/Invocation/CommandLineAction.cs b/src/System.CommandLine/Invocation/CommandLineAction.cs index 4980d44799..0a9cbe5ad9 100644 --- a/src/System.CommandLine/Invocation/CommandLineAction.cs +++ b/src/System.CommandLine/Invocation/CommandLineAction.cs @@ -17,5 +17,9 @@ private protected CommandLineAction() /// public bool Terminating { get; protected init; } = true; + /// + /// Indicates that the action clears any parse errors associated with symbols other than one that owns the . + /// + /// This property is ignored when is set to . public virtual bool ClearsParseErrors => false; } \ No newline at end of file