Skip to content

Commit 5d9ac09

Browse files
jonsequiturjonsequitur
authored
fix #2591 (#2644)
* fix #2591 * move LocalizationTests out of ApiCompatibility.Tests * rename IgnoresParseErrors to ClearsParseErrors * add a few tests for ClearsParseErrors * add test to verify preactions can't clear parse errors * add XML doc comment for ClearsParseErrors
1 parent 1decc9d commit 5d9ac09

File tree

11 files changed

+178
-32
lines changed

11 files changed

+178
-32
lines changed

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

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -179,6 +179,7 @@ System.CommandLine.Completions
179179
System.CommandLine.Help
180180
public class HelpAction : System.CommandLine.Invocation.SynchronousCommandLineAction
181181
.ctor()
182+
public System.Boolean ClearsParseErrors { get; }
182183
public System.Int32 MaxWidth { get; set; }
183184
public System.Int32 Invoke(System.CommandLine.ParseResult parseResult)
184185
public class HelpOption : System.CommandLine.Option
@@ -190,6 +191,7 @@ System.CommandLine.Invocation
190191
public abstract class AsynchronousCommandLineAction : CommandLineAction
191192
public System.Threading.Tasks.Task<System.Int32> InvokeAsync(System.CommandLine.ParseResult parseResult, System.Threading.CancellationToken cancellationToken = null)
192193
public abstract class CommandLineAction
194+
public System.Boolean ClearsParseErrors { get; }
193195
public System.Boolean Terminating { get; }
194196
protected System.Void set_Terminating(System.Boolean value)
195197
public class ParseErrorAction : SynchronousCommandLineAction

‎src/System.CommandLine.ApiCompatibility.Tests/LocalizationTests.cs‎ renamed to ‎src/System.CommandLine.Tests/LocalizationTests.cs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22
using System.Linq;
33
using Xunit;
44

5-
namespace System.CommandLine.ApiCompatibility.Tests
5+
namespace System.CommandLine.Tests
66
{
77
public class LocalizationTests
88
{

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

Lines changed: 61 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@ namespace System.CommandLine.Tests;
1515
public class ParseErrorReportingTests
1616
{
1717
[Fact] // https://github.com/dotnet/command-line-api/issues/817
18-
public void Parse_error_reporting_reports_error_when_help_is_used_and_required_subcommand_is_missing()
18+
public void Help_is_shown_when_required_subcommand_is_missing()
1919
{
2020
var root = new RootCommand
2121
{
@@ -112,4 +112,64 @@ public void When_no_help_option_is_present_then_help_is_not_shown_for_parse_erro
112112

113113
output.ToString().Should().NotShowHelp();
114114
}
115+
116+
[Fact]
117+
public void Custom_action_can_ignore_parse_errors_on_child_commands()
118+
{
119+
Command subcommand = new Command("subcommand");
120+
subcommand.Action = new SynchronousTestAction(
121+
_ => { },
122+
true,
123+
true);
124+
var rootCommand = new RootCommand
125+
{
126+
Subcommands = { subcommand }
127+
};
128+
129+
var result = rootCommand.Parse("subcommand --nonexistent option and other things");
130+
131+
result.Errors.Should().BeEmpty();
132+
}
133+
134+
[Fact]
135+
public void Custom_action_cannot_ignore_parse_errors_on_parent_commands()
136+
{
137+
Command subcommand = new Command("subcommand");
138+
subcommand.Action = new SynchronousTestAction(
139+
_ => { },
140+
true,
141+
true);
142+
var rootCommand = new RootCommand
143+
{
144+
Subcommands = { subcommand }
145+
};
146+
147+
var result = rootCommand.Parse("--what nope subcommand");
148+
149+
result.Errors.Should().NotBeEmpty();
150+
}
151+
152+
[Fact]
153+
public void Pre_actions_cannot_clear_parse_errors()
154+
{
155+
var rootCommand = new RootCommand
156+
{
157+
Directives =
158+
{
159+
new Directive("pre")
160+
{
161+
Action = new SynchronousTestAction(
162+
_ => { },
163+
false,
164+
true)
165+
}
166+
},
167+
};
168+
169+
rootCommand.SetAction(_ => { });
170+
171+
var result = rootCommand.Parse("[pre] --not valid");
172+
173+
result.Errors.Should().NotBeEmpty();
174+
}
115175
}

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1269,7 +1269,7 @@ public void Multiple_validators_on_the_same_argument_do_not_report_duplicate_err
12691269
}
12701270

12711271
[Fact] // https://github.com/dotnet/command-line-api/issues/1609
1272-
internal void When_there_is_an_arity_error_then_further_errors_are_not_reported()
1272+
public void When_there_is_an_arity_error_then_further_errors_are_not_reported()
12731273
{
12741274
var option = new Option<string>("-o");
12751275
option.Validators.Add(result =>

‎src/System.CommandLine.Tests/TestCliActions.cs‎ renamed to ‎src/System.CommandLine.Tests/TestActions.cs‎

Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -11,12 +11,18 @@ public class SynchronousTestAction : SynchronousCommandLineAction
1111
{
1212
private readonly Action<ParseResult> _invoke;
1313

14-
public SynchronousTestAction(Action<ParseResult> invoke, bool terminating = true)
14+
public SynchronousTestAction(
15+
Action<ParseResult> invoke,
16+
bool terminating = true,
17+
bool clearsParseErrors = false)
1518
{
19+
ClearsParseErrors = clearsParseErrors;
1620
_invoke = invoke;
1721
Terminating = terminating;
1822
}
1923

24+
public override bool ClearsParseErrors { get; }
25+
2026
public override int Invoke(ParseResult parseResult)
2127
{
2228
_invoke(parseResult);
@@ -28,12 +34,18 @@ public class AsynchronousTestAction : AsynchronousCommandLineAction
2834
{
2935
private readonly Action<ParseResult> _invoke;
3036

31-
public AsynchronousTestAction(Action<ParseResult> invoke, bool terminating = true)
37+
public AsynchronousTestAction(
38+
Action<ParseResult> invoke,
39+
bool terminating = true,
40+
bool clearsParseErrors = false)
3241
{
42+
ClearsParseErrors = clearsParseErrors;
3343
_invoke = invoke;
3444
Terminating = terminating;
3545
}
3646

47+
public override bool ClearsParseErrors { get; }
48+
3749
public override Task<int> InvokeAsync(ParseResult parseResult, CancellationToken cancellationToken = default)
3850
{
3951
_invoke(parseResult);

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

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -61,6 +61,24 @@ public void When_the_version_option_is_specified_then_there_are_no_parse_errors_
6161
parseResult.Errors.Should().BeEmpty();
6262
}
6363

64+
[Fact] // https://github.com/dotnet/command-line-api/issues/2591
65+
public void When_the_version_option_is_specified_then_there_are_no_parse_errors_due_to_missing_required_option()
66+
{
67+
Option<int> option = new("-x")
68+
{
69+
Required = true
70+
};
71+
RootCommand root = new()
72+
{
73+
option
74+
};
75+
root.SetAction(_ => 0);
76+
77+
var parseResult = root.Parse("--version");
78+
79+
parseResult.Errors.Should().BeEmpty();
80+
}
81+
6482
[Fact]
6583
public async Task Version_option_appears_in_help()
6684
{

‎src/System.CommandLine/Completions/CompletionAction.cs‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -39,4 +39,6 @@ public override int Invoke(ParseResult parseResult)
3939

4040
return 0;
4141
}
42+
43+
public override bool ClearsParseErrors => true;
4244
}

‎src/System.CommandLine/Help/HelpAction.cs‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -65,5 +65,7 @@ public override int Invoke(ParseResult parseResult)
6565

6666
return 0;
6767
}
68+
69+
public override bool ClearsParseErrors => true;
6870
}
6971
}

‎src/System.CommandLine/Invocation/CommandLineAction.cs‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,4 +16,10 @@ private protected CommandLineAction()
1616
/// Indicates that the action terminates a command line invocation, and later actions are skipped.
1717
/// </summary>
1818
public bool Terminating { get; protected init; } = true;
19+
20+
/// <summary>
21+
/// Indicates that the action clears any parse errors associated with symbols other than one that owns the <see cref="CommandLineAction"/>.
22+
/// </summary>
23+
/// <remarks>This property is ignored when <see cref="Terminating"/> is set to <see langword="false"/>.</remarks>
24+
public virtual bool ClearsParseErrors => false;
1925
}

‎src/System.CommandLine/Parsing/ParseOperation.cs‎

Lines changed: 69 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -2,8 +2,8 @@
22
// Licensed under the MIT license. See LICENSE file in the project root for full license information.
33

44
using System.Collections.Generic;
5-
using System.CommandLine.Help;
65
using System.CommandLine.Invocation;
6+
using System.Linq;
77

88
namespace System.CommandLine.Parsing
99
{
@@ -17,7 +17,6 @@ internal sealed class ParseOperation
1717

1818
private int _index;
1919
private CommandResult _innermostCommandResult;
20-
private bool _isHelpRequested;
2120
private bool _isTerminatingDirectiveSpecified;
2221
private CommandLineAction? _primaryAction;
2322
private List<CommandLineAction>? _preActions;
@@ -63,21 +62,7 @@ internal ParseResult Parse()
6362

6463
ValidateAndAddDefaultResults();
6564

66-
67-
if (_isHelpRequested)
68-
{
69-
_symbolResultTree.Errors?.Clear();
70-
}
71-
72-
if (_primaryAction is null)
73-
{
74-
if (_symbolResultTree.ErrorCount > 0)
75-
{
76-
_primaryAction = new ParseErrorAction();
77-
}
78-
}
79-
80-
return new (
65+
return new(
8166
_configuration,
8267
_rootCommandResult,
8368
_innermostCommandResult,
@@ -197,11 +182,6 @@ private void ParseOption()
197182
// directives have a precedence over --help and --version
198183
if (!_isTerminatingDirectiveSpecified)
199184
{
200-
if (option is HelpOption)
201-
{
202-
_isHelpRequested = true;
203-
}
204-
205185
if (option.Action.Terminating)
206186
{
207187
_primaryAction = option.Action;
@@ -386,11 +366,74 @@ private void ValidateAndAddDefaultResults()
386366
currentResult = currentResult.Parent as CommandResult;
387367
}
388368

389-
if (_primaryAction is null &&
390-
_innermostCommandResult is { Command: { Action: null, HasSubcommands: true } })
369+
if (_primaryAction is null)
391370
{
392-
_symbolResultTree.InsertFirstError(
393-
new ParseError(LocalizationResources.RequiredCommandWasNotProvided(), _innermostCommandResult));
371+
if (_innermostCommandResult is { Command: { Action: null, HasSubcommands: true } })
372+
{
373+
_symbolResultTree.InsertFirstError(
374+
new ParseError(LocalizationResources.RequiredCommandWasNotProvided(), _innermostCommandResult));
375+
}
376+
377+
if (_innermostCommandResult is { Command.Action.ClearsParseErrors: true } &&
378+
_symbolResultTree.Errors is not null)
379+
{
380+
var errorsNotUnderInnermostCommand = _symbolResultTree
381+
.Errors
382+
.Where(e => e.SymbolResult != _innermostCommandResult)
383+
.ToList();
384+
385+
_symbolResultTree.Errors = errorsNotUnderInnermostCommand;
386+
}
387+
else if (_symbolResultTree.ErrorCount > 0)
388+
{
389+
_primaryAction = new ParseErrorAction();
390+
}
391+
}
392+
else
393+
{
394+
if (_symbolResultTree.ErrorCount > 0 &&
395+
_primaryAction.ClearsParseErrors &&
396+
_symbolResultTree.Errors is not null)
397+
{
398+
foreach (var kvp in _symbolResultTree)
399+
{
400+
var symbol = kvp.Key;
401+
if (symbol is Option { Action: { } optionAction } option)
402+
{
403+
if (_primaryAction == optionAction)
404+
{
405+
var errorsForPrimarySymbol = _symbolResultTree
406+
.Errors
407+
.Where(e => e.SymbolResult is OptionResult r && r.Option == option)
408+
.ToList();
409+
410+
_symbolResultTree.Errors = errorsForPrimarySymbol;
411+
412+
return;
413+
}
414+
}
415+
416+
if (symbol is Command { Action: { } commandAction } command)
417+
{
418+
if (_primaryAction == commandAction)
419+
{
420+
var errorsForPrimarySymbol = _symbolResultTree
421+
.Errors
422+
.Where(e => e.SymbolResult is CommandResult r && r.Command == command)
423+
.ToList();
424+
425+
_symbolResultTree.Errors = errorsForPrimarySymbol;
426+
427+
return;
428+
}
429+
}
430+
}
431+
432+
if (_symbolResultTree.ErrorCount > 0)
433+
{
434+
_symbolResultTree.Errors?.Clear();
435+
}
436+
}
394437
}
395438
}
396439
}

0 commit comments

Comments
 (0)