Skip to content

Commit d3de878

Browse files
jonsequiturJon Sequeira
jonsequitur
and
Jon Sequeira
authored
fix #2771 and #2772 - Option action fixes (#2782)
* fix #2771 and #2772 * InvocationPipeline improvements based on PR feedback --------- Co-authored-by: Jon Sequeira <josequ+microsoft@microsoft.com>
1 parent baaad19 commit d3de878

3 files changed

Lines changed: 156 additions & 47 deletions

File tree

‎src/System.CommandLine.Tests/Invocation/InvocationTests.cs‎

Lines changed: 95 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -436,6 +436,101 @@ public async Task Nonterminating_option_actions_handle_exceptions_and_return_an_
436436
returnCode.Should().Be(1);
437437
}
438438

439+
[Theory] // https://github.com/dotnet/command-line-api/issues/2771
440+
[InlineData(true)]
441+
[InlineData(false)]
442+
public async Task Nonterminating_option_action_is_invoked_when_command_has_no_action(bool invokeAsync)
443+
{
444+
bool optionActionWasCalled = false;
445+
SynchronousTestAction optionAction = new(_ => optionActionWasCalled = true, terminating: false);
446+
447+
Option<bool> option = new("--test")
448+
{
449+
Action = optionAction
450+
};
451+
RootCommand command = new()
452+
{
453+
option
454+
};
455+
456+
ParseResult parseResult = command.Parse("--test");
457+
458+
if (invokeAsync)
459+
{
460+
await parseResult.InvokeAsync();
461+
}
462+
else
463+
{
464+
parseResult.Invoke();
465+
}
466+
467+
optionActionWasCalled.Should().BeTrue();
468+
}
469+
470+
[Theory] // https://github.com/dotnet/command-line-api/issues/2772
471+
[InlineData(true)]
472+
[InlineData(false)]
473+
public async Task Nonterminating_option_action_return_value_is_propagated(bool invokeAsync)
474+
{
475+
SynchronousTestAction optionAction = new(_ => { }, terminating: false, returnValue: 42);
476+
477+
Option<bool> option = new("--test")
478+
{
479+
Action = optionAction
480+
};
481+
RootCommand command = new()
482+
{
483+
option
484+
};
485+
command.SetAction(_ => { });
486+
487+
ParseResult parseResult = command.Parse("--test");
488+
489+
int result;
490+
if (invokeAsync)
491+
{
492+
result = await parseResult.InvokeAsync();
493+
}
494+
else
495+
{
496+
result = parseResult.Invoke();
497+
}
498+
499+
result.Should().Be(42);
500+
}
501+
502+
[Theory] // https://github.com/dotnet/command-line-api/issues/2772
503+
[InlineData(true)]
504+
[InlineData(false)]
505+
public async Task When_preaction_and_command_action_both_return_nonzero_then_preaction_value_wins(bool invokeAsync)
506+
{
507+
SynchronousTestAction optionAction = new(_ => { }, terminating: false, returnValue: 42);
508+
509+
Option<bool> option = new("--test")
510+
{
511+
Action = optionAction
512+
};
513+
RootCommand command = new()
514+
{
515+
option
516+
};
517+
command.SetAction(_ => 99);
518+
519+
ParseResult parseResult = command.Parse("--test");
520+
521+
int result;
522+
if (invokeAsync)
523+
{
524+
result = await parseResult.InvokeAsync();
525+
}
526+
else
527+
{
528+
result = parseResult.Invoke();
529+
}
530+
531+
result.Should().Be(42);
532+
}
533+
439534
[Fact]
440535
public async Task Command_InvokeAsync_with_cancelation_token_invokes_command_handler()
441536
{

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

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -10,14 +10,17 @@ namespace System.CommandLine.Tests;
1010
public class SynchronousTestAction : SynchronousCommandLineAction
1111
{
1212
private readonly Action<ParseResult> _invoke;
13+
private readonly int _returnValue;
1314

1415
public SynchronousTestAction(
1516
Action<ParseResult> invoke,
1617
bool terminating = true,
17-
bool clearsParseErrors = false)
18+
bool clearsParseErrors = false,
19+
int returnValue = 0)
1820
{
1921
ClearsParseErrors = clearsParseErrors;
2022
_invoke = invoke;
23+
_returnValue = returnValue;
2124
Terminating = terminating;
2225
}
2326

@@ -28,21 +31,24 @@ public SynchronousTestAction(
2831
public override int Invoke(ParseResult parseResult)
2932
{
3033
_invoke(parseResult);
31-
return 0;
34+
return _returnValue;
3235
}
3336
}
3437

3538
public class AsynchronousTestAction : AsynchronousCommandLineAction
3639
{
3740
private readonly Action<ParseResult> _invoke;
41+
private readonly int _returnValue;
3842

3943
public AsynchronousTestAction(
4044
Action<ParseResult> invoke,
4145
bool terminating = true,
42-
bool clearsParseErrors = false)
46+
bool clearsParseErrors = false,
47+
int returnValue = 0)
4348
{
4449
ClearsParseErrors = clearsParseErrors;
4550
_invoke = invoke;
51+
_returnValue = returnValue;
4652
Terminating = terminating;
4753
}
4854

@@ -53,6 +59,6 @@ public AsynchronousTestAction(
5359
public override Task<int> InvokeAsync(ParseResult parseResult, CancellationToken cancellationToken = default)
5460
{
5561
_invoke(parseResult);
56-
return Task.FromResult(0);
62+
return Task.FromResult(_returnValue);
5763
}
5864
}

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

Lines changed: 51 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -10,38 +10,49 @@ internal static class InvocationPipeline
1010
{
1111
internal static async Task<int> InvokeAsync(ParseResult parseResult, CancellationToken cancellationToken)
1212
{
13-
if (parseResult.Action is null)
14-
{
15-
return ReturnCodeForMissingAction(parseResult);
16-
}
17-
1813
ProcessTerminationHandler? terminationHandler = null;
1914
using CancellationTokenSource cts = CancellationTokenSource.CreateLinkedTokenSource(cancellationToken);
2015

2116
try
2217
{
18+
int actionResult = 0;
19+
int preActionResult = 0;
20+
2321
if (parseResult.PreActions is not null)
2422
{
2523
for (int i = 0; i < parseResult.PreActions.Count; i++)
2624
{
2725
var action = parseResult.PreActions[i];
26+
var result = 0;
2827

2928
switch (action)
3029
{
3130
case SynchronousCommandLineAction syncAction:
32-
syncAction.Invoke(parseResult);
31+
result = syncAction.Invoke(parseResult);
3332
break;
3433
case AsynchronousCommandLineAction asyncAction:
35-
await asyncAction.InvokeAsync(parseResult, cts.Token);
34+
result = await asyncAction.InvokeAsync(parseResult, cts.Token);
3635
break;
36+
37+
}
38+
39+
if (result != 0)
40+
{
41+
preActionResult = result;
3742
}
3843
}
3944
}
4045

46+
if (parseResult.Action is null)
47+
{
48+
return preActionResult != 0 ? preActionResult : ReturnCodeForMissingAction(parseResult);
49+
}
50+
4151
switch (parseResult.Action)
4252
{
4353
case SynchronousCommandLineAction syncAction:
44-
return syncAction.Invoke(parseResult);
54+
actionResult = syncAction.Invoke(parseResult);
55+
break;
4556

4657
case AsynchronousCommandLineAction asyncAction:
4758
var timeout = parseResult.InvocationConfiguration.ProcessTerminationTimeout;
@@ -55,7 +66,7 @@ internal static async Task<int> InvokeAsync(ParseResult parseResult, Cancellatio
5566

5667
if (terminationHandler is null)
5768
{
58-
return await startedInvocation;
69+
actionResult = await startedInvocation;
5970
}
6071
else
6172
{
@@ -64,12 +75,15 @@ internal static async Task<int> InvokeAsync(ParseResult parseResult, Cancellatio
6475
// In such cases, when CancelOnProcessTermination is configured and user presses Ctrl+C,
6576
// ProcessTerminationCompletionSource completes first, with the result equal to native exit code for given signal.
6677
Task<int> firstCompletedTask = await Task.WhenAny(startedInvocation, terminationHandler.ProcessTerminationCompletionSource.Task);
67-
return await firstCompletedTask; // return the result or propagate the exception
78+
actionResult = await firstCompletedTask; // return the result or propagate the exception
6879
}
80+
break;
6981

7082
default:
7183
throw new ArgumentOutOfRangeException(nameof(parseResult.Action));
7284
}
85+
86+
return preActionResult != 0 ? preActionResult : actionResult;
7387
}
7488
catch (Exception ex) when (parseResult.InvocationConfiguration.EnableDefaultExceptionHandler)
7589
{
@@ -83,48 +97,42 @@ internal static async Task<int> InvokeAsync(ParseResult parseResult, Cancellatio
8397

8498
internal static int Invoke(ParseResult parseResult)
8599
{
86-
switch (parseResult.Action)
100+
try
87101
{
88-
case null:
89-
return ReturnCodeForMissingAction(parseResult);
102+
int preActionResult = 0;
90103

91-
case SynchronousCommandLineAction syncAction:
92-
try
104+
if (parseResult.PreActions is not null)
105+
{
106+
for (var i = 0; i < parseResult.PreActions.Count; i++)
93107
{
94-
if (parseResult.PreActions is not null)
108+
if (parseResult.PreActions[i] is SynchronousCommandLineAction syncPreAction)
95109
{
96-
#if DEBUG
97-
for (var i = 0; i < parseResult.PreActions.Count; i++)
98-
{
99-
var action = parseResult.PreActions[i];
100-
101-
if (action is not SynchronousCommandLineAction)
102-
{
103-
parseResult.InvocationConfiguration.EnableDefaultExceptionHandler = false;
104-
throw new Exception(
105-
$"This should not happen. An instance of {nameof(AsynchronousCommandLineAction)} ({action}) was called within {nameof(InvocationPipeline)}.{nameof(Invoke)}. This is supposed to be detected earlier resulting in a call to {nameof(InvocationPipeline)}{nameof(InvokeAsync)}");
106-
}
107-
}
108-
#endif
109-
110-
for (var i = 0; i < parseResult.PreActions.Count; i++)
110+
int result = syncPreAction.Invoke(parseResult);
111+
112+
if (result != 0)
111113
{
112-
if (parseResult.PreActions[i] is SynchronousCommandLineAction syncPreAction)
113-
{
114-
syncPreAction.Invoke(parseResult);
115-
}
114+
preActionResult = result;
116115
}
117116
}
118-
119-
return syncAction.Invoke(parseResult);
120-
}
121-
catch (Exception ex) when (parseResult.InvocationConfiguration.EnableDefaultExceptionHandler)
122-
{
123-
return DefaultExceptionHandler(ex, parseResult);
124117
}
118+
}
119+
120+
switch (parseResult.Action)
121+
{
122+
case null:
123+
return preActionResult != 0 ? preActionResult : ReturnCodeForMissingAction(parseResult);
125124

126-
default:
127-
throw new InvalidOperationException($"{nameof(AsynchronousCommandLineAction)} called within non-async invocation.");
125+
case SynchronousCommandLineAction syncAction:
126+
int actionResult = syncAction.Invoke(parseResult);
127+
return preActionResult != 0 ? preActionResult : actionResult;
128+
129+
default:
130+
throw new InvalidOperationException($"{nameof(AsynchronousCommandLineAction)} called within non-async invocation.");
131+
}
132+
}
133+
catch (Exception ex) when (parseResult.InvocationConfiguration.EnableDefaultExceptionHandler)
134+
{
135+
return DefaultExceptionHandler(ex, parseResult);
128136
}
129137
}
130138

0 commit comments

Comments
 (0)