From 790646f97737f40b297ea5e5f3ce4f3b3a56d909 Mon Sep 17 00:00:00 2001 From: Adam Sitnik Date: Tue, 22 Feb 2022 13:01:40 +0100 Subject: [PATCH 01/18] extend ICommandHandler with synchronous Invoke method --- ...tionBinder_api_is_not_changed.approved.txt | 1 + ...ommandLine_api_is_not_changed.approved.txt | 1 + .../CommandHandlerSourceGenerator.cs | 3 ++ .../HostingHandlerTest.cs | 8 +++++ .../ParameterBindingTests.cs | 4 +++ .../ModelBindingCommandHandler.cs | 2 ++ .../Invocation/AnonymousCommandHandler.cs | 32 +++++++++++-------- .../Invocation/ICommandHandler.cs | 9 +++++- 8 files changed, 45 insertions(+), 15 deletions(-) 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 28565c243f..d01b397a21 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 @@ -98,6 +98,7 @@ public class ModelBindingCommandHandler, System.CommandLine.Invocation.ICommandHandler public System.Void BindParameter(System.Reflection.ParameterInfo param, System.CommandLine.Argument argument) public System.Void BindParameter(System.Reflection.ParameterInfo param, System.CommandLine.Option option) + public System.Int32 Invoke(System.CommandLine.Invocation.InvocationContext context) public System.Threading.Tasks.Task InvokeAsync(System.CommandLine.Invocation.InvocationContext context) public class ModelDescriptor public static ModelDescriptor FromType() 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 f39318f893..52b1dc3683 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 @@ -356,6 +356,7 @@ System.CommandLine.Help public System.Int32 GetHashCode() System.CommandLine.Invocation public interface ICommandHandler + public System.Int32 Invoke(InvocationContext context) public System.Threading.Tasks.Task InvokeAsync(InvocationContext context) public interface IInvocationResult public System.Void Apply(InvocationContext context) diff --git a/src/System.CommandLine.Generator/CommandHandlerSourceGenerator.cs b/src/System.CommandLine.Generator/CommandHandlerSourceGenerator.cs index cbae2f8a83..8d4166eb20 100644 --- a/src/System.CommandLine.Generator/CommandHandlerSourceGenerator.cs +++ b/src/System.CommandLine.Generator/CommandHandlerSourceGenerator.cs @@ -114,6 +114,9 @@ private class GeneratedHandler_{handlerCount} : {ICommandHandlerType} {propertyDeclaration}"); } + builder.Append($@" + public int Invoke(global::System.CommandLine.Invocation.InvocationContext context) => InvokeAsync(context).GetAwaiter().GetResult();"); + builder.Append($@" public async global::System.Threading.Tasks.Task InvokeAsync(global::System.CommandLine.Invocation.InvocationContext context) {{"); diff --git a/src/System.CommandLine.Hosting.Tests/HostingHandlerTest.cs b/src/System.CommandLine.Hosting.Tests/HostingHandlerTest.cs index 6191715e30..ab446b2d9c 100644 --- a/src/System.CommandLine.Hosting.Tests/HostingHandlerTest.cs +++ b/src/System.CommandLine.Hosting.Tests/HostingHandlerTest.cs @@ -132,6 +132,12 @@ public MyHandler(MyService service) public int IntOption { get; set; } // bound from option public IConsole Console { get; set; } // bound from DI + public int Invoke(InvocationContext context) + { + service.Value = IntOption; + return IntOption; + } + public Task InvokeAsync(InvocationContext context) { service.Value = IntOption; @@ -162,6 +168,8 @@ public MyHandler(MyService service) public string One { get; set; } + public int Invoke(InvocationContext context) => InvokeAsync(context).GetAwaiter().GetResult(); + public Task InvokeAsync(InvocationContext context) { service.Value = IntOption; diff --git a/src/System.CommandLine.NamingConventionBinder.Tests/ParameterBindingTests.cs b/src/System.CommandLine.NamingConventionBinder.Tests/ParameterBindingTests.cs index 23ad1e474b..2f0530845b 100644 --- a/src/System.CommandLine.NamingConventionBinder.Tests/ParameterBindingTests.cs +++ b/src/System.CommandLine.NamingConventionBinder.Tests/ParameterBindingTests.cs @@ -446,6 +446,8 @@ public abstract class AbstractTestCommandHandler : ICommandHandler { public abstract Task DoJobAsync(); + public int Invoke(InvocationContext context) => InvokeAsync(context).GetAwaiter().GetResult(); + public Task InvokeAsync(InvocationContext context) => DoJobAsync(); } @@ -458,6 +460,8 @@ public override Task DoJobAsync() public class VirtualTestCommandHandler : ICommandHandler { + public int Invoke(InvocationContext context) => 42; + public virtual Task InvokeAsync(InvocationContext context) => Task.FromResult(42); } diff --git a/src/System.CommandLine.NamingConventionBinder/ModelBindingCommandHandler.cs b/src/System.CommandLine.NamingConventionBinder/ModelBindingCommandHandler.cs index aca39c524a..292e545de9 100644 --- a/src/System.CommandLine.NamingConventionBinder/ModelBindingCommandHandler.cs +++ b/src/System.CommandLine.NamingConventionBinder/ModelBindingCommandHandler.cs @@ -128,4 +128,6 @@ private void BindValueSource(ParameterInfo param, IValueSource valueSource) : _methodDescriptor.ParameterDescriptors .FirstOrDefault(x => x.ValueName == param.Name && x.ValueType == param.ParameterType); + + public int Invoke(InvocationContext context) => InvokeAsync(context).GetAwaiter().GetResult(); } \ No newline at end of file diff --git a/src/System.CommandLine/Invocation/AnonymousCommandHandler.cs b/src/System.CommandLine/Invocation/AnonymousCommandHandler.cs index 521f65f945..962fb97e3e 100644 --- a/src/System.CommandLine/Invocation/AnonymousCommandHandler.cs +++ b/src/System.CommandLine/Invocation/AnonymousCommandHandler.cs @@ -7,32 +7,36 @@ namespace System.CommandLine.Invocation { internal class AnonymousCommandHandler : ICommandHandler { - private readonly Func _handle; + private readonly Func? _asyncHandle; + private readonly Action? _syncHandle; public AnonymousCommandHandler(Func handle) - { - _handle = handle ?? throw new ArgumentNullException(nameof(handle)); - } + => _asyncHandle = handle ?? throw new ArgumentNullException(nameof(handle)); public AnonymousCommandHandler(Action handle) + => _syncHandle = handle ?? throw new ArgumentNullException(nameof(handle)); + + public int Invoke(InvocationContext context) { - if (handle == null) + if (_syncHandle is not null) { - throw new ArgumentNullException(nameof(handle)); + _syncHandle(context); + return context.ExitCode; } - _handle = Handle; - - Task Handle(InvocationContext context) - { - handle(context); - return Task.FromResult(context.ExitCode); - } + return SyncUsingAsync(context); // kept in a separate method to avoid JITting } + private int SyncUsingAsync(InvocationContext context) => InvokeAsync(context).GetAwaiter().GetResult(); + public async Task InvokeAsync(InvocationContext context) { - object returnValue = _handle(context); + if (_syncHandle is not null) + { + return Invoke(context); + } + + object returnValue = _asyncHandle!(context); int ret; diff --git a/src/System.CommandLine/Invocation/ICommandHandler.cs b/src/System.CommandLine/Invocation/ICommandHandler.cs index 01b55adbd9..018dbe9b56 100644 --- a/src/System.CommandLine/Invocation/ICommandHandler.cs +++ b/src/System.CommandLine/Invocation/ICommandHandler.cs @@ -13,7 +13,14 @@ public interface ICommandHandler /// /// Performs an action when the associated command is invoked on the command line. /// - /// Provides context for the invocation, including parse results and binding support. + /// Provides context for the invocation, including parse results and binding support. + /// A value that can be used as the exit code for the process. + int Invoke(InvocationContext context); + + /// + /// Performs an action when the associated command is invoked on the command line. + /// + /// Provides context for the invocation, including parse results and binding support. /// A value that can be used as the exit code for the process. Task InvokeAsync(InvocationContext context); } From 7b15e52a393ad8870c020cc6cea48d50fcf08c95 Mon Sep 17 00:00:00 2001 From: Adam Sitnik Date: Tue, 22 Feb 2022 13:04:26 +0100 Subject: [PATCH 02/18] don't build invocation chain if there is no middleware --- .../Invocation/InvocationPipeline.cs | 42 +++++++++++++++---- 1 file changed, 33 insertions(+), 9 deletions(-) diff --git a/src/System.CommandLine/Invocation/InvocationPipeline.cs b/src/System.CommandLine/Invocation/InvocationPipeline.cs index 8e9864713e..08aba2febd 100644 --- a/src/System.CommandLine/Invocation/InvocationPipeline.cs +++ b/src/System.CommandLine/Invocation/InvocationPipeline.cs @@ -17,29 +17,51 @@ public InvocationPipeline(ParseResult parseResult) this.parseResult = parseResult ?? throw new ArgumentNullException(nameof(parseResult)); } - public async Task InvokeAsync(IConsole? console = null) + public Task InvokeAsync(IConsole? console = null) { var context = new InvocationContext(parseResult, console); - InvocationMiddleware invocationChain = BuildInvocationChain(context); + if (context.Parser.Configuration.Middleware.Count == 0 + && context.ParseResult.CommandResult.Command.Handler is ICommandHandler handler) + { + return handler.InvokeAsync(context); + } + + return FullInvocationChainAsync(context); + + static async Task FullInvocationChainAsync(InvocationContext context) + { + InvocationMiddleware invocationChain = BuildInvocationChain(context, true); - await invocationChain(context, _ => Task.CompletedTask); + await invocationChain(context, _ => Task.CompletedTask); - return GetExitCode(context); + return GetExitCode(context); + } } public int Invoke(IConsole? console = null) { var context = new InvocationContext(parseResult, console); - InvocationMiddleware invocationChain = BuildInvocationChain(context); + if (context.Parser.Configuration.Middleware.Count == 0 + && context.ParseResult.CommandResult.Command.Handler is ICommandHandler handler) + { + return handler.Invoke(context); + } + + return FullInvocationChain(context); // kept in a separate method to avoid JITting + + static int FullInvocationChain(InvocationContext context) + { + InvocationMiddleware invocationChain = BuildInvocationChain(context, false); - invocationChain(context, static _ => Task.CompletedTask).ConfigureAwait(false).GetAwaiter().GetResult(); + invocationChain(context, static _ => Task.CompletedTask).ConfigureAwait(false).GetAwaiter().GetResult(); - return GetExitCode(context); + return GetExitCode(context); + } } - private static InvocationMiddleware BuildInvocationChain(InvocationContext context) + private static InvocationMiddleware BuildInvocationChain(InvocationContext context, bool invokeAsync) { var invocations = new List(context.Parser.Configuration.Middleware.Count + 1); invocations.AddRange(context.Parser.Configuration.Middleware); @@ -55,7 +77,9 @@ private static InvocationMiddleware BuildInvocationChain(InvocationContext conte if (handler is not null) { - context.ExitCode = await handler.InvokeAsync(invocationContext); + context.ExitCode = invokeAsync + ? await handler.InvokeAsync(invocationContext) + : handler.Invoke(invocationContext); } } }); From e084081aa76aa310b464958461199b44d5dbb3b9 Mon Sep 17 00:00:00 2001 From: Adam Sitnik Date: Tue, 22 Feb 2022 14:41:44 +0100 Subject: [PATCH 03/18] _executablePath.Value already uses Environment.GetCommandLineArgs()[0] --- src/System.CommandLine/RootCommand.cs | 10 ---------- 1 file changed, 10 deletions(-) diff --git a/src/System.CommandLine/RootCommand.cs b/src/System.CommandLine/RootCommand.cs index df85644288..a8e46ba7bc 100644 --- a/src/System.CommandLine/RootCommand.cs +++ b/src/System.CommandLine/RootCommand.cs @@ -30,16 +30,6 @@ public class RootCommand : Command new(() => { var location = _executablePath.Value; - if (string.IsNullOrEmpty(location)) - { - var commandLineArgs = Environment.GetCommandLineArgs(); - - if (commandLineArgs.Length > 0) - { - location = commandLineArgs[0]; - } - } - return Path.GetFileNameWithoutExtension(location).Replace(" ", ""); }, LazyThreadSafetyMode.PublicationOnly); From e950bf073b7278a0026d181efb56a859b60d13d2 Mon Sep 17 00:00:00 2001 From: Adam Sitnik Date: Tue, 22 Feb 2022 15:04:47 +0100 Subject: [PATCH 04/18] Command.Validators: allocate when needed --- src/System.CommandLine/Command.cs | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/src/System.CommandLine/Command.cs b/src/System.CommandLine/Command.cs index 48382b05b4..dce000b5cf 100644 --- a/src/System.CommandLine/Command.cs +++ b/src/System.CommandLine/Command.cs @@ -23,6 +23,7 @@ public class Command : IdentifierSymbol, IEnumerable private List? _arguments; private List