From dc41a4a6916f2b175b28f56950f130be5e4791b2 Mon Sep 17 00:00:00 2001 From: Jon Sequeira Date: Wed, 9 Jul 2025 11:43:17 -0700 Subject: [PATCH 1/2] fix #2622: add ArgumentResult.Implicit property --- ...ommandLine_api_is_not_changed.approved.txt | 1 + src/System.CommandLine.Tests/ParserTests.cs | 42 +++++++++++++++++++ .../Parsing/ArgumentResult.cs | 11 ++++- 3 files changed, 53 insertions(+), 1 deletion(-) 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 1ef7bd503e..80214bd154 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 @@ -200,6 +200,7 @@ System.CommandLine.Invocation System.CommandLine.Parsing public class ArgumentResult : SymbolResult public System.CommandLine.Argument Argument { get; } + public System.Boolean Implicit { get; } public System.Void AddError(System.String errorMessage) public T GetValueOrDefault() public System.Void OnlyTake(System.Int32 numberOfTokens) diff --git a/src/System.CommandLine.Tests/ParserTests.cs b/src/System.CommandLine.Tests/ParserTests.cs index 09a3d8a397..ae37eaf084 100644 --- a/src/System.CommandLine.Tests/ParserTests.cs +++ b/src/System.CommandLine.Tests/ParserTests.cs @@ -926,6 +926,48 @@ public void When_an_argument_with_a_default_value_is_not_matched_then_there_are_ .BeEmpty(); } + [Fact] + public void When_an_argument_with_a_default_value_is_matched_then_the_option_result_is_implicit() + { + var argument = new Argument("the-arg") + { + DefaultValueFactory = _ => "the-default" + }; + + var command = new Command("command") + { + argument + }; + + var result = command.Parse("command the-explicit-value"); + + result.GetResult(argument) + .Implicit + .Should() + .BeFalse(); + } + + [Fact] + public void When_an_argument_with_a_default_value_is_not_matched_then_the_option_result_is_implicit() + { + var argument = new Argument("the-arg") + { + DefaultValueFactory = _ => "the-default" + }; + + var command = new Command("command") + { + argument + }; + + var result = command.Parse("command"); + + result.GetResult(argument) + .Implicit + .Should() + .BeTrue(); + } + [Fact] public void Command_default_argument_value_does_not_override_parsed_value() { diff --git a/src/System.CommandLine/Parsing/ArgumentResult.cs b/src/System.CommandLine/Parsing/ArgumentResult.cs index 1a9fc21939..ca969ab0ae 100644 --- a/src/System.CommandLine/Parsing/ArgumentResult.cs +++ b/src/System.CommandLine/Parsing/ArgumentResult.cs @@ -29,6 +29,8 @@ internal ArgumentResult( internal bool ArgumentLimitReached => Argument.Arity.MaximumNumberOfValues == (_tokens?.Count ?? 0); + public bool Implicit { get; private set; } + internal ArgumentConversionResult GetArgumentConversionResult() => _conversionResult ??= ValidateAndConvert(useValidators: true); @@ -155,8 +157,15 @@ private ArgumentConversionResult ValidateAndConvert(bool useValidators) { var defaultValue = Argument.GetDefaultValue(this); + Implicit = true; + // default value factory provided by the user might report an error, which sets _conversionResult - return _conversionResult ?? ArgumentConversionResult.Success(this, defaultValue); + if (_conversionResult is not null) + { + return _conversionResult; + } + + return ArgumentConversionResult.Success(this, defaultValue); } if (Argument.ConvertArguments is null) From 2bed9d891f2c3589bcee331765e6d3a0eb4cf9ba Mon Sep 17 00:00:00 2001 From: Jon Sequeira Date: Wed, 9 Jul 2025 11:53:41 -0700 Subject: [PATCH 2/2] some code cleanup --- .../Help/HelpBuilderTests.Customization.cs | 10 ++-- .../HelpOptionTests.cs | 1 - .../OptionTests.MultipleArgumentsPerToken.cs | 1 - .../ParseErrorReportingTests.cs | 1 - src/System.CommandLine.Tests/ParserTests.cs | 50 +++++++--------- .../ParsingValidationTests.cs | 58 ++++++++----------- 6 files changed, 50 insertions(+), 71 deletions(-) diff --git a/src/System.CommandLine.Tests/Help/HelpBuilderTests.Customization.cs b/src/System.CommandLine.Tests/Help/HelpBuilderTests.Customization.cs index 703f33d221..b103f18162 100644 --- a/src/System.CommandLine.Tests/Help/HelpBuilderTests.Customization.cs +++ b/src/System.CommandLine.Tests/Help/HelpBuilderTests.Customization.cs @@ -258,8 +258,8 @@ public void Option_can_fallback_to_default_when_customizing(bool conditionA, boo var helpBuilder = new HelpBuilder(LargeMaxWidth); helpBuilder.CustomizeSymbol(option, - firstColumnText: ctx => conditionA ? "custom 1st" : HelpBuilder.Default.GetOptionUsageLabel(option), - secondColumnText: ctx => conditionB ? "custom 2nd" : option.Description ?? string.Empty); + firstColumnText: _ => conditionA ? "custom 1st" : HelpBuilder.Default.GetOptionUsageLabel(option), + secondColumnText: _ => conditionB ? "custom 2nd" : option.Description ?? string.Empty); command.Options.Add(new HelpOption { @@ -300,9 +300,9 @@ public void Argument_can_fallback_to_default_when_customizing( var helpBuilder = new HelpBuilder(LargeMaxWidth); helpBuilder.CustomizeSymbol(argument, - firstColumnText: ctx => conditionA ? "custom 1st" : HelpBuilder.Default.GetArgumentUsageLabel(argument), - secondColumnText: ctx => conditionB ? "custom 2nd" : HelpBuilder.Default.GetArgumentDescription(argument), - defaultValue: ctx => conditionC ? "custom def" : HelpBuilder.Default.GetArgumentDefaultValue(argument)); + firstColumnText: _ => conditionA ? "custom 1st" : HelpBuilder.Default.GetArgumentUsageLabel(argument), + secondColumnText: _ => conditionB ? "custom 2nd" : HelpBuilder.Default.GetArgumentDescription(argument), + defaultValue: _ => conditionC ? "custom def" : HelpBuilder.Default.GetArgumentDefaultValue(argument)); command.Options.Add(new HelpOption { diff --git a/src/System.CommandLine.Tests/HelpOptionTests.cs b/src/System.CommandLine.Tests/HelpOptionTests.cs index 5a6261a48d..cfd85d795a 100644 --- a/src/System.CommandLine.Tests/HelpOptionTests.cs +++ b/src/System.CommandLine.Tests/HelpOptionTests.cs @@ -2,7 +2,6 @@ // Licensed under the MIT license. See LICENSE file in the project root for full license information. using FluentAssertions; -using Microsoft.VisualStudio.TestPlatform.Utilities; using System.CommandLine.Help; using System.CommandLine.Invocation; using System.CommandLine.Tests.Utility; diff --git a/src/System.CommandLine.Tests/OptionTests.MultipleArgumentsPerToken.cs b/src/System.CommandLine.Tests/OptionTests.MultipleArgumentsPerToken.cs index 994ecfd06e..a29b88184b 100644 --- a/src/System.CommandLine.Tests/OptionTests.MultipleArgumentsPerToken.cs +++ b/src/System.CommandLine.Tests/OptionTests.MultipleArgumentsPerToken.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.Parsing; using System.CommandLine.Tests.Utility; using FluentAssertions; using System.Linq; diff --git a/src/System.CommandLine.Tests/ParseErrorReportingTests.cs b/src/System.CommandLine.Tests/ParseErrorReportingTests.cs index d66f37d816..824029b5d3 100644 --- a/src/System.CommandLine.Tests/ParseErrorReportingTests.cs +++ b/src/System.CommandLine.Tests/ParseErrorReportingTests.cs @@ -7,7 +7,6 @@ using System.CommandLine.Tests.Utility; using System.IO; using System.Linq; -using System.Threading; using System.Threading.Tasks; using Xunit; diff --git a/src/System.CommandLine.Tests/ParserTests.cs b/src/System.CommandLine.Tests/ParserTests.cs index ae37eaf084..f93493e05a 100644 --- a/src/System.CommandLine.Tests/ParserTests.cs +++ b/src/System.CommandLine.Tests/ParserTests.cs @@ -15,12 +15,6 @@ namespace System.CommandLine.Tests { public partial class ParserTests { - private T GetValue(ParseResult parseResult, Option option) - => parseResult.GetValue(option); - - private T GetValue(ParseResult parseResult, Argument argument) - => parseResult.GetValue(argument); - [Fact] public void An_option_can_be_checked_by_object_instance() { @@ -816,7 +810,7 @@ public void Commands_can_have_default_argument_values() ParseResult result = command.Parse("command"); - GetValue(result, argument) + result.GetValue(argument) .Should() .Be("default"); @@ -861,7 +855,7 @@ public void When_an_option_with_a_default_value_is_not_matched_then_the_option_c ParseResult result = command.Parse("command"); result.GetResult(option).Should().NotBeNull(); - GetValue(result, option).Should().Be("the-default"); + result.GetValue(option).Should().Be("the-default"); } [Fact] @@ -983,7 +977,7 @@ public void Command_default_argument_value_does_not_override_parsed_value() var result = command.Parse("the-directory"); - GetValue(result, argument) + result.GetValue(argument) .Name .Should() .Be("the-directory"); @@ -1167,7 +1161,7 @@ public void Option_arguments_can_start_with_prefixes_that_make_them_look_like_op var result = command.Parse(input); - GetValue(result, optionX).Should().Be("-y"); + result.GetValue(optionX).Should().Be("-y"); } [Fact] @@ -1186,9 +1180,9 @@ public void Option_arguments_can_start_with_prefixes_that_make_them_look_like_bu var result = command.Parse("-a -bc"); - GetValue(result, optionA).Should().Be("-bc"); - GetValue(result, optionB).Should().BeFalse(); - GetValue(result, optionC).Should().BeFalse(); + result.GetValue(optionA).Should().Be("-bc"); + result.GetValue(optionB).Should().BeFalse(); + result.GetValue(optionC).Should().BeFalse(); } [Fact] @@ -1203,7 +1197,7 @@ public void Option_arguments_can_match_subcommands() var result = root.Parse("-a subcommand"); - GetValue(result, optionA).Should().Be("subcommand"); + result.GetValue(optionA).Should().Be("subcommand"); result.CommandResult.Command.Should().BeSameAs(root); } @@ -1224,7 +1218,7 @@ public void Arguments_can_match_subcommands() result.CommandResult.Command.Should().BeSameAs(subcommand); - GetValue(result, argument) + result.GetValue(argument) .Should() .BeEquivalentSequenceTo("one", "two", "three", "subcommand", "four"); @@ -1249,7 +1243,7 @@ public void Option_arguments_can_match_the_aliases_of_sibling_options_when_non_s var result = command.Parse(input); result.Errors.Should().BeEmpty(); - GetValue(result, optionX).Should().Be("-y"); + result.GetValue(optionX).Should().Be("-y"); } [Fact] @@ -1264,7 +1258,7 @@ public void Single_option_arguments_that_match_option_aliases_are_parsed_correct var result = command.Parse("-x -x"); - GetValue(result, optionX).Should().Be("-x"); + result.GetValue(optionX).Should().Be("-x"); } [Theory] @@ -1291,8 +1285,8 @@ public void Boolean_options_are_not_greedy(string commandLine) result.Errors.Should().BeEmpty(); - GetValue(result, optX).Should().BeTrue(); - GetValue(result, optY).Should().BeTrue(); + result.GetValue(optX).Should().BeTrue(); + result.GetValue(optY).Should().BeTrue(); } [Fact] @@ -1309,8 +1303,8 @@ public void Multiple_option_arguments_that_match_multiple_arity_option_aliases_a var result = command.Parse("-x -x -x -y -y -x -y -y -y -x -x -y"); - GetValue(result, optionX).Should().BeEquivalentTo(new[] { "-x", "-y", "-y" }); - GetValue(result, optionY).Should().BeEquivalentTo(new[] { "-x", "-y", "-x" }); + result.GetValue(optionX).Should().BeEquivalentTo(new[] { "-x", "-y", "-y" }); + result.GetValue(optionY).Should().BeEquivalentTo(new[] { "-x", "-y", "-x" }); } [Fact] @@ -1327,7 +1321,7 @@ public void Bundled_option_arguments_that_match_option_aliases_are_parsed_correc var result = command.Parse("-yxx"); - GetValue(result, optionX).Should().Be("x"); + result.GetValue(optionX).Should().Be("x"); } [Fact] @@ -1344,8 +1338,8 @@ public void Argument_name_is_not_matched_as_a_token() var result = command.Parse("name one two three"); - GetValue(result, nameArg).Should().Be("name"); - GetValue(result, columnsArg).Should().BeEquivalentTo("one", "two", "three"); + result.GetValue(nameArg).Should().Be("name"); + result.GetValue(columnsArg).Should().BeEquivalentTo("one", "two", "three"); } [Fact] @@ -1370,7 +1364,7 @@ public void Boolean_options_with_no_argument_specified_do_not_match_subsequent_a var result = command.Parse("-v an-argument"); - GetValue(result, option).Should().BeTrue(); + result.GetValue(option).Should().BeTrue(); } [Fact] @@ -1387,8 +1381,8 @@ public void When_a_command_line_has_unmatched_tokens_they_are_not_applied_to_sub var result = command.Parse("-x 23 unmatched-token -y 42"); - GetValue(result, optionX).Should().Be("23"); - GetValue(result, optionY).Should().Be("42"); + result.GetValue(optionX).Should().Be("23"); + result.GetValue(optionY).Should().Be("42"); result.UnmatchedTokens.Should().BeEquivalentTo("unmatched-token"); } @@ -1690,7 +1684,7 @@ public void Parsed_value_of_empty_string_arg_is_an_empty_string(string arg1, str var result = rootCommand.Parse(new[] { arg1, arg2 }); - GetValue(result, option).Should().BeEmpty(); + result.GetValue(option).Should().BeEmpty(); } } } diff --git a/src/System.CommandLine.Tests/ParsingValidationTests.cs b/src/System.CommandLine.Tests/ParsingValidationTests.cs index 097f6094b1..76a0c6c979 100644 --- a/src/System.CommandLine.Tests/ParsingValidationTests.cs +++ b/src/System.CommandLine.Tests/ParsingValidationTests.cs @@ -24,8 +24,7 @@ public ParsingValidationTests(ITestOutputHelper output) [Fact] public void When_an_option_accepts_only_specific_arguments_but_a_wrong_one_is_supplied_then_an_informative_error_is_returned() { - var option = new Option("-x"); - option.AcceptOnlyFromAmong("this", "that", "the-other-thing"); + var option = new Option("-x").AcceptOnlyFromAmong("this", "that", "the-other-thing"); var result = new RootCommand { option }.Parse("-x none-of-those"); @@ -40,8 +39,7 @@ public void When_an_option_accepts_only_specific_arguments_but_a_wrong_one_is_su [Fact] public void When_an_option_has_en_error_then_the_error_has_a_reference_to_the_option() { - var option = new Option("-x"); - option.AcceptOnlyFromAmong("this", "that"); + var option = new Option("-x").AcceptOnlyFromAmong("this", "that"); var result = new RootCommand { option }.Parse("-x something_else"); @@ -52,10 +50,9 @@ public void When_an_option_has_en_error_then_the_error_has_a_reference_to_the_op } [Fact] // https://github.com/dotnet/command-line-api/issues/1475 - public void When_FromAmong_is_used_then_the_OptionResult_ErrorMessage_is_set() + public void When_AcceptOnlyFromAmong_is_used_then_the_OptionResult_ErrorMessage_is_set() { - var option = new Option("--opt"); - option.AcceptOnlyFromAmong("a", "b"); + var option = new Option("--opt").AcceptOnlyFromAmong("a", "b"); var command = new Command("test") { option }; var parseResult = command.Parse("test --opt c"); @@ -74,10 +71,9 @@ public void When_FromAmong_is_used_then_the_OptionResult_ErrorMessage_is_set() } [Fact] // https://github.com/dotnet/command-line-api/issues/1475 - public void When_FromAmong_is_used_then_the_ArgumentResult_ErrorMessage_is_set() + public void When_AcceptOnlyFromAmong_is_used_then_the_ArgumentResult_ErrorMessage_is_set() { - var argument = new Argument("arg"); - argument.AcceptOnlyFromAmong("a", "b"); + var argument = new Argument("arg").AcceptOnlyFromAmong("a", "b"); var command = new Command("test") { argument }; @@ -96,12 +92,12 @@ public void When_FromAmong_is_used_then_the_ArgumentResult_ErrorMessage_is_set() } [Fact] // https://github.com/dotnet/command-line-api/issues/1556 - public void When_FromAmong_is_used_for_multiple_arguments_and_valid_input_is_provided_then_there_are_no_errors() + public void When_AcceptOnlyFromAmong_is_used_for_multiple_arguments_and_valid_input_is_provided_then_there_are_no_errors() { var command = new Command("set") { - CreateArgumentWithAcceptOnlyFromAmong(name: "key", "key1", "key2"), - CreateArgumentWithAcceptOnlyFromAmong(name : "value", "value1", "value2") + new Argument("key").AcceptOnlyFromAmong("key1"), + new Argument("value").AcceptOnlyFromAmong("value1") }; var result = command.Parse("set key1 value1"); @@ -110,12 +106,12 @@ public void When_FromAmong_is_used_for_multiple_arguments_and_valid_input_is_pro } [Fact] - public void When_FromAmong_is_used_for_multiple_arguments_and_invalid_input_is_provided_for_the_first_one_then_the_error_is_informative() + public void When_AcceptOnlyFromAmong_is_used_for_multiple_arguments_and_invalid_input_is_provided_for_the_first_one_then_the_error_is_informative() { var command = new Command("set") { - CreateArgumentWithAcceptOnlyFromAmong(name : "key", "key1", "key2"), - CreateArgumentWithAcceptOnlyFromAmong(name : "value", "value1", "value2") + new Argument("key").AcceptOnlyFromAmong(["key1", "key2"]), + new Argument("value").AcceptOnlyFromAmong("value1") }; var result = command.Parse("set not-key1 value1"); @@ -126,11 +122,11 @@ public void When_FromAmong_is_used_for_multiple_arguments_and_invalid_input_is_p .Which .Message .Should() - .Be(LocalizationResources.UnrecognizedArgument("not-key1", new[] { "key1", "key2" })); + .Be(LocalizationResources.UnrecognizedArgument("not-key1", ["key1", "key2"])); } [Fact] - public void When_FromAmong_is_used_multiple_times_only_the_most_recently_provided_values_are_taken_into_account() + public void When_AcceptOnlyFromAmong_is_used_multiple_times_only_the_most_recently_provided_values_are_taken_into_account() { Argument argument = new("key"); argument.AcceptOnlyFromAmong("key1"); @@ -158,12 +154,12 @@ public void When_FromAmong_is_used_multiple_times_only_the_most_recently_provide } [Fact] - public void When_FromAmong_is_used_for_multiple_arguments_and_invalid_input_is_provided_for_the_second_one_then_the_error_is_informative() + public void When_AcceptOnlyFromAmong_is_used_for_multiple_arguments_and_invalid_input_is_provided_for_the_second_one_then_the_error_is_informative() { var command = new Command("set") { - CreateArgumentWithAcceptOnlyFromAmong(name : "key", "key1", "key2"), - CreateArgumentWithAcceptOnlyFromAmong(name : "value", "value1", "value2") + new Argument("key").AcceptOnlyFromAmong("key1"), + new Argument("value").AcceptOnlyFromAmong("value1", "value2") }; var result = command.Parse("set key1 not-value1"); @@ -174,14 +170,13 @@ public void When_FromAmong_is_used_for_multiple_arguments_and_invalid_input_is_p .Which .Message .Should() - .Be(LocalizationResources.UnrecognizedArgument("not-value1", new[] { "value1", "value2" })); + .Be(LocalizationResources.UnrecognizedArgument("not-value1", ["value1", "value2"])); } [Fact] - public void When_FromAmong_is_used_and_multiple_invalid_inputs_are_provided_the_errors_mention_all_invalid_arguments() + public void When_AcceptOnlyFromAmong_is_used_and_multiple_invalid_inputs_are_provided_the_errors_mention_all_invalid_arguments() { - Option option = new("--columns"); - option.AcceptOnlyFromAmong("author", "language", "tags", "type"); + var option = new Option ("--columns").AcceptOnlyFromAmong("author", "language", "tags", "type"); option.Arity = new ArgumentArity(1, 4); option.AllowMultipleArgumentsPerToken = true; @@ -514,9 +509,9 @@ public async Task A_custom_validator_added_to_a_global_option_is_checked(string rootCommand.Options.Add(globalOption); - rootCommand.SetAction((ctx) => handlerWasCalled = true); - childCommand.SetAction((ctx) => handlerWasCalled = true); - grandchildCommand.SetAction((ctx) => handlerWasCalled = true); + rootCommand.SetAction(_ => handlerWasCalled = true); + childCommand.SetAction(_ => handlerWasCalled = true); + grandchildCommand.SetAction(_ => handlerWasCalled = true); var result = await rootCommand.Parse(commandLine).InvokeAsync(); @@ -1297,12 +1292,5 @@ internal void When_there_is_an_arity_error_then_further_errors_are_not_reported( .Should() .Be("Required argument missing for option: '-o'."); } - - private Argument CreateArgumentWithAcceptOnlyFromAmong(string name, params string[] values) - { - Argument argument = new(name); - argument.AcceptOnlyFromAmong(values); - return argument; - } } }