Skip to content

Commit 84d86f8

Browse files
author
Jon Sequeira
committed
fix #2771 and #2772 
1 parent 0b720d0 commit 84d86f8

File tree

3 files changed

+170
-45
lines changed

3 files changed

+170
-45
lines changed

‎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: 65 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -10,38 +10,52 @@ 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 exitCode = 0;
19+
2320
if (parseResult.PreActions is not null)
2421
{
2522
for (int i = 0; i < parseResult.PreActions.Count; i++)
2623
{
2724
var action = parseResult.PreActions[i];
25+
int preActionResult;
2826

2927
switch (action)
3028
{
3129
case SynchronousCommandLineAction syncAction:
32-
syncAction.Invoke(parseResult);
30+
preActionResult = syncAction.Invoke(parseResult);
3331
break;
3432
case AsynchronousCommandLineAction asyncAction:
35-
await asyncAction.InvokeAsync(parseResult, cts.Token);
33+
preActionResult = await asyncAction.InvokeAsync(parseResult, cts.Token);
34+
break;
35+
default:
36+
preActionResult = 0;
3637
break;
3738
}
39+
40+
if (exitCode == 0)
41+
{
42+
exitCode = preActionResult;
43+
}
3844
}
3945
}
4046

47+
if (parseResult.Action is null)
48+
{
49+
return exitCode != 0 ? exitCode : ReturnCodeForMissingAction(parseResult);
50+
}
51+
52+
int actionResult;
53+
4154
switch (parseResult.Action)
4255
{
4356
case SynchronousCommandLineAction syncAction:
44-
return syncAction.Invoke(parseResult);
57+
actionResult = syncAction.Invoke(parseResult);
58+
break;
4559

4660
case AsynchronousCommandLineAction asyncAction:
4761
var startedInvocation = asyncAction.InvokeAsync(parseResult, cts.Token);
@@ -55,20 +69,23 @@ internal static async Task<int> InvokeAsync(ParseResult parseResult, Cancellatio
5569

5670
if (terminationHandler is null)
5771
{
58-
return await startedInvocation;
72+
actionResult = await startedInvocation;
5973
}
6074
else
6175
{
6276
// Handlers may not implement cancellation.
6377
// In such cases, when CancelOnProcessTermination is configured and user presses Ctrl+C,
6478
// ProcessTerminationCompletionSource completes first, with the result equal to native exit code for given signal.
6579
Task<int> firstCompletedTask = await Task.WhenAny(startedInvocation, terminationHandler.ProcessTerminationCompletionSource.Task);
66-
return await firstCompletedTask; // return the result or propagate the exception
80+
actionResult = await firstCompletedTask; // return the result or propagate the exception
6781
}
82+
break;
6883

6984
default:
7085
throw new ArgumentOutOfRangeException(nameof(parseResult.Action));
7186
}
87+
88+
return exitCode != 0 ? exitCode : actionResult;
7289
}
7390
catch (Exception ex) when (parseResult.InvocationConfiguration.EnableDefaultExceptionHandler)
7491
{
@@ -82,48 +99,55 @@ internal static async Task<int> InvokeAsync(ParseResult parseResult, Cancellatio
8299

83100
internal static int Invoke(ParseResult parseResult)
84101
{
85-
switch (parseResult.Action)
102+
try
86103
{
87-
case null:
88-
return ReturnCodeForMissingAction(parseResult);
104+
int exitCode = 0;
89105

90-
case SynchronousCommandLineAction syncAction:
91-
try
106+
if (parseResult.PreActions is not null)
107+
{
108+
#if DEBUG
109+
for (var i = 0; i < parseResult.PreActions.Count; i++)
92110
{
93-
if (parseResult.PreActions is not null)
111+
var action = parseResult.PreActions[i];
112+
113+
if (action is not SynchronousCommandLineAction)
94114
{
95-
#if DEBUG
96-
for (var i = 0; i < parseResult.PreActions.Count; i++)
97-
{
98-
var action = parseResult.PreActions[i];
99-
100-
if (action is not SynchronousCommandLineAction)
101-
{
102-
parseResult.InvocationConfiguration.EnableDefaultExceptionHandler = false;
103-
throw new Exception(
104-
$"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)}");
105-
}
106-
}
115+
parseResult.InvocationConfiguration.EnableDefaultExceptionHandler = false;
116+
throw new Exception(
117+
$"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)}");
118+
}
119+
}
107120
#endif
108121

109-
for (var i = 0; i < parseResult.PreActions.Count; i++)
122+
for (var i = 0; i < parseResult.PreActions.Count; i++)
123+
{
124+
if (parseResult.PreActions[i] is SynchronousCommandLineAction syncPreAction)
125+
{
126+
int preActionResult = syncPreAction.Invoke(parseResult);
127+
if (exitCode == 0)
110128
{
111-
if (parseResult.PreActions[i] is SynchronousCommandLineAction syncPreAction)
112-
{
113-
syncPreAction.Invoke(parseResult);
114-
}
129+
exitCode = preActionResult;
115130
}
116131
}
117-
118-
return syncAction.Invoke(parseResult);
119-
}
120-
catch (Exception ex) when (parseResult.InvocationConfiguration.EnableDefaultExceptionHandler)
121-
{
122-
return DefaultExceptionHandler(ex, parseResult);
123132
}
133+
}
134+
135+
switch (parseResult.Action)
136+
{
137+
case null:
138+
return exitCode != 0 ? exitCode : ReturnCodeForMissingAction(parseResult);
124139

125-
default:
126-
throw new InvalidOperationException($"{nameof(AsynchronousCommandLineAction)} called within non-async invocation.");
140+
case SynchronousCommandLineAction syncAction:
141+
int actionResult = syncAction.Invoke(parseResult);
142+
return exitCode != 0 ? exitCode : actionResult;
143+
144+
default:
145+
throw new InvalidOperationException($"{nameof(AsynchronousCommandLineAction)} called within non-async invocation.");
146+
}
147+
}
148+
catch (Exception ex) when (parseResult.InvocationConfiguration.EnableDefaultExceptionHandler)
149+
{
150+
return DefaultExceptionHandler(ex, parseResult);
127151
}
128152
}
129153

0 commit comments

Comments
 (0)