From 8df1d607c17012f0f8adfc63fc2f48af994400f8 Mon Sep 17 00:00:00 2001 From: Tyrrrz Date: Fri, 23 Oct 2020 20:52:26 +0300 Subject: [PATCH] Refactor & improve argument conversion feature --- .../CommandSchemaAnalyzerTests.cs | 128 +++++++++-- CliFx.Analyzers/CommandSchemaAnalyzer.cs | 104 +++++++-- CliFx.Analyzers/DiagnosticDescriptors.cs | 50 ++++- CliFx.Analyzers/KnownSymbols.cs | 5 +- CliFx.Tests/ApplicationSpecs.cs | 40 ++++ CliFx.Tests/ArgumentConversionSpecs.cs | 207 +++++++----------- .../CommandWithParameterOfCustomType.cs | 27 --- .../Converters/CustomTypeConverter.cs | 8 - .../InvalidCustomConverterOptionCommand.cs | 16 ++ .../InvalidCustomConverterParameterCommand.cs | 16 ++ .../Commands/SupportedArgumentTypesCommand.cs | 21 +- CliFx.Tests/DirectivesSpecs.cs | 3 +- CliFx/Attributes/CommandOptionAttribute.cs | 3 +- CliFx/Attributes/CommandParameterAttribute.cs | 3 +- CliFx/Domain/CommandArgumentSchema.cs | 17 +- CliFx/Domain/CommandOptionSchema.cs | 8 +- CliFx/Domain/CommandParameterSchema.cs | 9 +- CliFx/Domain/RootSchema.cs | 24 ++ CliFx/Exceptions/CliFxException.cs | 26 +++ CliFx/IArgumentValueConverter.cs | 2 +- CliFx/Internal/Extensions/TypeExtensions.cs | 16 +- 21 files changed, 490 insertions(+), 243 deletions(-) delete mode 100644 CliFx.Tests/Commands/CommandWithParameterOfCustomType.cs delete mode 100644 CliFx.Tests/Commands/Converters/CustomTypeConverter.cs create mode 100644 CliFx.Tests/Commands/Invalid/InvalidCustomConverterOptionCommand.cs create mode 100644 CliFx.Tests/Commands/Invalid/InvalidCustomConverterParameterCommand.cs diff --git a/CliFx.Analyzers.Tests/CommandSchemaAnalyzerTests.cs b/CliFx.Analyzers.Tests/CommandSchemaAnalyzerTests.cs index d0ce6dc..2d2b0df 100644 --- a/CliFx.Analyzers.Tests/CommandSchemaAnalyzerTests.cs +++ b/CliFx.Analyzers.Tests/CommandSchemaAnalyzerTests.cs @@ -140,6 +140,30 @@ public class MyCommand : ICommand [CommandParameter(2)] public IReadOnlyList ParamB { get; set; } + public ValueTask ExecuteAsync(IConsole console) => default; +}" + ) + }; + + yield return new object[] + { + new AnalyzerTestCase( + "Parameter with valid converter", + Analyzer.SupportedDiagnostics, + + // language=cs + @" +public class MyConverter : IArgumentValueConverter +{ + public object ConvertFrom(string value) => value; +} + +[Command] +public class MyCommand : ICommand +{ + [CommandParameter(0, Converter = typeof(MyConverter))] + public string Param { get; set; } + public ValueTask ExecuteAsync(IConsole console) => default; }" ) @@ -157,7 +181,7 @@ public class MyCommand : ICommand public class MyCommand : ICommand { [CommandOption(""foo"")] - public string Param { get; set; } + public string Option { get; set; } public ValueTask ExecuteAsync(IConsole console) => default; }" @@ -176,7 +200,7 @@ public class MyCommand : ICommand public class MyCommand : ICommand { [CommandOption(""foo"", 'f')] - public string Param { get; set; } + public string Option { get; set; } public ValueTask ExecuteAsync(IConsole console) => default; }" @@ -195,10 +219,10 @@ public class MyCommand : ICommand public class MyCommand : ICommand { [CommandOption(""foo"")] - public string ParamA { get; set; } + public string OptionA { get; set; } [CommandOption(""bar"")] - public string ParamB { get; set; } + public string OptionB { get; set; } public ValueTask ExecuteAsync(IConsole console) => default; }" @@ -217,10 +241,10 @@ public class MyCommand : ICommand public class MyCommand : ICommand { [CommandOption('f')] - public string ParamA { get; set; } + public string OptionA { get; set; } [CommandOption('x')] - public string ParamB { get; set; } + public string OptionB { get; set; } public ValueTask ExecuteAsync(IConsole console) => default; }" @@ -239,11 +263,35 @@ public class MyCommand : ICommand public class MyCommand : ICommand { [CommandOption('a', EnvironmentVariableName = ""env_var_a"")] - public string ParamA { get; set; } + public string OptionA { get; set; } [CommandOption('b', EnvironmentVariableName = ""env_var_b"")] - public string ParamB { get; set; } + public string OptionB { get; set; } + public ValueTask ExecuteAsync(IConsole console) => default; +}" + ) + }; + + yield return new object[] + { + new AnalyzerTestCase( + "Option with valid converter", + Analyzer.SupportedDiagnostics, + + // language=cs + @" +public class MyConverter : IArgumentValueConverter +{ + public object ConvertFrom(string value) => value; +} + +[Command] +public class MyCommand : ICommand +{ + [CommandOption('o', Converter = typeof(MyConverter))] + public string Option { get; set; } + public ValueTask ExecuteAsync(IConsole console) => default; }" ) @@ -366,6 +414,30 @@ public class MyCommand : ICommand [CommandParameter(2)] public string ParamB { get; set; } + public ValueTask ExecuteAsync(IConsole console) => default; +}" + ) + }; + + yield return new object[] + { + new AnalyzerTestCase( + "Parameter with invalid converter", + DiagnosticDescriptors.CliFx0025, + + // language=cs + @" +public class MyConverter +{ + public object ConvertFrom(string value) => value; +} + +[Command] +public class MyCommand : ICommand +{ + [CommandParameter(0, Converter = typeof(MyConverter))] + public string Param { get; set; } + public ValueTask ExecuteAsync(IConsole console) => default; }" ) @@ -383,7 +455,7 @@ public class MyCommand : ICommand public class MyCommand : ICommand { [CommandOption("""")] - public string Param { get; set; } + public string Option { get; set; } public ValueTask ExecuteAsync(IConsole console) => default; }" @@ -402,7 +474,7 @@ public class MyCommand : ICommand public class MyCommand : ICommand { [CommandOption(""a"")] - public string Param { get; set; } + public string Option { get; set; } public ValueTask ExecuteAsync(IConsole console) => default; }" @@ -421,10 +493,10 @@ public class MyCommand : ICommand public class MyCommand : ICommand { [CommandOption(""foo"")] - public string ParamA { get; set; } + public string OptionA { get; set; } [CommandOption(""foo"")] - public string ParamB { get; set; } + public string OptionB { get; set; } public ValueTask ExecuteAsync(IConsole console) => default; }" @@ -443,10 +515,10 @@ public class MyCommand : ICommand public class MyCommand : ICommand { [CommandOption('f')] - public string ParamA { get; set; } + public string OptionA { get; set; } [CommandOption('f')] - public string ParamB { get; set; } + public string OptionB { get; set; } public ValueTask ExecuteAsync(IConsole console) => default; }" @@ -465,11 +537,35 @@ public class MyCommand : ICommand public class MyCommand : ICommand { [CommandOption('a', EnvironmentVariableName = ""env_var"")] - public string ParamA { get; set; } + public string OptionA { get; set; } [CommandOption('b', EnvironmentVariableName = ""env_var"")] - public string ParamB { get; set; } + public string OptionB { get; set; } + public ValueTask ExecuteAsync(IConsole console) => default; +}" + ) + }; + + yield return new object[] + { + new AnalyzerTestCase( + "Option with invalid converter", + DiagnosticDescriptors.CliFx0046, + + // language=cs + @" +public class MyConverter +{ + public object ConvertFrom(string value) => value; +} + +[Command] +public class MyCommand : ICommand +{ + [CommandOption('o', Converter = typeof(MyConverter))] + public string Option { get; set; } + public ValueTask ExecuteAsync(IConsole console) => default; }" ) diff --git a/CliFx.Analyzers/CommandSchemaAnalyzer.cs b/CliFx.Analyzers/CommandSchemaAnalyzer.cs index f299279..a688d34 100644 --- a/CliFx.Analyzers/CommandSchemaAnalyzer.cs +++ b/CliFx.Analyzers/CommandSchemaAnalyzer.cs @@ -17,16 +17,19 @@ namespace CliFx.Analyzers DiagnosticDescriptors.CliFx0022, DiagnosticDescriptors.CliFx0023, DiagnosticDescriptors.CliFx0024, + DiagnosticDescriptors.CliFx0025, DiagnosticDescriptors.CliFx0041, DiagnosticDescriptors.CliFx0042, DiagnosticDescriptors.CliFx0043, DiagnosticDescriptors.CliFx0044, - DiagnosticDescriptors.CliFx0045 + DiagnosticDescriptors.CliFx0045, + DiagnosticDescriptors.CliFx0046 ); private static bool IsScalarType(ITypeSymbol typeSymbol) => KnownSymbols.IsSystemString(typeSymbol) || - !typeSymbol.AllInterfaces.Select(i => i.ConstructedFrom).Any(KnownSymbols.IsSystemCollectionsGenericIEnumerable); + !typeSymbol.AllInterfaces.Select(i => i.ConstructedFrom) + .Any(KnownSymbols.IsSystemCollectionsGenericIEnumerable); private static void CheckCommandParameterProperties( SymbolAnalysisContext context, @@ -50,11 +53,18 @@ namespace CliFx.Analyzers .Select(a => a.Value.Value) .FirstOrDefault() as string; + var converter = attribute + .NamedArguments + .Where(a => a.Key == "Converter") + .Select(a => a.Value.Value) + .FirstOrDefault() as ITypeSymbol; + return new { Property = p, Order = order, - Name = name + Name = name, + Converter = converter }; }) .ToArray(); @@ -69,8 +79,9 @@ namespace CliFx.Analyzers foreach (var parameter in duplicateOrderParameters) { - context.ReportDiagnostic( - Diagnostic.Create(DiagnosticDescriptors.CliFx0021, parameter.Property.Locations.First())); + context.ReportDiagnostic(Diagnostic.Create( + DiagnosticDescriptors.CliFx0021, parameter.Property.Locations.First() + )); } // Duplicate name @@ -83,8 +94,9 @@ namespace CliFx.Analyzers foreach (var parameter in duplicateNameParameters) { - context.ReportDiagnostic( - Diagnostic.Create(DiagnosticDescriptors.CliFx0022, parameter.Property.Locations.First())); + context.ReportDiagnostic(Diagnostic.Create( + DiagnosticDescriptors.CliFx0022, parameter.Property.Locations.First() + )); } // Multiple non-scalar @@ -96,8 +108,9 @@ namespace CliFx.Analyzers { foreach (var parameter in nonScalarParameters) { - context.ReportDiagnostic( - Diagnostic.Create(DiagnosticDescriptors.CliFx0023, parameter.Property.Locations.First())); + context.ReportDiagnostic(Diagnostic.Create( + DiagnosticDescriptors.CliFx0023, parameter.Property.Locations.First() + )); } } @@ -109,8 +122,23 @@ namespace CliFx.Analyzers if (nonLastNonScalarParameter != null) { - context.ReportDiagnostic( - Diagnostic.Create(DiagnosticDescriptors.CliFx0024, nonLastNonScalarParameter.Property.Locations.First())); + context.ReportDiagnostic(Diagnostic.Create( + DiagnosticDescriptors.CliFx0024, nonLastNonScalarParameter.Property.Locations.First() + )); + } + + // Invalid converter + var invalidConverterParameters = parameters + .Where(p => + p.Converter != null && + !p.Converter.AllInterfaces.Any(KnownSymbols.IsArgumentValueConverterInterface)) + .ToArray(); + + foreach (var parameter in invalidConverterParameters) + { + context.ReportDiagnostic(Diagnostic.Create( + DiagnosticDescriptors.CliFx0025, parameter.Property.Locations.First() + )); } } @@ -143,12 +171,19 @@ namespace CliFx.Analyzers .Select(a => a.Value.Value) .FirstOrDefault() as string; + var converter = attribute + .NamedArguments + .Where(a => a.Key == "Converter") + .Select(a => a.Value.Value) + .FirstOrDefault() as ITypeSymbol; + return new { Property = p, Name = name, ShortName = shortName, - EnvironmentVariableName = envVarName + EnvironmentVariableName = envVarName, + Converter = converter }; }) .ToArray(); @@ -160,8 +195,9 @@ namespace CliFx.Analyzers foreach (var option in noNameOptions) { - context.ReportDiagnostic( - Diagnostic.Create(DiagnosticDescriptors.CliFx0041, option.Property.Locations.First())); + context.ReportDiagnostic(Diagnostic.Create( + DiagnosticDescriptors.CliFx0041, option.Property.Locations.First() + )); } // Too short name @@ -171,8 +207,9 @@ namespace CliFx.Analyzers foreach (var option in invalidNameLengthOptions) { - context.ReportDiagnostic( - Diagnostic.Create(DiagnosticDescriptors.CliFx0042, option.Property.Locations.First())); + context.ReportDiagnostic(Diagnostic.Create( + DiagnosticDescriptors.CliFx0042, option.Property.Locations.First() + )); } // Duplicate name @@ -185,8 +222,9 @@ namespace CliFx.Analyzers foreach (var option in duplicateNameOptions) { - context.ReportDiagnostic( - Diagnostic.Create(DiagnosticDescriptors.CliFx0043, option.Property.Locations.First())); + context.ReportDiagnostic(Diagnostic.Create( + DiagnosticDescriptors.CliFx0043, option.Property.Locations.First() + )); } // Duplicate name @@ -199,8 +237,9 @@ namespace CliFx.Analyzers foreach (var option in duplicateShortNameOptions) { - context.ReportDiagnostic( - Diagnostic.Create(DiagnosticDescriptors.CliFx0044, option.Property.Locations.First())); + context.ReportDiagnostic(Diagnostic.Create( + DiagnosticDescriptors.CliFx0044, option.Property.Locations.First() + )); } // Duplicate environment variable name @@ -213,8 +252,23 @@ namespace CliFx.Analyzers foreach (var option in duplicateEnvironmentVariableNameOptions) { - context.ReportDiagnostic( - Diagnostic.Create(DiagnosticDescriptors.CliFx0045, option.Property.Locations.First())); + context.ReportDiagnostic(Diagnostic.Create( + DiagnosticDescriptors.CliFx0045, option.Property.Locations.First() + )); + } + + // Invalid converter + var invalidConverterOptions = options + .Where(o => + o.Converter != null && + !o.Converter.AllInterfaces.Any(KnownSymbols.IsArgumentValueConverterInterface)) + .ToArray(); + + foreach (var option in invalidConverterOptions) + { + context.ReportDiagnostic(Diagnostic.Create( + DiagnosticDescriptors.CliFx0046, option.Property.Locations.First() + )); } } @@ -252,10 +306,12 @@ namespace CliFx.Analyzers var isAlmostValidCommandType = implementsCommandInterface ^ hasCommandAttribute; if (isAlmostValidCommandType && !implementsCommandInterface) - context.ReportDiagnostic(Diagnostic.Create(DiagnosticDescriptors.CliFx0001, namedTypeSymbol.Locations.First())); + context.ReportDiagnostic(Diagnostic.Create(DiagnosticDescriptors.CliFx0001, + namedTypeSymbol.Locations.First())); if (isAlmostValidCommandType && !hasCommandAttribute) - context.ReportDiagnostic(Diagnostic.Create(DiagnosticDescriptors.CliFx0002, namedTypeSymbol.Locations.First())); + context.ReportDiagnostic(Diagnostic.Create(DiagnosticDescriptors.CliFx0002, + namedTypeSymbol.Locations.First())); return; } diff --git a/CliFx.Analyzers/DiagnosticDescriptors.cs b/CliFx.Analyzers/DiagnosticDescriptors.cs index 19f7a68..d6a20d4 100644 --- a/CliFx.Analyzers/DiagnosticDescriptors.cs +++ b/CliFx.Analyzers/DiagnosticDescriptors.cs @@ -8,72 +8,98 @@ namespace CliFx.Analyzers new DiagnosticDescriptor(nameof(CliFx0001), "Type must implement the 'CliFx.ICommand' interface in order to be a valid command", "Type must implement the 'CliFx.ICommand' interface in order to be a valid command", - "Usage", DiagnosticSeverity.Error, true); + "Usage", DiagnosticSeverity.Error, true + ); public static readonly DiagnosticDescriptor CliFx0002 = new DiagnosticDescriptor(nameof(CliFx0002), "Type must be annotated with the 'CliFx.Attributes.CommandAttribute' in order to be a valid command", "Type must be annotated with the 'CliFx.Attributes.CommandAttribute' in order to be a valid command", - "Usage", DiagnosticSeverity.Error, true); + "Usage", DiagnosticSeverity.Error, true + ); public static readonly DiagnosticDescriptor CliFx0021 = new DiagnosticDescriptor(nameof(CliFx0021), "Parameter order must be unique within its command", "Parameter order must be unique within its command", - "Usage", DiagnosticSeverity.Error, true); + "Usage", DiagnosticSeverity.Error, true + ); public static readonly DiagnosticDescriptor CliFx0022 = new DiagnosticDescriptor(nameof(CliFx0022), "Parameter order must have unique name within its command", "Parameter order must have unique name within its command", - "Usage", DiagnosticSeverity.Error, true); + "Usage", DiagnosticSeverity.Error, true + ); public static readonly DiagnosticDescriptor CliFx0023 = new DiagnosticDescriptor(nameof(CliFx0023), "Only one non-scalar parameter per command is allowed", "Only one non-scalar parameter per command is allowed", - "Usage", DiagnosticSeverity.Error, true); + "Usage", DiagnosticSeverity.Error, true + ); public static readonly DiagnosticDescriptor CliFx0024 = new DiagnosticDescriptor(nameof(CliFx0024), "Non-scalar parameter must be last in order", "Non-scalar parameter must be last in order", - "Usage", DiagnosticSeverity.Error, true); + "Usage", DiagnosticSeverity.Error, true + ); + + public static readonly DiagnosticDescriptor CliFx0025 = + new DiagnosticDescriptor(nameof(CliFx0025), + "Parameter converter must implement 'CliFx.IArgumentValueConverter'", + "Parameter converter must implement 'CliFx.IArgumentValueConverter'", + "Usage", DiagnosticSeverity.Error, true + ); public static readonly DiagnosticDescriptor CliFx0041 = new DiagnosticDescriptor(nameof(CliFx0041), "Option must have a name or short name specified", "Option must have a name or short name specified", - "Usage", DiagnosticSeverity.Error, true); + "Usage", DiagnosticSeverity.Error, true + ); public static readonly DiagnosticDescriptor CliFx0042 = new DiagnosticDescriptor(nameof(CliFx0042), "Option name must be at least 2 characters long", "Option name must be at least 2 characters long", - "Usage", DiagnosticSeverity.Error, true); + "Usage", DiagnosticSeverity.Error, true + ); public static readonly DiagnosticDescriptor CliFx0043 = new DiagnosticDescriptor(nameof(CliFx0043), "Option name must be unique within its command", "Option name must be unique within its command", - "Usage", DiagnosticSeverity.Error, true); + "Usage", DiagnosticSeverity.Error, true + ); public static readonly DiagnosticDescriptor CliFx0044 = new DiagnosticDescriptor(nameof(CliFx0044), "Option short name must be unique within its command", "Option short name must be unique within its command", - "Usage", DiagnosticSeverity.Error, true); + "Usage", DiagnosticSeverity.Error, true + ); public static readonly DiagnosticDescriptor CliFx0045 = new DiagnosticDescriptor(nameof(CliFx0045), "Option environment variable name must be unique within its command", "Option environment variable name must be unique within its command", - "Usage", DiagnosticSeverity.Error, true); + "Usage", DiagnosticSeverity.Error, true + ); + + public static readonly DiagnosticDescriptor CliFx0046 = + new DiagnosticDescriptor(nameof(CliFx0046), + "Option converter must implement 'CliFx.IArgumentValueConverter'", + "Option converter must implement 'CliFx.IArgumentValueConverter'", + "Usage", DiagnosticSeverity.Error, true + ); public static readonly DiagnosticDescriptor CliFx0100 = new DiagnosticDescriptor(nameof(CliFx0100), "Use the provided IConsole abstraction instead of System.Console to ensure that the command can be tested in isolation", "Use the provided IConsole abstraction instead of System.Console to ensure that the command can be tested in isolation", - "Usage", DiagnosticSeverity.Warning, true); + "Usage", DiagnosticSeverity.Warning, true + ); } } \ No newline at end of file diff --git a/CliFx.Analyzers/KnownSymbols.cs b/CliFx.Analyzers/KnownSymbols.cs index 88e1aa6..2510d47 100644 --- a/CliFx.Analyzers/KnownSymbols.cs +++ b/CliFx.Analyzers/KnownSymbols.cs @@ -3,7 +3,7 @@ using Microsoft.CodeAnalysis; namespace CliFx.Analyzers { - public static class KnownSymbols + internal static class KnownSymbols { public static bool IsSystemString(ISymbol symbol) => symbol.DisplayNameMatches("string") || @@ -25,6 +25,9 @@ namespace CliFx.Analyzers public static bool IsCommandInterface(ISymbol symbol) => symbol.DisplayNameMatches("CliFx.ICommand"); + public static bool IsArgumentValueConverterInterface(ISymbol symbol) => + symbol.DisplayNameMatches("CliFx.IArgumentValueConverter"); + public static bool IsCommandAttribute(ISymbol symbol) => symbol.DisplayNameMatches("CliFx.Attributes.CommandAttribute"); diff --git a/CliFx.Tests/ApplicationSpecs.cs b/CliFx.Tests/ApplicationSpecs.cs index eec5c96..8a2a7c7 100644 --- a/CliFx.Tests/ApplicationSpecs.cs +++ b/CliFx.Tests/ApplicationSpecs.cs @@ -232,6 +232,26 @@ namespace CliFx.Tests _output.WriteLine(stdErr.GetString()); } + [Fact] + public async Task Command_parameter_custom_converter_must_implement_the_corresponding_interface() + { + var (console, _, stdErr) = VirtualConsole.CreateBuffered(); + + var application = new CliApplicationBuilder() + .AddCommand() + .UseConsole(console) + .Build(); + + // Act + var exitCode = await application.RunAsync(Array.Empty()); + + // Assert + exitCode.Should().NotBe(0); + stdErr.GetString().Should().NotBeNullOrWhiteSpace(); + + _output.WriteLine(stdErr.GetString()); + } + [Fact] public async Task Command_options_must_have_names_that_are_not_empty() { @@ -371,5 +391,25 @@ namespace CliFx.Tests _output.WriteLine(stdErr.GetString()); } + + [Fact] + public async Task Command_option_custom_converter_must_implement_the_corresponding_interface() + { + var (console, _, stdErr) = VirtualConsole.CreateBuffered(); + + var application = new CliApplicationBuilder() + .AddCommand() + .UseConsole(console) + .Build(); + + // Act + var exitCode = await application.RunAsync(Array.Empty()); + + // Assert + exitCode.Should().NotBe(0); + stdErr.GetString().Should().NotBeNullOrWhiteSpace(); + + _output.WriteLine(stdErr.GetString()); + } } } \ No newline at end of file diff --git a/CliFx.Tests/ArgumentConversionSpecs.cs b/CliFx.Tests/ArgumentConversionSpecs.cs index 9be90e4..7be6731 100644 --- a/CliFx.Tests/ArgumentConversionSpecs.cs +++ b/CliFx.Tests/ArgumentConversionSpecs.cs @@ -3,7 +3,6 @@ using System.Collections.Generic; using System.Globalization; using System.Threading.Tasks; using CliFx.Tests.Commands; -using CliFx.Tests.Commands.Converters; using CliFx.Tests.Internal; using FluentAssertions; using Xunit; @@ -18,7 +17,7 @@ namespace CliFx.Tests public ArgumentConversionSpecs(ITestOutputHelper output) => _output = output; [Fact] - public async Task Property_of_type_object_is_bound_directly_from_the_argument_value() + public async Task Argument_value_can_be_bound_to_object() { // Arrange var (console, stdOut, _) = VirtualConsole.CreateBuffered(); @@ -46,7 +45,7 @@ namespace CliFx.Tests } [Fact] - public async Task Property_of_type_object_array_is_bound_directly_from_the_argument_values() + public async Task Argument_values_can_be_bound_to_array_of_object() { // Arrange var (console, stdOut, _) = VirtualConsole.CreateBuffered(); @@ -74,7 +73,7 @@ namespace CliFx.Tests } [Fact] - public async Task Property_of_type_string_is_bound_directly_from_the_argument_value() + public async Task Argument_value_can_be_bound_to_string() { // Arrange var (console, stdOut, _) = VirtualConsole.CreateBuffered(); @@ -102,7 +101,7 @@ namespace CliFx.Tests } [Fact] - public async Task Property_of_type_string_array_is_bound_directly_from_the_argument_values() + public async Task Argument_values_can_be_bound_to_array_of_string() { // Arrange var (console, stdOut, _) = VirtualConsole.CreateBuffered(); @@ -130,7 +129,7 @@ namespace CliFx.Tests } [Fact] - public async Task Property_of_type_string_IEnumerable_is_bound_directly_from_the_argument_values() + public async Task Argument_values_can_be_bound_to_IEnumerable_of_string() { // Arrange var (console, stdOut, _) = VirtualConsole.CreateBuffered(); @@ -158,7 +157,7 @@ namespace CliFx.Tests } [Fact] - public async Task Property_of_type_string_IReadOnlyList_is_bound_directly_from_the_argument_values() + public async Task Argument_values_can_be_bound_to_IReadOnlyList_of_string() { // Arrange var (console, stdOut, _) = VirtualConsole.CreateBuffered(); @@ -186,7 +185,7 @@ namespace CliFx.Tests } [Fact] - public async Task Property_of_type_string_List_is_bound_directly_from_the_argument_values() + public async Task Argument_values_can_be_bound_to_List_of_string() { // Arrange var (console, stdOut, _) = VirtualConsole.CreateBuffered(); @@ -214,7 +213,7 @@ namespace CliFx.Tests } [Fact] - public async Task Property_of_type_string_HashSet_is_bound_directly_from_the_argument_values() + public async Task Argument_values_can_be_bound_to_HashSet_of_string() { // Arrange var (console, stdOut, _) = VirtualConsole.CreateBuffered(); @@ -242,7 +241,7 @@ namespace CliFx.Tests } [Fact] - public async Task Property_of_type_bool_is_bound_as_true_if_the_argument_value_is_true() + public async Task Argument_value_can_be_bound_to_boolean_as_true_if_the_value_is_true() { // Arrange var (console, stdOut, _) = VirtualConsole.CreateBuffered(); @@ -270,7 +269,7 @@ namespace CliFx.Tests } [Fact] - public async Task Property_of_type_bool_is_bound_as_false_if_the_argument_value_is_false() + public async Task Argument_value_can_be_bound_to_boolean_as_false_if_the_value_is_false() { // Arrange var (console, stdOut, _) = VirtualConsole.CreateBuffered(); @@ -298,7 +297,7 @@ namespace CliFx.Tests } [Fact] - public async Task Property_of_type_bool_is_bound_as_true_if_the_argument_value_is_not_set() + public async Task Argument_value_can_be_bound_to_boolean_as_true_if_the_value_is_not_set() { // Arrange var (console, stdOut, _) = VirtualConsole.CreateBuffered(); @@ -326,7 +325,7 @@ namespace CliFx.Tests } [Fact] - public async Task Property_of_type_char_is_bound_directly_from_the_argument_value_if_it_contains_only_one_character() + public async Task Argument_value_can_be_bound_to_char_if_the_value_contains_a_single_character() { // Arrange var (console, stdOut, _) = VirtualConsole.CreateBuffered(); @@ -354,7 +353,7 @@ namespace CliFx.Tests } [Fact] - public async Task Property_of_type_sbyte_is_bound_by_parsing_the_argument_value() + public async Task Argument_value_can_be_bound_to_sbyte() { // Arrange var (console, stdOut, _) = VirtualConsole.CreateBuffered(); @@ -382,7 +381,7 @@ namespace CliFx.Tests } [Fact] - public async Task Property_of_type_byte_is_bound_by_parsing_the_argument_value() + public async Task Argument_value_can_be_bound_to_byte() { // Arrange var (console, stdOut, _) = VirtualConsole.CreateBuffered(); @@ -410,7 +409,7 @@ namespace CliFx.Tests } [Fact] - public async Task Property_of_type_short_is_bound_by_parsing_the_argument_value() + public async Task Argument_value_can_be_bound_to_short() { // Arrange var (console, stdOut, _) = VirtualConsole.CreateBuffered(); @@ -438,7 +437,7 @@ namespace CliFx.Tests } [Fact] - public async Task Property_of_type_ushort_is_bound_by_parsing_the_argument_value() + public async Task Argument_value_can_be_bound_to_ushort() { // Arrange var (console, stdOut, _) = VirtualConsole.CreateBuffered(); @@ -466,7 +465,7 @@ namespace CliFx.Tests } [Fact] - public async Task Property_of_type_int_is_bound_by_parsing_the_argument_value() + public async Task Argument_value_can_be_bound_to_int() { // Arrange var (console, stdOut, _) = VirtualConsole.CreateBuffered(); @@ -494,7 +493,7 @@ namespace CliFx.Tests } [Fact] - public async Task Property_of_type_nullable_int_is_bound_by_parsing_the_argument_value_if_it_is_set() + public async Task Argument_value_can_be_bound_to_nullable_of_int_as_actual_value_if_it_is_set() { // Arrange var (console, stdOut, _) = VirtualConsole.CreateBuffered(); @@ -522,7 +521,7 @@ namespace CliFx.Tests } [Fact] - public async Task Property_of_type_nullable_int_is_bound_as_null_if_the_argument_value_is_not_set() + public async Task Argument_value_can_be_bound_to_nullable_of_int_as_null_if_it_is_not_set() { // Arrange var (console, stdOut, _) = VirtualConsole.CreateBuffered(); @@ -550,7 +549,7 @@ namespace CliFx.Tests } [Fact] - public async Task Property_of_type_int_array_is_bound_by_parsing_the_argument_values() + public async Task Argument_values_can_be_bound_to_array_of_int() { // Arrange var (console, stdOut, _) = VirtualConsole.CreateBuffered(); @@ -578,7 +577,7 @@ namespace CliFx.Tests } [Fact] - public async Task Property_of_type_nullable_int_array_is_bound_by_parsing_the_argument_values() + public async Task Argument_values_can_be_bound_to_array_of_nullable_of_int() { // Arrange var (console, stdOut, _) = VirtualConsole.CreateBuffered(); @@ -606,7 +605,7 @@ namespace CliFx.Tests } [Fact] - public async Task Property_of_type_uint_is_bound_by_parsing_the_argument_value() + public async Task Argument_value_can_be_bound_to_uint() { // Arrange var (console, stdOut, _) = VirtualConsole.CreateBuffered(); @@ -634,7 +633,7 @@ namespace CliFx.Tests } [Fact] - public async Task Property_of_type_long_is_bound_by_parsing_the_argument_value() + public async Task Argument_value_can_be_bound_to_long() { // Arrange var (console, stdOut, _) = VirtualConsole.CreateBuffered(); @@ -662,7 +661,7 @@ namespace CliFx.Tests } [Fact] - public async Task Property_of_type_ulong_is_bound_by_parsing_the_argument_value() + public async Task Argument_value_can_be_bound_to_ulong() { // Arrange var (console, stdOut, _) = VirtualConsole.CreateBuffered(); @@ -690,7 +689,7 @@ namespace CliFx.Tests } [Fact] - public async Task Property_of_type_float_is_bound_by_parsing_the_argument_value() + public async Task Argument_value_can_be_bound_to_float() { // Arrange var (console, stdOut, _) = VirtualConsole.CreateBuffered(); @@ -718,7 +717,7 @@ namespace CliFx.Tests } [Fact] - public async Task Property_of_type_double_is_bound_by_parsing_the_argument_value() + public async Task Argument_value_can_be_bound_to_double() { // Arrange var (console, stdOut, _) = VirtualConsole.CreateBuffered(); @@ -746,7 +745,7 @@ namespace CliFx.Tests } [Fact] - public async Task Property_of_type_decimal_is_bound_by_parsing_the_argument_value() + public async Task Argument_value_can_be_bound_to_decimal() { // Arrange var (console, stdOut, _) = VirtualConsole.CreateBuffered(); @@ -774,7 +773,7 @@ namespace CliFx.Tests } [Fact] - public async Task Property_of_type_DateTime_is_bound_by_parsing_the_argument_value() + public async Task Argument_value_can_be_bound_to_DateTime() { // Arrange var (console, stdOut, _) = VirtualConsole.CreateBuffered(); @@ -802,7 +801,7 @@ namespace CliFx.Tests } [Fact] - public async Task Property_of_type_DateTimeOffset_is_bound_by_parsing_the_argument_value() + public async Task Argument_value_can_be_bound_to_DateTimeOffset() { // Arrange var (console, stdOut, _) = VirtualConsole.CreateBuffered(); @@ -830,7 +829,7 @@ namespace CliFx.Tests } [Fact] - public async Task Property_of_type_TimeSpan_is_bound_by_parsing_the_argument_value() + public async Task Argument_value_can_be_bound_to_TimeSpan() { // Arrange var (console, stdOut, _) = VirtualConsole.CreateBuffered(); @@ -858,7 +857,7 @@ namespace CliFx.Tests } [Fact] - public async Task Property_of_type_nullable_TimeSpan_is_bound_by_parsing_the_argument_value_if_it_is_set() + public async Task Argument_value_can_be_bound_to_nullable_of_TimeSpan_as_actual_value_if_it_is_set() { // Arrange var (console, stdOut, _) = VirtualConsole.CreateBuffered(); @@ -886,7 +885,7 @@ namespace CliFx.Tests } [Fact] - public async Task Property_of_type_nullable_TimeSpan_is_bound_as_null_if_the_argument_value_is_not_set() + public async Task Argument_value_can_be_bound_to_nullable_of_TimeSpan_as_null_if_it_is_not_set() { // Arrange var (console, stdOut, _) = VirtualConsole.CreateBuffered(); @@ -914,7 +913,7 @@ namespace CliFx.Tests } [Fact] - public async Task Property_of_an_enum_type_is_bound_by_parsing_the_argument_value_as_name() + public async Task Argument_value_can_be_bound_to_an_enum_type_by_name() { // Arrange var (console, stdOut, _) = VirtualConsole.CreateBuffered(); @@ -942,7 +941,7 @@ namespace CliFx.Tests } [Fact] - public async Task Property_of_an_enum_type_is_bound_by_parsing_the_argument_value_as_id() + public async Task Argument_value_can_be_bound_to_an_enum_type_by_id() { // Arrange var (console, stdOut, _) = VirtualConsole.CreateBuffered(); @@ -970,7 +969,7 @@ namespace CliFx.Tests } [Fact] - public async Task Property_of_a_nullable_enum_type_is_bound_by_parsing_the_argument_value_as_name_if_it_is_set() + public async Task Argument_value_can_be_bound_to_nullable_of_enum_type_by_name_if_it_is_set() { // Arrange var (console, stdOut, _) = VirtualConsole.CreateBuffered(); @@ -998,7 +997,7 @@ namespace CliFx.Tests } [Fact] - public async Task Property_of_a_nullable_enum_type_is_bound_by_parsing_the_argument_value_as_id_if_it_is_set() + public async Task Argument_value_can_be_bound_to_nullable_of_enum_type_by_id_if_it_is_set() { // Arrange var (console, stdOut, _) = VirtualConsole.CreateBuffered(); @@ -1026,7 +1025,7 @@ namespace CliFx.Tests } [Fact] - public async Task Property_of_a_nullable_enum_type_is_bound_as_null_if_the_argument_value_is_not_set() + public async Task Argument_value_can_be_bound_to_nullable_of_enum_type_as_null_if_it_is_not_set() { // Arrange var (console, stdOut, _) = VirtualConsole.CreateBuffered(); @@ -1054,7 +1053,7 @@ namespace CliFx.Tests } [Fact] - public async Task Property_of_an_enum_array_type_is_bound_by_parsing_the_argument_values_as_names() + public async Task Argument_values_can_be_bound_to_array_of_enum_type_by_names() { // Arrange var (console, stdOut, _) = VirtualConsole.CreateBuffered(); @@ -1082,7 +1081,7 @@ namespace CliFx.Tests } [Fact] - public async Task Property_of_an_enum_array_type_is_bound_by_parsing_the_argument_values_as_ids() + public async Task Argument_values_can_be_bound_to_array_of_enum_type_by_ids() { // Arrange var (console, stdOut, _) = VirtualConsole.CreateBuffered(); @@ -1110,7 +1109,7 @@ namespace CliFx.Tests } [Fact] - public async Task Property_of_an_enum_array_type_is_bound_by_parsing_the_argument_values_as_either_names_or_ids() + public async Task Argument_values_can_be_bound_to_array_of_enum_type_by_either_names_or_ids() { // Arrange var (console, stdOut, _) = VirtualConsole.CreateBuffered(); @@ -1138,7 +1137,7 @@ namespace CliFx.Tests } [Fact] - public async Task Property_of_a_type_that_has_a_constructor_accepting_a_string_is_bound_by_invoking_the_constructor_with_the_argument_value() + public async Task Argument_value_can_be_bound_to_a_custom_type_if_it_has_a_constructor_accepting_a_string() { // Arrange var (console, stdOut, _) = VirtualConsole.CreateBuffered(); @@ -1166,7 +1165,7 @@ namespace CliFx.Tests } [Fact] - public async Task Property_of_an_array_of_type_that_has_a_constructor_accepting_a_string_is_bound_by_invoking_the_constructor_with_the_argument_values() + public async Task Argument_values_can_be_bound_to_array_of_custom_type_if_it_has_a_constructor_accepting_a_string() { // Arrange var (console, stdOut, _) = VirtualConsole.CreateBuffered(); @@ -1198,7 +1197,7 @@ namespace CliFx.Tests } [Fact] - public async Task Property_of_a_type_that_has_a_static_Parse_method_accepting_a_string_is_bound_by_invoking_the_method() + public async Task Argument_value_can_be_bound_to_a_custom_type_if_it_has_a_static_Parse_method() { // Arrange var (console, stdOut, _) = VirtualConsole.CreateBuffered(); @@ -1226,7 +1225,7 @@ namespace CliFx.Tests } [Fact] - public async Task Property_of_a_type_that_has_a_static_Parse_method_accepting_a_string_and_format_provider_is_bound_by_invoking_the_method() + public async Task Argument_value_can_be_bound_to_a_custom_type_if_it_has_a_static_Parse_method_with_format_provider() { // Arrange var (console, stdOut, _) = VirtualConsole.CreateBuffered(); @@ -1255,55 +1254,72 @@ namespace CliFx.Tests } [Fact] - public async Task Property_of_custom_type_must_be_string_initializable_in_order_to_be_bound() + public async Task Argument_value_can_be_bound_to_a_custom_type_if_a_converter_has_been_specified() { // Arrange - var (console, _, stdErr) = VirtualConsole.CreateBuffered(); + var (console, stdOut, _) = VirtualConsole.CreateBuffered(); var application = new CliApplicationBuilder() - .AddCommand() + .AddCommand() .UseConsole(console) .Build(); // Act var exitCode = await application.RunAsync(new[] { - "cmd", "--str-non-initializable", "foobar" + "cmd", "--convertible", "13" }); - // Assert - exitCode.Should().NotBe(0); - stdErr.GetString().Should().NotBeNullOrWhiteSpace(); + var commandInstance = stdOut.GetString().DeserializeJson(); - _output.WriteLine(stdErr.GetString()); + // Assert + exitCode.Should().Be(0); + + commandInstance.Should().BeEquivalentTo(new SupportedArgumentTypesCommand + { + Convertible = + (SupportedArgumentTypesCommand.CustomConvertible) + new SupportedArgumentTypesCommand.CustomConvertibleConverter().ConvertFrom("13") + }); } [Fact] - public async Task Property_of_custom_type_that_implements_IEnumerable_can_only_be_bound_if_that_type_has_a_constructor_accepting_an_array() + public async Task Argument_values_can_be_bound_to_array_of_custom_type_if_a_converter_has_been_specified() { // Arrange - var (console, _, stdErr) = VirtualConsole.CreateBuffered(); + var (console, stdOut, _) = VirtualConsole.CreateBuffered(); var application = new CliApplicationBuilder() - .AddCommand() + .AddCommand() .UseConsole(console) .Build(); // Act var exitCode = await application.RunAsync(new[] { - "cmd", "--str-enumerable-non-initializable", "foobar" + "cmd", "--convertible-array", "13", "42" }); - // Assert - exitCode.Should().NotBe(0); - stdErr.GetString().Should().NotBeNullOrWhiteSpace(); + var commandInstance = stdOut.GetString().DeserializeJson(); - _output.WriteLine(stdErr.GetString()); + // Assert + exitCode.Should().Be(0); + + commandInstance.Should().BeEquivalentTo(new SupportedArgumentTypesCommand + { + ConvertibleArray = new[] + { + (SupportedArgumentTypesCommand.CustomConvertible) + new SupportedArgumentTypesCommand.CustomConvertibleConverter().ConvertFrom("13"), + + (SupportedArgumentTypesCommand.CustomConvertible) + new SupportedArgumentTypesCommand.CustomConvertibleConverter().ConvertFrom("42") + } + }); } [Fact] - public async Task Property_of_non_nullable_type_can_only_be_bound_if_the_argument_value_is_set() + public async Task Argument_value_can_only_be_bound_to_non_nullable_type_if_it_is_set() { // Arrange var (console, _, stdErr) = VirtualConsole.CreateBuffered(); @@ -1327,7 +1343,7 @@ namespace CliFx.Tests } [Fact] - public async Task Property_must_have_a_type_that_implements_IEnumerable_in_order_to_be_bound_from_multiple_argument_values() + public async Task Argument_values_can_only_be_bound_to_a_type_that_implements_IEnumerable() { // Arrange var (console, _, stdErr) = VirtualConsole.CreateBuffered(); @@ -1349,70 +1365,5 @@ namespace CliFx.Tests _output.WriteLine(stdErr.GetString()); } - - [Fact] - public async Task Property_of_custom_type_is_bound_when_the_valid_converter_type_is_specified() - { - // Arrange - const string foo = "foo"; - - var (console, stdOut, _) = VirtualConsole.CreateBuffered(); - - var application = new CliApplicationBuilder() - .AddCommand() - .UseConsole(console) - .Build(); - - // Act - var exitCode = await application.RunAsync(new[] - { - "cmd", "--prop", foo - }); - - // Assert - exitCode.Should().Be(0); - - var commandInstance = stdOut.GetString().DeserializeJson(); - - commandInstance.Should().BeEquivalentTo(new CommandWithParameterOfCustomType() - { - MyProperty = (CustomType) new CustomTypeConverter().ConvertFrom(foo) - }); - } - - [Fact] - public async Task Enumerable_of_the_custom_type_is_bound_when_the_valid_converter_type_is_specified() - { - // Arrange - string foo = "foo"; - string bar = "bar"; - - var (console, stdOut, _) = VirtualConsole.CreateBuffered(); - - var application = new CliApplicationBuilder() - .AddCommand() - .UseConsole(console) - .Build(); - - // Act - var exitCode = await application.RunAsync(new[] - { - "cmd", "--prop", foo, bar - }); - - // Assert - exitCode.Should().Be(0); - - var commandInstance = stdOut.GetString().DeserializeJson(); - - commandInstance.Should().BeEquivalentTo(new CommandWithEnumerableOfParametersOfCustomType() - { - MyProperties = new List - { - (CustomType) new CustomTypeConverter().ConvertFrom(foo), - (CustomType) new CustomTypeConverter().ConvertFrom(bar) - } - }); - } } } \ No newline at end of file diff --git a/CliFx.Tests/Commands/CommandWithParameterOfCustomType.cs b/CliFx.Tests/Commands/CommandWithParameterOfCustomType.cs deleted file mode 100644 index ac1d14b..0000000 --- a/CliFx.Tests/Commands/CommandWithParameterOfCustomType.cs +++ /dev/null @@ -1,27 +0,0 @@ -using CliFx.Attributes; -using System; -using System.Threading.Tasks; -using CliFx.Tests.Commands.Converters; -using System.Collections.Generic; - -namespace CliFx.Tests.Commands -{ - public class CustomType - { - public int SomeValue { get; set; } - } - - [Command("cmd")] - public class CommandWithParameterOfCustomType : SelfSerializeCommandBase - { - [CommandOption("prop", Converter = typeof(CustomTypeConverter))] - public CustomType? MyProperty { get; set; } - } - - [Command("cmd")] - public class CommandWithEnumerableOfParametersOfCustomType : SelfSerializeCommandBase - { - [CommandOption("prop", Converter = typeof(CustomTypeConverter))] - public List? MyProperties { get; set; } - } -} diff --git a/CliFx.Tests/Commands/Converters/CustomTypeConverter.cs b/CliFx.Tests/Commands/Converters/CustomTypeConverter.cs deleted file mode 100644 index 3b17a3f..0000000 --- a/CliFx.Tests/Commands/Converters/CustomTypeConverter.cs +++ /dev/null @@ -1,8 +0,0 @@ -namespace CliFx.Tests.Commands.Converters -{ - public class CustomTypeConverter : IArgumentValueConverter - { - public object ConvertFrom(string value) => - new CustomType { SomeValue = value.Length }; - } -} diff --git a/CliFx.Tests/Commands/Invalid/InvalidCustomConverterOptionCommand.cs b/CliFx.Tests/Commands/Invalid/InvalidCustomConverterOptionCommand.cs new file mode 100644 index 0000000..3b59e08 --- /dev/null +++ b/CliFx.Tests/Commands/Invalid/InvalidCustomConverterOptionCommand.cs @@ -0,0 +1,16 @@ +using CliFx.Attributes; + +namespace CliFx.Tests.Commands.Invalid +{ + [Command] + public class InvalidCustomConverterOptionCommand : SelfSerializeCommandBase + { + [CommandOption('f', Converter = typeof(Converter))] + public string? Option { get; set; } + + public class Converter + { + public object ConvertFrom(string value) => value; + } + } +} \ No newline at end of file diff --git a/CliFx.Tests/Commands/Invalid/InvalidCustomConverterParameterCommand.cs b/CliFx.Tests/Commands/Invalid/InvalidCustomConverterParameterCommand.cs new file mode 100644 index 0000000..68ce67d --- /dev/null +++ b/CliFx.Tests/Commands/Invalid/InvalidCustomConverterParameterCommand.cs @@ -0,0 +1,16 @@ +using CliFx.Attributes; + +namespace CliFx.Tests.Commands.Invalid +{ + [Command] + public class InvalidCustomConverterParameterCommand : SelfSerializeCommandBase + { + [CommandParameter(0, Converter = typeof(Converter))] + public string? Param { get; set; } + + public class Converter + { + public object ConvertFrom(string value) => value; + } + } +} \ No newline at end of file diff --git a/CliFx.Tests/Commands/SupportedArgumentTypesCommand.cs b/CliFx.Tests/Commands/SupportedArgumentTypesCommand.cs index 8258159..8969904 100644 --- a/CliFx.Tests/Commands/SupportedArgumentTypesCommand.cs +++ b/CliFx.Tests/Commands/SupportedArgumentTypesCommand.cs @@ -1,6 +1,6 @@ using System; -using System.Collections; using System.Collections.Generic; +using System.Globalization; using CliFx.Attributes; using Newtonsoft.Json; @@ -84,6 +84,9 @@ namespace CliFx.Tests.Commands [CommandOption("str-parseable-format")] public CustomStringParseableWithFormatProvider? StringParseableWithFormatProvider { get; set; } + [CommandOption("convertible", Converter = typeof(CustomConvertibleConverter))] + public CustomConvertible? Convertible { get; set; } + [CommandOption("obj-array")] public object[]? ObjectArray { get; set; } @@ -102,6 +105,9 @@ namespace CliFx.Tests.Commands [CommandOption("str-constructible-array")] public CustomStringConstructible[]? StringConstructibleArray { get; set; } + [CommandOption("convertible-array", Converter = typeof(CustomConvertibleConverter))] + public CustomConvertible[]? ConvertibleArray { get; set; } + [CommandOption("str-enumerable")] public IEnumerable? StringEnumerable { get; set; } @@ -151,5 +157,18 @@ namespace CliFx.Tests.Commands public static CustomStringParseableWithFormatProvider Parse(string value, IFormatProvider formatProvider) => new CustomStringParseableWithFormatProvider(value + " " + formatProvider); } + + public class CustomConvertible + { + public int Value { get; } + + public CustomConvertible(int value) => Value = value; + } + + public class CustomConvertibleConverter : IArgumentValueConverter + { + public object ConvertFrom(string value) => + new CustomConvertible(int.Parse(value, CultureInfo.InvariantCulture)); + } } } \ No newline at end of file diff --git a/CliFx.Tests/DirectivesSpecs.cs b/CliFx.Tests/DirectivesSpecs.cs index 2c79c7e..c47ffeb 100644 --- a/CliFx.Tests/DirectivesSpecs.cs +++ b/CliFx.Tests/DirectivesSpecs.cs @@ -29,7 +29,8 @@ namespace CliFx.Tests // Act var exitCode = await application.RunAsync( new[] {"[preview]", "named", "param", "-abc", "--option", "foo"}, - new Dictionary()); + new Dictionary() + ); // Assert exitCode.Should().Be(0); diff --git a/CliFx/Attributes/CommandOptionAttribute.cs b/CliFx/Attributes/CommandOptionAttribute.cs index 787ba53..ac0c0da 100644 --- a/CliFx/Attributes/CommandOptionAttribute.cs +++ b/CliFx/Attributes/CommandOptionAttribute.cs @@ -38,7 +38,8 @@ namespace CliFx.Attributes public string? EnvironmentVariableName { get; set; } /// - /// Type of a converter to use for the option value evaluating. + /// Type of converter to use when mapping the argument value. + /// Converter must implement . /// public Type? Converter { get; set; } diff --git a/CliFx/Attributes/CommandParameterAttribute.cs b/CliFx/Attributes/CommandParameterAttribute.cs index 67f5c87..afcfa3c 100644 --- a/CliFx/Attributes/CommandParameterAttribute.cs +++ b/CliFx/Attributes/CommandParameterAttribute.cs @@ -27,7 +27,8 @@ namespace CliFx.Attributes public string? Description { get; set; } /// - /// Type of a converter to use for the parameter value evaluating. + /// Type of converter to use when mapping the argument value. + /// Converter must implement . /// public Type? Converter { get; set; } diff --git a/CliFx/Domain/CommandArgumentSchema.cs b/CliFx/Domain/CommandArgumentSchema.cs index dc726fb..c2300e7 100644 --- a/CliFx/Domain/CommandArgumentSchema.cs +++ b/CliFx/Domain/CommandArgumentSchema.cs @@ -17,13 +17,13 @@ namespace CliFx.Domain public bool IsScalar => TryGetEnumerableArgumentUnderlyingType() == null; - protected Type? Converter { get; set; } + public Type? ConverterType { get; } - protected CommandArgumentSchema(PropertyInfo? property, string? description, Type? converter = null) + protected CommandArgumentSchema(PropertyInfo? property, string? description, Type? converterType) { Property = property; Description = description; - Converter = converter; + ConverterType = converterType; } private Type? TryGetEnumerableArgumentUnderlyingType() => @@ -35,6 +35,10 @@ namespace CliFx.Domain { try { + // Custom conversion + if (ConverterType != null) + return ConverterType.CreateInstance().ConvertFrom(value!); + // Primitive var primitiveConverter = PrimitiveConverters.GetValueOrDefault(targetType); if (primitiveConverter != null) @@ -57,17 +61,14 @@ namespace CliFx.Domain return stringConstructor.Invoke(new object[] {value!}); // String-parseable (with format provider) - var parseMethodWithFormatProvider = targetType.GetStaticParseMethod(true); + var parseMethodWithFormatProvider = targetType.TryGetStaticParseMethod(true); if (parseMethodWithFormatProvider != null) return parseMethodWithFormatProvider.Invoke(null, new object[] {value!, FormatProvider}); // String-parseable (without format provider) - var parseMethod = targetType.GetStaticParseMethod(); + var parseMethod = targetType.TryGetStaticParseMethod(); if (parseMethod != null) return parseMethod.Invoke(null, new object[] {value!}); - - if (Converter != null) - return Converter.InstanceOf().ConvertFrom(value!); } catch (Exception ex) { diff --git a/CliFx/Domain/CommandOptionSchema.cs b/CliFx/Domain/CommandOptionSchema.cs index f5476c3..070bcf7 100644 --- a/CliFx/Domain/CommandOptionSchema.cs +++ b/CliFx/Domain/CommandOptionSchema.cs @@ -24,8 +24,8 @@ namespace CliFx.Domain string? environmentVariableName, bool isRequired, string? description, - Type? converter = null) - : base(property, description, converter) + Type? converterType) + : base(property, description, converterType) { Name = name; ShortName = shortName; @@ -107,9 +107,9 @@ namespace CliFx.Domain internal partial class CommandOptionSchema { public static CommandOptionSchema HelpOption { get; } = - new CommandOptionSchema(null, "help", 'h', null, false, "Shows help text."); + new CommandOptionSchema(null, "help", 'h', null, false, "Shows help text.", null); public static CommandOptionSchema VersionOption { get; } = - new CommandOptionSchema(null, "version", null, null, false, "Shows version information."); + new CommandOptionSchema(null, "version", null, null, false, "Shows version information.", null); } } \ No newline at end of file diff --git a/CliFx/Domain/CommandParameterSchema.cs b/CliFx/Domain/CommandParameterSchema.cs index ea70137..b54ada4 100644 --- a/CliFx/Domain/CommandParameterSchema.cs +++ b/CliFx/Domain/CommandParameterSchema.cs @@ -12,8 +12,13 @@ namespace CliFx.Domain public string Name { get; } - public CommandParameterSchema(PropertyInfo? property, int order, string name, string? description, Type? converter = null) - : base(property, description, converter) + public CommandParameterSchema( + PropertyInfo? property, + int order, + string name, + string? description, + Type? converterType) + : base(property, description, converterType) { Order = order; Name = name; diff --git a/CliFx/Domain/RootSchema.cs b/CliFx/Domain/RootSchema.cs index de4821f..431bc43 100644 --- a/CliFx/Domain/RootSchema.cs +++ b/CliFx/Domain/RootSchema.cs @@ -114,6 +114,18 @@ namespace CliFx.Domain nonLastNonScalarParameter ); } + + var invalidConverterParameters = command.Parameters + .Where(p => p.ConverterType != null && !p.ConverterType.Implements(typeof(IArgumentValueConverter))) + .ToArray(); + + if (invalidConverterParameters.Any()) + { + throw CliFxException.ParametersWithInvalidConverters( + command, + invalidConverterParameters + ); + } } private static void ValidateOptions(CommandSchema command) @@ -184,6 +196,18 @@ namespace CliFx.Domain duplicateEnvironmentVariableNameGroup.ToArray() ); } + + var invalidConverterOptions = command.Options + .Where(o => o.ConverterType != null && !o.ConverterType.Implements(typeof(IArgumentValueConverter))) + .ToArray(); + + if (invalidConverterOptions.Any()) + { + throw CliFxException.OptionsWithInvalidConverters( + command, + invalidConverterOptions + ); + } } private static void ValidateCommands(IReadOnlyList commands) diff --git a/CliFx/Exceptions/CliFxException.cs b/CliFx/Exceptions/CliFxException.cs index f02c7f8..9b39849 100644 --- a/CliFx/Exceptions/CliFxException.cs +++ b/CliFx/Exceptions/CliFxException.cs @@ -172,6 +172,19 @@ If it's not feasible to fit into these constraints, consider using options inste return new CliFxException(message.Trim()); } + internal static CliFxException ParametersWithInvalidConverters( + CommandSchema command, + IReadOnlyList invalidParameters) + { + var message = $@" +Command '{command.Type.FullName}' is invalid because it contains {invalidParameters.Count} parameter(s) with invalid converters: +{invalidParameters.JoinToString(Environment.NewLine)} + +Specified converter must implement {typeof(IArgumentValueConverter).FullName}."; + + return new CliFxException(message.Trim()); + } + internal static CliFxException OptionsWithNoName( CommandSchema command, IReadOnlyList invalidOptions) @@ -243,6 +256,19 @@ Environment variable names are not case-sensitive."; return new CliFxException(message.Trim()); } + + internal static CliFxException OptionsWithInvalidConverters( + CommandSchema command, + IReadOnlyList invalidOptions) + { + var message = $@" +Command '{command.Type.FullName}' is invalid because it contains {invalidOptions.Count} option(s) with invalid converters: +{invalidOptions.JoinToString(Environment.NewLine)} + +Specified converter must implement {typeof(IArgumentValueConverter).FullName}."; + + return new CliFxException(message.Trim()); + } } // End-user-facing exceptions diff --git a/CliFx/IArgumentValueConverter.cs b/CliFx/IArgumentValueConverter.cs index 27c0c21..9929d42 100644 --- a/CliFx/IArgumentValueConverter.cs +++ b/CliFx/IArgumentValueConverter.cs @@ -1,7 +1,7 @@ namespace CliFx { /// - /// Used as an interface for implementing custom parameter/option converters. + /// Implements custom conversion logic that maps an argument value to a domain type. /// public interface IArgumentValueConverter { diff --git a/CliFx/Internal/Extensions/TypeExtensions.cs b/CliFx/Internal/Extensions/TypeExtensions.cs index ba289f1..42ac2f9 100644 --- a/CliFx/Internal/Extensions/TypeExtensions.cs +++ b/CliFx/Internal/Extensions/TypeExtensions.cs @@ -8,6 +8,8 @@ namespace CliFx.Internal.Extensions { internal static class TypeExtensions { + public static T CreateInstance(this Type type) => (T) Activator.CreateInstance(type); + public static bool Implements(this Type type, Type interfaceType) => type.GetInterfaces().Contains(interfaceType); public static Type? GetNullableUnderlyingType(this Type type) => Nullable.GetUnderlyingType(type); @@ -31,11 +33,14 @@ namespace CliFx.Internal.Extensions .FirstOrDefault(); } - public static MethodInfo GetToStringMethod(this Type type) => type.GetMethod(nameof(ToString), Type.EmptyTypes); + public static MethodInfo GetToStringMethod(this Type type) => + // ToString() with no params always exists + type.GetMethod(nameof(ToString), Type.EmptyTypes)!; - public static bool IsToStringOverriden(this Type type) => type.GetToStringMethod() != typeof(object).GetToStringMethod(); + public static bool IsToStringOverriden(this Type type) => + type.GetToStringMethod() != typeof(object).GetToStringMethod(); - public static MethodInfo GetStaticParseMethod(this Type type, bool withFormatProvider = false) + public static MethodInfo? TryGetStaticParseMethod(this Type type, bool withFormatProvider = false) { var argumentTypes = withFormatProvider ? new[] {typeof(string), typeof(IFormatProvider)} @@ -56,10 +61,5 @@ namespace CliFx.Internal.Extensions return array; } - - public static T InstanceOf(this Type type) => - type.Implements(typeof(T)) - ? (T) Activator.CreateInstance(type) - : throw new ArgumentException(); } } \ No newline at end of file