diff --git a/src/System.CommandLine.Tests/ArgumentTests.cs b/src/System.CommandLine.Tests/ArgumentTests.cs index 10e2483805..92dc160c3b 100644 --- a/src/System.CommandLine.Tests/ArgumentTests.cs +++ b/src/System.CommandLine.Tests/ArgumentTests.cs @@ -67,13 +67,13 @@ public void When_argument_type_is_set_to_null_then_it_throws() } [Fact] - public void By_default_the_argument_type_is_void() + public void By_default_the_argument_type_is_string() { var argument = new Argument(); argument.ArgumentType - .Should() - .Be(typeof(void)); + .Should() + .Be(typeof(string)); } public class CustomParsing @@ -203,8 +203,7 @@ public void custom_parsing_of_scalar_value_from_an_argument_with_one_token() var argument = new Argument(result => int.Parse(result.Tokens.Single().Value)); argument.Parse("123") - .FindResultFor(argument) - .GetValueOrDefault() + .ValueForArgument(argument) .Should() .Be(123); } @@ -215,8 +214,7 @@ public void custom_parsing_of_sequence_value_from_an_argument_with_one_token() var argument = new Argument>(result => result.Tokens.Single().Value.Split(',').Select(int.Parse)); argument.Parse("1,2,3") - .FindResultFor(argument) - .GetValueOrDefault() + .ValueForArgument(argument) .Should() .BeEquivalentTo(new[] { 1, 2, 3 }); } @@ -230,8 +228,7 @@ public void custom_parsing_of_sequence_value_from_an_argument_with_multiple_toke }); argument.Parse("1 2 3") - .FindResultFor(argument) - .GetValueOrDefault() + .ValueForArgument(argument) .Should() .BeEquivalentTo(new[] { 1, 2, 3 }); } @@ -245,8 +242,7 @@ public void custom_parsing_of_scalar_value_from_an_argument_with_multiple_tokens }; argument.Parse("1 2 3") - .FindResultFor(argument) - .GetValueOrDefault() + .ValueForArgument(argument) .Should() .Be(6); } diff --git a/src/System.CommandLine.Tests/Binding/TypeConversionTests.cs b/src/System.CommandLine.Tests/Binding/TypeConversionTests.cs index 046ad166d4..66f3b8a050 100644 --- a/src/System.CommandLine.Tests/Binding/TypeConversionTests.cs +++ b/src/System.CommandLine.Tests/Binding/TypeConversionTests.cs @@ -15,18 +15,12 @@ public class TypeConversionTests [Fact] public void Option_argument_with_arity_of_one_can_be_bound_without_custom_conversion_logic_if_the_type_has_a_constructor_that_takes_a_single_string() { - var option = new Option("--file") - { - Argument = new Argument() - }; + var option = new Option("--file"); var file = new FileInfo(Path.Combine(new DirectoryInfo("temp").FullName, "the-file.txt")); var result = option.Parse($"--file {file.FullName}"); - result.ValueForOption("--file") - .Should() - .BeOfType() - .Which + result.ValueForOption(option) .Name .Should() .Be("the-file.txt"); @@ -35,19 +29,17 @@ public void Option_argument_with_arity_of_one_can_be_bound_without_custom_conver [Fact] public void Command_argument_with_arity_of_one_can_be_bound_without_custom_conversion_logic_if_the_type_has_a_constructor_that_takes_a_single_string() { - var option = new Command("the-command") + var argument = new Argument("the-arg"); + + var command = new Command("the-command") { - new Argument("the-arg") + argument }; var file = new FileInfo(Path.Combine(new DirectoryInfo("temp").FullName, "the-file.txt")); - var result = option.Parse($"{file.FullName}"); + var result = command.Parse($"{file.FullName}"); - result.CommandResult - .GetArgumentValueOrDefault("the-arg") - .Should() - .BeOfType() - .Which + result.ValueForArgument(argument) .Name .Should() .Be("the-file.txt"); @@ -56,18 +48,18 @@ public void Command_argument_with_arity_of_one_can_be_bound_without_custom_conve [Fact] public void Command_argument_with_arity_of_zero_or_one_when_type_has_a_constructor_that_takes_a_single_string_returns_null_when_argument_is_not_provided() { + var argument = new Argument("the-arg") + { + Arity = ArgumentArity.ZeroOrOne + }; var command = new Command("the-command") { - new Argument("the-arg") - { - Arity = ArgumentArity.ZeroOrOne - } + argument }; var result = command.Parse(""); - result.CommandResult - .GetArgumentValueOrDefault("the-arg") + result.ValueForArgument(argument) .Should() .BeNull(); } @@ -75,19 +67,13 @@ public void Command_argument_with_arity_of_zero_or_one_when_type_has_a_construct [Fact] public void Argument_with_arity_of_many_can_be_called_without_custom_conversion_logic_if_the_item_type_has_a_constructor_that_takes_a_single_string() { - var option = new Option("--file") - { - Argument = new Argument() - }; + var option = new Option("--file"); var file1 = new FileInfo(Path.Combine(new DirectoryInfo("temp").FullName, "file1.txt")); var file2 = new FileInfo(Path.Combine(new DirectoryInfo("temp").FullName, "file2.txt")); var result = option.Parse($"--file {file1.FullName} --file {file2.FullName}"); - result.ValueForOption("--file") - .Should() - .BeOfType() - .Which + result.ValueForOption(option) .Select(fi => fi.Name) .Should() .BeEquivalentTo("file1.txt", "file2.txt"); @@ -145,15 +131,13 @@ public void Argument_infers_arity_of_IEnumerable_types_as_OneOrMore(Type type) [Fact] public void Argument_bool_will_default_to_true_when_no_argument_is_passed() { - var parser = new Parser(new Option("-x") - { - Argument = new Argument() - }); + var option = new Option("-x"); + var parser = new Parser(option); var result = parser.Parse("-x"); result.Errors.Should().BeEmpty(); - result.ValueForOption("x").Should().Be(true); + result.ValueForOption(option).Should().Be(true); } [Fact] @@ -562,17 +546,19 @@ public void A_default_value_with_a_custom_constructor_can_be_specified_for_a_com [Fact] public void An_option_argument_with_a_default_argument_can_be_converted_to_the_requested_type() { + var option = new Option("-x") + { + Argument = new Argument(() => "123") + }; + var command = new Command("something") { - new Option("-x") - { - Argument = new Argument(() => "123") - } + option }; var result = command.Parse("something"); - var value = result.CommandResult.ValueForOption("x"); + var value = result.ValueForOption(option); value.Should().Be(123); } @@ -606,7 +592,7 @@ public void Values_can_be_correctly_converted_to_int_without_the_parser_specifyi } }; - var value = option.Parse("-x 123").ValueForOption("x"); + var value = option.Parse("-x 123").ValueForOption(option); value.Should().Be(123); } diff --git a/src/System.CommandLine.Tests/CommandTests.cs b/src/System.CommandLine.Tests/CommandTests.cs index 970901fdc9..a93f6736a0 100644 --- a/src/System.CommandLine.Tests/CommandTests.cs +++ b/src/System.CommandLine.Tests/CommandTests.cs @@ -265,20 +265,6 @@ public void Subcommands_can_have_aliases() result.Errors.Should().BeEmpty(); } - [Fact] - public void It_defaults_argument_to_alias_name_when_it_is_not_provided() - { - var command = new Command("-alias") - { - new Argument - { - Arity = ArgumentArity.ZeroOrOne - } - }; - - command.Arguments.Single().Name.Should().Be("alias"); - } - [Fact] public void It_retains_argument_name_when_it_is_provided() { diff --git a/src/System.CommandLine.Tests/Invocation/CommandHandlerTests.cs b/src/System.CommandLine.Tests/Invocation/CommandHandlerTests.cs index 770976f657..be2fda93dd 100644 --- a/src/System.CommandLine.Tests/Invocation/CommandHandlerTests.cs +++ b/src/System.CommandLine.Tests/Invocation/CommandHandlerTests.cs @@ -303,18 +303,16 @@ public async Task Method_parameters_of_type_ParseResult_receive_the_current_Bind { BindingContext boundContext = default; + var option = new Option("-x"); var command = new Command("command") { - new Option("-x") - { - Argument = new Argument() - } + option }; command.Handler = CommandHandler.Create(context => { boundContext = context; }); await command.InvokeAsync("command -x 123", _console); - boundContext.ParseResult.ValueForOption("-x").Should().Be(123); + boundContext.ParseResult.ValueForOption(option).Should().Be(123); } [Fact] diff --git a/src/System.CommandLine.Tests/Invocation/TypoCorrectionTests.cs b/src/System.CommandLine.Tests/Invocation/TypoCorrectionTests.cs index 717e49c642..1c1dd40c41 100644 --- a/src/System.CommandLine.Tests/Invocation/TypoCorrectionTests.cs +++ b/src/System.CommandLine.Tests/Invocation/TypoCorrectionTests.cs @@ -108,7 +108,7 @@ public async Task Arguments_are_not_suggested() { var parser = new CommandLineBuilder() - .AddArgument(new Argument()) + .AddArgument(new Argument("the-argument")) .AddCommand(new Command("been")) .UseTypoCorrections() .Build(); @@ -117,7 +117,7 @@ public async Task Arguments_are_not_suggested() await result.InvokeAsync(_console); - _console.Out.ToString().Should().Contain("'een' was not matched. Did you mean 'been'?"); + _console.Out.ToString().Should().NotContain("the-argument"); } [Fact] diff --git a/src/System.CommandLine.Tests/ParserTests.MultipleArguments.cs b/src/System.CommandLine.Tests/ParserTests.MultipleArguments.cs index ceeaf85dd8..b44c822dad 100644 --- a/src/System.CommandLine.Tests/ParserTests.MultipleArguments.cs +++ b/src/System.CommandLine.Tests/ParserTests.MultipleArguments.cs @@ -5,6 +5,7 @@ using System.CommandLine.Parsing; using System.CommandLine.Tests.Utility; using FluentAssertions; +using FluentAssertions.Execution; using Xunit; namespace System.CommandLine.Tests @@ -148,6 +149,33 @@ public void Multiple_arguments_of_unspecified_type_are_parsed_correctly() .Should() .Be("dest.txt"); } + + [Fact] + public void arity_ambiguities_can_be_differentiated_by_type_convertibility() + { + var ints = new Argument(); + var strings = new Argument(); + + var root = new RootCommand + { + ints, + strings + }; + + var result = root.Parse("1 2 3 one", "two"); + + var _ = new AssertionScope(); + + result.ValueForArgument(ints) + .Should() + .BeEquivalentTo(new[] { 1, 2, 3 }, + options => options.WithStrictOrdering()); + + result.ValueForArgument(strings) + .Should() + .BeEquivalentTo(new[] { "one", "two" }, + options => options.WithStrictOrdering()); + } } } } diff --git a/src/System.CommandLine.Tests/ParserTests.cs b/src/System.CommandLine.Tests/ParserTests.cs index d018022c56..87804bdf70 100644 --- a/src/System.CommandLine.Tests/ParserTests.cs +++ b/src/System.CommandLine.Tests/ParserTests.cs @@ -1948,14 +1948,11 @@ public void When_option_arguments_are_greater_than_maximum_arity_then_an_error_i [Fact] public void Argument_with_custom_type_converter_can_be_bound() { - var option = new Option("--value") - { - Argument = new Argument() - }; + var option = new Option("--value"); var parseResult = option.Parse("--value a;b;c"); - var instance = (ClassWithCustomTypeConverter)parseResult.ValueForOption("--value"); + var instance = parseResult.ValueForOption(option); instance.Values.Should().BeEquivalentTo("a", "b", "c"); } diff --git a/src/System.CommandLine.Tests/ParsingValidationTests.cs b/src/System.CommandLine.Tests/ParsingValidationTests.cs index 7592b34434..7307212a74 100644 --- a/src/System.CommandLine.Tests/ParsingValidationTests.cs +++ b/src/System.CommandLine.Tests/ParsingValidationTests.cs @@ -7,7 +7,6 @@ using System.IO; using FluentAssertions; using System.Linq; -using System.Reflection; using Xunit; using Xunit.Abstractions; @@ -281,7 +280,7 @@ public void LegalFilePathsOnly_rejects_command_arguments_containing_invalid_path result.Errors .Should() - .Contain(e => e.SymbolResult.Symbol.Name == "the-command" && + .Contain(e => e.SymbolResult.Symbol == command.Arguments.First() && e.Message == $"Character not allowed in a path: {invalidCharacter}"); } @@ -429,7 +428,7 @@ public void A_command_argument_can_be_invalid_based_on_file_or_directory_existen .Should() .HaveCount(1) .And - .Contain(e => e.SymbolResult.Symbol.Name == "move" && + .Contain(e => e.SymbolResult.Symbol == command.Arguments.First() && e.Message == $"File or directory does not exist: {path}"); } diff --git a/src/System.CommandLine/Argument.cs b/src/System.CommandLine/Argument.cs index 65d2042a3e..54ac9b75d5 100644 --- a/src/System.CommandLine/Argument.cs +++ b/src/System.CommandLine/Argument.cs @@ -14,13 +14,13 @@ public class Argument : Symbol, IArgument private Func? _defaultValueFactory; private IArgumentArity? _arity; private TryConvertArgument? _convertArguments; - private Type _argumentType = typeof(void); + private Type _argumentType = typeof(string); public Argument() { } - public Argument(string? name) + public Argument(string name) { if (!string.IsNullOrWhiteSpace(name)) { @@ -36,14 +36,10 @@ public IArgumentArity Arity { if (_arity is null) { - if (ArgumentType != typeof(void)) - { - return ArgumentArity.Default(ArgumentType, this, Parents.FirstOrDefault()); - } - else - { - return ArgumentArity.Zero; - } + return ArgumentArity.Default( + ArgumentType, + this, + Parents.FirstOrDefault()); } return _arity; @@ -55,8 +51,7 @@ internal TryConvertArgument? ConvertArguments { get { - if (_convertArguments == null && - ArgumentType != typeof(void)) + if (_convertArguments == null) { if (ArgumentType.CanBeBoundFromScalarValue()) { @@ -202,5 +197,9 @@ internal void AddAllowedValues(IEnumerable values) string IValueDescriptor.ValueName => Name; Type IValueDescriptor.ValueType => ArgumentType; + + private protected override void ChooseNameForUnnamedArgument(Argument argument) + { + } } } diff --git a/src/System.CommandLine/Argument{T}.cs b/src/System.CommandLine/Argument{T}.cs index 8895886961..20cb210638 100644 --- a/src/System.CommandLine/Argument{T}.cs +++ b/src/System.CommandLine/Argument{T}.cs @@ -7,7 +7,7 @@ namespace System.CommandLine { public class Argument : Argument { - public Argument() : base(null) + public Argument() { ArgumentType = typeof(T); } diff --git a/src/System.CommandLine/Binding/Binder.cs b/src/System.CommandLine/Binding/Binder.cs index ea7b90de6d..08f7bcb5f4 100644 --- a/src/System.CommandLine/Binding/Binder.cs +++ b/src/System.CommandLine/Binding/Binder.cs @@ -15,7 +15,8 @@ internal static bool IsMatch(this string parameterName, string alias) => StringComparison.OrdinalIgnoreCase); internal static bool IsMatch(this string parameterName, ISymbol symbol) => - parameterName.IsMatch(symbol.Name) || symbol.Aliases.Any(parameterName.IsMatch); + parameterName.IsMatch(symbol.Name) || + symbol.HasAlias(parameterName); internal static bool IsNullable(this Type t) { diff --git a/src/System.CommandLine/Binding/IValueDescriptor.cs b/src/System.CommandLine/Binding/IValueDescriptor.cs index 786c7e2337..4468f42063 100644 --- a/src/System.CommandLine/Binding/IValueDescriptor.cs +++ b/src/System.CommandLine/Binding/IValueDescriptor.cs @@ -5,7 +5,7 @@ namespace System.CommandLine.Binding { public interface IValueDescriptor { - string? ValueName { get; } + string ValueName { get; } Type ValueType { get; } diff --git a/src/System.CommandLine/Binding/ModelBinder.cs b/src/System.CommandLine/Binding/ModelBinder.cs index 0979e8c574..0f7a80f325 100644 --- a/src/System.CommandLine/Binding/ModelBinder.cs +++ b/src/System.CommandLine/Binding/ModelBinder.cs @@ -289,7 +289,7 @@ public AnonymousValueDescriptor(Type modelType) ValueType = modelType; } - public string? ValueName => null; + public string ValueName => ""; public bool HasDefaultValue => false; diff --git a/src/System.CommandLine/Binding/SpecificSymbolValueSource.cs b/src/System.CommandLine/Binding/SpecificSymbolValueSource.cs index abaaa10115..746aae3d2b 100644 --- a/src/System.CommandLine/Binding/SpecificSymbolValueSource.cs +++ b/src/System.CommandLine/Binding/SpecificSymbolValueSource.cs @@ -2,7 +2,6 @@ // Licensed under the MIT license. See LICENSE file in the project root for full license information. using System.CommandLine.Parsing; -using System.Linq; namespace System.CommandLine.Binding { diff --git a/src/System.CommandLine/Command.cs b/src/System.CommandLine/Command.cs index c381fbe148..d15c3f712d 100644 --- a/src/System.CommandLine/Command.cs +++ b/src/System.CommandLine/Command.cs @@ -83,5 +83,10 @@ private protected override void AddSymbol(Symbol symbol) IEnumerable ICommand.Options => Options; internal Parser? ImplicitParser { get; set; } + + private protected override void ChooseNameForUnnamedArgument(Argument argument) + { + argument.Name = argument.ArgumentType.Name.ToLower(); + } } } diff --git a/src/System.CommandLine/Option.cs b/src/System.CommandLine/Option.cs index 74efa6be3c..21bac3c7d1 100644 --- a/src/System.CommandLine/Option.cs +++ b/src/System.CommandLine/Option.cs @@ -53,5 +53,10 @@ public virtual Argument Argument bool IValueDescriptor.HasDefaultValue => Argument.HasDefaultValue; object? IValueDescriptor.GetDefaultValue() => Argument.GetDefaultValue(); + + private protected override void ChooseNameForUnnamedArgument(Argument argument) + { + argument.Name = Aliases[0].ToLower(); + } } } diff --git a/src/System.CommandLine/Parsing/ArgumentResultExtensions.cs b/src/System.CommandLine/Parsing/ArgumentResultExtensions.cs index 9a35cff0f8..005f562148 100644 --- a/src/System.CommandLine/Parsing/ArgumentResultExtensions.cs +++ b/src/System.CommandLine/Parsing/ArgumentResultExtensions.cs @@ -8,6 +8,7 @@ namespace System.CommandLine.Parsing { public static class ArgumentResultExtensions { + [return: MaybeNull] public static object? GetValueOrDefault(this ArgumentResult argumentResult) => argumentResult.GetValueOrDefault(); diff --git a/src/System.CommandLine/Parsing/CommandResult.cs b/src/System.CommandLine/Parsing/CommandResult.cs index f62791012f..7daeaf3b64 100644 --- a/src/System.CommandLine/Parsing/CommandResult.cs +++ b/src/System.CommandLine/Parsing/CommandResult.cs @@ -35,14 +35,13 @@ internal CommandResult( public Token Token { get; } - internal virtual RootCommandResult? Root => (Parent as CommandResult)?.Root; internal bool TryGetValueForArgument( IValueDescriptor valueDescriptor, out object? value) { - if (valueDescriptor.ValueName is string valueName) + if (valueDescriptor.ValueName is { } valueName) { foreach (var argument in Command.Arguments) { diff --git a/src/System.CommandLine/Parsing/ParseResult.cs b/src/System.CommandLine/Parsing/ParseResult.cs index e476af5c2f..04b302f085 100644 --- a/src/System.CommandLine/Parsing/ParseResult.cs +++ b/src/System.CommandLine/Parsing/ParseResult.cs @@ -85,6 +85,54 @@ public T ValueForOption(string alias) } } + [return: MaybeNull] + public T ValueForArgument(Argument argument) + { + if (FindResultFor(argument) is {} result && + result.GetValueOrDefault() is {} t) + { + return t; + } + + return default!; + } + + [return: MaybeNull] + public T ValueForArgument(Argument argument) + { + if (FindResultFor(argument) is {} result && + result.GetValueOrDefault() is {} t) + { + return t; + } + + return default!; + } + + [return: MaybeNull] + public T ValueForOption(Option option) + { + if (FindResultFor(option) is {} result && + result.GetValueOrDefault() is {} t) + { + return t; + } + + return default!; + } + + [return: MaybeNull] + public T ValueForOption(Option option) + { + if (FindResultFor(option) is {} result && + result.GetValueOrDefault() is {} t) + { + return t; + } + + return default!; + } + public SymbolResult? this[string alias] => CommandResult.Children[alias]; public override string ToString() => $"{nameof(ParseResult)}: {this.Diagram()}"; diff --git a/src/System.CommandLine/Parsing/ParseResultExtensions.cs b/src/System.CommandLine/Parsing/ParseResultExtensions.cs index b7f5301459..4a34927537 100644 --- a/src/System.CommandLine/Parsing/ParseResultExtensions.cs +++ b/src/System.CommandLine/Parsing/ParseResultExtensions.cs @@ -115,8 +115,8 @@ private static void Diagram( { var includeArgumentName = argumentResult.Argument is Argument argument && - argument.Parents.First() is ICommand command && - command.Name != argument.Name; + argument.Parents[0] is ICommand command && + command.Arguments.Count() > 1; if (includeArgumentName) { diff --git a/src/System.CommandLine/Parsing/ParseResultVisitor.cs b/src/System.CommandLine/Parsing/ParseResultVisitor.cs index 529e2fd81a..e606ddb1c6 100644 --- a/src/System.CommandLine/Parsing/ParseResultVisitor.cs +++ b/src/System.CommandLine/Parsing/ParseResultVisitor.cs @@ -69,8 +69,6 @@ protected override void VisitCommandNode(CommandNode commandNode) commandNode.Token, _innermostCommandResult); - Debug.Assert(_innermostCommandResult != null); - _innermostCommandResult! .Children .Add(commandResult); diff --git a/src/System.CommandLine/Symbol.cs b/src/System.CommandLine/Symbol.cs index 8010190a52..478918b135 100644 --- a/src/System.CommandLine/Symbol.cs +++ b/src/System.CommandLine/Symbol.cs @@ -87,12 +87,14 @@ private protected void AddArgumentInner(Argument argument) if (string.IsNullOrEmpty(argument.Name)) { - argument.Name = _aliases.First().ToLower(); + ChooseNameForUnnamedArgument(argument); } Children.Add(argument); } + private protected abstract void ChooseNameForUnnamedArgument(Argument argument); + public SymbolSet Children { get; } = new SymbolSet(); public void AddAlias(string alias)