From de847b90e45d642bb202b3d5214297ee195f721b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?C=C3=A9dric=20Luthi?= Date: Tue, 24 Jan 2023 19:38:03 +0100 Subject: [PATCH] Improve conversion error messages When a conversion to an enum fails, list all the valid enum values in the error message. Message before this commit: > Error: heimday is not a valid value for DayOfWeek. Message after this commit: > Error: Failed to convert 'heimday' to DayOfWeek. Valid values are 'Sunday', 'Monday', 'Tuesday', 'Wednesday', 'Thursday', 'Friday', 'Saturday' --- .../CommandRuntimeException.cs | 7 +++ .../Internal/Binding/CommandValueResolver.cs | 12 ++++- .../Data/Commands/HorseCommand.cs | 4 +- .../Data/Settings/HorseSettings.cs | 7 +++ .../Xml/Test_1.Output.verified.txt | 6 +-- .../Xml/Test_3.Output.verified.txt | 5 +- .../Xml/Test_6.Output.verified.txt | 5 +- .../Unit/CommandAppTests.TypeConverters.cs | 47 +++++++++++++++++++ 8 files changed, 83 insertions(+), 10 deletions(-) create mode 100644 test/Spectre.Console.Cli.Tests/Data/Settings/HorseSettings.cs diff --git a/src/Spectre.Console.Cli/CommandRuntimeException.cs b/src/Spectre.Console.Cli/CommandRuntimeException.cs index 786d294..669842f 100644 --- a/src/Spectre.Console.Cli/CommandRuntimeException.cs +++ b/src/Spectre.Console.Cli/CommandRuntimeException.cs @@ -42,6 +42,13 @@ public class CommandRuntimeException : CommandAppException return new CommandRuntimeException($"Could not find converter for type '{parameter.ParameterType.FullName}'."); } + internal static CommandRuntimeException ConversionFailed(MappedCommandParameter parameter, TypeConverter typeConverter, Exception exception) + { + var standardValues = typeConverter.GetStandardValuesSupported() ? typeConverter.GetStandardValues() : null; + var validValues = standardValues == null ? string.Empty : $" Valid values are '{string.Join("', '", standardValues.Cast().Select(Convert.ToString))}'"; + return new CommandRuntimeException($"Failed to convert '{parameter.Value}' to {parameter.Parameter.ParameterType.Name}.{validValues}", exception); + } + internal static CommandRuntimeException ValidationFailed(ValidationResult result) { return new CommandRuntimeException(result.Message ?? "Unknown validation error."); diff --git a/src/Spectre.Console.Cli/Internal/Binding/CommandValueResolver.cs b/src/Spectre.Console.Cli/Internal/Binding/CommandValueResolver.cs index d2d5334..e277028 100644 --- a/src/Spectre.Console.Cli/Internal/Binding/CommandValueResolver.cs +++ b/src/Spectre.Console.Cli/Internal/Binding/CommandValueResolver.cs @@ -84,8 +84,18 @@ internal static class CommandValueResolver throw CommandRuntimeException.NoConverterFound(mapped.Parameter); } + object? value; + try + { + value = converter.ConvertFromInvariantString(mapped.Value ?? string.Empty); + } + catch (Exception exception) + { + throw CommandRuntimeException.ConversionFailed(mapped, converter, exception); + } + // Assign the value to the parameter. - binder.Bind(mapped.Parameter, resolver, converter.ConvertFromInvariantString(mapped.Value ?? string.Empty)); + binder.Bind(mapped.Parameter, resolver, value); } } diff --git a/test/Spectre.Console.Cli.Tests/Data/Commands/HorseCommand.cs b/test/Spectre.Console.Cli.Tests/Data/Commands/HorseCommand.cs index 4b10806..e352a09 100644 --- a/test/Spectre.Console.Cli.Tests/Data/Commands/HorseCommand.cs +++ b/test/Spectre.Console.Cli.Tests/Data/Commands/HorseCommand.cs @@ -1,9 +1,9 @@ namespace Spectre.Console.Tests.Data; [Description("The horse command.")] -public class HorseCommand : AnimalCommand +public class HorseCommand : AnimalCommand { - public override int Execute(CommandContext context, MammalSettings settings) + public override int Execute(CommandContext context, HorseSettings settings) { DumpSettings(context, settings); return 0; diff --git a/test/Spectre.Console.Cli.Tests/Data/Settings/HorseSettings.cs b/test/Spectre.Console.Cli.Tests/Data/Settings/HorseSettings.cs new file mode 100644 index 0000000..7231b5e --- /dev/null +++ b/test/Spectre.Console.Cli.Tests/Data/Settings/HorseSettings.cs @@ -0,0 +1,7 @@ +namespace Spectre.Console.Tests.Data; + +public class HorseSettings : MammalSettings +{ + [CommandOption("-d|--day")] + public DayOfWeek Day { get; set; } +} \ No newline at end of file diff --git a/test/Spectre.Console.Cli.Tests/Expectations/Xml/Test_1.Output.verified.txt b/test/Spectre.Console.Cli.Tests/Expectations/Xml/Test_1.Output.verified.txt index 13ed69d..c161f02 100644 --- a/test/Spectre.Console.Cli.Tests/Expectations/Xml/Test_1.Output.verified.txt +++ b/test/Spectre.Console.Cli.Tests/Expectations/Xml/Test_1.Output.verified.txt @@ -1,4 +1,4 @@ - + @@ -27,9 +27,9 @@ - + - diff --git a/test/Spectre.Console.Cli.Tests/Expectations/Xml/Test_3.Output.verified.txt b/test/Spectre.Console.Cli.Tests/Expectations/Xml/Test_3.Output.verified.txt index f2a9c3f..cf55628 100644 --- a/test/Spectre.Console.Cli.Tests/Expectations/Xml/Test_3.Output.verified.txt +++ b/test/Spectre.Console.Cli.Tests/Expectations/Xml/Test_3.Output.verified.txt @@ -1,4 +1,4 @@ - + @@ -23,8 +23,9 @@ - + + diff --git a/test/Spectre.Console.Cli.Tests/Expectations/Xml/Test_6.Output.verified.txt b/test/Spectre.Console.Cli.Tests/Expectations/Xml/Test_6.Output.verified.txt index 8ece874..c0a5c45 100644 --- a/test/Spectre.Console.Cli.Tests/Expectations/Xml/Test_6.Output.verified.txt +++ b/test/Spectre.Console.Cli.Tests/Expectations/Xml/Test_6.Output.verified.txt @@ -1,4 +1,4 @@ - + @@ -19,7 +19,7 @@ - + The number of legs. @@ -31,6 +31,7 @@ + diff --git a/test/Spectre.Console.Cli.Tests/Unit/CommandAppTests.TypeConverters.cs b/test/Spectre.Console.Cli.Tests/Unit/CommandAppTests.TypeConverters.cs index 86db2eb..a115c0e 100644 --- a/test/Spectre.Console.Cli.Tests/Unit/CommandAppTests.TypeConverters.cs +++ b/test/Spectre.Console.Cli.Tests/Unit/CommandAppTests.TypeConverters.cs @@ -29,5 +29,52 @@ public sealed partial class CommandAppTests cat.Agility.ShouldBe(6); }); } + + [Fact] + public void Should_Convert_Enum_Ignoring_Case() + { + // Given + var app = new CommandAppTester(); + app.Configure(config => + { + config.AddCommand("horse"); + }); + + // When + var result = app.Run(new[] { "horse", "--day", "friday" }); + + // Then + result.ExitCode.ShouldBe(0); + result.Settings.ShouldBeOfType().And(horse => + { + horse.Day.ShouldBe(DayOfWeek.Friday); + }); + } + + [Fact] + public void Should_List_All_Valid_Enum_Values_On_Conversion_Error() + { + // Given + var app = new CommandAppTester(); + app.Configure(config => + { + config.AddCommand("horse"); + }); + + // When + var result = app.Run(new[] { "horse", "--day", "heimday" }); + + // Then + result.ExitCode.ShouldBe(-1); + result.Output.ShouldStartWith("Error"); + result.Output.ShouldContain("heimday"); + result.Output.ShouldContain(nameof(DayOfWeek.Sunday)); + result.Output.ShouldContain(nameof(DayOfWeek.Monday)); + result.Output.ShouldContain(nameof(DayOfWeek.Tuesday)); + result.Output.ShouldContain(nameof(DayOfWeek.Wednesday)); + result.Output.ShouldContain(nameof(DayOfWeek.Thursday)); + result.Output.ShouldContain(nameof(DayOfWeek.Friday)); + result.Output.ShouldContain(nameof(DayOfWeek.Saturday)); + } } }