From c322b7029cd18dfee515797dfa23c85e69755352 Mon Sep 17 00:00:00 2001 From: Nikiforov Alexey Date: Thu, 22 Oct 2020 16:02:58 +0300 Subject: [PATCH 01/16] Add child command usage in help text (#77) --- CliFx/Domain/HelpTextWriter.cs | 45 +++++++++++++++++++++++++--------- 1 file changed, 34 insertions(+), 11 deletions(-) diff --git a/CliFx/Domain/HelpTextWriter.cs b/CliFx/Domain/HelpTextWriter.cs index 7724425..f2b452b 100644 --- a/CliFx/Domain/HelpTextWriter.cs +++ b/CliFx/Domain/HelpTextWriter.cs @@ -105,31 +105,52 @@ namespace CliFx.Domain WriteLine(); } - private void WriteCommandUsage(CommandSchema command, IReadOnlyList childCommands) + private void WriteCommandUsage( + CommandSchema command, + IReadOnlyList childCommands) { if (!IsEmpty) WriteVerticalMargin(); WriteHeader("Usage"); - // Exe name - WriteHorizontalMargin(); - Write(_metadata.ExecutableName); + WriteCommandUsageLineItem(command, childCommands.Count > 0); + if (!IsEmpty) + WriteVerticalMargin(); + + foreach (var childCommand in childCommands) + { + WriteCommandUsageLineItem(childCommand, compactCommand: false, size: 4); + } + } + + private void WriteCommandUsageLineItem( + CommandSchema command, + bool compactCommand = true, + int size = 2) + { + WriteHorizontalMargin(size); + if (compactCommand) + { + // Exe name + Write(_metadata.ExecutableName); + Write(' '); + + } // Command name if (!string.IsNullOrWhiteSpace(command.Name)) { - Write(' '); + // this is fragile, because we rely that subcommand name consists + // of all required tokens Write(ConsoleColor.Cyan, command.Name); } - // Child command placeholder - if (childCommands.Any()) + if (compactCommand) { Write(' '); Write(ConsoleColor.Cyan, "[command]"); } - // Parameters foreach (var parameter in command.Parameters) { @@ -334,7 +355,9 @@ namespace CliFx.Domain CommandSchema command, IReadOnlyDictionary defaultValues) { - var childCommands = root.GetChildCommands(command.Name); + var commandName = command.Name; + var childCommands = root.GetChildCommands(commandName); + var descendantCommands = root.GetDescendantCommands(commandName); _console.ResetColor(); @@ -342,7 +365,7 @@ namespace CliFx.Domain WriteApplicationInfo(); WriteCommandDescription(command); - WriteCommandUsage(command, childCommands); + WriteCommandUsage(command, descendantCommands); WriteCommandParameters(command); WriteCommandOptions(command, defaultValues); WriteCommandChildren(command, childCommands); @@ -385,4 +408,4 @@ namespace CliFx.Domain } } } -} \ No newline at end of file +} From 0ec12e57c1bfe320f703f483c91f5cc1964a0616 Mon Sep 17 00:00:00 2001 From: Tyrrrz Date: Fri, 23 Oct 2020 18:01:40 +0300 Subject: [PATCH 02/16] Refactor pretty stack traces --- CliFx/CliApplication.cs | 39 +-- CliFx/Domain/ErrorTextWriter.cs | 226 ------------------ CliFx/IConsole.cs | 126 +++++++++- CliFx/Internal/Extensions/StringExtensions.cs | 5 + CliFx/Internal/Polyfills.cs | 3 + CliFx/Internal/StackFrame.cs | 130 ++++++++++ 6 files changed, 286 insertions(+), 243 deletions(-) delete mode 100644 CliFx/Domain/ErrorTextWriter.cs create mode 100644 CliFx/Internal/StackFrame.cs diff --git a/CliFx/CliApplication.cs b/CliFx/CliApplication.cs index 24d29c1..dedfaee 100644 --- a/CliFx/CliApplication.cs +++ b/CliFx/CliApplication.cs @@ -22,7 +22,6 @@ namespace CliFx private readonly ITypeActivator _typeActivator; private readonly HelpTextWriter _helpTextWriter; - private readonly ErrorTextWriter _errorTextWriter; /// /// Initializes an instance of . @@ -37,12 +36,8 @@ namespace CliFx _typeActivator = typeActivator; _helpTextWriter = new HelpTextWriter(metadata, console); - _errorTextWriter = new ErrorTextWriter(console); } - private void WriteError(string message) => _console.WithForegroundColor(ConsoleColor.Red, () => - _console.Error.WriteLine(message)); - private async ValueTask LaunchAndWaitForDebuggerAsync() { var processId = ProcessEx.GetCurrentProcessId(); @@ -53,7 +48,9 @@ namespace CliFx Debugger.Launch(); while (!Debugger.IsAttached) + { await Task.Delay(100); + } } private void WriteCommandLineInput(CommandInput input) @@ -105,9 +102,9 @@ namespace CliFx } private ICommand GetCommandInstance(CommandSchema command) => - command != StubDefaultCommand.Schema + command != FallbackDefaultCommand.Schema ? (ICommand) _typeActivator.CreateInstance(command.Type) - : new StubDefaultCommand(); + : new FallbackDefaultCommand(); /// /// Runs the application with specified command line arguments and environment variables, and returns the exit code. @@ -143,7 +140,7 @@ namespace CliFx var command = root.TryFindCommand(input.CommandName) ?? root.TryFindDefaultCommand() ?? - StubDefaultCommand.Schema; + FallbackDefaultCommand.Schema; // Version option if (command.IsVersionOptionAvailable && input.IsVersionOptionSpecified) @@ -161,7 +158,7 @@ namespace CliFx // Help option if (command.IsHelpOptionAvailable && input.IsHelpOptionSpecified || - command == StubDefaultCommand.Schema && !input.Parameters.Any() && !input.Options.Any()) + command == FallbackDefaultCommand.Schema && !input.Parameters.Any() && !input.Options.Any()) { _helpTextWriter.Write(root, command, defaultValues); return ExitCode.Success; @@ -175,7 +172,10 @@ namespace CliFx // This may throw exceptions which are useful only to the end-user catch (CliFxException ex) { - WriteError(ex.ToString()); + _console.WithForegroundColor(ConsoleColor.Red, () => + _console.Error.WriteLine(ex.ToString()) + ); + _helpTextWriter.Write(root, command, defaultValues); return ExitCode.FromException(ex); @@ -190,10 +190,14 @@ namespace CliFx // Swallow command exceptions and route them to the console catch (CommandException ex) { - WriteError(ex.ToString()); + _console.WithForegroundColor(ConsoleColor.Red, () => + _console.Error.WriteLine(ex.ToString()) + ); if (ex.ShowHelp) + { _helpTextWriter.Write(root, command, defaultValues); + } return ex.ExitCode; } @@ -204,7 +208,13 @@ namespace CliFx // because we still want the IDE to show them to the developer. catch (Exception ex) when (!Debugger.IsAttached) { - _errorTextWriter.WriteError(ex); + _console.WithColors(ConsoleColor.White, ConsoleColor.DarkRed, () => + _console.Error.Write("ERROR:") + ); + + _console.Error.Write(" "); + _console.WriteException(ex); + return ExitCode.FromException(ex); } } @@ -259,10 +269,11 @@ namespace CliFx : 1; } + // Fallback default command used when none is defined in the application [Command] - private class StubDefaultCommand : ICommand + private class FallbackDefaultCommand : ICommand { - public static CommandSchema Schema { get; } = CommandSchema.TryResolve(typeof(StubDefaultCommand))!; + public static CommandSchema Schema { get; } = CommandSchema.TryResolve(typeof(FallbackDefaultCommand))!; public ValueTask ExecuteAsync(IConsole console) => default; } diff --git a/CliFx/Domain/ErrorTextWriter.cs b/CliFx/Domain/ErrorTextWriter.cs deleted file mode 100644 index 9cff91a..0000000 --- a/CliFx/Domain/ErrorTextWriter.cs +++ /dev/null @@ -1,226 +0,0 @@ -using System; -using System.Collections.Generic; -using System.Text.RegularExpressions; - -namespace CliFx.Domain -{ - internal class ErrorTextWriter - { - private const int indent = 4; - - private static readonly ConsoleColor NameColor = ConsoleColor.DarkGray; - private static readonly ConsoleColor SpecificNameColor = ConsoleColor.White; - private static readonly ConsoleColor MessageColor = ConsoleColor.Red; - private static readonly ConsoleColor MethodColor = ConsoleColor.Yellow; - private static readonly ConsoleColor ParameterTypeColor = ConsoleColor.Blue; - private static readonly ConsoleColor FileColor = ConsoleColor.Yellow; - private static readonly ConsoleColor LineColor = ConsoleColor.Blue; - - private static readonly Regex MethodMatcher = new Regex(@"(?\S+) (?.*?)(?[^\.]+)\("); - private static readonly Regex ParameterMatcher = new Regex(@"(?.+? )(?.+?)(?:(?, )|\))"); - private static readonly Regex FileMatcher = new Regex(@"(?\S+?) (?.*?)(?[^\\/]+?(?:\.\w*)?):[^:]+? (?\d+).*"); - - private readonly IConsole _console; - - public ErrorTextWriter(IConsole console) - { - _console = console; - } - - public void WriteError(Exception ex) => WriteError(ex, 0); - private void WriteError(Exception ex, int indentLevel) - { - var indentation = new string(' ', indent * indentLevel); - var extraIndentation = new string(' ', indent / 2); - - var exType = ex.GetType(); - - // (Fully qualified) type of the exception - Write(NameColor, indentation + exType.Namespace + "."); - Write(SpecificNameColor, exType.Name); - _console.Error.Write(": "); - - // Message - Write(MessageColor, ex.Message); - _console.Error.WriteLine(); - - // Prints the inner exception - // with one higher indentation level - if (ex.InnerException is Exception innerException) - { - WriteError(innerException, indentLevel + 1); - } - - // Print with formatting when successfully parsing all entries in the stack trace - if (ParseStackTrace(ex.StackTrace) is IEnumerable parsedStackTrace) - { - // Each step in the stack trace is formated and printed - foreach (var entry in parsedStackTrace) - { - _console.Error.Write(indentation + extraIndentation); - - WriteMethodDescriptor(entry.MethodPrefix, entry.MethodName, entry.MethodSpecificName); - - WriteParameters(entry.Parameters); - - _console.Error.Write(entry.FilePrefix); - _console.Error.Write("\n" + indentation + extraIndentation + extraIndentation); - WriteFileDescriptor(entry.FilePath, entry.FileName, entry.FileLine); - - _console.Error.WriteLine(); - } - } - else - { - // Parsing failed. Print without formatting. - foreach (var trace in ex.StackTrace.Split('\n')) - { - _console.Error.WriteLine(indentation + trace); - } - } - } - - private void WriteMethodDescriptor(string prefix, string name, string methodName) - { - _console.Error.Write(prefix + " "); - Write(NameColor, name); - Write(MethodColor, methodName); - } - - private void WriteParameters(IEnumerable parameters) - { - _console.Error.Write("("); - foreach (var parameter in parameters) - { - Write(ParameterTypeColor, parameter.Type); - Write(SpecificNameColor, parameter.Name); - - if (parameter.Separator is string separator) - { - _console.Error.Write(separator); - } - } - _console.Error.Write(") "); - } - - private void WriteFileDescriptor(string path, string fileName, string lineNumber) - { - Write(NameColor, path); - - Write(FileColor, fileName); - _console.Error.Write(":"); - Write(LineColor, lineNumber); - } - - private void Write(ConsoleColor color, string value) - { - _console.WithForegroundColor(color, () => _console.Error.Write(value)); - } - - private IEnumerable? ParseStackTrace(string stackTrace) - { - IList stackTraceEntries = new List(); - foreach (var trace in stackTrace.Split('\n')) - { - var methodMatch = MethodMatcher.Match(trace); - var parameterMatches = ParameterMatcher.Matches(trace, methodMatch.Index + methodMatch.Length); - var fileMatch = FileMatcher.Match( - trace, - parameterMatches.Count switch - { - 0 => methodMatch.Index + methodMatch.Length + 1, - int c => parameterMatches[c - 1].Index + parameterMatches[c - 1].Length - } - ); - - if (fileMatch.Index + fileMatch.Length != trace.Length) - { - // Didnt match the whole trace - return null; - } - - try - { - IList parameters = new List(); - foreach (Match match in parameterMatches) - { - parameters.Add(new ParameterEntry( - match.Groups["type"].Success ? match.Groups["type"].Value : throw new Exception("type"), - match.Groups["name"].Success ? match.Groups["name"].Value : throw new Exception("name"), - match.Groups["separator"]?.Value // If this is null, it's just the last parameter - )); - } - - stackTraceEntries.Add(new StackTraceEntry( - methodMatch.Groups["prefix"].Success ? methodMatch.Groups["prefix"].Value : throw new Exception("prefix"), - methodMatch.Groups["name"].Success ? methodMatch.Groups["name"].Value : throw new Exception("name"), - methodMatch.Groups["methodName"].Success ? methodMatch.Groups["methodName"].Value : throw new Exception("methodName"), - parameters, - fileMatch.Groups["prefix"].Success ? fileMatch.Groups["prefix"].Value : throw new Exception("prefix"), - fileMatch.Groups["path"].Success ? fileMatch.Groups["path"].Value : throw new Exception("path"), - fileMatch.Groups["file"].Success ? fileMatch.Groups["file"].Value : throw new Exception("file"), - fileMatch.Groups["line"].Success ? fileMatch.Groups["line"].Value : throw new Exception("line") - )); - } - catch - { - // One of the required groups failed to match - return null; - } - } - - return stackTraceEntries; - } - - private readonly struct StackTraceEntry - { - public string MethodPrefix { get; } - public string MethodName { get; } - public string MethodSpecificName { get; } - - public IEnumerable Parameters { get; } - - public string FilePrefix { get; } - public string FilePath { get; } - public string FileName { get; } - public string FileLine { get; } - - public StackTraceEntry( - string methodPrefix, - string methodName, - string methodSpecificName, - IEnumerable parameters, - string filePrefix, - string filePath, - string fileName, - string fileLine) - { - MethodPrefix = methodPrefix; - MethodName = methodName; - MethodSpecificName = methodSpecificName; - Parameters = parameters; - FilePrefix = filePrefix; - FilePath = filePath; - FileName = fileName; - FileLine = fileLine; - } - } - - private readonly struct ParameterEntry - { - public string Type { get; } - public string Name { get; } - public string? Separator { get; } - - public ParameterEntry( - string type, - string name, - string? separator) - { - Type = type; - Name = name; - Separator = separator; - } - } - } -} diff --git a/CliFx/IConsole.cs b/CliFx/IConsole.cs index 8181fd6..2adab83 100644 --- a/CliFx/IConsole.cs +++ b/CliFx/IConsole.cs @@ -1,6 +1,7 @@ using System; using System.IO; using System.Threading; +using CliFx.Internal; namespace CliFx { @@ -85,7 +86,10 @@ namespace CliFx /// /// Sets console foreground color, executes specified action, and sets the color back to the original value. /// - public static void WithForegroundColor(this IConsole console, ConsoleColor foregroundColor, Action action) + public static void WithForegroundColor( + this IConsole console, + ConsoleColor foregroundColor, + Action action) { var lastColor = console.ForegroundColor; console.ForegroundColor = foregroundColor; @@ -98,7 +102,10 @@ namespace CliFx /// /// Sets console background color, executes specified action, and sets the color back to the original value. /// - public static void WithBackgroundColor(this IConsole console, ConsoleColor backgroundColor, Action action) + public static void WithBackgroundColor( + this IConsole console, + ConsoleColor backgroundColor, + Action action) { var lastColor = console.BackgroundColor; console.BackgroundColor = backgroundColor; @@ -111,7 +118,120 @@ namespace CliFx /// /// Sets console foreground and background colors, executes specified action, and sets the colors back to the original values. /// - public static void WithColors(this IConsole console, ConsoleColor foregroundColor, ConsoleColor backgroundColor, Action action) => + public static void WithColors( + this IConsole console, + ConsoleColor foregroundColor, + ConsoleColor backgroundColor, + Action action) => console.WithForegroundColor(foregroundColor, () => console.WithBackgroundColor(backgroundColor, action)); + + private static void WriteException( + this IConsole console, + Exception exception, + int indentLevel) + { + var exceptionType = exception.GetType(); + + var indentationShared = new string(' ', 4 * indentLevel); + var indentationLocal = new string(' ', 2); + + // Fully qualified exception type + console.Error.Write(indentationShared); + console.WithForegroundColor(ConsoleColor.DarkGray, () => + console.Error.Write(exceptionType.Namespace + ".") + ); + console.WithForegroundColor(ConsoleColor.White, () => + console.Error.Write(exceptionType.Name) + ); + console.Error.Write(": "); + + // Exception message + console.WithForegroundColor(ConsoleColor.Red, () => console.Error.WriteLine(exception.Message)); + + // Recurse into inner exceptions + if (exception.InnerException != null) + { + console.WriteException(exception.InnerException, indentLevel + 1); + } + + // Try to parse and pretty-print the stack trace + try + { + foreach (var stackFrame in StackFrame.ParseMany(exception.StackTrace)) + { + console.Error.Write(indentationShared + indentationLocal); + + // "at" + console.Error.Write(stackFrame.Prefix + " "); + + // "CliFx.Demo.Commands.BookAddCommand." + console.WithForegroundColor(ConsoleColor.DarkGray, () => + console.Error.Write(stackFrame.ParentType) + ); + + // "ExecuteAsync" + console.WithForegroundColor(ConsoleColor.Yellow, () => + console.Error.Write(stackFrame.MethodName) + ); + + console.Error.Write("("); + + foreach (var parameter in stackFrame.Parameters) + { + // "IConsole" + console.WithForegroundColor(ConsoleColor.Blue, () => + console.Error.Write(parameter.Type) + ); + + // "console" + console.WithForegroundColor(ConsoleColor.White, () => + console.Error.Write(parameter.Name) + ); + + // ", ' + if (parameter.Separator != null) + { + console.Error.Write(parameter.Separator); + } + } + + console.Error.Write(") "); + + // "in" + console.Error.Write(stackFrame.LocationPrefix); + console.Error.Write("\n" + indentationShared + indentationLocal + indentationLocal); + + // "E:\Projects\Softdev\CliFx\CliFx.Demo\Commands\" + console.WithForegroundColor(ConsoleColor.DarkGray, () => + console.Error.Write(stackFrame.DirectoryPath) + ); + + // "BookAddCommand.cs" + console.WithForegroundColor(ConsoleColor.Yellow, () => + console.Error.Write(stackFrame.FileName) + ); + + console.Error.Write(":"); + + // "35" + console.WithForegroundColor(ConsoleColor.Blue, () => + console.Error.Write(stackFrame.LineNumber) + ); + + console.Error.WriteLine(); + } + } + // If any point of parsing has failed - print the stack trace without any formatting + catch + { + console.Error.WriteLine(exception.StackTrace); + } + } + + //Should this be public? + internal static void WriteException( + this IConsole console, + Exception exception) => + console.WriteException(exception, 0); } } \ No newline at end of file diff --git a/CliFx/Internal/Extensions/StringExtensions.cs b/CliFx/Internal/Extensions/StringExtensions.cs index fbafbe0..dd7f8d4 100644 --- a/CliFx/Internal/Extensions/StringExtensions.cs +++ b/CliFx/Internal/Extensions/StringExtensions.cs @@ -6,6 +6,11 @@ namespace CliFx.Internal.Extensions { internal static class StringExtensions { + public static string? NullIfWhiteSpace(this string str) => + !string.IsNullOrWhiteSpace(str) + ? str + : null; + public static string Repeat(this char c, int count) => new string(c, count); public static string AsString(this char c) => c.Repeat(1); diff --git a/CliFx/Internal/Polyfills.cs b/CliFx/Internal/Polyfills.cs index 6c0a2e3..10f6987 100644 --- a/CliFx/Internal/Polyfills.cs +++ b/CliFx/Internal/Polyfills.cs @@ -17,6 +17,9 @@ namespace System public static bool EndsWith(this string str, char c) => str.Length > 0 && str[str.Length - 1] == c; + + public static string[] Split(this string str, char separator, StringSplitOptions splitOptions) => + str.Split(new[] {separator}, splitOptions); } } diff --git a/CliFx/Internal/StackFrame.cs b/CliFx/Internal/StackFrame.cs new file mode 100644 index 0000000..2336185 --- /dev/null +++ b/CliFx/Internal/StackFrame.cs @@ -0,0 +1,130 @@ +using System; +using System.Collections.Generic; +using System.Linq; +using System.Text.RegularExpressions; +using CliFx.Internal.Extensions; + +namespace CliFx.Internal +{ + internal class StackFrameParameter + { + public string Type { get; } + + public string Name { get; } + + public string? Separator { get; } + + public StackFrameParameter( + string type, + string name, + string? separator) + { + Type = type; + Name = name; + Separator = separator; + } + } + + internal partial class StackFrame + { + public string Prefix { get; } + + public string ParentType { get; } + + public string MethodName { get; } + + public IReadOnlyList Parameters { get; } + + public string LocationPrefix { get; } + + public string DirectoryPath { get; } + + public string FileName { get; } + + public string LineNumber { get; } + + public StackFrame( + string prefix, + string parentType, + string methodName, + IReadOnlyList parameters, + string locationPrefix, + string directoryPath, + string fileName, + string lineNumber) + { + Prefix = prefix; + ParentType = parentType; + MethodName = methodName; + Parameters = parameters; + LocationPrefix = locationPrefix; + DirectoryPath = directoryPath; + FileName = fileName; + LineNumber = lineNumber; + } + } + + internal partial class StackFrame + { + private static readonly Regex MethodMatcher = + new Regex(@"(?\S+) (?.*?)(?[^\.]+)\("); + + private static readonly Regex ParameterMatcher = + new Regex(@"(?.+? )(?.+?)(?:(?, )|\))"); + + private static readonly Regex FileMatcher = + new Regex(@"(?\S+?) (?.*?)(?[^\\/]+?(?:\.\w*)?):[^:]+? (?\d+).*"); + + public static StackFrame Parse(string stackFrame) + { + var methodMatch = MethodMatcher.Match(stackFrame); + + var parameterMatches = ParameterMatcher.Matches(stackFrame, methodMatch.Index + methodMatch.Length) + .Cast() + .ToArray(); + + var fileMatch = FileMatcher.Match( + stackFrame, + parameterMatches.Length switch + { + 0 => methodMatch.Index + methodMatch.Length + 1, + _ => parameterMatches[parameterMatches.Length - 1].Index + + parameterMatches[parameterMatches.Length - 1].Length + } + ); + + // Ensure everything was parsed successfully + var isSuccessful = + methodMatch.Success && + parameterMatches.All(m => m.Success) && + fileMatch.Success && + fileMatch.Index + fileMatch.Length == stackFrame.Length; + + if (!isSuccessful) + { + throw new FormatException("Failed to parse stack frame."); + } + + var parameters = parameterMatches + .Select(match => new StackFrameParameter( + match.Groups["type"].Value, + match.Groups["name"].Value, + match.Groups["separator"].Value.NullIfWhiteSpace() + )).ToArray(); + + return new StackFrame( + methodMatch.Groups["prefix"].Value, + methodMatch.Groups["name"].Value, + methodMatch.Groups["methodName"].Value, + parameters, + fileMatch.Groups["prefix"].Value, + fileMatch.Groups["path"].Value, + fileMatch.Groups["file"].Value, + fileMatch.Groups["line"].Value + ); + } + + public static IReadOnlyList ParseMany(string stackTrace) => + stackTrace.Split('\n', StringSplitOptions.RemoveEmptyEntries).Select(Parse).ToArray(); + } +} \ No newline at end of file From d52a205f130d3136c06caa200b4fea4a16f8b7e4 Mon Sep 17 00:00:00 2001 From: Tyrrrz Date: Fri, 23 Oct 2020 18:17:17 +0300 Subject: [PATCH 03/16] Improve coverage slightly --- .../Commands/GenericInnerExceptionCommand.cs | 19 +++++++++++++ CliFx.Tests/ErrorReportingSpecs.cs | 28 +++++++++++++++++++ CliFx/CliApplication.cs | 3 ++ 3 files changed, 50 insertions(+) create mode 100644 CliFx.Tests/Commands/GenericInnerExceptionCommand.cs diff --git a/CliFx.Tests/Commands/GenericInnerExceptionCommand.cs b/CliFx.Tests/Commands/GenericInnerExceptionCommand.cs new file mode 100644 index 0000000..479f811 --- /dev/null +++ b/CliFx.Tests/Commands/GenericInnerExceptionCommand.cs @@ -0,0 +1,19 @@ +using System; +using System.Threading.Tasks; +using CliFx.Attributes; + +namespace CliFx.Tests.Commands +{ + [Command("cmd")] + public class GenericInnerExceptionCommand : ICommand + { + [CommandOption("msg", 'm')] + public string? Message { get; set; } + + [CommandOption("inner-msg", 'i')] + public string? InnerMessage { get; set; } + + public ValueTask ExecuteAsync(IConsole console) => + throw new Exception(Message, new Exception(InnerMessage)); + } +} \ No newline at end of file diff --git a/CliFx.Tests/ErrorReportingSpecs.cs b/CliFx.Tests/ErrorReportingSpecs.cs index 302d6b1..d07766d 100644 --- a/CliFx.Tests/ErrorReportingSpecs.cs +++ b/CliFx.Tests/ErrorReportingSpecs.cs @@ -39,6 +39,34 @@ namespace CliFx.Tests _output.WriteLine(stdErr.GetString()); } + [Fact] + public async Task Command_may_throw_a_generic_exception_with_inner_exception_which_exits_and_prints_error_message_and_stack_trace() + { + // Arrange + var (console, stdOut, stdErr) = VirtualConsole.CreateBuffered(); + + var application = new CliApplicationBuilder() + .AddCommand() + .UseConsole(console) + .Build(); + + // Act + var exitCode = await application.RunAsync(new[] {"cmd", "-m", "Kaput", "-i", "FooBar"}); + + // Assert + exitCode.Should().NotBe(0); + stdOut.GetString().Should().BeEmpty(); + stdErr.GetString().Should().ContainAll( + "System.Exception:", + "FooBar", + "Kaput", "at", + "CliFx.Tests" + ); + + _output.WriteLine(stdOut.GetString()); + _output.WriteLine(stdErr.GetString()); + } + [Fact] public async Task Command_may_throw_a_specialized_exception_which_exits_with_custom_code_and_prints_minimal_error_details() { diff --git a/CliFx/CliApplication.cs b/CliFx/CliApplication.cs index dedfaee..dd14211 100644 --- a/CliFx/CliApplication.cs +++ b/CliFx/CliApplication.cs @@ -2,6 +2,7 @@ using System; using System.Collections; using System.Collections.Generic; using System.Diagnostics; +using System.Diagnostics.CodeAnalysis; using System.Linq; using System.Threading.Tasks; using CliFx.Attributes; @@ -275,6 +276,8 @@ namespace CliFx { public static CommandSchema Schema { get; } = CommandSchema.TryResolve(typeof(FallbackDefaultCommand))!; + // Never actually executed + [ExcludeFromCodeCoverage] public ValueTask ExecuteAsync(IConsole console) => default; } } From c06f2810b9643144d7b889fd6c94b56b2e12c88e Mon Sep 17 00:00:00 2001 From: Tyrrrz Date: Fri, 23 Oct 2020 18:23:58 +0300 Subject: [PATCH 04/16] Cleanup analyzers --- CliFx.Analyzers/ConsoleUsageAnalyzer.cs | 76 ++++++++++++------------- 1 file changed, 35 insertions(+), 41 deletions(-) diff --git a/CliFx.Analyzers/ConsoleUsageAnalyzer.cs b/CliFx.Analyzers/ConsoleUsageAnalyzer.cs index cbcf2ce..31e3e3c 100644 --- a/CliFx.Analyzers/ConsoleUsageAnalyzer.cs +++ b/CliFx.Analyzers/ConsoleUsageAnalyzer.cs @@ -18,55 +18,49 @@ namespace CliFx.Analyzers SyntaxNodeAnalysisContext context, InvocationExpressionSyntax invocationSyntax) { - // Get the method member access (Console.WriteLine or Console.Error.WriteLine) - if (!(invocationSyntax.Expression is MemberAccessExpressionSyntax memberAccessSyntax)) - return false; + if (invocationSyntax.Expression is MemberAccessExpressionSyntax memberAccessSyntax && + context.SemanticModel.GetSymbolInfo(memberAccessSyntax).Symbol is IMethodSymbol methodSymbol) + { + // Direct call to System.Console (e.g. System.Console.WriteLine()) + if (KnownSymbols.IsSystemConsole(methodSymbol.ContainingType)) + { + return true; + } - // Get the semantic model for the invoked method - if (!(context.SemanticModel.GetSymbolInfo(memberAccessSyntax).Symbol is IMethodSymbol methodSymbol)) - return false; - - // Check if contained within System.Console - if (KnownSymbols.IsSystemConsole(methodSymbol.ContainingType)) - return true; - - // In case with Console.Error.WriteLine that wouldn't work, we need to check parent member access too - if (!(memberAccessSyntax.Expression is MemberAccessExpressionSyntax parentMemberAccessSyntax)) - return false; - - // Get the semantic model for the parent member - if (!(context.SemanticModel.GetSymbolInfo(parentMemberAccessSyntax).Symbol is IPropertySymbol propertySymbol)) - return false; - - // Check if contained within System.Console - if (KnownSymbols.IsSystemConsole(propertySymbol.ContainingType)) - return true; + // Indirect call to System.Console (e.g. System.Console.Error.WriteLine()) + if (memberAccessSyntax.Expression is MemberAccessExpressionSyntax parentMemberAccessSyntax && + context.SemanticModel.GetSymbolInfo(parentMemberAccessSyntax).Symbol is IPropertySymbol propertySymbol) + { + return KnownSymbols.IsSystemConsole(propertySymbol.ContainingType); + } + } return false; } private static void CheckSystemConsoleUsage(SyntaxNodeAnalysisContext context) { - if (!(context.Node is InvocationExpressionSyntax invocationSyntax)) - return; + if (context.Node is InvocationExpressionSyntax invocationSyntax && + IsSystemConsoleInvocation(context, invocationSyntax)) + { + // Check if IConsole is available in scope as alternative to System.Console + var isConsoleInterfaceAvailable = invocationSyntax + .Ancestors() + .OfType() + .SelectMany(m => m.ParameterList.Parameters) + .Select(p => p.Type) + .Select(t => context.SemanticModel.GetSymbolInfo(t).Symbol) + .Where(s => s != null) + .Any(KnownSymbols.IsConsoleInterface!); - if (!IsSystemConsoleInvocation(context, invocationSyntax)) - return; - - // Check if IConsole is available in the scope as a viable alternative - var isConsoleInterfaceAvailable = invocationSyntax - .Ancestors() - .OfType() - .SelectMany(m => m.ParameterList.Parameters) - .Select(p => p.Type) - .Select(t => context.SemanticModel.GetSymbolInfo(t).Symbol) - .Where(s => s != null) - .Any(KnownSymbols.IsConsoleInterface!); - - if (!isConsoleInterfaceAvailable) - return; - - context.ReportDiagnostic(Diagnostic.Create(DiagnosticDescriptors.CliFx0100, invocationSyntax.GetLocation())); + if (isConsoleInterfaceAvailable) + { + context.ReportDiagnostic(Diagnostic.Create( + DiagnosticDescriptors.CliFx0100, + invocationSyntax.GetLocation() + )); + } + } } public override void Initialize(AnalysisContext context) From 8df1d607c17012f0f8adfc63fc2f48af994400f8 Mon Sep 17 00:00:00 2001 From: Tyrrrz Date: Fri, 23 Oct 2020 20:52:26 +0300 Subject: [PATCH 05/16] 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 From b120138de36bf6e5f9894bd0736a1b1d7fc4781e Mon Sep 17 00:00:00 2001 From: Tyrrrz Date: Fri, 23 Oct 2020 20:53:27 +0300 Subject: [PATCH 06/16] Update github actions --- .github/workflows/CD.yml | 4 ++-- .github/workflows/CI.yml | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/.github/workflows/CD.yml b/.github/workflows/CD.yml index 3d4ffed..8ddad3d 100644 --- a/.github/workflows/CD.yml +++ b/.github/workflows/CD.yml @@ -11,10 +11,10 @@ jobs: steps: - name: Checkout - uses: actions/checkout@v2 + uses: actions/checkout@v2.3.3 - name: Install .NET Core - uses: actions/setup-dotnet@v1.4.0 + uses: actions/setup-dotnet@v1.7.2 with: dotnet-version: 3.1.100 diff --git a/.github/workflows/CI.yml b/.github/workflows/CI.yml index 7e7d3a0..d92e8aa 100644 --- a/.github/workflows/CI.yml +++ b/.github/workflows/CI.yml @@ -12,10 +12,10 @@ jobs: steps: - name: Checkout - uses: actions/checkout@v2 + uses: actions/checkout@v2.3.3 - name: Install .NET Core - uses: actions/setup-dotnet@v1.4.0 + uses: actions/setup-dotnet@v1.7.2 with: dotnet-version: 3.1.100 From 14ad9d5738620ff1771a82965b7abffa44eb28d0 Mon Sep 17 00:00:00 2001 From: Tyrrrz Date: Fri, 23 Oct 2020 21:18:57 +0300 Subject: [PATCH 07/16] Improve tests --- CliFx.Analyzers/CommandSchemaAnalyzer.cs | 7 +- CliFx.Tests/ArgumentConversionSpecs.cs | 76 ++++++++++++++++++- .../UnsupportedArgumentTypesCommand.cs | 8 +- CliFx/CliApplicationBuilder.cs | 24 +++--- CliFx/DefaultTypeActivator.cs | 3 +- CliFx/Internal/Extensions/TypeExtensions.cs | 4 +- 6 files changed, 100 insertions(+), 22 deletions(-) diff --git a/CliFx.Analyzers/CommandSchemaAnalyzer.cs b/CliFx.Analyzers/CommandSchemaAnalyzer.cs index a688d34..cf652fb 100644 --- a/CliFx.Analyzers/CommandSchemaAnalyzer.cs +++ b/CliFx.Analyzers/CommandSchemaAnalyzer.cs @@ -275,11 +275,8 @@ namespace CliFx.Analyzers private static void CheckCommandType(SymbolAnalysisContext context) { // Named type: MyCommand - if (!(context.Symbol is INamedTypeSymbol namedTypeSymbol)) - return; - - // Only classes - if (namedTypeSymbol.TypeKind != TypeKind.Class) + if (!(context.Symbol is INamedTypeSymbol namedTypeSymbol) || + namedTypeSymbol.TypeKind != TypeKind.Class) return; // Implements ICommand? diff --git a/CliFx.Tests/ArgumentConversionSpecs.cs b/CliFx.Tests/ArgumentConversionSpecs.cs index 7be6731..097dc4c 100644 --- a/CliFx.Tests/ArgumentConversionSpecs.cs +++ b/CliFx.Tests/ArgumentConversionSpecs.cs @@ -913,7 +913,7 @@ namespace CliFx.Tests } [Fact] - public async Task Argument_value_can_be_bound_to_an_enum_type_by_name() + public async Task Argument_value_can_be_bound_to_enum_type_by_name() { // Arrange var (console, stdOut, _) = VirtualConsole.CreateBuffered(); @@ -941,7 +941,7 @@ namespace CliFx.Tests } [Fact] - public async Task Argument_value_can_be_bound_to_an_enum_type_by_id() + public async Task Argument_value_can_be_bound_to_enum_type_by_id() { // Arrange var (console, stdOut, _) = VirtualConsole.CreateBuffered(); @@ -1318,6 +1318,54 @@ namespace CliFx.Tests }); } + [Fact] + public async Task Argument_value_can_only_be_bound_if_the_target_type_is_supported() + { + // Arrange + var (console, _, stdErr) = VirtualConsole.CreateBuffered(); + + var application = new CliApplicationBuilder() + .AddCommand() + .UseConsole(console) + .Build(); + + // Act + var exitCode = await application.RunAsync(new[] + { + "cmd", "--custom" + }); + + // Assert + exitCode.Should().NotBe(0); + stdErr.GetString().Should().NotBeNullOrWhiteSpace(); + + _output.WriteLine(stdErr.GetString()); + } + + [Fact] + public async Task Argument_value_can_only_be_bound_if_the_provided_value_can_be_converted_to_the_target_type() + { + // Arrange + var (console, _, stdErr) = VirtualConsole.CreateBuffered(); + + var application = new CliApplicationBuilder() + .AddCommand() + .UseConsole(console) + .Build(); + + // Act + var exitCode = await application.RunAsync(new[] + { + "cmd", "--int", "foo" + }); + + // Assert + exitCode.Should().NotBe(0); + stdErr.GetString().Should().NotBeNullOrWhiteSpace(); + + _output.WriteLine(stdErr.GetString()); + } + [Fact] public async Task Argument_value_can_only_be_bound_to_non_nullable_type_if_it_is_set() { @@ -1365,5 +1413,29 @@ namespace CliFx.Tests _output.WriteLine(stdErr.GetString()); } + + [Fact] + public async Task Argument_values_can_only_be_bound_to_a_type_that_implements_IEnumerable_and_can_be_converted_from_an_array() + { + // Arrange + var (console, _, stdErr) = VirtualConsole.CreateBuffered(); + + var application = new CliApplicationBuilder() + .AddCommand() + .UseConsole(console) + .Build(); + + // Act + var exitCode = await application.RunAsync(new[] + { + "cmd", "--custom-enumerable" + }); + + // Assert + exitCode.Should().NotBe(0); + stdErr.GetString().Should().NotBeNullOrWhiteSpace(); + + _output.WriteLine(stdErr.GetString()); + } } } \ No newline at end of file diff --git a/CliFx.Tests/Commands/UnsupportedArgumentTypesCommand.cs b/CliFx.Tests/Commands/UnsupportedArgumentTypesCommand.cs index 4dc9648..f6efda9 100644 --- a/CliFx.Tests/Commands/UnsupportedArgumentTypesCommand.cs +++ b/CliFx.Tests/Commands/UnsupportedArgumentTypesCommand.cs @@ -8,11 +8,11 @@ namespace CliFx.Tests.Commands [Command("cmd")] public partial class UnsupportedArgumentTypesCommand : SelfSerializeCommandBase { - [CommandOption("str-non-initializable")] - public CustomType? StringNonInitializable { get; set; } + [CommandOption("custom")] + public CustomType? CustomNonConvertible { get; set; } - [CommandOption("str-enumerable-non-initializable")] - public CustomEnumerable? StringEnumerableNonInitializable { get; set; } + [CommandOption("custom-enumerable")] + public CustomEnumerable? CustomEnumerableNonConvertible { get; set; } } public partial class UnsupportedArgumentTypesCommand diff --git a/CliFx/CliApplicationBuilder.cs b/CliFx/CliApplicationBuilder.cs index dcce7e5..65e0e59 100644 --- a/CliFx/CliApplicationBuilder.cs +++ b/CliFx/CliApplicationBuilder.cs @@ -165,9 +165,9 @@ namespace CliFx /// public CliApplication Build() { - _title ??= TryGetDefaultTitle() ?? "App"; - _executableName ??= TryGetDefaultExecutableName() ?? "app"; - _versionText ??= TryGetDefaultVersionText() ?? "v1.0"; + _title ??= GetDefaultTitle(); + _executableName ??= GetDefaultExecutableName(); + _versionText ??= GetDefaultVersionText(); _console ??= new SystemConsole(); _typeActivator ??= new DefaultTypeActivator(); @@ -185,23 +185,29 @@ namespace CliFx // Entry assembly is null in tests private static Assembly? EntryAssembly => LazyEntryAssembly.Value; - private static string? TryGetDefaultTitle() => EntryAssembly?.GetName().Name; + private static string GetDefaultTitle() => EntryAssembly?.GetName().Name?? "App"; - private static string? TryGetDefaultExecutableName() + private static string GetDefaultExecutableName() { var entryAssemblyLocation = EntryAssembly?.Location; // The assembly can be an executable or a dll, depending on how it was packaged - var isDll = string.Equals(Path.GetExtension(entryAssemblyLocation), ".dll", StringComparison.OrdinalIgnoreCase); + var isDll = string.Equals( + Path.GetExtension(entryAssemblyLocation), + ".dll", + StringComparison.OrdinalIgnoreCase + ); - return isDll + var name = isDll ? "dotnet " + Path.GetFileName(entryAssemblyLocation) : Path.GetFileNameWithoutExtension(entryAssemblyLocation); + + return name ?? "app"; } - private static string? TryGetDefaultVersionText() => + private static string GetDefaultVersionText() => EntryAssembly != null ? $"v{EntryAssembly.GetName().Version.ToSemanticString()}" - : null; + : "v1.0"; } } \ No newline at end of file diff --git a/CliFx/DefaultTypeActivator.cs b/CliFx/DefaultTypeActivator.cs index dc945d2..5a6d27e 100644 --- a/CliFx/DefaultTypeActivator.cs +++ b/CliFx/DefaultTypeActivator.cs @@ -1,5 +1,6 @@ using System; using CliFx.Exceptions; +using CliFx.Internal.Extensions; namespace CliFx { @@ -13,7 +14,7 @@ namespace CliFx { try { - return Activator.CreateInstance(type); + return type.CreateInstance(); } catch (Exception ex) { diff --git a/CliFx/Internal/Extensions/TypeExtensions.cs b/CliFx/Internal/Extensions/TypeExtensions.cs index 42ac2f9..6a05cdb 100644 --- a/CliFx/Internal/Extensions/TypeExtensions.cs +++ b/CliFx/Internal/Extensions/TypeExtensions.cs @@ -8,7 +8,9 @@ namespace CliFx.Internal.Extensions { internal static class TypeExtensions { - public static T CreateInstance(this Type type) => (T) Activator.CreateInstance(type); + public static object CreateInstance(this Type type) => Activator.CreateInstance(type); + + public static T CreateInstance(this Type type) => (T) type.CreateInstance(); public static bool Implements(this Type type, Type interfaceType) => type.GetInterfaces().Contains(interfaceType); From 7f2202e8692581e4956dc9456dadd24d32d796aa Mon Sep 17 00:00:00 2001 From: Tyrrrz Date: Fri, 23 Oct 2020 21:40:53 +0300 Subject: [PATCH 08/16] Cleanup --- CliFx/ApplicationMetadata.cs | 6 ++- CliFx/Domain/CommandArgumentSchema.cs | 44 ++++++++++++--------- CliFx/Domain/HelpTextWriter.cs | 2 +- CliFx/Internal/Extensions/TypeExtensions.cs | 6 +-- CliFx/Internal/Polyfills.cs | 2 +- 5 files changed, 36 insertions(+), 24 deletions(-) diff --git a/CliFx/ApplicationMetadata.cs b/CliFx/ApplicationMetadata.cs index 5674724..0f778c1 100644 --- a/CliFx/ApplicationMetadata.cs +++ b/CliFx/ApplicationMetadata.cs @@ -28,7 +28,11 @@ /// /// Initializes an instance of . /// - public ApplicationMetadata(string title, string executableName, string versionText, string? description) + public ApplicationMetadata( + string title, + string executableName, + string versionText, + string? description) { Title = title; ExecutableName = executableName; diff --git a/CliFx/Domain/CommandArgumentSchema.cs b/CliFx/Domain/CommandArgumentSchema.cs index c2300e7..d9fab28 100644 --- a/CliFx/Domain/CommandArgumentSchema.cs +++ b/CliFx/Domain/CommandArgumentSchema.cs @@ -28,44 +28,52 @@ namespace CliFx.Domain private Type? TryGetEnumerableArgumentUnderlyingType() => Property != null && Property.PropertyType != typeof(string) - ? Property.PropertyType.GetEnumerableUnderlyingType() + ? Property.PropertyType.TryGetEnumerableUnderlyingType() : null; private object? ConvertScalar(string? value, Type targetType) { try { - // Custom conversion + // Custom conversion (always takes highest priority) if (ConverterType != null) return ConverterType.CreateInstance().ConvertFrom(value!); - // Primitive + // No conversion necessary + if (targetType == typeof(object) || targetType == typeof(string)) + return value; + + // Bool conversion (special case) + if (targetType == typeof(bool)) + return string.IsNullOrWhiteSpace(value) || bool.Parse(value); + + // Primitive conversion var primitiveConverter = PrimitiveConverters.GetValueOrDefault(targetType); - if (primitiveConverter != null) + if (primitiveConverter != null && !string.IsNullOrWhiteSpace(value)) return primitiveConverter(value); - // Enum - if (targetType.IsEnum) + // Enum conversion + if (targetType.IsEnum && !string.IsNullOrWhiteSpace(value)) return Enum.Parse(targetType, value, true); - // Nullable - var nullableUnderlyingType = targetType.GetNullableUnderlyingType(); + // Nullable conversion + var nullableUnderlyingType = targetType.TryGetNullableUnderlyingType(); if (nullableUnderlyingType != null) return !string.IsNullOrWhiteSpace(value) ? ConvertScalar(value, nullableUnderlyingType) : null; - // String-constructible + // String-constructible conversion var stringConstructor = targetType.GetConstructor(new[] {typeof(string)}); if (stringConstructor != null) return stringConstructor.Invoke(new object[] {value!}); - // String-parseable (with format provider) + // String-parseable conversion (with format provider) var parseMethodWithFormatProvider = targetType.TryGetStaticParseMethod(true); if (parseMethodWithFormatProvider != null) return parseMethodWithFormatProvider.Invoke(null, new object[] {value!, FormatProvider}); - // String-parseable (without format provider) + // String-parseable conversion (without format provider) var parseMethod = targetType.TryGetStaticParseMethod(); if (parseMethod != null) return parseMethod.Invoke(null, new object[] {value!}); @@ -78,7 +86,10 @@ namespace CliFx.Domain throw CliFxException.CannotConvertToType(this, value, targetType); } - private object ConvertNonScalar(IReadOnlyList values, Type targetEnumerableType, Type targetElementType) + private object ConvertNonScalar( + IReadOnlyList values, + Type targetEnumerableType, + Type targetElementType) { var array = values .Select(v => ConvertScalar(v, targetElementType)) @@ -133,7 +144,7 @@ namespace CliFx.Domain return Array.Empty(); var underlyingType = - Property.PropertyType.GetNullableUnderlyingType() ?? + Property.PropertyType.TryGetNullableUnderlyingType() ?? Property.PropertyType; // Enum @@ -148,12 +159,9 @@ namespace CliFx.Domain { private static readonly IFormatProvider FormatProvider = CultureInfo.InvariantCulture; - private static readonly IReadOnlyDictionary> PrimitiveConverters = - new Dictionary> + private static readonly IReadOnlyDictionary> PrimitiveConverters = + new Dictionary> { - [typeof(object)] = v => v, - [typeof(string)] = v => v, - [typeof(bool)] = v => string.IsNullOrWhiteSpace(v) || bool.Parse(v), [typeof(char)] = v => v.Single(), [typeof(sbyte)] = v => sbyte.Parse(v, FormatProvider), [typeof(byte)] = v => byte.Parse(v, FormatProvider), diff --git a/CliFx/Domain/HelpTextWriter.cs b/CliFx/Domain/HelpTextWriter.cs index f2b452b..5a05b95 100644 --- a/CliFx/Domain/HelpTextWriter.cs +++ b/CliFx/Domain/HelpTextWriter.cs @@ -385,7 +385,7 @@ namespace CliFx.Domain // Enumerable if (!(defaultValue is string) && defaultValue is IEnumerable defaultValues) { - var elementType = defaultValues.GetType().GetEnumerableUnderlyingType() ?? typeof(object); + var elementType = defaultValues.GetType().TryGetEnumerableUnderlyingType() ?? typeof(object); // If the ToString() method is not overriden, the default value can't be formatted nicely if (!elementType.IsToStringOverriden()) diff --git a/CliFx/Internal/Extensions/TypeExtensions.cs b/CliFx/Internal/Extensions/TypeExtensions.cs index 6a05cdb..4c93a90 100644 --- a/CliFx/Internal/Extensions/TypeExtensions.cs +++ b/CliFx/Internal/Extensions/TypeExtensions.cs @@ -14,9 +14,9 @@ namespace CliFx.Internal.Extensions public static bool Implements(this Type type, Type interfaceType) => type.GetInterfaces().Contains(interfaceType); - public static Type? GetNullableUnderlyingType(this Type type) => Nullable.GetUnderlyingType(type); + public static Type? TryGetNullableUnderlyingType(this Type type) => Nullable.GetUnderlyingType(type); - public static Type? GetEnumerableUnderlyingType(this Type type) + public static Type? TryGetEnumerableUnderlyingType(this Type type) { if (type.IsPrimitive) return null; @@ -29,7 +29,7 @@ namespace CliFx.Internal.Extensions return type .GetInterfaces() - .Select(GetEnumerableUnderlyingType) + .Select(TryGetEnumerableUnderlyingType) .Where(t => t != null) .OrderByDescending(t => t != typeof(object)) // prioritize more specific types .FirstOrDefault(); diff --git a/CliFx/Internal/Polyfills.cs b/CliFx/Internal/Polyfills.cs index 10f6987..daa3965 100644 --- a/CliFx/Internal/Polyfills.cs +++ b/CliFx/Internal/Polyfills.cs @@ -34,7 +34,7 @@ namespace System.Collections.Generic } public static TValue GetValueOrDefault(this IReadOnlyDictionary dic, TKey key) => - dic.TryGetValue(key, out var result) ? result! : default!; + dic.TryGetValue(key!, out var result) ? result! : default!; } } From f765af60613bf99e0d39b6132f65b2e605d172bf Mon Sep 17 00:00:00 2001 From: Tyrrrz Date: Fri, 23 Oct 2020 22:03:20 +0300 Subject: [PATCH 09/16] Update readme with info about custom converters --- Readme.md | 54 +++++++++++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 53 insertions(+), 1 deletion(-) diff --git a/Readme.md b/Readme.md index b1c07b0..069619e 100644 --- a/Readme.md +++ b/Readme.md @@ -249,6 +249,7 @@ Parameters and options can have different underlying types: - String-initializable types - Types with a constructor that accepts a single `string` parameter (`FileInfo`, `DirectoryInfo`, etc.) - Types with a static method `Parse` that accepts a single `string` parameter (and optionally `IFormatProvider`) +- Any other type if a custom converter is specified - Nullable versions of all above types (`decimal?`, `TimeSpan?`, etc.) - Collections of all above types - Array types (`T[]`) @@ -257,7 +258,58 @@ Parameters and options can have different underlying types: When defining a parameter of an enumerable type, keep in mind that it has to be the only such parameter and it must be the last in order. Options, on the other hand, don't have this limitation. -Example command with an array-backed parameter: +- Example command with a custom converter: + +```c# +// Maps 2D vectors from AxB notation +public class VectorConverter : IArgumentValueConverter +{ + public object ConvertFrom(string value) + { + var components = value.Split('x', 'X', ';'); + var x = int.Parse(components[0], CultureInfo.InvariantCulture); + var y = int.Parse(components[1], CultureInfo.InvariantCulture); + return new Vector2(x, y); + } +} + +[Command] +public class SurfaceCalculatorCommand : ICommand +{ + // Custom converter is used to map raw argument values + [CommandParameter(0, Converter = typeof(VectorConverter))] + public Vector2 PointA { get; set; } + + [CommandParameter(1, Converter = typeof(VectorConverter))] + public Vector2 PointB { get; set; } + + [CommandParameter(2, Converter = typeof(VectorConverter))] + public Vector2 PointC { get; set; } + + public ValueTask ExecuteAsync(IConsole console) + { + var a = (PointB - PointA).Length(); + var b = (PointC - PointB).Length(); + var c = (PointA - PointC).Length(); + + // Heron's formula + var p = (a + b + c) / 2; + var surface = Math.Sqrt(p * (p - a) * (p - b) * (p - c)); + + console.Output.WriteLine($"Triangle surface area: {surface}"); + + return default; + } +} +``` + +```sh +> myapp.exe 0x0 0x18 24x0 + +Triangle surface area: 216 +``` + +- Example command with an array-backed parameter: ```c# [Command] From d0d024c42752f0148bfdb20d9b4922f769013248 Mon Sep 17 00:00:00 2001 From: Tyrrrz Date: Fri, 23 Oct 2020 22:57:48 +0300 Subject: [PATCH 10/16] Improve stack trace parsing --- CliFx/IConsole.cs | 71 ++++++++++------- CliFx/Internal/StackFrame.cs | 143 ++++++++++++++++------------------- 2 files changed, 108 insertions(+), 106 deletions(-) diff --git a/CliFx/IConsole.cs b/CliFx/IConsole.cs index 2adab83..c05d151 100644 --- a/CliFx/IConsole.cs +++ b/CliFx/IConsole.cs @@ -160,13 +160,11 @@ namespace CliFx foreach (var stackFrame in StackFrame.ParseMany(exception.StackTrace)) { console.Error.Write(indentationShared + indentationLocal); - - // "at" - console.Error.Write(stackFrame.Prefix + " "); + console.Error.Write("at "); // "CliFx.Demo.Commands.BookAddCommand." console.WithForegroundColor(ConsoleColor.DarkGray, () => - console.Error.Write(stackFrame.ParentType) + console.Error.Write(stackFrame.ParentType + ".") ); // "ExecuteAsync" @@ -176,47 +174,62 @@ namespace CliFx console.Error.Write("("); - foreach (var parameter in stackFrame.Parameters) + for (var i = 0; i < stackFrame.Parameters.Count; i++) { + var parameter = stackFrame.Parameters[i]; + // "IConsole" console.WithForegroundColor(ConsoleColor.Blue, () => console.Error.Write(parameter.Type) ); - // "console" - console.WithForegroundColor(ConsoleColor.White, () => - console.Error.Write(parameter.Name) - ); - - // ", ' - if (parameter.Separator != null) + if (!string.IsNullOrWhiteSpace(parameter.Name)) { - console.Error.Write(parameter.Separator); + console.Error.Write(" "); + + // "console" + console.WithForegroundColor(ConsoleColor.White, () => + console.Error.Write(parameter.Name) + ); + } + + // Separator + if (stackFrame.Parameters.Count > 1 && i < stackFrame.Parameters.Count - 1) + { + console.Error.Write(", "); } } console.Error.Write(") "); - // "in" - console.Error.Write(stackFrame.LocationPrefix); - console.Error.Write("\n" + indentationShared + indentationLocal + indentationLocal); + // Location + if (!string.IsNullOrWhiteSpace(stackFrame.FilePath)) + { + console.Error.Write("in"); + console.Error.Write("\n" + indentationShared + indentationLocal + indentationLocal); - // "E:\Projects\Softdev\CliFx\CliFx.Demo\Commands\" - console.WithForegroundColor(ConsoleColor.DarkGray, () => - console.Error.Write(stackFrame.DirectoryPath) - ); + // "E:\Projects\Softdev\CliFx\CliFx.Demo\Commands\" + var stackFrameDirectoryPath = Path.GetDirectoryName(stackFrame.FilePath); + console.WithForegroundColor(ConsoleColor.DarkGray, () => + console.Error.Write(stackFrameDirectoryPath + Path.DirectorySeparatorChar) + ); - // "BookAddCommand.cs" - console.WithForegroundColor(ConsoleColor.Yellow, () => - console.Error.Write(stackFrame.FileName) - ); + // "BookAddCommand.cs" + var stackFrameFileName = Path.GetFileName(stackFrame.FilePath); + console.WithForegroundColor(ConsoleColor.Yellow, () => + console.Error.Write(stackFrameFileName) + ); - console.Error.Write(":"); + if (!string.IsNullOrWhiteSpace(stackFrame.LineNumber)) + { + console.Error.Write(":"); - // "35" - console.WithForegroundColor(ConsoleColor.Blue, () => - console.Error.Write(stackFrame.LineNumber) - ); + // "35" + console.WithForegroundColor(ConsoleColor.Blue, () => + console.Error.Write(stackFrame.LineNumber) + ); + } + } console.Error.WriteLine(); } diff --git a/CliFx/Internal/StackFrame.cs b/CliFx/Internal/StackFrame.cs index 2336185..37a2f8f 100644 --- a/CliFx/Internal/StackFrame.cs +++ b/CliFx/Internal/StackFrame.cs @@ -10,121 +10,110 @@ namespace CliFx.Internal { public string Type { get; } - public string Name { get; } + public string? Name { get; } - public string? Separator { get; } - - public StackFrameParameter( - string type, - string name, - string? separator) + public StackFrameParameter(string type, string? name) { Type = type; Name = name; - Separator = separator; } } internal partial class StackFrame { - public string Prefix { get; } - public string ParentType { get; } public string MethodName { get; } public IReadOnlyList Parameters { get; } - public string LocationPrefix { get; } + public string? FilePath { get; } - public string DirectoryPath { get; } - - public string FileName { get; } - - public string LineNumber { get; } + public string? LineNumber { get; } public StackFrame( - string prefix, string parentType, string methodName, IReadOnlyList parameters, - string locationPrefix, - string directoryPath, - string fileName, - string lineNumber) + string? filePath, + string? lineNumber) { - Prefix = prefix; ParentType = parentType; MethodName = methodName; Parameters = parameters; - LocationPrefix = locationPrefix; - DirectoryPath = directoryPath; - FileName = fileName; + FilePath = filePath; LineNumber = lineNumber; } } internal partial class StackFrame { - private static readonly Regex MethodMatcher = - new Regex(@"(?\S+) (?.*?)(?[^\.]+)\("); + private const string Space = @"[\x20\t]"; + private const string NotSpace = @"[^\x20\t]"; - private static readonly Regex ParameterMatcher = - new Regex(@"(?.+? )(?.+?)(?:(?, )|\))"); + // Taken from https://github.com/atifaziz/StackTraceParser + private static readonly Regex Pattern = new Regex(@" + ^ + " + Space + @"* + \w+ " + Space + @"+ + (? + (? " + NotSpace + @"+ ) \. + (? " + NotSpace + @"+? ) " + Space + @"* + (? \( ( " + Space + @"* \) + | (? .+?) " + Space + @"+ (? .+?) + (, " + Space + @"* (? .+?) " + Space + @"+ (? .+?) )* \) ) ) + ( " + Space + @"+ + ( # Microsoft .NET stack traces + \w+ " + Space + @"+ + (? ( [a-z] \: # Windows rooted path starting with a drive letter + | / ) # *nix rooted path starting with a forward-slash + .+? ) + \: \w+ " + Space + @"+ + (? [0-9]+ ) \p{P}? + | # Mono stack traces + \[0x[0-9a-f]+\] " + Space + @"+ \w+ " + Space + @"+ + <(? [^>]+ )> + :(? [0-9]+ ) + ) + )? + ) + \s* + $", + RegexOptions.IgnoreCase | + RegexOptions.Multiline | + RegexOptions.ExplicitCapture | + RegexOptions.CultureInvariant | + RegexOptions.IgnorePatternWhitespace, + TimeSpan.FromSeconds(5) + ); - private static readonly Regex FileMatcher = - new Regex(@"(?\S+?) (?.*?)(?[^\\/]+?(?:\.\w*)?):[^:]+? (?\d+).*"); - - public static StackFrame Parse(string stackFrame) + public static IEnumerable ParseMany(string stackTrace) { - var methodMatch = MethodMatcher.Match(stackFrame); + var matches = Pattern.Matches(stackTrace).Cast().ToArray(); - var parameterMatches = ParameterMatcher.Matches(stackFrame, methodMatch.Index + methodMatch.Length) - .Cast() - .ToArray(); - - var fileMatch = FileMatcher.Match( - stackFrame, - parameterMatches.Length switch - { - 0 => methodMatch.Index + methodMatch.Length + 1, - _ => parameterMatches[parameterMatches.Length - 1].Index + - parameterMatches[parameterMatches.Length - 1].Length - } - ); - - // Ensure everything was parsed successfully - var isSuccessful = - methodMatch.Success && - parameterMatches.All(m => m.Success) && - fileMatch.Success && - fileMatch.Index + fileMatch.Length == stackFrame.Length; - - if (!isSuccessful) + // Ensure success + var lastMatch = matches.LastOrDefault(); + if (lastMatch == null || + lastMatch.Index + lastMatch.Length < stackTrace.Length) { - throw new FormatException("Failed to parse stack frame."); + throw new FormatException("Could not parse stack trace."); } - var parameters = parameterMatches - .Select(match => new StackFrameParameter( - match.Groups["type"].Value, - match.Groups["name"].Value, - match.Groups["separator"].Value.NullIfWhiteSpace() - )).ToArray(); - - return new StackFrame( - methodMatch.Groups["prefix"].Value, - methodMatch.Groups["name"].Value, - methodMatch.Groups["methodName"].Value, - parameters, - fileMatch.Groups["prefix"].Value, - fileMatch.Groups["path"].Value, - fileMatch.Groups["file"].Value, - fileMatch.Groups["line"].Value - ); + return from m in matches + select m.Groups + into groups + let pt = groups["pt"].Captures + let pn = groups["pn"].Captures + select new StackFrame( + groups["type"].Value, + groups["method"].Value, + ( + from i in Enumerable.Range(0, pt.Count) + select new StackFrameParameter(pt[i].Value, pn[i].Value.NullIfWhiteSpace()) + ).ToArray(), + groups["file"].Value.NullIfWhiteSpace(), + groups["line"].Value.NullIfWhiteSpace() + ); } - - public static IReadOnlyList ParseMany(string stackTrace) => - stackTrace.Split('\n', StringSplitOptions.RemoveEmptyEntries).Select(Parse).ToArray(); } } \ No newline at end of file From 9557d386e206b9ff86c391c4cc1bc852e4a28af8 Mon Sep 17 00:00:00 2001 From: Tyrrrz Date: Fri, 23 Oct 2020 23:00:29 +0300 Subject: [PATCH 11/16] Better success check in stack trace parsing --- CliFx/Internal/StackFrame.cs | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/CliFx/Internal/StackFrame.cs b/CliFx/Internal/StackFrame.cs index 37a2f8f..3163db9 100644 --- a/CliFx/Internal/StackFrame.cs +++ b/CliFx/Internal/StackFrame.cs @@ -91,10 +91,12 @@ namespace CliFx.Internal { var matches = Pattern.Matches(stackTrace).Cast().ToArray(); - // Ensure success - var lastMatch = matches.LastOrDefault(); - if (lastMatch == null || - lastMatch.Index + lastMatch.Length < stackTrace.Length) + // Ensure success (all lines should be parsed) + var isSuccess = + matches.Length == + stackTrace.Split('\n', StringSplitOptions.RemoveEmptyEntries).Length; + + if (!isSuccess) { throw new FormatException("Could not parse stack trace."); } From 3abdfb1acfcb0bb8f9bdaec5ef9c0a8e9a507217 Mon Sep 17 00:00:00 2001 From: Tyrrrz Date: Fri, 23 Oct 2020 23:36:36 +0300 Subject: [PATCH 12/16] Improve child command usage info in help text --- CliFx.Tests/HelpTextSpecs.cs | 27 +++++++ CliFx/Domain/HelpTextWriter.cs | 134 ++++++++++++++++----------------- CliFx/IConsole.cs | 12 +-- 3 files changed, 100 insertions(+), 73 deletions(-) diff --git a/CliFx.Tests/HelpTextSpecs.cs b/CliFx.Tests/HelpTextSpecs.cs index 0939236..722eabe 100644 --- a/CliFx.Tests/HelpTextSpecs.cs +++ b/CliFx.Tests/HelpTextSpecs.cs @@ -64,6 +64,33 @@ namespace CliFx.Tests _output.WriteLine(stdOut.GetString()); } + [Fact] + public async Task Help_text_shows_usage_format_which_lists_available_sub_commands() + { + // Arrange + var (console, stdOut, _) = VirtualConsole.CreateBuffered(); + + var application = new CliApplicationBuilder() + .AddCommand() + .AddCommand() + .AddCommand() + .UseConsole(console) + .Build(); + + // Act + var exitCode = await application.RunAsync(new[] {"--help"}); + + // Assert + exitCode.Should().Be(0); + stdOut.GetString().Should().ContainAll( + "Usage", + "... named", + "... named sub" + ); + + _output.WriteLine(stdOut.GetString()); + } + [Fact] public async Task Help_text_shows_all_valid_values_for_enum_arguments() { diff --git a/CliFx/Domain/HelpTextWriter.cs b/CliFx/Domain/HelpTextWriter.cs index 5a05b95..b7528c6 100644 --- a/CliFx/Domain/HelpTextWriter.cs +++ b/CliFx/Domain/HelpTextWriter.cs @@ -105,6 +105,54 @@ namespace CliFx.Domain WriteLine(); } + private void WriteCommandUsageLineItem(CommandSchema command, bool showChildCommandsPlaceholder) + { + // Command name + if (!string.IsNullOrWhiteSpace(command.Name)) + { + Write(ConsoleColor.Cyan, command.Name); + Write(' '); + } + + // Child command placeholder + if (showChildCommandsPlaceholder) + { + Write(ConsoleColor.Cyan, "[command]"); + Write(' '); + } + + // Parameters + foreach (var parameter in command.Parameters) + { + Write(parameter.IsScalar + ? $"<{parameter.Name}>" + : $"<{parameter.Name}...>" + ); + Write(' '); + } + + // Required options + foreach (var option in command.Options.Where(o => o.IsRequired)) + { + Write(ConsoleColor.White, !string.IsNullOrWhiteSpace(option.Name) + ? $"--{option.Name}" + : $"-{option.ShortName}" + ); + Write(' '); + + Write(option.IsScalar + ? "" + : "" + ); + Write(' '); + } + + // Options placeholder + Write(ConsoleColor.White, "[options]"); + + WriteLine(); + } + private void WriteCommandUsage( CommandSchema command, IReadOnlyList childCommands) @@ -114,76 +162,28 @@ namespace CliFx.Domain WriteHeader("Usage"); - WriteCommandUsageLineItem(command, childCommands.Count > 0); + // Exe name + WriteHorizontalMargin(); + Write(_metadata.ExecutableName); + Write(' '); - if (!IsEmpty) + // Current command usage + WriteCommandUsageLineItem(command, childCommands.Any()); + + // Sub commands usage + if (childCommands.Any()) + { WriteVerticalMargin(); - foreach (var childCommand in childCommands) - { - WriteCommandUsageLineItem(childCommand, compactCommand: false, size: 4); + foreach (var childCommand in childCommands) + { + WriteHorizontalMargin(); + Write("... "); + WriteCommandUsageLineItem(childCommand, false); + } } } - private void WriteCommandUsageLineItem( - CommandSchema command, - bool compactCommand = true, - int size = 2) - { - WriteHorizontalMargin(size); - if (compactCommand) - { - // Exe name - Write(_metadata.ExecutableName); - Write(' '); - - } - // Command name - if (!string.IsNullOrWhiteSpace(command.Name)) - { - // this is fragile, because we rely that subcommand name consists - // of all required tokens - Write(ConsoleColor.Cyan, command.Name); - } - // Child command placeholder - if (compactCommand) - { - Write(' '); - Write(ConsoleColor.Cyan, "[command]"); - } - // Parameters - foreach (var parameter in command.Parameters) - { - Write(' '); - Write(parameter.IsScalar - ? $"<{parameter.Name}>" - : $"<{parameter.Name}...>" - ); - } - - // Required options - foreach (var option in command.Options.Where(o => o.IsRequired)) - { - Write(' '); - Write(ConsoleColor.White, !string.IsNullOrWhiteSpace(option.Name) - ? $"--{option.Name}" - : $"-{option.ShortName}" - ); - - Write(' '); - Write(option.IsScalar - ? "" - : "" - ); - } - - // Options placeholder - Write(' '); - Write(ConsoleColor.White, "[options]"); - - WriteLine(); - } - private void WriteCommandParameters(CommandSchema command) { if (!command.Parameters.Any()) @@ -285,7 +285,7 @@ namespace CliFx.Domain if (!option.IsRequired) { var defaultValue = argumentDefaultValues.GetValueOrDefault(option); - var defaultValueFormatted = FormatDefaultValue(defaultValue); + var defaultValueFormatted = TryFormatDefaultValue(defaultValue); if (defaultValueFormatted != null) { Write($"Default: {defaultValueFormatted}."); @@ -377,7 +377,7 @@ namespace CliFx.Domain private static string FormatValidValues(IReadOnlyList values) => values.Select(v => v.Quote()).JoinToString(", "); - private static string? FormatDefaultValue(object? defaultValue) + private static string? TryFormatDefaultValue(object? defaultValue) { if (defaultValue == null) return null; @@ -408,4 +408,4 @@ namespace CliFx.Domain } } } -} +} \ No newline at end of file diff --git a/CliFx/IConsole.cs b/CliFx/IConsole.cs index c05d151..72a2153 100644 --- a/CliFx/IConsole.cs +++ b/CliFx/IConsole.cs @@ -178,6 +178,12 @@ namespace CliFx { var parameter = stackFrame.Parameters[i]; + // Separator + if (i > 0) + { + console.Error.Write(", "); + } + // "IConsole" console.WithForegroundColor(ConsoleColor.Blue, () => console.Error.Write(parameter.Type) @@ -192,12 +198,6 @@ namespace CliFx console.Error.Write(parameter.Name) ); } - - // Separator - if (stackFrame.Parameters.Count > 1 && i < stackFrame.Parameters.Count - 1) - { - console.Error.Write(", "); - } } console.Error.Write(") "); From 60a3b26fd147d29ec94927d3bdb73433bd35c347 Mon Sep 17 00:00:00 2001 From: Tyrrrz Date: Fri, 23 Oct 2020 23:46:57 +0300 Subject: [PATCH 13/16] Update version --- Changelog.md | 6 ++++++ CliFx.props | 2 +- 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/Changelog.md b/Changelog.md index ac61673..39d72c2 100644 --- a/Changelog.md +++ b/Changelog.md @@ -1,3 +1,9 @@ +### v1.5 (23-Oct-2020) + +- Added pretty-printing for unhandled exceptions thrown from within the application. This makes the errors easier to parse visually and should help in troubleshooting. This change does not affect `CommandException`, as it already has special treatment. (Thanks [@Mårten Åsberg](https://github.com/89netraM)) +- Added support for custom value converters. You can now create a type that implements `CliFx.IArgumentValueConverter` and specify it as a converter for your parameters or options via the `Converter` named property. This should enable conversion between raw argument values and custom types which are not string-initializable. (Thanks [@Oleksandr Shustov](https://github.com/AlexandrShustov)) +- Improved help text so that it also shows minimal usage examples for child and descendant commands, besides the actual command it was requested on. This should improve user experience for applications with many nested commands. (Thanks [@Nikiforov Alexey](https://github.com/NikiforovAll)) + ### v1.4 (20-Aug-2020) - Added `VirtualConsole.CreateBuffered()` method to simplify test setup when using in-memory backing stores for output and error streams. Please refer to the readme for updated recommendations on how to test applications built with CliFx. diff --git a/CliFx.props b/CliFx.props index 07dbc94..017e2b6 100644 --- a/CliFx.props +++ b/CliFx.props @@ -1,7 +1,7 @@ - 1.4 + 1.5 Tyrrrz Copyright (C) Alexey Golub latest From 11579f11b13832b5250c089721d16ce43168780c Mon Sep 17 00:00:00 2001 From: Tyrrrz Date: Sun, 25 Oct 2020 01:55:12 +0300 Subject: [PATCH 14/16] Update readme --- Readme.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/Readme.md b/Readme.md index 069619e..bfd80f2 100644 --- a/Readme.md +++ b/Readme.md @@ -6,6 +6,8 @@ [![Downloads](https://img.shields.io/nuget/dt/CliFx.svg)](https://nuget.org/packages/CliFx) [![Donate](https://img.shields.io/badge/donate-$$$-purple.svg)](https://tyrrrz.me/donate) +**Project status: active**. + CliFx is a simple to use, yet powerful framework for building command line applications. Its primary goal is to completely take over the user input layer, letting you forget about the infrastructure and instead focus on writing your application. This framework uses a declarative class-first approach for defining commands, avoiding excessive boilerplate code and complex configurations. ## Download From 6a378ad946312153459fbdcfe7e7dc176511b591 Mon Sep 17 00:00:00 2001 From: Tyrrrz Date: Tue, 27 Oct 2020 17:20:07 +0200 Subject: [PATCH 15/16] Update nuget packages --- CliFx.Analyzers.Tests/CliFx.Analyzers.Tests.csproj | 2 +- CliFx.Demo/CliFx.Demo.csproj | 2 +- CliFx.Tests/CliFx.Tests.csproj | 4 ++-- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/CliFx.Analyzers.Tests/CliFx.Analyzers.Tests.csproj b/CliFx.Analyzers.Tests/CliFx.Analyzers.Tests.csproj index c7d9515..80e6260 100644 --- a/CliFx.Analyzers.Tests/CliFx.Analyzers.Tests.csproj +++ b/CliFx.Analyzers.Tests/CliFx.Analyzers.Tests.csproj @@ -12,7 +12,7 @@ - + diff --git a/CliFx.Demo/CliFx.Demo.csproj b/CliFx.Demo/CliFx.Demo.csproj index 4ac93f9..3166c75 100644 --- a/CliFx.Demo/CliFx.Demo.csproj +++ b/CliFx.Demo/CliFx.Demo.csproj @@ -7,7 +7,7 @@ - + diff --git a/CliFx.Tests/CliFx.Tests.csproj b/CliFx.Tests/CliFx.Tests.csproj index cfd4d45..5bdae4c 100644 --- a/CliFx.Tests/CliFx.Tests.csproj +++ b/CliFx.Tests/CliFx.Tests.csproj @@ -15,9 +15,9 @@ - + - + From fec6850c397df5dad74553ec48ceb0b16c287592 Mon Sep 17 00:00:00 2001 From: Rene Escalante Date: Thu, 29 Oct 2020 12:31:03 -0600 Subject: [PATCH 16/16] Change format for the command section in help text (#83) --- CliFx.Tests/HelpTextSpecs.cs | 12 ++++++---- CliFx/Domain/HelpTextWriter.cs | 43 ++++++++++++---------------------- 2 files changed, 22 insertions(+), 33 deletions(-) diff --git a/CliFx.Tests/HelpTextSpecs.cs b/CliFx.Tests/HelpTextSpecs.cs index 722eabe..e254a7d 100644 --- a/CliFx.Tests/HelpTextSpecs.cs +++ b/CliFx.Tests/HelpTextSpecs.cs @@ -65,7 +65,7 @@ namespace CliFx.Tests } [Fact] - public async Task Help_text_shows_usage_format_which_lists_available_sub_commands() + public async Task Help_text_shows_commands_list_with_description_and_usage_info() { // Arrange var (console, stdOut, _) = VirtualConsole.CreateBuffered(); @@ -78,14 +78,16 @@ namespace CliFx.Tests .Build(); // Act - var exitCode = await application.RunAsync(new[] {"--help"}); + var exitCode = await application.RunAsync(new[] { "--help" }); // Assert exitCode.Should().Be(0); stdOut.GetString().Should().ContainAll( - "Usage", - "... named", - "... named sub" + "Commands", + "Named command description", + "named [options]", + "Named sub command description", + "named sub [options]" ); _output.WriteLine(stdOut.GetString()); diff --git a/CliFx/Domain/HelpTextWriter.cs b/CliFx/Domain/HelpTextWriter.cs index b7528c6..deb0808 100644 --- a/CliFx/Domain/HelpTextWriter.cs +++ b/CliFx/Domain/HelpTextWriter.cs @@ -169,19 +169,6 @@ namespace CliFx.Domain // Current command usage WriteCommandUsageLineItem(command, childCommands.Any()); - - // Sub commands usage - if (childCommands.Any()) - { - WriteVerticalMargin(); - - foreach (var childCommand in childCommands) - { - WriteHorizontalMargin(); - Write("... "); - WriteCommandUsageLineItem(childCommand, false); - } - } } private void WriteCommandParameters(CommandSchema command) @@ -298,9 +285,9 @@ namespace CliFx.Domain private void WriteCommandChildren( CommandSchema command, - IReadOnlyList childCommands) + IReadOnlyList descendantCommands) { - if (!childCommands.Any()) + if (!descendantCommands.Any()) return; if (!IsEmpty) @@ -308,28 +295,29 @@ namespace CliFx.Domain WriteHeader("Commands"); - foreach (var childCommand in childCommands) + foreach (var descendantCommand in descendantCommands) { var relativeCommandName = !string.IsNullOrWhiteSpace(command.Name) - ? childCommand.Name!.Substring(command.Name.Length).Trim() - : childCommand.Name!; - - // Name - WriteHorizontalMargin(); - Write(ConsoleColor.Cyan, relativeCommandName); + ? descendantCommand.Name!.Substring(command.Name.Length).Trim() + : descendantCommand.Name!; // Description - if (!string.IsNullOrWhiteSpace(childCommand.Description)) + + if (!string.IsNullOrWhiteSpace(descendantCommand.Description)) { - WriteColumnMargin(); - Write(childCommand.Description); + WriteHorizontalMargin(); + Write(descendantCommand.Description); + WriteVerticalMargin(); } + // Name + WriteHorizontalMargin(4); + WriteCommandUsageLineItem(descendantCommand, false); + WriteLine(); } // Child command help tip - WriteVerticalMargin(); Write("You can run `"); Write(_metadata.ExecutableName); @@ -356,7 +344,6 @@ namespace CliFx.Domain IReadOnlyDictionary defaultValues) { var commandName = command.Name; - var childCommands = root.GetChildCommands(commandName); var descendantCommands = root.GetDescendantCommands(commandName); _console.ResetColor(); @@ -368,7 +355,7 @@ namespace CliFx.Domain WriteCommandUsage(command, descendantCommands); WriteCommandParameters(command); WriteCommandOptions(command, defaultValues); - WriteCommandChildren(command, childCommands); + WriteCommandChildren(command, descendantCommands); } }