From 6355ad797dcdc2dd40ad69f03d331ad5527d3077 Mon Sep 17 00:00:00 2001 From: Adam Sitnik Date: Fri, 17 Feb 2023 15:59:50 +0100 Subject: [PATCH 1/2] remove non-generic GetValue overloads --- ...ommandLine_api_is_not_changed.approved.txt | 6 ---- .../ModelBinderTests.cs | 32 ------------------- .../Binding/TypeConversionTests.cs | 15 --------- .../Invocation/InvocationContext.cs | 12 ++----- src/System.CommandLine/ParseResult.cs | 20 ++++-------- .../Parsing/SymbolResult.cs | 28 ++-------------- 6 files changed, 10 insertions(+), 103 deletions(-) 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 26cf6ad038..f16015d6b7 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 @@ -224,8 +224,6 @@ System.CommandLine public System.CommandLine.Parsing.SymbolResult FindResultFor(Symbol symbol) public System.CommandLine.Completions.CompletionContext GetCompletionContext() public System.Collections.Generic.IEnumerable GetCompletions(System.Nullable position = null) - public System.Object GetValue(Option option) - public System.Object GetValue(Argument argument) public T GetValue(Argument argument) public T GetValue(Option option) public System.String ToString() @@ -335,9 +333,7 @@ System.CommandLine.Invocation public System.CommandLine.Parsing.Parser Parser { get; } public System.CommandLine.ParseResult ParseResult { get; set; } public System.Threading.CancellationToken GetCancellationToken() - public System.Object GetValue(System.CommandLine.Option option) public T GetValue(Option option) - public System.Object GetValue(System.CommandLine.Argument argument) public T GetValue(Argument argument) public System.Void LinkToken(System.Threading.CancellationToken token) public delegate InvocationMiddleware : System.MulticastDelegate, System.ICloneable, System.Runtime.Serialization.ISerializable @@ -434,9 +430,7 @@ System.CommandLine.Parsing public CommandResult FindResultFor(System.CommandLine.Command command) public OptionResult FindResultFor(System.CommandLine.Option option) public T GetValue(Argument argument) - public System.Object GetValue(System.CommandLine.Argument argument) public T GetValue(Option option) - public System.Object GetValue(System.CommandLine.Option option) public System.String ToString() public class Token, System.IEquatable public static System.Boolean op_Equality(Token left, Token right) diff --git a/src/System.CommandLine.NamingConventionBinder.Tests/ModelBinderTests.cs b/src/System.CommandLine.NamingConventionBinder.Tests/ModelBinderTests.cs index 950122041a..064b93a9e4 100644 --- a/src/System.CommandLine.NamingConventionBinder.Tests/ModelBinderTests.cs +++ b/src/System.CommandLine.NamingConventionBinder.Tests/ModelBinderTests.cs @@ -790,22 +790,6 @@ public void InvocationContext_GetValue_with_generic_option_returns_value() .Be(42); } - [Fact] - public void InvocationContext_GetValue_with_non_generic_option_returns_value() - { - Option option = new Option("--number"); - Command command = new("the-command") - { - option - }; - - InvocationContext invocationContext = new(command.Parse("the-command --number 42")); - - invocationContext.GetValue(option) - .Should() - .Be(42); - } - [Fact] public void InvocationContext_GetValue_with_generic_argument_returns_value() { @@ -822,22 +806,6 @@ public void InvocationContext_GetValue_with_generic_argument_returns_value() .Be(42); } - [Fact] - public void InvocationContext_GetValue_with_non_generic_argument_returns_value() - { - Argument option = new Argument(); - Command command = new("the-command") - { - option - }; - - InvocationContext invocationContext = new(command.Parse("the-command 42")); - - invocationContext.GetValue(option) - .Should() - .Be(42); - } - class DeployOptions { public string Bundle { get; set; } diff --git a/src/System.CommandLine.Tests/Binding/TypeConversionTests.cs b/src/System.CommandLine.Tests/Binding/TypeConversionTests.cs index 891b9a7121..419d8b2fb9 100644 --- a/src/System.CommandLine.Tests/Binding/TypeConversionTests.cs +++ b/src/System.CommandLine.Tests/Binding/TypeConversionTests.cs @@ -238,21 +238,6 @@ public void Nullable_bool_parses_as_null_when_the_option_has_not_been_applied() .Be(null); } - [Fact] // https://github.com/dotnet/command-line-api/issues/1647 - public void Generic_option_bool_parses_when_passed_to_non_generic_GetValueForOption() - { - var option = new Option("-b"); - - var cmd = new RootCommand - { - option - }; - - var parseResult = cmd.Parse("-b"); - - parseResult.GetValue((Option)option).Should().Be(true); - } - [Fact] public void When_exactly_one_argument_is_expected_and_none_are_provided_then_getting_value_throws() { diff --git a/src/System.CommandLine/Invocation/InvocationContext.cs b/src/System.CommandLine/Invocation/InvocationContext.cs index 39076d22e8..58debb158a 100644 --- a/src/System.CommandLine/Invocation/InvocationContext.cs +++ b/src/System.CommandLine/Invocation/InvocationContext.cs @@ -123,19 +123,11 @@ public void LinkToken(CancellationToken token) _registrations.AddLast(token.Register(Cancel)); } - /// - public object? GetValue(Option option) => - ParseResult.GetValue(option); - - /// + /// public T? GetValue(Option option) => ParseResult.GetValue(option); - /// - public object? GetValue(Argument argument) => - ParseResult.GetValue(argument); - - /// + /// public T GetValue(Argument argument) => ParseResult.GetValue(argument); diff --git a/src/System.CommandLine/ParseResult.cs b/src/System.CommandLine/ParseResult.cs index 9bab338295..5b4fd372ea 100644 --- a/src/System.CommandLine/ParseResult.cs +++ b/src/System.CommandLine/ParseResult.cs @@ -119,27 +119,19 @@ CommandLineText is null _ => throw new ArgumentOutOfRangeException() }; - /// - /// Gets the parsed or default value for the specified option. - /// - /// The option for which to get a value. - /// The parsed value or a configured default. - public object? GetValue(Option option) => - RootCommandResult.GetValue(option); - /// /// Gets the parsed or default value for the specified argument. /// /// The argument for which to get a value. /// The parsed value or a configured default. - public object? GetValue(Argument argument) => - RootCommandResult.GetValue(argument); - - /// public T GetValue(Argument argument) => RootCommandResult.GetValue(argument); - - /// + + /// + /// Gets the parsed or default value for the specified option. + /// + /// The option for which to get a value. + /// The parsed value or a configured default. public T? GetValue(Option option) => RootCommandResult.GetValue(option); diff --git a/src/System.CommandLine/Parsing/SymbolResult.cs b/src/System.CommandLine/Parsing/SymbolResult.cs index a7ae8d5202..d597d49116 100644 --- a/src/System.CommandLine/Parsing/SymbolResult.cs +++ b/src/System.CommandLine/Parsing/SymbolResult.cs @@ -65,7 +65,7 @@ private protected SymbolResult(SymbolResultTree symbolResultTree, SymbolResult? /// An option result if the option was matched by the parser or has a default value; otherwise, null. public OptionResult? FindResultFor(Option option) => SymbolResultTree.FindResultFor(option); - /// + /// public T GetValue(Argument argument) { if (FindResultFor(argument) is { } result && @@ -77,19 +77,7 @@ public T GetValue(Argument argument) return (T)ArgumentConverter.GetDefaultValue(argument.ValueType)!; } - /// - public object? GetValue(Argument argument) - { - if (FindResultFor(argument) is { } result && - result.GetValueOrDefault() is { } t) - { - return t; - } - - return ArgumentConverter.GetDefaultValue(argument.ValueType); - } - - /// + /// public T? GetValue(Option option) { if (FindResultFor(option) is { } result && @@ -101,18 +89,6 @@ public T GetValue(Argument argument) return (T)ArgumentConverter.GetDefaultValue(option.Argument.ValueType)!; } - /// - public object? GetValue(Option option) - { - if (FindResultFor(option) is { } result && - result.GetValueOrDefault() is { } t) - { - return t; - } - - return ArgumentConverter.GetDefaultValue(option.Argument.ValueType); - } - internal virtual bool UseDefaultValueFor(ArgumentResult argumentResult) => false; /// From 875d322b4b326b8973987c44bd88676bc48f459a Mon Sep 17 00:00:00 2001 From: Adam Sitnik Date: Fri, 17 Feb 2023 16:11:31 +0100 Subject: [PATCH 2/2] fix nullable annotations for GetValue(Argument) --- src/System.CommandLine/Invocation/InvocationContext.cs | 2 +- src/System.CommandLine/ParseResult.cs | 2 +- src/System.CommandLine/Parsing/SymbolResult.cs | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/System.CommandLine/Invocation/InvocationContext.cs b/src/System.CommandLine/Invocation/InvocationContext.cs index 58debb158a..601d152cc1 100644 --- a/src/System.CommandLine/Invocation/InvocationContext.cs +++ b/src/System.CommandLine/Invocation/InvocationContext.cs @@ -128,7 +128,7 @@ public void LinkToken(CancellationToken token) => ParseResult.GetValue(option); /// - public T GetValue(Argument argument) + public T? GetValue(Argument argument) => ParseResult.GetValue(argument); /// diff --git a/src/System.CommandLine/ParseResult.cs b/src/System.CommandLine/ParseResult.cs index 5b4fd372ea..7c377a6d51 100644 --- a/src/System.CommandLine/ParseResult.cs +++ b/src/System.CommandLine/ParseResult.cs @@ -124,7 +124,7 @@ CommandLineText is null /// /// The argument for which to get a value. /// The parsed value or a configured default. - public T GetValue(Argument argument) + public T? GetValue(Argument argument) => RootCommandResult.GetValue(argument); /// diff --git a/src/System.CommandLine/Parsing/SymbolResult.cs b/src/System.CommandLine/Parsing/SymbolResult.cs index d597d49116..73f5b75b93 100644 --- a/src/System.CommandLine/Parsing/SymbolResult.cs +++ b/src/System.CommandLine/Parsing/SymbolResult.cs @@ -66,7 +66,7 @@ private protected SymbolResult(SymbolResultTree symbolResultTree, SymbolResult? public OptionResult? FindResultFor(Option option) => SymbolResultTree.FindResultFor(option); /// - public T GetValue(Argument argument) + public T? GetValue(Argument argument) { if (FindResultFor(argument) is { } result && result.GetValueOrDefault() is { } t)