From 90abe049fff0dbad5e4c769bdd858abb99d34097 Mon Sep 17 00:00:00 2001 From: Jon Sequeira Date: Tue, 15 Nov 2022 08:15:28 -0800 Subject: [PATCH 1/8] wip --- samples/DragonFruit/DragonFruit.csproj | 2 +- ....CommandLine.NamingConventionBinder.csproj | 2 +- .../UseExceptionHandlerTests.cs | 21 ++++++++++++++++++- .../Builder/CommandLineBuilderExtensions.cs | 2 -- .../Invocation/InvocationPipeline.cs | 3 +++ 5 files changed, 25 insertions(+), 5 deletions(-) diff --git a/samples/DragonFruit/DragonFruit.csproj b/samples/DragonFruit/DragonFruit.csproj index 2a9cb34d9c..7c72c4da2b 100644 --- a/samples/DragonFruit/DragonFruit.csproj +++ b/samples/DragonFruit/DragonFruit.csproj @@ -2,7 +2,7 @@ Exe - net5.0 + net7.0 true diff --git a/src/System.CommandLine.NamingConventionBinder/System.CommandLine.NamingConventionBinder.csproj b/src/System.CommandLine.NamingConventionBinder/System.CommandLine.NamingConventionBinder.csproj index 567e87f251..3e9813d5f8 100644 --- a/src/System.CommandLine.NamingConventionBinder/System.CommandLine.NamingConventionBinder.csproj +++ b/src/System.CommandLine.NamingConventionBinder/System.CommandLine.NamingConventionBinder.csproj @@ -3,7 +3,7 @@ true System.CommandLine.NamingConventionBinder - netstandard2.0 + net7.0;netstandard2.0 10 enable This package provides command handler support for System.CommandLine performs parameter and model binding by matching option and argument names to parameter and property names. diff --git a/src/System.CommandLine.Tests/UseExceptionHandlerTests.cs b/src/System.CommandLine.Tests/UseExceptionHandlerTests.cs index d0e3c1d93b..b012cb22b6 100644 --- a/src/System.CommandLine.Tests/UseExceptionHandlerTests.cs +++ b/src/System.CommandLine.Tests/UseExceptionHandlerTests.cs @@ -1,9 +1,11 @@ // 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.Binding; using System.CommandLine.IO; using System.CommandLine.Parsing; using System.Threading.Tasks; +using ApprovalUtilities.SimpleLogger; using FluentAssertions; using Xunit; @@ -167,5 +169,22 @@ public async Task UseExceptionHandler_set_custom_result_code() resultCode.Should().Be(42); } + + [Fact(Skip = "for later")] + public void issue_796() + { + var rootCmd = new RootCommand(); + rootCmd.SetHandler(_ => throw new Exception()); + + var parser = new CommandLineBuilder(rootCmd) + .UseExceptionHandler( + (_, _) => { }, + errorExitCode: 42) + .Build(); + + var exitCode = parser.Invoke(""); + + exitCode.Should().Be(42); + } } -} \ No newline at end of file +} diff --git a/src/System.CommandLine/Builder/CommandLineBuilderExtensions.cs b/src/System.CommandLine/Builder/CommandLineBuilderExtensions.cs index 0fb3088725..209a72f96d 100644 --- a/src/System.CommandLine/Builder/CommandLineBuilderExtensions.cs +++ b/src/System.CommandLine/Builder/CommandLineBuilderExtensions.cs @@ -1,7 +1,6 @@ // 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; using System.CommandLine.Binding; using System.CommandLine.Help; using System.CommandLine.Invocation; @@ -37,7 +36,6 @@ public static class CommandLineBuilderExtensions { return assemblyVersionAttribute.InformationalVersion; } - }); /// diff --git a/src/System.CommandLine/Invocation/InvocationPipeline.cs b/src/System.CommandLine/Invocation/InvocationPipeline.cs index 4cbcfad873..3e0a9f305c 100644 --- a/src/System.CommandLine/Invocation/InvocationPipeline.cs +++ b/src/System.CommandLine/Invocation/InvocationPipeline.cs @@ -99,3 +99,6 @@ private static int GetExitCode(InvocationContext context) } } } + + +// myapp.exe [parse] subcommand -h \ No newline at end of file From eee98910e2727c69660b577d15b4516ce1058515 Mon Sep 17 00:00:00 2001 From: Jon Sequeira Date: Tue, 15 Nov 2022 09:20:06 -0800 Subject: [PATCH 2/8] move version behavior out of middleware --- .../VersionOptionTests.cs | 2 +- .../Builder/CommandLineBuilderExtensions.cs | 22 ++--------- .../Invocation/InvocationPipeline.cs | 21 +++------- src/System.CommandLine/ParseResult.cs | 20 ++++++++++ .../Parsing/ParseResultVisitor.cs | 39 ++++++++++++++----- src/System.CommandLine/RootCommand.cs | 19 +++++++++ 6 files changed, 79 insertions(+), 44 deletions(-) diff --git a/src/System.CommandLine.Tests/VersionOptionTests.cs b/src/System.CommandLine.Tests/VersionOptionTests.cs index 5d3a5ce7b7..b72b62aed6 100644 --- a/src/System.CommandLine.Tests/VersionOptionTests.cs +++ b/src/System.CommandLine.Tests/VersionOptionTests.cs @@ -73,7 +73,7 @@ public async Task When_the_version_option_is_specified_and_there_are_default_opt { var rootCommand = new RootCommand { - new Option("-x") + new Option("-x", defaultValueFactory: () => true) }; rootCommand.SetHandler(() => { }); diff --git a/src/System.CommandLine/Builder/CommandLineBuilderExtensions.cs b/src/System.CommandLine/Builder/CommandLineBuilderExtensions.cs index 209a72f96d..1bf52b3587 100644 --- a/src/System.CommandLine/Builder/CommandLineBuilderExtensions.cs +++ b/src/System.CommandLine/Builder/CommandLineBuilderExtensions.cs @@ -8,7 +8,6 @@ using System.CommandLine.Parsing; using System.IO; using System.Linq; -using System.Reflection; using System.Threading; using System.Threading.Tasks; using static System.Environment; @@ -21,23 +20,6 @@ namespace System.CommandLine /// public static class CommandLineBuilderExtensions { - private static readonly Lazy _assemblyVersion = - new(() => - { - var assembly = RootCommand.GetAssembly(); - - var assemblyVersionAttribute = assembly.GetCustomAttribute(); - - if (assemblyVersionAttribute is null) - { - return assembly.GetName().Version?.ToString() ?? ""; - } - else - { - return assemblyVersionAttribute.InformationalVersion; - } - }); - /// /// Enables signaling and handling of process termination via a that can be passed to a during invocation. /// @@ -641,6 +623,7 @@ public static CommandLineBuilder UseVersionOption( builder.VersionOption = versionOption; builder.Command.AddOption(versionOption); +#if false builder.AddMiddleware(async (context, next) => { if (context.ParseResult.FindResultFor(versionOption) is { }) @@ -659,6 +642,7 @@ public static CommandLineBuilder UseVersionOption( await next(context); } }, MiddlewareOrderInternal.VersionOption); +#endif return builder; } @@ -682,6 +666,7 @@ public static CommandLineBuilder UseVersionOption( builder.VersionOption = versionOption; command.AddOption(versionOption); +#if false builder.AddMiddleware(async (context, next) => { if (context.ParseResult.FindResultFor(versionOption) is { }) @@ -700,6 +685,7 @@ public static CommandLineBuilder UseVersionOption( await next(context); } }, MiddlewareOrderInternal.VersionOption); +#endif return builder; } diff --git a/src/System.CommandLine/Invocation/InvocationPipeline.cs b/src/System.CommandLine/Invocation/InvocationPipeline.cs index 3e0a9f305c..8700d2a80a 100644 --- a/src/System.CommandLine/Invocation/InvocationPipeline.cs +++ b/src/System.CommandLine/Invocation/InvocationPipeline.cs @@ -68,19 +68,11 @@ private static InvocationMiddleware BuildInvocationChain(InvocationContext conte invocations.Add(async (invocationContext, _) => { - if (invocationContext - .ParseResult - .CommandResult - .Command is Command command) + if (invocationContext.ParseResult.Handler is { } handler) { - var handler = command.Handler; - - if (handler is not null) - { - context.ExitCode = invokeAsync - ? await handler.InvokeAsync(invocationContext) - : handler.Invoke(invocationContext); - } + context.ExitCode = invokeAsync + ? await handler.InvokeAsync(invocationContext) + : handler.Invoke(invocationContext); } }); @@ -88,7 +80,7 @@ private static InvocationMiddleware BuildInvocationChain(InvocationContext conte (first, second) => (ctx, next) => first(ctx, - c => second(c, next))); + c => second(c, next))); } private static int GetExitCode(InvocationContext context) @@ -99,6 +91,3 @@ private static int GetExitCode(InvocationContext context) } } } - - -// myapp.exe [parse] subcommand -h \ No newline at end of file diff --git a/src/System.CommandLine/ParseResult.cs b/src/System.CommandLine/ParseResult.cs index 8c0b61b167..7ad1b5305a 100644 --- a/src/System.CommandLine/ParseResult.cs +++ b/src/System.CommandLine/ParseResult.cs @@ -18,6 +18,7 @@ public class ParseResult private readonly RootCommandResult _rootCommandResult; private readonly IReadOnlyList _unmatchedTokens; private CompletionContext? _completionContext; + private ICommandHandler? _handler; internal ParseResult( Parser parser, @@ -239,6 +240,25 @@ static IEnumerable OptionsWithArgumentLimitReached(SymbolResult symbolRe .SelectMany(c => c.Aliases); } + internal ICommandHandler? Handler + { + get + { + if (_handler is not null) + { + return _handler; + } + + if (CommandResult.Command is { } command) + { + return command.Handler; + } + + return null; + } + set => _handler = value; + } + private SymbolResult SymbolToComplete(int? position = null) { var commandResult = CommandResult; diff --git a/src/System.CommandLine/Parsing/ParseResultVisitor.cs b/src/System.CommandLine/Parsing/ParseResultVisitor.cs index 97834fe642..2880c3b999 100644 --- a/src/System.CommandLine/Parsing/ParseResultVisitor.cs +++ b/src/System.CommandLine/Parsing/ParseResultVisitor.cs @@ -4,6 +4,8 @@ using System.Collections.Generic; using System.CommandLine.Binding; using System.CommandLine.Help; +using System.CommandLine.Invocation; +using System.CommandLine.IO; using System.Linq; namespace System.CommandLine.Parsing @@ -26,6 +28,7 @@ internal sealed class ParseResultVisitor private RootCommandResult? _rootCommandResult; private CommandResult? _innermostCommandResult; private bool _isHelpRequested; + private bool _isVersionRequested; public ParseResultVisitor( Parser parser, @@ -183,6 +186,11 @@ private void VisitOptionNode(OptionNode optionNode) _isHelpRequested = true; } + if (optionNode.Option is VersionOption) + { + _isVersionRequested = true; + } + var optionResult = new OptionResult( optionNode.Option, optionNode.Token, @@ -622,14 +630,27 @@ private void AddErrorToResult(SymbolResult symbolResult, ParseError parseError) _errors.Add(parseError); } - public ParseResult GetResult() => - new(_parser, - _rootCommandResult ?? throw new InvalidOperationException("No root command was found"), - _innermostCommandResult ?? throw new InvalidOperationException("No command was found"), - _directives, - _tokenizeResult, - _unmatchedTokens, - _errors, - _rawInput); + public ParseResult GetResult() + { + var parseResult = new ParseResult(_parser, + _rootCommandResult ?? throw new InvalidOperationException("No root command was found"), + _innermostCommandResult ?? throw new InvalidOperationException("No command was found"), + _directives, + _tokenizeResult, + _unmatchedTokens, + _errors, + _rawInput); + + if (_isVersionRequested) + { + // FIX: (GetResult) use the ActiveOption's handler + parseResult.Handler = new AnonymousCommandHandler(context => + { + context.Console.Out.WriteLine(RootCommand.ExecutableVersion); + }); + } + + return parseResult; + } } } diff --git a/src/System.CommandLine/RootCommand.cs b/src/System.CommandLine/RootCommand.cs index 3fe653c1cc..59443d33b5 100644 --- a/src/System.CommandLine/RootCommand.cs +++ b/src/System.CommandLine/RootCommand.cs @@ -19,6 +19,23 @@ public class RootCommand : Command private static Assembly? _assembly; private static string? _executablePath; private static string? _executableName; + private static string? _executableVersion; + + private static string GetExecutableVersion() + { + var assembly = GetAssembly(); + + var assemblyVersionAttribute = assembly.GetCustomAttribute(); + + if (assemblyVersionAttribute is null) + { + return assembly.GetName().Version?.ToString() ?? ""; + } + else + { + return assemblyVersionAttribute.InformationalVersion; + } + } /// The description of the command, shown in help. public RootCommand(string description = "") : base(ExecutableName, description) @@ -38,6 +55,8 @@ public static string ExecutableName /// The path to the currently running executable. /// public static string ExecutablePath => _executablePath ??= Environment.GetCommandLineArgs()[0]; + + internal static string ExecutableVersion => _executableVersion ??= GetExecutableVersion(); private protected override void RemoveAlias(string alias) { From aaf9c64cd46bf67bb7b402350698f911111e99bc Mon Sep 17 00:00:00 2001 From: Jon Sequeira Date: Tue, 15 Nov 2022 09:35:08 -0800 Subject: [PATCH 3/8] remove DisallowBinding --- src/System.CommandLine/Help/HelpOption.cs | 1 - src/System.CommandLine/Help/VersionOption.cs | 4 ---- src/System.CommandLine/Option.cs | 2 -- 3 files changed, 7 deletions(-) diff --git a/src/System.CommandLine/Help/HelpOption.cs b/src/System.CommandLine/Help/HelpOption.cs index b85d5abd95..616156cff1 100644 --- a/src/System.CommandLine/Help/HelpOption.cs +++ b/src/System.CommandLine/Help/HelpOption.cs @@ -12,7 +12,6 @@ public HelpOption(string[] aliases, Func getLocalizationR : base(aliases, null, new Argument { Arity = ArgumentArity.Zero }) { _localizationResources = getLocalizationResources; - DisallowBinding = true; } public HelpOption(Func getLocalizationResources) : this(new[] diff --git a/src/System.CommandLine/Help/VersionOption.cs b/src/System.CommandLine/Help/VersionOption.cs index ab906b8d01..9baf20693b 100644 --- a/src/System.CommandLine/Help/VersionOption.cs +++ b/src/System.CommandLine/Help/VersionOption.cs @@ -15,8 +15,6 @@ internal class VersionOption : Option { _builder = builder; - DisallowBinding = true; - AddValidators(); } @@ -24,8 +22,6 @@ public VersionOption(string[] aliases, CommandLineBuilder builder) : base(aliase { _builder = builder; - DisallowBinding = true; - AddValidators(); } diff --git a/src/System.CommandLine/Option.cs b/src/System.CommandLine/Option.cs index 3a4447fb89..b6df1fba2c 100644 --- a/src/System.CommandLine/Option.cs +++ b/src/System.CommandLine/Option.cs @@ -80,8 +80,6 @@ public ArgumentArity Arity /// internal bool IsGlobal { get; set; } - internal bool DisallowBinding { get; init; } - /// public override string Name { From a84fb80cabd721dfeb1143125a5d8d0a436ca0c4 Mon Sep 17 00:00:00 2001 From: Jon Sequeira Date: Tue, 15 Nov 2022 13:28:57 -0800 Subject: [PATCH 4/8] use ParseResult instead of middleware --- .../VersionOptionTests.cs | 38 ++++--------------- .../Invocation/InvocationPipeline.cs | 15 ++++---- 2 files changed, 15 insertions(+), 38 deletions(-) diff --git a/src/System.CommandLine.Tests/VersionOptionTests.cs b/src/System.CommandLine.Tests/VersionOptionTests.cs index b72b62aed6..fcebf7c1f9 100644 --- a/src/System.CommandLine.Tests/VersionOptionTests.cs +++ b/src/System.CommandLine.Tests/VersionOptionTests.cs @@ -126,21 +126,9 @@ public void Version_is_not_valid_with_other_tokens(string commandLine) .UseVersionOption() .Build(); - var console = new TestConsole(); - - var result = parser.Invoke(commandLine, console); - - console.Out - .ToString() - .Should() - .NotContain(version); + var result = parser.Parse(commandLine); - console.Error - .ToString() - .Should() - .Contain("--version option cannot be combined with other arguments."); - - result.Should().NotBe(0); + result.Errors.Should().Contain(e => e.Message == "--version option cannot be combined with other arguments."); } [Fact] @@ -206,7 +194,7 @@ public async Task Version_can_specify_additional_alias() [Fact] public void Version_is_not_valid_with_other_tokens_uses_custom_alias() { - var childCommand = new Command("subcommand"); + var childCommand = new Command("subcommand"); childCommand.SetHandler(() => { }); var rootCommand = new RootCommand { @@ -215,24 +203,12 @@ public void Version_is_not_valid_with_other_tokens_uses_custom_alias() rootCommand.SetHandler(() => { }); var parser = new CommandLineBuilder(rootCommand) - .UseVersionOption("-v") - .Build(); - - var console = new TestConsole(); - - var result = parser.Invoke("-v subcommand", console); - - console.Out - .ToString() - .Should() - .NotContain(version); + .UseVersionOption("-v") + .Build(); - console.Error - .ToString() - .Should() - .Contain("-v option cannot be combined with other arguments."); + var result = parser.Parse("-v subcommand"); - result.Should().NotBe(0); + result.Errors.Should().ContainSingle(e => e.Message == "-v option cannot be combined with other arguments."); } } } diff --git a/src/System.CommandLine/Invocation/InvocationPipeline.cs b/src/System.CommandLine/Invocation/InvocationPipeline.cs index 8700d2a80a..a16b8a2832 100644 --- a/src/System.CommandLine/Invocation/InvocationPipeline.cs +++ b/src/System.CommandLine/Invocation/InvocationPipeline.cs @@ -2,6 +2,7 @@ // Licensed under the MIT license. See LICENSE file in the project root for full license information. using System.Collections.Generic; +using System.CommandLine.Parsing; using System.Linq; using System.Threading; using System.Threading.Tasks; @@ -21,15 +22,15 @@ public Task InvokeAsync(IConsole? console = null, CancellationToken cancell { var context = new InvocationContext(_parseResult, console, cancellationToken); - if (context.Parser.Configuration.Middleware.Count == 0 - && context.ParseResult.CommandResult.Command.Handler is ICommandHandler handler) + if (context.Parser.Configuration.Middleware.Count == 0 && + _parseResult.Handler is not null) { - return handler.InvokeAsync(context); + return _parseResult.Handler.InvokeAsync(context); } - return FullInvocationChainAsync(context); + return InvokeHandlerWithMiddleware(context); - static async Task FullInvocationChainAsync(InvocationContext context) + static async Task InvokeHandlerWithMiddleware(InvocationContext context) { InvocationMiddleware invocationChain = BuildInvocationChain(context, true); @@ -49,9 +50,9 @@ public int Invoke(IConsole? console = null) return handler.Invoke(context); } - return FullInvocationChain(context); // kept in a separate method to avoid JITting + return InvokeHandlerWithMiddleware(context); // kept in a separate method to avoid JITting - static int FullInvocationChain(InvocationContext context) + static int InvokeHandlerWithMiddleware(InvocationContext context) { InvocationMiddleware invocationChain = BuildInvocationChain(context, false); From a8f2844159823e175cb58152349a598c2e654761 Mon Sep 17 00:00:00 2001 From: Jon Sequeira Date: Tue, 15 Nov 2022 14:09:51 -0800 Subject: [PATCH 5/8] Update TFMs --- samples/HostingPlayground/HostingPlayground.csproj | 2 +- samples/RenderingPlayground/RenderingPlayground.csproj | 2 +- .../System.CommandLine.Hosting.csproj | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/samples/HostingPlayground/HostingPlayground.csproj b/samples/HostingPlayground/HostingPlayground.csproj index b939d5bedd..54bc18bb67 100644 --- a/samples/HostingPlayground/HostingPlayground.csproj +++ b/samples/HostingPlayground/HostingPlayground.csproj @@ -2,7 +2,7 @@ Exe - net5.0 + net7.0 true $(NoWarn);CS1591 diff --git a/samples/RenderingPlayground/RenderingPlayground.csproj b/samples/RenderingPlayground/RenderingPlayground.csproj index e43b0a77b8..0f9aba9a8f 100644 --- a/samples/RenderingPlayground/RenderingPlayground.csproj +++ b/samples/RenderingPlayground/RenderingPlayground.csproj @@ -2,7 +2,7 @@ Exe - net5.0 + net7.0 true diff --git a/src/System.CommandLine.Hosting/System.CommandLine.Hosting.csproj b/src/System.CommandLine.Hosting/System.CommandLine.Hosting.csproj index 9433f93a0c..acab3a977e 100644 --- a/src/System.CommandLine.Hosting/System.CommandLine.Hosting.csproj +++ b/src/System.CommandLine.Hosting/System.CommandLine.Hosting.csproj @@ -2,7 +2,7 @@ true - netstandard2.0;netstandard2.1 + netstandard2.0;netstandard2.1;net7.0 This package provides support for using System.CommandLine with Microsoft.Extensions.Hosting. From b1cdaa6e491d021c494584b016b745a7dd41fa40 Mon Sep 17 00:00:00 2001 From: Jon Sequeira Date: Tue, 15 Nov 2022 14:10:08 -0800 Subject: [PATCH 6/8] clean up --- .../UseExceptionHandlerTests.cs | 21 +------------------ .../Builder/CommandLineBuilderExtensions.cs | 1 + 2 files changed, 2 insertions(+), 20 deletions(-) diff --git a/src/System.CommandLine.Tests/UseExceptionHandlerTests.cs b/src/System.CommandLine.Tests/UseExceptionHandlerTests.cs index b012cb22b6..d0e3c1d93b 100644 --- a/src/System.CommandLine.Tests/UseExceptionHandlerTests.cs +++ b/src/System.CommandLine.Tests/UseExceptionHandlerTests.cs @@ -1,11 +1,9 @@ // 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.Binding; using System.CommandLine.IO; using System.CommandLine.Parsing; using System.Threading.Tasks; -using ApprovalUtilities.SimpleLogger; using FluentAssertions; using Xunit; @@ -169,22 +167,5 @@ public async Task UseExceptionHandler_set_custom_result_code() resultCode.Should().Be(42); } - - [Fact(Skip = "for later")] - public void issue_796() - { - var rootCmd = new RootCommand(); - rootCmd.SetHandler(_ => throw new Exception()); - - var parser = new CommandLineBuilder(rootCmd) - .UseExceptionHandler( - (_, _) => { }, - errorExitCode: 42) - .Build(); - - var exitCode = parser.Invoke(""); - - exitCode.Should().Be(42); - } } -} +} \ No newline at end of file diff --git a/src/System.CommandLine/Builder/CommandLineBuilderExtensions.cs b/src/System.CommandLine/Builder/CommandLineBuilderExtensions.cs index 1bf52b3587..0ac055d7f3 100644 --- a/src/System.CommandLine/Builder/CommandLineBuilderExtensions.cs +++ b/src/System.CommandLine/Builder/CommandLineBuilderExtensions.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; using System.CommandLine.Binding; using System.CommandLine.Help; using System.CommandLine.Invocation; From 196961243c1b7bfb69a5744500735af956169d1a Mon Sep 17 00:00:00 2001 From: Jon Sequeira Date: Tue, 15 Nov 2022 14:42:54 -0800 Subject: [PATCH 7/8] fix warnings --- .../CommandHandler.cs | 2 +- .../MethodInfoHandlerDescriptor.cs | 2 +- .../ModelBinder.cs | 4 ++-- .../ModelBinder{T}.cs | 19 +++++++++++-------- .../ModelBindingCommandHandler.cs | 6 +++--- .../ModelDescriptor.cs | 2 +- .../ParameterDescriptor.cs | 2 +- 7 files changed, 20 insertions(+), 17 deletions(-) diff --git a/src/System.CommandLine.NamingConventionBinder/CommandHandler.cs b/src/System.CommandLine.NamingConventionBinder/CommandHandler.cs index a0fa5a5d5e..dbcf4daed9 100644 --- a/src/System.CommandLine.NamingConventionBinder/CommandHandler.cs +++ b/src/System.CommandLine.NamingConventionBinder/CommandHandler.cs @@ -605,7 +605,7 @@ public static ICommandHandler Create> action) => HandlerDescriptor.FromDelegate(action).GetCommandHandler(); - internal static async Task GetExitCodeAsync(object returnValue, InvocationContext context) + internal static async Task GetExitCodeAsync(object? returnValue, InvocationContext context) { switch (returnValue) { diff --git a/src/System.CommandLine.NamingConventionBinder/MethodInfoHandlerDescriptor.cs b/src/System.CommandLine.NamingConventionBinder/MethodInfoHandlerDescriptor.cs index c94b951149..6f4e09e744 100644 --- a/src/System.CommandLine.NamingConventionBinder/MethodInfoHandlerDescriptor.cs +++ b/src/System.CommandLine.NamingConventionBinder/MethodInfoHandlerDescriptor.cs @@ -38,7 +38,7 @@ public override ICommandHandler GetCommandHandler() } } - public override ModelDescriptor Parent => ModelDescriptor.FromType(_handlerMethodInfo.ReflectedType); + public override ModelDescriptor Parent => ModelDescriptor.FromType(_handlerMethodInfo.ReflectedType!); private protected override IEnumerable InitializeParameterDescriptors() => _handlerMethodInfo.GetParameters() diff --git a/src/System.CommandLine.NamingConventionBinder/ModelBinder.cs b/src/System.CommandLine.NamingConventionBinder/ModelBinder.cs index 56885c878a..1054d5ef60 100644 --- a/src/System.CommandLine.NamingConventionBinder/ModelBinder.cs +++ b/src/System.CommandLine.NamingConventionBinder/ModelBinder.cs @@ -141,7 +141,7 @@ private bool ShortCutTheBinding() private (bool success, object? newInstance, bool anyNonDefaults) InstanceFromSpecificConstructor( BindingContext bindingContext, ConstructorDescriptor constructor, IReadOnlyList? boundValues, ref bool nonDefaultsUsed) { - var values = boundValues.Select(x => x.Value).ToArray(); + var values = boundValues?.Select(x => x.Value).ToArray() ?? Array.Empty(); object? newInstance = null; try { @@ -332,7 +332,7 @@ private static BoundValue DefaultForValueDescriptor( valueSource); } - private protected IValueDescriptor FindModelPropertyDescriptor(Type propertyType, string propertyName) + private protected IValueDescriptor? FindModelPropertyDescriptor(Type propertyType, string propertyName) { return ModelDescriptor.PropertyDescriptors .FirstOrDefault(desc => diff --git a/src/System.CommandLine.NamingConventionBinder/ModelBinder{T}.cs b/src/System.CommandLine.NamingConventionBinder/ModelBinder{T}.cs index 58acb092ae..98b7d95eba 100644 --- a/src/System.CommandLine.NamingConventionBinder/ModelBinder{T}.cs +++ b/src/System.CommandLine.NamingConventionBinder/ModelBinder{T}.cs @@ -25,10 +25,12 @@ public void BindMemberFromValue( IValueDescriptor valueDescriptor) { var (propertyType, propertyName) = property.MemberTypeAndName(); - var propertyDescriptor = FindModelPropertyDescriptor( - propertyType, propertyName); - MemberBindingSources[propertyDescriptor] = - new SpecificSymbolValueSource(valueDescriptor); + var propertyDescriptor = FindModelPropertyDescriptor(propertyType, propertyName); + + if (propertyDescriptor is not null) + { + MemberBindingSources[propertyDescriptor] = new SpecificSymbolValueSource(valueDescriptor); + } } /// @@ -42,9 +44,10 @@ public void BindMemberFromValue( Func getValue) { var (propertyType, propertyName) = property.MemberTypeAndName(); - var propertyDescriptor = FindModelPropertyDescriptor( - propertyType, propertyName); - MemberBindingSources[propertyDescriptor] = - new DelegateValueSource(c => getValue(c)); + var propertyDescriptor = FindModelPropertyDescriptor(propertyType, propertyName); + if (propertyDescriptor is not null) + { + MemberBindingSources[propertyDescriptor] = new DelegateValueSource(c => getValue(c)); + } } } \ No newline at end of file diff --git a/src/System.CommandLine.NamingConventionBinder/ModelBindingCommandHandler.cs b/src/System.CommandLine.NamingConventionBinder/ModelBindingCommandHandler.cs index 72a53338f6..451cb1591a 100644 --- a/src/System.CommandLine.NamingConventionBinder/ModelBindingCommandHandler.cs +++ b/src/System.CommandLine.NamingConventionBinder/ModelBindingCommandHandler.cs @@ -31,7 +31,7 @@ internal ModelBindingCommandHandler( _handlerMethodInfo = handlerMethodInfo ?? throw new ArgumentNullException(nameof(handlerMethodInfo)); _invocationTargetBinder = _handlerMethodInfo.IsStatic ? null - : new ModelBinder(_handlerMethodInfo.ReflectedType); + : new ModelBinder(_handlerMethodInfo.ReflectedType!); _methodDescriptor = methodDescriptor ?? throw new ArgumentNullException(nameof(methodDescriptor)); _invocationTarget = invocationTarget; } @@ -69,11 +69,11 @@ public async Task InvokeAsync(InvocationContext context) .Select(x => x.Value) .ToArray(); - object result; + object? result; if (_handlerDelegate is null) { var invocationTarget = _invocationTarget ?? - bindingContext.GetService(_handlerMethodInfo!.ReflectedType); + bindingContext.GetService(_handlerMethodInfo!.ReflectedType!); if(invocationTarget is { }) { _invocationTargetBinder?.UpdateInstance(invocationTarget, bindingContext); diff --git a/src/System.CommandLine.NamingConventionBinder/ModelDescriptor.cs b/src/System.CommandLine.NamingConventionBinder/ModelDescriptor.cs index d47cee393f..341919493c 100644 --- a/src/System.CommandLine.NamingConventionBinder/ModelDescriptor.cs +++ b/src/System.CommandLine.NamingConventionBinder/ModelDescriptor.cs @@ -46,7 +46,7 @@ protected ModelDescriptor(Type modelType) public IReadOnlyList PropertyDescriptors => _propertyDescriptors ??= ModelType.GetProperties(CommonBindingFlags) - .Where(p => p.CanWrite && p.SetMethod.IsPublic) + .Where(p => p.CanWrite && p.SetMethod?.IsPublic == true) .Select(i => new PropertyDescriptor(i, this)) .ToList(); diff --git a/src/System.CommandLine.NamingConventionBinder/ParameterDescriptor.cs b/src/System.CommandLine.NamingConventionBinder/ParameterDescriptor.cs index c7ae5fcb65..4cc264117b 100644 --- a/src/System.CommandLine.NamingConventionBinder/ParameterDescriptor.cs +++ b/src/System.CommandLine.NamingConventionBinder/ParameterDescriptor.cs @@ -23,7 +23,7 @@ internal ParameterDescriptor( } /// - public string ValueName => _parameterInfo.Name; + public string ValueName => _parameterInfo.Name!; /// /// The method descriptor that this constructor belongs to. From 3ed169624a14576622bb8fc06830f8015be2d8dd Mon Sep 17 00:00:00 2001 From: Jon Sequeira Date: Thu, 17 Nov 2022 11:48:40 -0800 Subject: [PATCH 8/8] remove dead code --- .../Builder/CommandLineBuilderExtensions.cs | 42 ------------------- 1 file changed, 42 deletions(-) diff --git a/src/System.CommandLine/Builder/CommandLineBuilderExtensions.cs b/src/System.CommandLine/Builder/CommandLineBuilderExtensions.cs index 0ac055d7f3..b20832ba20 100644 --- a/src/System.CommandLine/Builder/CommandLineBuilderExtensions.cs +++ b/src/System.CommandLine/Builder/CommandLineBuilderExtensions.cs @@ -624,27 +624,6 @@ public static CommandLineBuilder UseVersionOption( builder.VersionOption = versionOption; builder.Command.AddOption(versionOption); -#if false - builder.AddMiddleware(async (context, next) => - { - if (context.ParseResult.FindResultFor(versionOption) is { }) - { - if (context.ParseResult.Errors.Any(e => e.SymbolResult?.Symbol is VersionOption)) - { - context.InvocationResult = static ctx => ParseErrorResult.Apply(ctx, null); - } - else - { - context.Console.Out.WriteLine(_assemblyVersion.Value); - } - } - else - { - await next(context); - } - }, MiddlewareOrderInternal.VersionOption); -#endif - return builder; } @@ -667,27 +646,6 @@ public static CommandLineBuilder UseVersionOption( builder.VersionOption = versionOption; command.AddOption(versionOption); -#if false - builder.AddMiddleware(async (context, next) => - { - if (context.ParseResult.FindResultFor(versionOption) is { }) - { - if (context.ParseResult.Errors.Any(e => e.SymbolResult?.Symbol is VersionOption)) - { - context.InvocationResult = static ctx => ParseErrorResult.Apply(ctx, null); - } - else - { - context.Console.Out.WriteLine(_assemblyVersion.Value); - } - } - else - { - await next(context); - } - }, MiddlewareOrderInternal.VersionOption); -#endif - return builder; }