dotnet · adamsitnik · Apr 12, 2023
diff --git a/...tyApprovalTests.System_CommandLine_NamingConventionBinder_api_is_not_changed.approved.txt b/...tyApprovalTests.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<System.Int32> InvokeAsync(System.CommandLine.ParseResult parseResult, System.Threading.CancellationToken cancellationToken = null)
+    protected System.Int32 Invoke(System.CommandLine.ParseResult parseResult)
+    protected System.Threading.Tasks.Task<System.Int32> InvokeAsync(System.CommandLine.ParseResult parseResult, System.Threading.CancellationToken cancellationToken = null)
   public class ModelDescriptor
     public static ModelDescriptor FromType<T>()
     public static ModelDescriptor FromType(System.Type type)

diff --git a/...ty.Tests/ApiCompatibilityApprovalTests.System_CommandLine_api_is_not_changed.approved.txt b/...ty.Tests/ApiCompatibilityApprovalTests.System_CommandLine_api_is_not_changed.approved.txt
@@ -18,8 +18,8 @@ System.CommandLine
     public static CliArgument<T> AcceptExistingOnly<T>(this CliArgument<T> argument)
   public abstract class CliAction
     public System.Boolean Exclusive { get; }
-    public System.Int32 Invoke(ParseResult parseResult)
-    public System.Threading.Tasks.Task<System.Int32> InvokeAsync(ParseResult parseResult, System.Threading.CancellationToken cancellationToken = null)
+    protected System.Int32 Invoke(ParseResult parseResult)
+    protected System.Threading.Tasks.Task<System.Int32> 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<System.Int32> InvokeAsync(System.CommandLine.ParseResult parseResult, System.Threading.CancellationToken cancellationToken = null)
+    protected System.Int32 Invoke(System.CommandLine.ParseResult parseResult)
+    protected System.Threading.Tasks.Task<System.Int32> 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
@@ -60,7 +60,7 @@ public string ParseResult_Diagram(BdnParam<ParseResult> 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<string> ParseResult_DiagramAsync(BdnParam<ParseResult> 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
@@ -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<int> InvokeAsync(global::System.CommandLine.ParseResult context, global::System.Threading.CancellationToken cancellationToken)
+            protected override async global::System.Threading.Tasks.Task<int> 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
@@ -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<int> InvokeAsync(ParseResult context, CancellationToken cancellationToken)
+            protected override Task<int> 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<int> InvokeAsync(ParseResult context, CancellationToken cancellationToken)
+                protected override Task<int> 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<int> InvokeAsync(ParseResult context, CancellationToken cancellationToken)
+                protected override Task<int> 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
@@ -38,7 +38,7 @@ public override BindingContext GetBindingContext(ParseResult parseResult)
                 ? bindingHandler.GetBindingContext(parseResult)
                 : base.GetBindingContext(parseResult);
 
-        public async override Task<int> InvokeAsync(ParseResult parseResult, CancellationToken cancellationToken = default)
+        protected async override Task<int> InvokeAsync(ParseResult parseResult, CancellationToken cancellationToken = default)
         {
             var argsRemaining = parseResult.UnmatchedTokens;
             var hostBuilder = _hostBuilderFactory?.Invoke(argsRemaining)
@@ -86,7 +86,8 @@ public async override Task<int> 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<int>)_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<int> 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
@@ -54,7 +54,7 @@ public static OptionsBuilder<TOptions> BindCommandLine<TOptions>(
         public static CliCommand UseCommandHandler<THandler>(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
@@ -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
@@ -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<int> 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<int> InvokeAsync(ParseResult context, CancellationToken cancellationToken)
+        protected override Task<int> InvokeAsync(ParseResult context, CancellationToken cancellationToken)
             => DoJobAsync();
     }
 
@@ -422,15 +422,15 @@ public override Task<int> DoJobAsync()
 
     public class VirtualTestCommandHandler : CliAction
     {
-        public override int Invoke(ParseResult context) => 42;
+        protected override int Invoke(ParseResult context) => 42;
 
-        public override Task<int> InvokeAsync(ParseResult context, CancellationToken cancellationToken)
+        protected override Task<int> InvokeAsync(ParseResult context, CancellationToken cancellationToken)
             => Task.FromResult(42);
     }
 
     public class OverridenVirtualTestCommandHandler : VirtualTestCommandHandler
     {
-        public override Task<int> InvokeAsync(ParseResult context, CancellationToken cancellationToken)
+        protected override Task<int> InvokeAsync(ParseResult context, CancellationToken cancellationToken)
             => Task.FromResult(41);
     }
 }
diff --git 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<int> InvokeAsync(ParseResult parseResult, CancellationToken cancellationToken = default) => Task.FromResult(0);
+        protected override Task<int> 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
@@ -56,7 +56,7 @@ internal ModelBindingCommandHandler(
     /// <param name="parseResult">The current parse result.</param>
     /// <param name="cancellationToken">A token that can be used to cancel the invocation.</param>
     /// <returns>A task whose value can be used to set the process exit code.</returns>
-    public override async Task<int> InvokeAsync(ParseResult parseResult, CancellationToken cancellationToken = default)
+    protected override async Task<int> 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);
 
     /// <inheritdoc />
-    public override int Invoke(ParseResult parseResult) => InvokeAsync(parseResult, CancellationToken.None).GetAwaiter().GetResult();
+    protected override int Invoke(ParseResult parseResult) => InvokeAsync(parseResult, CancellationToken.None).GetAwaiter().GetResult();
 }
diff --git a/src/System.CommandLine.Tests/DirectiveTests.cs b/src/System.CommandLine.Tests/DirectiveTests.cs
@@ -153,13 +153,13 @@ public NonexclusiveTestAction(Action<ParseResult> invoke)
                 Exclusive = false;
             }
 
-            public override int Invoke(ParseResult parseResult)
+            protected override int Invoke(ParseResult parseResult)
             {
                 _invoke(parseResult);
                 return 0;
             }
 
-            public override Task<int> InvokeAsync(ParseResult parseResult, CancellationToken cancellationToken = default)
+            protected override Task<int> 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
@@ -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<int> InvokeAsync(ParseResult parseResult, CancellationToken cancellationToken = default)
+            protected override Task<int> 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
@@ -63,9 +63,9 @@ private static Task<int> 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<int> InvokeAsync(ParseResult context, CancellationToken cancellationToken = default)
+            protected async override Task<int> 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
@@ -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<int> InvokeAsync(ParseResult context, CancellationToken cancellationToken = default)
+            protected override Task<int> 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<int> InvokeAsync(ParseResult parseResult, CancellationToken cancellationToken = default)
+            protected override Task<int> 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
@@ -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<int> InvokeAsync(ParseResult context, CancellationToken cancellationToken = default)
+            protected override Task<int> 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
@@ -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
@@ -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
     }
 }