From e004c5983a1c248716ca9bdd2bf8513fe94999ce Mon Sep 17 00:00:00 2001 From: Adam Sitnik Date: Wed, 12 Apr 2023 20:36:09 +0200 Subject: [PATCH] make CliAction.Invoke* methods protected, so users can not invoke them move InvocationPipeline logic into CliAction --- ...tionBinder_api_is_not_changed.approved.txt | 4 +- ...ommandLine_api_is_not_changed.approved.txt | 8 +- .../CommandLine/Perf_Parser_ParseResult.cs | 4 +- .../CommandHandlerSourceGenerator.cs | 4 +- .../HostingHandlerTest.cs | 12 +- .../HostingAction.cs | 7 +- .../HostingExtensions.cs | 2 +- .../ModelBindingCommandHandlerTests.cs | 10 +- .../ParameterBindingTests.cs | 12 +- .../BindingContextExtensions.cs | 4 +- .../ModelBindingCommandHandler.cs | 4 +- .../DirectiveTests.cs | 4 +- .../EnvironmentVariableDirectiveTests.cs | 4 +- .../CancelOnProcessTerminationTests.cs | 4 +- .../Invocation/InvocationTests.cs | 8 +- src/System.CommandLine.Tests/OptionTests.cs | 4 +- .../ParseResultTests.cs | 2 +- .../Utility/ParseResultExtensions.cs | 21 +--- src/System.CommandLine/CliAction.cs | 107 ++++++++++++++++- .../Completions/SuggestDirective.cs | 4 +- .../EnvironmentVariablesDirective.cs | 4 +- .../Help/HelpOptionAction.cs | 4 +- .../Invocation/AnonymousCliAction.cs | 4 +- .../Invocation/InvocationPipeline.cs | 113 ------------------ .../Invocation/ParseErrorAction.cs | 13 +- .../Invocation/TypoCorrectionAction.cs | 4 +- src/System.CommandLine/ParseResult.cs | 5 +- .../Parsing/ParseDiagramAction.cs | 4 +- src/System.CommandLine/VersionOption.cs | 4 +- 29 files changed, 182 insertions(+), 202 deletions(-) delete mode 100644 src/System.CommandLine/Invocation/InvocationPipeline.cs diff --git a/src/System.CommandLine.ApiCompatibility.Tests/ApiCompatibilityApprovalTests.System_CommandLine_NamingConventionBinder_api_is_not_changed.approved.txt b/src/System.CommandLine.ApiCompatibility.Tests/ApiCompatibilityApprovalTests.System_CommandLine_NamingConventionBinder_api_is_not_changed.approved.txt index 3b24de558b..f2c57e4b6d 100644 --- a/src/System.CommandLine.ApiCompatibility.Tests/ApiCompatibilityApprovalTests.System_CommandLine_NamingConventionBinder_api_is_not_changed.approved.txt +++ b/src/System.CommandLine.ApiCompatibility.Tests/ApiCompatibilityApprovalTests.System_CommandLine_NamingConventionBinder_api_is_not_changed.approved.txt @@ -119,8 +119,8 @@ System.CommandLine.NamingConventionBinder public class ModelBindingCommandHandler : BindingHandler public System.Void BindParameter(System.Reflection.ParameterInfo param, System.CommandLine.CliArgument argument) public System.Void BindParameter(System.Reflection.ParameterInfo param, System.CommandLine.CliOption option) - public System.Int32 Invoke(System.CommandLine.ParseResult parseResult) - public System.Threading.Tasks.Task InvokeAsync(System.CommandLine.ParseResult parseResult, System.Threading.CancellationToken cancellationToken = null) + protected System.Int32 Invoke(System.CommandLine.ParseResult parseResult) + protected System.Threading.Tasks.Task InvokeAsync(System.CommandLine.ParseResult parseResult, System.Threading.CancellationToken cancellationToken = null) public class ModelDescriptor public static ModelDescriptor FromType() public static ModelDescriptor FromType(System.Type type) 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 388b9f26fa..5a0a41e816 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 @@ -18,8 +18,8 @@ System.CommandLine public static CliArgument AcceptExistingOnly(this CliArgument argument) public abstract class CliAction public System.Boolean Exclusive { get; } - public System.Int32 Invoke(ParseResult parseResult) - public System.Threading.Tasks.Task InvokeAsync(ParseResult parseResult, System.Threading.CancellationToken cancellationToken = null) + protected System.Int32 Invoke(ParseResult parseResult) + protected System.Threading.Tasks.Task InvokeAsync(ParseResult parseResult, System.Threading.CancellationToken cancellationToken = null) protected System.Void set_Exclusive(System.Boolean value) public abstract class CliArgument : CliSymbol public ArgumentArity Arity { get; set; } @@ -183,8 +183,8 @@ System.CommandLine.Help public class HelpAction : System.CommandLine.CliAction .ctor() public HelpBuilder Builder { get; set; } - public System.Int32 Invoke(System.CommandLine.ParseResult parseResult) - public System.Threading.Tasks.Task InvokeAsync(System.CommandLine.ParseResult parseResult, System.Threading.CancellationToken cancellationToken = null) + protected System.Int32 Invoke(System.CommandLine.ParseResult parseResult) + protected System.Threading.Tasks.Task InvokeAsync(System.CommandLine.ParseResult parseResult, System.Threading.CancellationToken cancellationToken = null) public class HelpBuilder .ctor(System.Int32 maxWidth = 2147483647) public System.Int32 MaxWidth { get; } diff --git a/src/System.CommandLine.Benchmarks/CommandLine/Perf_Parser_ParseResult.cs b/src/System.CommandLine.Benchmarks/CommandLine/Perf_Parser_ParseResult.cs index 9b1e5fbe65..5acada4a1e 100644 --- a/src/System.CommandLine.Benchmarks/CommandLine/Perf_Parser_ParseResult.cs +++ b/src/System.CommandLine.Benchmarks/CommandLine/Perf_Parser_ParseResult.cs @@ -60,7 +60,7 @@ public string ParseResult_Diagram(BdnParam parseResult) // clear the contents, so each benchmark has the same starting state stringBuilder.Clear(); - parseResult.Value.Action!.Invoke(parseResult.Value); + parseResult.Value.Invoke(); return stringBuilder.ToString(); } @@ -74,7 +74,7 @@ public async Task ParseResult_DiagramAsync(BdnParam parseRe // clear the contents, so each benchmark has the same starting state stringBuilder.Clear(); - await parseResult.Value.Action!.InvokeAsync(parseResult.Value); + await parseResult.Value.InvokeAsync(); return stringBuilder.ToString(); } diff --git a/src/System.CommandLine.Generator/CommandHandlerSourceGenerator.cs b/src/System.CommandLine.Generator/CommandHandlerSourceGenerator.cs index 378c107267..d34c4470e6 100644 --- a/src/System.CommandLine.Generator/CommandHandlerSourceGenerator.cs +++ b/src/System.CommandLine.Generator/CommandHandlerSourceGenerator.cs @@ -115,10 +115,10 @@ private class GeneratedHandler_{handlerCount} : {CliActionType} } builder.Append($@" - public override int Invoke(global::System.CommandLine.ParseResult context) => InvokeAsync(context, global::System.Threading.CancellationToken.None).GetAwaiter().GetResult();"); + protected override int Invoke(global::System.CommandLine.ParseResult context) => InvokeAsync(context, global::System.Threading.CancellationToken.None).GetAwaiter().GetResult();"); builder.Append($@" - public override async global::System.Threading.Tasks.Task InvokeAsync(global::System.CommandLine.ParseResult context, global::System.Threading.CancellationToken cancellationToken) + protected override async global::System.Threading.Tasks.Task InvokeAsync(global::System.CommandLine.ParseResult context, global::System.Threading.CancellationToken cancellationToken) {{"); builder.Append($@" {invocation.InvokeContents()}"); diff --git a/src/System.CommandLine.Hosting.Tests/HostingHandlerTest.cs b/src/System.CommandLine.Hosting.Tests/HostingHandlerTest.cs index 76d59d3f8b..ed7a1d1e36 100644 --- a/src/System.CommandLine.Hosting.Tests/HostingHandlerTest.cs +++ b/src/System.CommandLine.Hosting.Tests/HostingHandlerTest.cs @@ -122,12 +122,12 @@ public abstract class MyBaseHandler : CliAction { public int IntOption { get; set; } // bound from option - public override int Invoke(ParseResult context) + protected override int Invoke(ParseResult context) { return Act(); } - public override Task InvokeAsync(ParseResult context, CancellationToken cancellationToken) + protected override Task InvokeAsync(ParseResult context, CancellationToken cancellationToken) { return Task.FromResult(Act()); } @@ -153,13 +153,13 @@ public MyHandler(MyService service) public int IntOption { get; set; } // bound from option - public override int Invoke(ParseResult context) + protected override int Invoke(ParseResult context) { service.Value = IntOption; return IntOption; } - public override Task InvokeAsync(ParseResult context, CancellationToken cancellationToken) + protected override Task InvokeAsync(ParseResult context, CancellationToken cancellationToken) { service.Value = IntOption; return Task.FromResult(IntOption); @@ -204,9 +204,9 @@ public MyHandler(MyService service) public string One { get; set; } - public override int Invoke(ParseResult context) => InvokeAsync(context, CancellationToken.None).GetAwaiter().GetResult(); + protected override int Invoke(ParseResult context) => InvokeAsync(context, CancellationToken.None).GetAwaiter().GetResult(); - public override Task InvokeAsync(ParseResult context, CancellationToken cancellationToken) + protected override Task InvokeAsync(ParseResult context, CancellationToken cancellationToken) { service.Value = IntOption; service.StringValue = One; diff --git a/src/System.CommandLine.Hosting/HostingAction.cs b/src/System.CommandLine.Hosting/HostingAction.cs index 489cf8c3b4..d3388d3083 100644 --- a/src/System.CommandLine.Hosting/HostingAction.cs +++ b/src/System.CommandLine.Hosting/HostingAction.cs @@ -38,7 +38,7 @@ public override BindingContext GetBindingContext(ParseResult parseResult) ? bindingHandler.GetBindingContext(parseResult) : base.GetBindingContext(parseResult); - public async override Task InvokeAsync(ParseResult parseResult, CancellationToken cancellationToken = default) + protected async override Task InvokeAsync(ParseResult parseResult, CancellationToken cancellationToken = default) { var argsRemaining = parseResult.UnmatchedTokens; var hostBuilder = _hostBuilderFactory?.Invoke(argsRemaining) @@ -86,7 +86,8 @@ public async override Task InvokeAsync(ParseResult parseResult, Cancellatio { if (_actualAction is not null) { - return await _actualAction.InvokeAsync(parseResult, cancellationToken); + // This is dirty, as the whole HostingAction concept. + return await (Task)_actualAction.GetType().GetMethod(nameof(InvokeAsync), Reflection.BindingFlags.NonPublic | Reflection.BindingFlags.Instance).Invoke(_actualAction, new object[] { parseResult, cancellationToken }); } return 0; } @@ -96,6 +97,6 @@ public async override Task InvokeAsync(ParseResult parseResult, Cancellatio } } - public override int Invoke(ParseResult parseResult) => InvokeAsync(parseResult).GetAwaiter().GetResult(); + protected override int Invoke(ParseResult parseResult) => InvokeAsync(parseResult).GetAwaiter().GetResult(); } } diff --git a/src/System.CommandLine.Hosting/HostingExtensions.cs b/src/System.CommandLine.Hosting/HostingExtensions.cs index 3444e24c41..627949bd0c 100644 --- a/src/System.CommandLine.Hosting/HostingExtensions.cs +++ b/src/System.CommandLine.Hosting/HostingExtensions.cs @@ -54,7 +54,7 @@ public static OptionsBuilder BindCommandLine( public static CliCommand UseCommandHandler(this CliCommand command) where THandler : CliAction { - command.Action = CommandHandler.Create(typeof(THandler).GetMethod(nameof(CliAction.InvokeAsync))); + command.Action = CommandHandler.Create(typeof(THandler).GetMethod("InvokeAsync", Reflection.BindingFlags.NonPublic | Reflection.BindingFlags.Instance)); return command; } diff --git a/src/System.CommandLine.NamingConventionBinder.Tests/ModelBindingCommandHandlerTests.cs b/src/System.CommandLine.NamingConventionBinder.Tests/ModelBindingCommandHandlerTests.cs index d1a59df6b0..b4d55ebcca 100644 --- a/src/System.CommandLine.NamingConventionBinder.Tests/ModelBindingCommandHandlerTests.cs +++ b/src/System.CommandLine.NamingConventionBinder.Tests/ModelBindingCommandHandlerTests.cs @@ -41,7 +41,7 @@ public async Task Unspecified_option_arguments_with_no_default_value_are_bound_t var parseResult = command.Parse(""); - await handler.InvokeAsync(parseResult, CancellationToken.None); + await parseResult.InvokeAsync(CancellationToken.None); BoundValueCapturer.GetBoundValue(parseResult).Should().Be(expectedValue); } @@ -116,7 +116,7 @@ public async Task Handler_method_receives_option_arguments_bound_to_the_specifie var commandLine = string.Join(" ", testCase.CommandLineTokens.Select(t => $"--value {t}")); var parseResult = command.Parse(commandLine); - await handler.InvokeAsync(parseResult, CancellationToken.None); + await parseResult.InvokeAsync(CancellationToken.None); var boundValue = BoundValueCapturer.GetBoundValue(parseResult); @@ -176,7 +176,7 @@ public async Task Handler_method_receives_command_arguments_bound_to_the_specifi var commandLine = string.Join(" ", testCase.CommandLineTokens); var parseResult = command.Parse(commandLine); - await handler.InvokeAsync(parseResult, CancellationToken.None); + await parseResult.InvokeAsync(CancellationToken.None); var boundValue = BoundValueCapturer.GetBoundValue(parseResult); @@ -226,7 +226,7 @@ public async Task Handler_method_receives_command_arguments_explicitly_bound_to_ var commandLine = string.Join(" ", testCase.CommandLineTokens); var parseResult = command.Parse(commandLine); - await handler.InvokeAsync(parseResult, CancellationToken.None); + await parseResult.InvokeAsync(CancellationToken.None); var boundValue = BoundValueCapturer.GetBoundValue(parseResult); @@ -277,7 +277,7 @@ public async Task Handler_method_receive_option_arguments_explicitly_bound_to_th var commandLine = string.Join(" ", testCase.CommandLineTokens.Select(t => $"--value {t}")); var parseResult = command.Parse(commandLine); - await handler.InvokeAsync(parseResult, CancellationToken.None); + await parseResult.InvokeAsync(CancellationToken.None); var boundValue = BoundValueCapturer.GetBoundValue(parseResult); diff --git a/src/System.CommandLine.NamingConventionBinder.Tests/ParameterBindingTests.cs b/src/System.CommandLine.NamingConventionBinder.Tests/ParameterBindingTests.cs index 471d1efeba..338b0279cd 100644 --- a/src/System.CommandLine.NamingConventionBinder.Tests/ParameterBindingTests.cs +++ b/src/System.CommandLine.NamingConventionBinder.Tests/ParameterBindingTests.cs @@ -397,7 +397,7 @@ void Execute(string fullnameOrNickname, int age) public async Task Method_invoked_is_matching_to_the_interface_implementation(Type type, int expectedResult) { var command = new CliCommand("command"); - command.Action = CommandHandler.Create(type.GetMethod(nameof(CliAction.InvokeAsync))); + command.Action = CommandHandler.Create(type.GetMethod("InvokeAsync", Reflection.BindingFlags.NonPublic | Reflection.BindingFlags.Instance)); int result = await command.Parse("command").InvokeAsync(); @@ -408,9 +408,9 @@ public abstract class AbstractTestCommandHandler : CliAction { public abstract Task DoJobAsync(); - public override int Invoke(ParseResult context) => InvokeAsync(context, CancellationToken.None).GetAwaiter().GetResult(); + protected override int Invoke(ParseResult context) => InvokeAsync(context, CancellationToken.None).GetAwaiter().GetResult(); - public override Task InvokeAsync(ParseResult context, CancellationToken cancellationToken) + protected override Task InvokeAsync(ParseResult context, CancellationToken cancellationToken) => DoJobAsync(); } @@ -422,15 +422,15 @@ public override Task DoJobAsync() public class VirtualTestCommandHandler : CliAction { - public override int Invoke(ParseResult context) => 42; + protected override int Invoke(ParseResult context) => 42; - public override Task InvokeAsync(ParseResult context, CancellationToken cancellationToken) + protected override Task InvokeAsync(ParseResult context, CancellationToken cancellationToken) => Task.FromResult(42); } public class OverridenVirtualTestCommandHandler : VirtualTestCommandHandler { - public override Task InvokeAsync(ParseResult context, CancellationToken cancellationToken) + protected override Task InvokeAsync(ParseResult context, CancellationToken cancellationToken) => Task.FromResult(41); } } \ No newline at end of file diff --git a/src/System.CommandLine.NamingConventionBinder/BindingContextExtensions.cs b/src/System.CommandLine.NamingConventionBinder/BindingContextExtensions.cs index d72b2647bd..2d58f9f90f 100644 --- a/src/System.CommandLine.NamingConventionBinder/BindingContextExtensions.cs +++ b/src/System.CommandLine.NamingConventionBinder/BindingContextExtensions.cs @@ -14,9 +14,9 @@ public static class BindingContextExtensions { private sealed class DummyStateHoldingHandler : BindingHandler { - public override int Invoke(ParseResult parseResult) => 0; + protected override int Invoke(ParseResult parseResult) => 0; - public override Task InvokeAsync(ParseResult parseResult, CancellationToken cancellationToken = default) => Task.FromResult(0); + protected override Task InvokeAsync(ParseResult parseResult, CancellationToken cancellationToken = default) => Task.FromResult(0); } public static BindingContext GetBindingContext(this ParseResult parseResult) diff --git a/src/System.CommandLine.NamingConventionBinder/ModelBindingCommandHandler.cs b/src/System.CommandLine.NamingConventionBinder/ModelBindingCommandHandler.cs index 7f7aa88180..c72b5c91c9 100644 --- a/src/System.CommandLine.NamingConventionBinder/ModelBindingCommandHandler.cs +++ b/src/System.CommandLine.NamingConventionBinder/ModelBindingCommandHandler.cs @@ -56,7 +56,7 @@ internal ModelBindingCommandHandler( /// The current parse result. /// A token that can be used to cancel the invocation. /// A task whose value can be used to set the process exit code. - public override async Task InvokeAsync(ParseResult parseResult, CancellationToken cancellationToken = default) + protected override async Task InvokeAsync(ParseResult parseResult, CancellationToken cancellationToken = default) { var bindingContext = GetBindingContext(parseResult); @@ -131,5 +131,5 @@ private void BindValueSource(ParameterInfo param, IValueSource valueSource) x.ValueType == param.ParameterType); /// - public override int Invoke(ParseResult parseResult) => InvokeAsync(parseResult, CancellationToken.None).GetAwaiter().GetResult(); + protected override int Invoke(ParseResult parseResult) => InvokeAsync(parseResult, CancellationToken.None).GetAwaiter().GetResult(); } \ No newline at end of file diff --git a/src/System.CommandLine.Tests/DirectiveTests.cs b/src/System.CommandLine.Tests/DirectiveTests.cs index 58398be4a7..83f51a2558 100644 --- a/src/System.CommandLine.Tests/DirectiveTests.cs +++ b/src/System.CommandLine.Tests/DirectiveTests.cs @@ -153,13 +153,13 @@ public NonexclusiveTestAction(Action invoke) Exclusive = false; } - public override int Invoke(ParseResult parseResult) + protected override int Invoke(ParseResult parseResult) { _invoke(parseResult); return 0; } - public override Task InvokeAsync(ParseResult parseResult, CancellationToken cancellationToken = default) + protected override Task InvokeAsync(ParseResult parseResult, CancellationToken cancellationToken = default) { ; return Task.FromResult(Invoke(parseResult)); diff --git a/src/System.CommandLine.Tests/EnvironmentVariableDirectiveTests.cs b/src/System.CommandLine.Tests/EnvironmentVariableDirectiveTests.cs index cb47806f31..59a0b5e2e9 100644 --- a/src/System.CommandLine.Tests/EnvironmentVariableDirectiveTests.cs +++ b/src/System.CommandLine.Tests/EnvironmentVariableDirectiveTests.cs @@ -130,13 +130,13 @@ private class CustomHelpAction : CliAction { public bool WasCalled { get; private set; } - public override int Invoke(ParseResult parseResult) + protected override int Invoke(ParseResult parseResult) { WasCalled = true; return 0; } - public override Task InvokeAsync(ParseResult parseResult, CancellationToken cancellationToken = default) + protected override Task InvokeAsync(ParseResult parseResult, CancellationToken cancellationToken = default) { WasCalled = true; return Task.FromResult(0); diff --git a/src/System.CommandLine.Tests/Invocation/CancelOnProcessTerminationTests.cs b/src/System.CommandLine.Tests/Invocation/CancelOnProcessTerminationTests.cs index 6815736886..97d283f56b 100644 --- a/src/System.CommandLine.Tests/Invocation/CancelOnProcessTerminationTests.cs +++ b/src/System.CommandLine.Tests/Invocation/CancelOnProcessTerminationTests.cs @@ -63,9 +63,9 @@ private static Task Program(string[] args) private sealed class CustomCliAction : CliAction { - public override int Invoke(ParseResult context) => throw new NotImplementedException(); + protected override int Invoke(ParseResult context) => throw new NotImplementedException(); - public async override Task InvokeAsync(ParseResult context, CancellationToken cancellationToken = default) + protected async override Task InvokeAsync(ParseResult context, CancellationToken cancellationToken = default) { Console.WriteLine(ChildProcessWaiting); diff --git a/src/System.CommandLine.Tests/Invocation/InvocationTests.cs b/src/System.CommandLine.Tests/Invocation/InvocationTests.cs index b3f07344f9..f43243e846 100644 --- a/src/System.CommandLine.Tests/Invocation/InvocationTests.cs +++ b/src/System.CommandLine.Tests/Invocation/InvocationTests.cs @@ -241,10 +241,10 @@ public async Task Command_InvokeAsync_with_cancelation_token_invokes_command_han private class CustomExitCodeAction : CliAction { - public override int Invoke(ParseResult context) + protected override int Invoke(ParseResult context) => 123; - public override Task InvokeAsync(ParseResult context, CancellationToken cancellationToken = default) + protected override Task InvokeAsync(ParseResult context, CancellationToken cancellationToken = default) => Task.FromResult(456); } @@ -259,7 +259,7 @@ public NonexclusiveTestAction() public bool HasBeenInvoked { get; private set; } - public override int Invoke(ParseResult parseResult) + protected override int Invoke(ParseResult parseResult) { HasBeenInvoked = true; if (ThrowOnInvoke) @@ -270,7 +270,7 @@ public override int Invoke(ParseResult parseResult) return 0; } - public override Task InvokeAsync(ParseResult parseResult, CancellationToken cancellationToken = default) + protected override Task InvokeAsync(ParseResult parseResult, CancellationToken cancellationToken = default) { HasBeenInvoked = true; if (ThrowOnInvoke) diff --git a/src/System.CommandLine.Tests/OptionTests.cs b/src/System.CommandLine.Tests/OptionTests.cs index d0cc6f804c..018e12d90c 100644 --- a/src/System.CommandLine.Tests/OptionTests.cs +++ b/src/System.CommandLine.Tests/OptionTests.cs @@ -381,13 +381,13 @@ internal sealed class OptionAction : CliAction { internal bool WasCalled = false; - public override int Invoke(ParseResult context) + protected override int Invoke(ParseResult context) { WasCalled = true; return 0; } - public override Task InvokeAsync(ParseResult context, CancellationToken cancellationToken = default) + protected override Task InvokeAsync(ParseResult context, CancellationToken cancellationToken = default) => Task.FromResult(Invoke(context)); } diff --git a/src/System.CommandLine.Tests/ParseResultTests.cs b/src/System.CommandLine.Tests/ParseResultTests.cs index bac682ba67..94c3b8fbdf 100644 --- a/src/System.CommandLine.Tests/ParseResultTests.cs +++ b/src/System.CommandLine.Tests/ParseResultTests.cs @@ -139,7 +139,7 @@ public void Handler_is_not_null_when_parsed_command_specified_handler() parseResult.Action.Should().NotBeNull(); handlerWasCalled.Should().BeFalse(); - parseResult.Action.Invoke(null!).Should().Be(0); + parseResult.Invoke().Should().Be(0); handlerWasCalled.Should().BeTrue(); } } diff --git a/src/System.CommandLine.Tests/Utility/ParseResultExtensions.cs b/src/System.CommandLine.Tests/Utility/ParseResultExtensions.cs index d423e3b0a0..040bf5869a 100644 --- a/src/System.CommandLine.Tests/Utility/ParseResultExtensions.cs +++ b/src/System.CommandLine.Tests/Utility/ParseResultExtensions.cs @@ -1,25 +1,8 @@ -using System.IO; - -namespace System.CommandLine.Tests +namespace System.CommandLine.Tests { internal static class ParseResultExtensions { internal static string Diagram(this ParseResult parseResult) - { - TextWriter outputBefore = parseResult.Configuration.Output; - - try - { - parseResult.Configuration.Output = new StringWriter(); - new DiagramDirective().Action.Invoke(parseResult); - return parseResult.Configuration.Output.ToString() - .TrimEnd(); // the directive adds a new line, tests that used to rely on Diagram extension method don't expect it - } - finally - { - // some of the tests check the Output after getting the Diagram - parseResult.Configuration.Output = outputBefore; - } - } + => parseResult.ToString().TrimEnd(); // the directive adds a new line, tests that used to rely on Diagram extension method don't expect it } } diff --git a/src/System.CommandLine/CliAction.cs b/src/System.CommandLine/CliAction.cs index c857811287..50a6392573 100644 --- a/src/System.CommandLine/CliAction.cs +++ b/src/System.CommandLine/CliAction.cs @@ -1,6 +1,7 @@ // 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.Invocation; using System.Threading; using System.Threading.Tasks; @@ -18,7 +19,7 @@ public abstract class CliAction /// /// Provides the parse results. /// A value that can be used as the exit code for the process. - public abstract int Invoke(ParseResult parseResult); + protected abstract int Invoke(ParseResult parseResult); /// /// Performs an action when the associated symbol is invoked on the command line. @@ -26,6 +27,108 @@ public abstract class CliAction /// Provides the parse results. /// The token to monitor for cancellation requests. /// A value that can be used as the exit code for the process. - public abstract Task InvokeAsync(ParseResult parseResult, CancellationToken cancellationToken = default); + protected abstract Task InvokeAsync(ParseResult parseResult, CancellationToken cancellationToken = default); + + internal static async Task InvokePipelineAsync(ParseResult parseResult, CancellationToken cancellationToken) + { + if (parseResult.Action is null) + { + return ReturnCodeForMissingAction(parseResult); + } + + ProcessTerminationHandler? terminationHandler = null; + using CancellationTokenSource cts = CancellationTokenSource.CreateLinkedTokenSource(cancellationToken); + + try + { + if (parseResult.NonexclusiveActions is not null) + { + for (int i = 0; i < parseResult.NonexclusiveActions.Count; i++) + { + await parseResult.NonexclusiveActions[i].InvokeAsync(parseResult, cts.Token); + } + } + + Task startedInvocation = parseResult.Action.InvokeAsync(parseResult, cts.Token); + + if (parseResult.Configuration.ProcessTerminationTimeout.HasValue) + { + terminationHandler = new(cts, startedInvocation, parseResult.Configuration.ProcessTerminationTimeout.Value); + } + + if (terminationHandler is null) + { + return await startedInvocation; + } + else + { + // Handlers may not implement cancellation. + // In such cases, when CancelOnProcessTermination is configured and user presses Ctrl+C, + // ProcessTerminationCompletionSource completes first, with the result equal to native exit code for given signal. + Task firstCompletedTask = await Task.WhenAny(startedInvocation, terminationHandler.ProcessTerminationCompletionSource.Task); + return await firstCompletedTask; // return the result or propagate the exception + } + } + catch (Exception ex) when (parseResult.Configuration.EnableDefaultExceptionHandler) + { + return DefaultExceptionHandler(ex, parseResult.Configuration); + } + finally + { + terminationHandler?.Dispose(); + } + } + + internal static int InvokePipeline(ParseResult parseResult) + { + if (parseResult.Action is null) + { + return ReturnCodeForMissingAction(parseResult); + } + + try + { + if (parseResult.NonexclusiveActions is not null) + { + for (var i = 0; i < parseResult.NonexclusiveActions.Count; i++) + { + parseResult.NonexclusiveActions[i].Invoke(parseResult); + } + } + + return parseResult.Action.Invoke(parseResult); + } + catch (Exception ex) when (parseResult.Configuration.EnableDefaultExceptionHandler) + { + return DefaultExceptionHandler(ex, parseResult.Configuration); + } + } + + private static int DefaultExceptionHandler(Exception exception, CliConfiguration config) + { + if (exception is not OperationCanceledException) + { + ConsoleHelpers.ResetTerminalForegroundColor(); + ConsoleHelpers.SetTerminalForegroundRed(); + + config.Error.Write(LocalizationResources.ExceptionHandlerHeader()); + config.Error.WriteLine(exception.ToString()); + + ConsoleHelpers.ResetTerminalForegroundColor(); + } + return 1; + } + + private static int ReturnCodeForMissingAction(ParseResult parseResult) + { + if (parseResult.Errors.Count > 0) + { + return 1; + } + else + { + return 0; + } + } } } diff --git a/src/System.CommandLine/Completions/SuggestDirective.cs b/src/System.CommandLine/Completions/SuggestDirective.cs index 6a47097874..1421aa018b 100644 --- a/src/System.CommandLine/Completions/SuggestDirective.cs +++ b/src/System.CommandLine/Completions/SuggestDirective.cs @@ -30,7 +30,7 @@ private sealed class SuggestDirectiveAction : CliAction internal SuggestDirectiveAction(SuggestDirective suggestDirective) => _directive = suggestDirective; - public override int Invoke(ParseResult parseResult) + protected override int Invoke(ParseResult parseResult) { string? parsedValues = parseResult.FindResultFor(_directive)!.Values.SingleOrDefault(); string? rawInput = parseResult.CommandLineText; @@ -51,7 +51,7 @@ public override int Invoke(ParseResult parseResult) return 0; } - public override Task InvokeAsync(ParseResult parseResult, CancellationToken cancellationToken = default) + protected override Task InvokeAsync(ParseResult parseResult, CancellationToken cancellationToken = default) => cancellationToken.IsCancellationRequested ? Task.FromCanceled(cancellationToken) : Task.FromResult(Invoke(parseResult)); diff --git a/src/System.CommandLine/EnvironmentVariablesDirective.cs b/src/System.CommandLine/EnvironmentVariablesDirective.cs index d65ae37fe5..862deda32a 100644 --- a/src/System.CommandLine/EnvironmentVariablesDirective.cs +++ b/src/System.CommandLine/EnvironmentVariablesDirective.cs @@ -32,14 +32,14 @@ internal EnvironmentVariablesDirectiveAction(EnvironmentVariablesDirective direc Exclusive = false; } - public override int Invoke(ParseResult parseResult) + protected override int Invoke(ParseResult parseResult) { SetEnvVars(parseResult); return 0; } - public override Task InvokeAsync(ParseResult parseResult, CancellationToken cancellationToken = default) + protected override Task InvokeAsync(ParseResult parseResult, CancellationToken cancellationToken = default) { SetEnvVars(parseResult); diff --git a/src/System.CommandLine/Help/HelpOptionAction.cs b/src/System.CommandLine/Help/HelpOptionAction.cs index 185a6fc94f..ee2c99204c 100644 --- a/src/System.CommandLine/Help/HelpOptionAction.cs +++ b/src/System.CommandLine/Help/HelpOptionAction.cs @@ -16,7 +16,7 @@ public HelpBuilder Builder set => _builder = value ?? throw new ArgumentNullException(nameof(value)); } - public override int Invoke(ParseResult parseResult) + protected override int Invoke(ParseResult parseResult) { var output = parseResult.Configuration.Output; @@ -30,7 +30,7 @@ public override int Invoke(ParseResult parseResult) return 0; } - public override Task InvokeAsync(ParseResult parseResult, CancellationToken cancellationToken = default) + protected override Task InvokeAsync(ParseResult parseResult, CancellationToken cancellationToken = default) => cancellationToken.IsCancellationRequested ? Task.FromCanceled(cancellationToken) : Task.FromResult(Invoke(parseResult)); diff --git a/src/System.CommandLine/Invocation/AnonymousCliAction.cs b/src/System.CommandLine/Invocation/AnonymousCliAction.cs index 5b0b048792..a5073c638c 100644 --- a/src/System.CommandLine/Invocation/AnonymousCliAction.cs +++ b/src/System.CommandLine/Invocation/AnonymousCliAction.cs @@ -17,7 +17,7 @@ internal AnonymousCliAction(Func action) internal AnonymousCliAction(Func> action) => _asyncAction = action; - public override int Invoke(ParseResult parseResult) + protected override int Invoke(ParseResult parseResult) { if (_syncAction is not null) { @@ -32,7 +32,7 @@ int SyncUsingAsync(ParseResult parseResult) => _asyncAction!(parseResult, CancellationToken.None).GetAwaiter().GetResult(); } - public override async Task InvokeAsync(ParseResult parseResult, CancellationToken cancellationToken) + protected override async Task InvokeAsync(ParseResult parseResult, CancellationToken cancellationToken) { if (_asyncAction is not null) { diff --git a/src/System.CommandLine/Invocation/InvocationPipeline.cs b/src/System.CommandLine/Invocation/InvocationPipeline.cs deleted file mode 100644 index a9b6d90feb..0000000000 --- a/src/System.CommandLine/Invocation/InvocationPipeline.cs +++ /dev/null @@ -1,113 +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. - -using System.Threading; -using System.Threading.Tasks; - -namespace System.CommandLine.Invocation -{ - internal static class InvocationPipeline - { - internal static async Task InvokeAsync(ParseResult parseResult, CancellationToken cancellationToken) - { - if (parseResult.Action is null) - { - return ReturnCodeForMissingAction(parseResult); - } - - ProcessTerminationHandler? terminationHandler = null; - using CancellationTokenSource cts = CancellationTokenSource.CreateLinkedTokenSource(cancellationToken); - - try - { - if (parseResult.NonexclusiveActions is not null) - { - for (int i = 0; i < parseResult.NonexclusiveActions.Count; i++) - { - await parseResult.NonexclusiveActions[i].InvokeAsync(parseResult, cts.Token); - } - } - - Task startedInvocation = parseResult.Action.InvokeAsync(parseResult, cts.Token); - - if (parseResult.Configuration.ProcessTerminationTimeout.HasValue) - { - terminationHandler = new(cts, startedInvocation, parseResult.Configuration.ProcessTerminationTimeout.Value); - } - - if (terminationHandler is null) - { - return await startedInvocation; - } - else - { - // Handlers may not implement cancellation. - // In such cases, when CancelOnProcessTermination is configured and user presses Ctrl+C, - // ProcessTerminationCompletionSource completes first, with the result equal to native exit code for given signal. - Task firstCompletedTask = await Task.WhenAny(startedInvocation, terminationHandler.ProcessTerminationCompletionSource.Task); - return await firstCompletedTask; // return the result or propagate the exception - } - } - catch (Exception ex) when (parseResult.Configuration.EnableDefaultExceptionHandler) - { - return DefaultExceptionHandler(ex, parseResult.Configuration); - } - finally - { - terminationHandler?.Dispose(); - } - } - - internal static int Invoke(ParseResult parseResult) - { - if (parseResult.Action is null) - { - return ReturnCodeForMissingAction(parseResult); - } - - try - { - if (parseResult.NonexclusiveActions is not null) - { - for (var i = 0; i < parseResult.NonexclusiveActions.Count; i++) - { - parseResult.NonexclusiveActions[i].Invoke(parseResult); - } - } - - return parseResult.Action.Invoke(parseResult); - } - catch (Exception ex) when (parseResult.Configuration.EnableDefaultExceptionHandler) - { - return DefaultExceptionHandler(ex, parseResult.Configuration); - } - } - - private static int DefaultExceptionHandler(Exception exception, CliConfiguration config) - { - if (exception is not OperationCanceledException) - { - ConsoleHelpers.ResetTerminalForegroundColor(); - ConsoleHelpers.SetTerminalForegroundRed(); - - config.Error.Write(LocalizationResources.ExceptionHandlerHeader()); - config.Error.WriteLine(exception.ToString()); - - ConsoleHelpers.ResetTerminalForegroundColor(); - } - return 1; - } - - private static int ReturnCodeForMissingAction(ParseResult parseResult) - { - if (parseResult.Errors.Count > 0) - { - return 1; - } - else - { - return 0; - } - } - } -} diff --git a/src/System.CommandLine/Invocation/ParseErrorAction.cs b/src/System.CommandLine/Invocation/ParseErrorAction.cs index 04845a29da..6c81fc2c1c 100644 --- a/src/System.CommandLine/Invocation/ParseErrorAction.cs +++ b/src/System.CommandLine/Invocation/ParseErrorAction.cs @@ -9,7 +9,7 @@ namespace System.CommandLine.Invocation { internal sealed class ParseErrorAction : CliAction { - public override int Invoke(ParseResult parseResult) + protected override int Invoke(ParseResult parseResult) { ConsoleHelpers.ResetTerminalForegroundColor(); ConsoleHelpers.SetTerminalForegroundRed(); @@ -23,12 +23,19 @@ public override int Invoke(ParseResult parseResult) ConsoleHelpers.ResetTerminalForegroundColor(); - new HelpOption().Action!.Invoke(parseResult); + var helpBuilder = new HelpAction().Builder; + + var helpContext = new HelpContext(helpBuilder, + parseResult.CommandResult.Command, + parseResult.Configuration.Output, + parseResult); + + helpBuilder.Write(helpContext); return 1; } - public override Task InvokeAsync(ParseResult parseResult, CancellationToken cancellationToken = default) + protected override Task InvokeAsync(ParseResult parseResult, CancellationToken cancellationToken = default) => cancellationToken.IsCancellationRequested ? Task.FromCanceled(cancellationToken) : Task.FromResult(Invoke(parseResult)); diff --git a/src/System.CommandLine/Invocation/TypoCorrectionAction.cs b/src/System.CommandLine/Invocation/TypoCorrectionAction.cs index 8daead5693..fe693cb93b 100644 --- a/src/System.CommandLine/Invocation/TypoCorrectionAction.cs +++ b/src/System.CommandLine/Invocation/TypoCorrectionAction.cs @@ -12,10 +12,10 @@ internal sealed class TypoCorrectionAction : CliAction { private const int MaxLevenshteinDistance = 3; - public override int Invoke(ParseResult parseResult) + protected override int Invoke(ParseResult parseResult) => ProvideSuggestions(parseResult); - public override Task InvokeAsync(ParseResult parseResult, CancellationToken cancellationToken = default) + protected override Task InvokeAsync(ParseResult parseResult, CancellationToken cancellationToken = default) => cancellationToken.IsCancellationRequested ? Task.FromCanceled(cancellationToken) : Task.FromResult(ProvideSuggestions(parseResult)); diff --git a/src/System.CommandLine/ParseResult.cs b/src/System.CommandLine/ParseResult.cs index a540cace89..2f0c188947 100644 --- a/src/System.CommandLine/ParseResult.cs +++ b/src/System.CommandLine/ParseResult.cs @@ -289,14 +289,13 @@ static string[] OptionsWithArgumentLimitReached(CommandResult commandResult) => /// /// A token that can be used to cancel an invocation. /// A task whose result can be used as a process exit code. - public Task InvokeAsync(CancellationToken cancellationToken = default) - => InvocationPipeline.InvokeAsync(this, cancellationToken); + public Task InvokeAsync(CancellationToken cancellationToken = default) => CliAction.InvokePipelineAsync(this, cancellationToken); /// /// Invokes the appropriate command handler for a parsed command line input. /// /// A value that can be used as a process exit code. - public int Invoke() => InvocationPipeline.Invoke(this); + public int Invoke() => CliAction.InvokePipeline(this); /// /// Gets the for parsed result. The handler represents the action diff --git a/src/System.CommandLine/Parsing/ParseDiagramAction.cs b/src/System.CommandLine/Parsing/ParseDiagramAction.cs index d9932b2f42..23647b881a 100644 --- a/src/System.CommandLine/Parsing/ParseDiagramAction.cs +++ b/src/System.CommandLine/Parsing/ParseDiagramAction.cs @@ -20,13 +20,13 @@ internal sealed class DiagramAction : CliAction internal DiagramAction(int parseErrorReturnValue) => _parseErrorReturnValue = parseErrorReturnValue; - public override int Invoke(ParseResult parseResult) + protected override int Invoke(ParseResult parseResult) { parseResult.Configuration.Output.WriteLine(Diagram(parseResult)); return parseResult.Errors.Count == 0 ? 0 : _parseErrorReturnValue; } - public override async Task InvokeAsync(ParseResult parseResult, CancellationToken cancellationToken = default) + protected override async Task InvokeAsync(ParseResult parseResult, CancellationToken cancellationToken = default) { cancellationToken.ThrowIfCancellationRequested(); diff --git a/src/System.CommandLine/VersionOption.cs b/src/System.CommandLine/VersionOption.cs index d6f35c5b1f..a26cdf21b8 100644 --- a/src/System.CommandLine/VersionOption.cs +++ b/src/System.CommandLine/VersionOption.cs @@ -63,13 +63,13 @@ private static bool NotImplicit(SymbolResult symbolResult) private sealed class VersionOptionAction : CliAction { - public override int Invoke(ParseResult parseResult) + protected override int Invoke(ParseResult parseResult) { parseResult.Configuration.Output.WriteLine(CliRootCommand.ExecutableVersion); return 0; } - public override Task InvokeAsync(ParseResult parseResult, CancellationToken cancellationToken = default) + protected override Task InvokeAsync(ParseResult parseResult, CancellationToken cancellationToken = default) => cancellationToken.IsCancellationRequested ? Task.FromCanceled(cancellationToken) : Task.FromResult(Invoke(parseResult));