From d6da687170ccf0d43fff135d0f608f0871b69342 Mon Sep 17 00:00:00 2001 From: Tyrrrz Date: Wed, 18 Nov 2020 18:00:43 +0200 Subject: [PATCH] Refactor recent PRs --- CliFx.Demo/Program.cs | 2 +- CliFx/ArgumentValueValidator.cs | 49 +++++++++++++++--- CliFx/Attributes/CommandArgumentAttribute.cs | 28 ++++++++++ CliFx/Attributes/CommandOptionAttribute.cs | 18 +------ CliFx/Attributes/CommandParameterAttribute.cs | 18 +------ CliFx/Domain/CommandArgumentSchema.cs | 51 ++++++++++--------- CliFx/Domain/CommandOptionSchema.cs | 28 +++++++--- CliFx/Domain/CommandParameterSchema.cs | 4 +- CliFx/Domain/HelpTextWriter.cs | 11 ++-- CliFx/Domain/RootSchema.cs | 4 +- CliFx/Exceptions/CliFxException.cs | 35 +++++++++++-- CliFx/IArgumentValueValidator.cs | 7 --- .../Extensions/CollectionExtensions.cs | 7 --- CliFx/ValidationResult.cs | 30 ----------- 14 files changed, 159 insertions(+), 133 deletions(-) create mode 100644 CliFx/Attributes/CommandArgumentAttribute.cs delete mode 100644 CliFx/IArgumentValueValidator.cs delete mode 100644 CliFx/ValidationResult.cs diff --git a/CliFx.Demo/Program.cs b/CliFx.Demo/Program.cs index 4bb0b64..8c3dbd7 100644 --- a/CliFx.Demo/Program.cs +++ b/CliFx.Demo/Program.cs @@ -28,7 +28,7 @@ namespace CliFx.Demo public static async Task Main() => await new CliApplicationBuilder() .AddCommandsFromThisAssembly() - .UseTypeActivator(GetServiceProvider().GetService) + .UseTypeActivator(GetServiceProvider().GetRequiredService) .Build() .RunAsync(); } diff --git a/CliFx/ArgumentValueValidator.cs b/CliFx/ArgumentValueValidator.cs index 69797bf..d0cc3eb 100644 --- a/CliFx/ArgumentValueValidator.cs +++ b/CliFx/ArgumentValueValidator.cs @@ -1,18 +1,55 @@ namespace CliFx { /// - /// A base type for custom validators. + /// Represents a result of a validation. + /// + public partial class ValidationResult + { + /// + /// Whether validation was successful. + /// + public bool IsValid => ErrorMessage == null; + + /// + /// If validation has failed, contains the associated error, otherwise null. + /// + public string? ErrorMessage { get; } + + /// + /// Initializes an instance of . + /// + public ValidationResult(string? errorMessage = null) => + ErrorMessage = errorMessage; + } + + public partial class ValidationResult + { + /// + /// Creates successful result, meaning that the validation has passed. + /// + public static ValidationResult Ok() => new ValidationResult(); + + /// + /// Creates an error result, meaning that the validation has failed. + /// + public static ValidationResult Error(string message) => new ValidationResult(message); + } + + internal interface IArgumentValueValidator + { + ValidationResult Validate(object value); + } + + /// + /// A base type for custom argument validators. /// public abstract class ArgumentValueValidator : IArgumentValueValidator { /// - /// Your validation logic have to be implemented in this method. + /// Validates the value. /// public abstract ValidationResult Validate(T value); - /// - /// Non-generic method, will be called by the framework. - /// - public ValidationResult Validate(object value) => Validate((T) value); + ValidationResult IArgumentValueValidator.Validate(object value) => Validate((T) value); } } diff --git a/CliFx/Attributes/CommandArgumentAttribute.cs b/CliFx/Attributes/CommandArgumentAttribute.cs new file mode 100644 index 0000000..ef275a9 --- /dev/null +++ b/CliFx/Attributes/CommandArgumentAttribute.cs @@ -0,0 +1,28 @@ +using System; + +namespace CliFx.Attributes +{ + /// + /// Properties shared by parameter and option arguments. + /// + [AttributeUsage(AttributeTargets.Property)] + public abstract class CommandArgumentAttribute : Attribute + { + /// + /// Option description, which is used in help text. + /// + public string? Description { get; set; } + + /// + /// Type of converter to use when mapping the argument value. + /// Converter must implement . + /// + public Type? Converter { get; set; } + + /// + /// Types of validators to use when mapping the argument value. + /// Validators must derive from . + /// + public Type[] Validators { get; set; } = Array.Empty(); + } +} \ No newline at end of file diff --git a/CliFx/Attributes/CommandOptionAttribute.cs b/CliFx/Attributes/CommandOptionAttribute.cs index 2f61471..efddfea 100644 --- a/CliFx/Attributes/CommandOptionAttribute.cs +++ b/CliFx/Attributes/CommandOptionAttribute.cs @@ -6,7 +6,7 @@ namespace CliFx.Attributes /// Annotates a property that defines a command option. /// [AttributeUsage(AttributeTargets.Property)] - public class CommandOptionAttribute : Attribute + public class CommandOptionAttribute : CommandArgumentAttribute { /// /// Option name (must be longer than a single character). @@ -27,27 +27,11 @@ namespace CliFx.Attributes /// public bool IsRequired { get; set; } - /// - /// Option description, which is used in help text. - /// - public string? Description { get; set; } - /// /// Environment variable that will be used as fallback if no option value is specified. /// public string? EnvironmentVariableName { get; set; } - /// - /// Type of converter to use when mapping the argument value. - /// Converter must implement . - /// - public Type? Converter { get; set; } - - /// - /// Type of a converter to use for the option value evaluating. - /// - public Type[]? Validators { get; set; } - /// /// Initializes an instance of . /// diff --git a/CliFx/Attributes/CommandParameterAttribute.cs b/CliFx/Attributes/CommandParameterAttribute.cs index 5d15d29..85624e4 100644 --- a/CliFx/Attributes/CommandParameterAttribute.cs +++ b/CliFx/Attributes/CommandParameterAttribute.cs @@ -6,7 +6,7 @@ namespace CliFx.Attributes /// Annotates a property that defines a command parameter. /// [AttributeUsage(AttributeTargets.Property)] - public class CommandParameterAttribute : Attribute + public class CommandParameterAttribute : CommandArgumentAttribute { /// /// Order of this parameter compared to other parameters. @@ -21,22 +21,6 @@ namespace CliFx.Attributes /// public string? Name { get; set; } - /// - /// Parameter description, which is used in help text. - /// - public string? Description { get; set; } - - /// - /// Type of converter to use when mapping the argument value. - /// Converter must implement . - /// - public Type? Converter { get; set; } - - /// - /// Type of a converter to use for the option value evaluating. - /// - public Type[]? Validators { get; set; } - /// /// Initializes an instance of . /// diff --git a/CliFx/Domain/CommandArgumentSchema.cs b/CliFx/Domain/CommandArgumentSchema.cs index 9f524a6..ae6f37c 100644 --- a/CliFx/Domain/CommandArgumentSchema.cs +++ b/CliFx/Domain/CommandArgumentSchema.cs @@ -19,14 +19,18 @@ namespace CliFx.Domain public Type? ConverterType { get; } - public readonly Type[]? ValidatorTypes; + public Type[] ValidatorTypes { get; } - protected CommandArgumentSchema(PropertyInfo? property, string? description, Type? converterType = null, Type[]? validators = null) + protected CommandArgumentSchema( + PropertyInfo? property, + string? description, + Type? converterType, + Type[] validatorTypes) { Property = property; Description = description; ConverterType = converterType; - ValidatorTypes = validators; + ValidatorTypes = validatorTypes; } private Type? TryGetEnumerableArgumentUnderlyingType() => @@ -135,12 +139,28 @@ namespace CliFx.Domain } } + private void Validate(object? value) + { + if (value is null) + return; + + var validators = ValidatorTypes + .Select(t => t.CreateInstance()) + .ToArray(); + + var failedValidations = validators + .Select(v => v.Validate(value)) + .Where(result => !result.IsValid) + .ToArray(); + + if (failedValidations.Any()) + throw CliFxException.ValidationFailed(this, failedValidations); + } + public void BindOn(ICommand command, IReadOnlyList values) { var value = Convert(values); - - if (ValidatorTypes.NotEmpty()) - Validate(value); + Validate(value); Property?.SetValue(command, value); } @@ -163,25 +183,6 @@ namespace CliFx.Domain return Array.Empty(); } - - private void Validate(object? value) - { - if (value is null) - return; - - var failed = new List(); - foreach (var validator in ValidatorTypes!) - { - var result = validator.CreateInstance().Validate(value!); - if (result.IsValid) - continue; - - failed.Add(result); - } - - if (failed.NotEmpty()) - throw CliFxException.ValueValidationFailed(this, failed.Select(x => x.ErrorMessage!)); - } } internal partial class CommandArgumentSchema diff --git a/CliFx/Domain/CommandOptionSchema.cs b/CliFx/Domain/CommandOptionSchema.cs index 941a23f..18a2e9d 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? converterType = null, - Type[]? validatorTypes = null) + Type? converterType, + Type[] validatorTypes) : base(property, description, converterType, validatorTypes) { Name = name; @@ -108,10 +108,26 @@ namespace CliFx.Domain internal partial class CommandOptionSchema { - public static CommandOptionSchema HelpOption { get; } = - new CommandOptionSchema(null, "help", 'h', null, false, "Shows help text.", converterType: null, validatorTypes: null); + public static CommandOptionSchema HelpOption { get; } = new CommandOptionSchema( + null, + "help", + 'h', + null, + false, + "Shows help text.", + null, + Array.Empty() + ); - public static CommandOptionSchema VersionOption { get; } = - new CommandOptionSchema(null, "version", null, null, false, "Shows version information.", converterType: null, validatorTypes: null); + public static CommandOptionSchema VersionOption { get; } = new CommandOptionSchema( + null, + "version", + null, + null, + false, + "Shows version information.", + null, + Array.Empty() + ); } } \ No newline at end of file diff --git a/CliFx/Domain/CommandParameterSchema.cs b/CliFx/Domain/CommandParameterSchema.cs index 814c8d6..9283186 100644 --- a/CliFx/Domain/CommandParameterSchema.cs +++ b/CliFx/Domain/CommandParameterSchema.cs @@ -17,8 +17,8 @@ namespace CliFx.Domain int order, string name, string? description, - Type? converterType = null, - Type[]? validatorTypes = null) + Type? converterType, + Type[] validatorTypes) : base(property, description, converterType, validatorTypes) { Order = order; diff --git a/CliFx/Domain/HelpTextWriter.cs b/CliFx/Domain/HelpTextWriter.cs index deb0808..6177c23 100644 --- a/CliFx/Domain/HelpTextWriter.cs +++ b/CliFx/Domain/HelpTextWriter.cs @@ -283,7 +283,7 @@ namespace CliFx.Domain } } - private void WriteCommandChildren( + private void WriteCommandDescendants( CommandSchema command, IReadOnlyList descendantCommands) { @@ -297,12 +297,7 @@ namespace CliFx.Domain foreach (var descendantCommand in descendantCommands) { - var relativeCommandName = !string.IsNullOrWhiteSpace(command.Name) - ? descendantCommand.Name!.Substring(command.Name.Length).Trim() - : descendantCommand.Name!; - // Description - if (!string.IsNullOrWhiteSpace(descendantCommand.Description)) { WriteHorizontalMargin(); @@ -310,7 +305,7 @@ namespace CliFx.Domain WriteVerticalMargin(); } - // Name + // Usage WriteHorizontalMargin(4); WriteCommandUsageLineItem(descendantCommand, false); @@ -355,7 +350,7 @@ namespace CliFx.Domain WriteCommandUsage(command, descendantCommands); WriteCommandParameters(command); WriteCommandOptions(command, defaultValues); - WriteCommandChildren(command, descendantCommands); + WriteCommandDescendants(command, descendantCommands); } } diff --git a/CliFx/Domain/RootSchema.cs b/CliFx/Domain/RootSchema.cs index 98fff68..84362ad 100644 --- a/CliFx/Domain/RootSchema.cs +++ b/CliFx/Domain/RootSchema.cs @@ -128,7 +128,7 @@ namespace CliFx.Domain } var invalidValidatorParameters = command.Parameters - .Where(p => p.ValidatorTypes != null && !p.ValidatorTypes.All(x => x.Implements(typeof(IArgumentValueValidator)))) + .Where(p => !p.ValidatorTypes.All(x => x.Implements(typeof(IArgumentValueValidator)))) .ToArray(); if (invalidValidatorParameters.Any()) @@ -222,7 +222,7 @@ namespace CliFx.Domain } var invalidValidatorOptions = command.Options - .Where(o => o.ValidatorTypes != null && !o.ValidatorTypes.All(x => x.Implements(typeof(IArgumentValueValidator)))) + .Where(o => !o.ValidatorTypes.All(x => x.Implements(typeof(IArgumentValueValidator)))) .ToArray(); if (invalidValidatorOptions.Any()) diff --git a/CliFx/Exceptions/CliFxException.cs b/CliFx/Exceptions/CliFxException.cs index 8535621..55535a0 100644 --- a/CliFx/Exceptions/CliFxException.cs +++ b/CliFx/Exceptions/CliFxException.cs @@ -36,7 +36,8 @@ namespace CliFx.Exceptions { internal static CliFxException DefaultActivatorFailed(Type type, Exception? innerException = null) { - var configureActivatorMethodName = $"{nameof(CliApplicationBuilder)}.{nameof(CliApplicationBuilder.UseTypeActivator)}(...)"; + var configureActivatorMethodName = + $"{nameof(CliApplicationBuilder)}.{nameof(CliApplicationBuilder.UseTypeActivator)}(...)"; var message = $@" Failed to create an instance of type '{type.FullName}'. @@ -193,7 +194,7 @@ Specified converter must implement {typeof(IArgumentValueConverter).FullName}."; Command '{command.Type.FullName}' is invalid because it contains {invalidParameters.Count} parameter(s) with invalid value validators: {invalidParameters.JoinToString(Environment.NewLine)} -Specified validator(s) must inherit from {typeof(ArgumentValueValidator<>).FullName}."; +Specified validators must inherit from {typeof(ArgumentValueValidator<>).FullName}."; return new CliFxException(message.Trim()); } @@ -424,7 +425,8 @@ Missing values for one or more required options: return new CliFxException(message.Trim()); } - internal static CliFxException UnrecognizedParametersProvided(IReadOnlyList parameterInputs) + internal static CliFxException UnrecognizedParametersProvided( + IReadOnlyList parameterInputs) { var message = $@" Unrecognized parameters provided: @@ -442,12 +444,35 @@ Unrecognized options provided: return new CliFxException(message.Trim()); } - internal static CliFxException ValueValidationFailed(CommandArgumentSchema argument, IEnumerable errors) + internal static CliFxException ValidationFailed( + CommandParameterSchema parameter, + IReadOnlyList failedResults) { var message = $@" -The validation of the provided value for {argument.Property!.Name} is failed because: {errors.JoinToString(Environment.NewLine)}"; +Value provided for parameter {parameter.GetUserFacingDisplayString()}: +{failedResults.Select(r => r.ErrorMessage).JoinToString(Environment.NewLine)}"; return new CliFxException(message.Trim()); } + + internal static CliFxException ValidationFailed( + CommandOptionSchema option, + IReadOnlyList failedResults) + { + var message = $@" +Value provided for option {option.GetUserFacingDisplayString()}: +{failedResults.Select(r => r.ErrorMessage).JoinToString(Environment.NewLine)}"; + + return new CliFxException(message.Trim()); + } + + internal static CliFxException ValidationFailed( + CommandArgumentSchema argument, + IReadOnlyList failedResults) => argument switch + { + CommandParameterSchema parameter => ValidationFailed(parameter, failedResults), + CommandOptionSchema option => ValidationFailed(option, failedResults), + _ => throw new ArgumentOutOfRangeException(nameof(argument)) + }; } } \ No newline at end of file diff --git a/CliFx/IArgumentValueValidator.cs b/CliFx/IArgumentValueValidator.cs deleted file mode 100644 index 8c45b37..0000000 --- a/CliFx/IArgumentValueValidator.cs +++ /dev/null @@ -1,7 +0,0 @@ -namespace CliFx -{ - internal interface IArgumentValueValidator - { - ValidationResult Validate(object value); - } -} diff --git a/CliFx/Internal/Extensions/CollectionExtensions.cs b/CliFx/Internal/Extensions/CollectionExtensions.cs index 38e29ec..9ef38fd 100644 --- a/CliFx/Internal/Extensions/CollectionExtensions.cs +++ b/CliFx/Internal/Extensions/CollectionExtensions.cs @@ -1,5 +1,4 @@ using System.Collections.Generic; -using System.Linq; namespace CliFx.Internal.Extensions { @@ -10,11 +9,5 @@ namespace CliFx.Internal.Extensions foreach (var item in items) source.Remove(item); } - - public static bool IsNullOrEmpty(this IEnumerable? source) => - !source?.Any() ?? true; - - public static bool NotEmpty(this IEnumerable? source) => - !source.IsNullOrEmpty(); } } \ No newline at end of file diff --git a/CliFx/ValidationResult.cs b/CliFx/ValidationResult.cs deleted file mode 100644 index 4a04f4e..0000000 --- a/CliFx/ValidationResult.cs +++ /dev/null @@ -1,30 +0,0 @@ -namespace CliFx -{ - /// - /// A tiny object that represents a result of the validation. - /// - public class ValidationResult - { - /// - /// False if there is no error message, otherwise - true. - /// - public bool IsValid => ErrorMessage == null; - - /// - /// Contains an information about the reasons of failed validation. - /// - public string? ErrorMessage { get; private set; } - - private ValidationResult() { } - - /// - /// Creates Ok result, means that the validation is passed. - /// - public static ValidationResult Ok() => new ValidationResult() { }; - - /// - /// Creates Error result, means that the validation failed. - /// - public static ValidationResult Error(string message) => new ValidationResult() { ErrorMessage = message }; - } -}