From 8e96d2701dbace76b38832dc7ca6fd6bd3075e60 Mon Sep 17 00:00:00 2001 From: AliReZa Sabouri <7004080+alirezanet@users.noreply.github.com> Date: Tue, 11 Jan 2022 00:41:04 +0330 Subject: [PATCH] Add support for optional parameters (#119) --- ...terMustBeLastIfNotRequiredAnalyzerSpecs.cs | 76 +++++++++++++++++++ .../ObjectModel/CommandParameterSymbol.cs | 21 +++-- ...arameterMustBeLastIfNotRequiredAnalyzer.cs | 59 ++++++++++++++ CliFx.Tests/ParameterBindingSpecs.cs | 4 +- CliFx/Attributes/CommandParameterAttribute.cs | 7 ++ CliFx/CommandBinder.cs | 10 +-- CliFx/Formatting/HelpConsoleFormatter.cs | 10 ++- CliFx/Schema/ParameterSchema.cs | 10 ++- 8 files changed, 180 insertions(+), 17 deletions(-) create mode 100644 CliFx.Analyzers.Tests/ParameterMustBeLastIfNotRequiredAnalyzerSpecs.cs create mode 100644 CliFx.Analyzers/ParameterMustBeLastIfNotRequiredAnalyzer.cs diff --git a/CliFx.Analyzers.Tests/ParameterMustBeLastIfNotRequiredAnalyzerSpecs.cs b/CliFx.Analyzers.Tests/ParameterMustBeLastIfNotRequiredAnalyzerSpecs.cs new file mode 100644 index 0000000..935eefc --- /dev/null +++ b/CliFx.Analyzers.Tests/ParameterMustBeLastIfNotRequiredAnalyzerSpecs.cs @@ -0,0 +1,76 @@ +using CliFx.Analyzers.Tests.Utils; +using Microsoft.CodeAnalysis.Diagnostics; +using Xunit; + +namespace CliFx.Analyzers.Tests; + +public class ParameterMustBeLastIfNotRequiredAnalyzerSpecs +{ + private static DiagnosticAnalyzer Analyzer { get; } = new ParameterMustBeLastIfNotRequiredAnalyzer(); + + [Fact] + public void Analyzer_reports_an_error_if_a_parameter_is_not_required_and_is_not_last_parameter() + { + // Arrange + // language=cs + const string code = @" +[Command] +public class MyCommand : ICommand +{ + [CommandParameter(0, Name = ""foo"", IsRequired = false)] + public string Foo { get; set; } + + [CommandParameter(1, Name = ""bar"", IsRequired = false)] + public string Bar { get; set; } + + public ValueTask ExecuteAsync(IConsole console) => default; +}"; + + // Act & assert + Analyzer.Should().ProduceDiagnostics(code); + } + + [Fact] + public void Analyzer_does_not_report_an_error_if_isRequired_is_false_on_last_parameter() + { + // Arrange + // language=cs + const string code = @" +[Command] +public class MyCommand : ICommand +{ + [CommandParameter(0, Name = ""foo"", IsRequired = true)] + public string Foo { get; set; } + + [CommandParameter(1, Name = ""bar"", IsRequired = false)] + public string Bar { get; set; } + + public ValueTask ExecuteAsync(IConsole console) => default; +}"; + + // Act & assert + Analyzer.Should().NotProduceDiagnostics(code); + } + + [Fact] + public void Analyzer_does_not_report_an_error_on_a_property_when_isRequired_is_not_set() + { + // Arrange + // language=cs + const string code = @" +[Command] +public class MyCommand : ICommand +{ + [CommandParameter(0, Name = ""foo"")] + public string Foo { get; set; } + + [CommandParameter(1, Name = ""bar"")] + public string Bar { get; set; } + + public ValueTask ExecuteAsync(IConsole console) => default; +}"; + + // Act & assert + Analyzer.Should().NotProduceDiagnostics(code); + } +} \ No newline at end of file diff --git a/CliFx.Analyzers/ObjectModel/CommandParameterSymbol.cs b/CliFx.Analyzers/ObjectModel/CommandParameterSymbol.cs index 2fe4ba5..04712bb 100644 --- a/CliFx.Analyzers/ObjectModel/CommandParameterSymbol.cs +++ b/CliFx.Analyzers/ObjectModel/CommandParameterSymbol.cs @@ -10,7 +10,9 @@ internal partial class CommandParameterSymbol public int Order { get; } public string? Name { get; } - + + public bool? IsRequired { get; } + public ITypeSymbol? ConverterType { get; } public IReadOnlyList ValidatorTypes { get; } @@ -18,11 +20,13 @@ internal partial class CommandParameterSymbol public CommandParameterSymbol( int order, string? name, + bool? isRequired, ITypeSymbol? converterType, IReadOnlyList validatorTypes) { Order = order; Name = name; + IsRequired = isRequired; ConverterType = converterType; ValidatorTypes = validatorTypes; } @@ -47,6 +51,12 @@ internal partial class CommandParameterSymbol .Where(a => a.Key == "Name") .Select(a => a.Value.Value) .FirstOrDefault() as string; + + var isRequired = attribute + .NamedArguments + .Where(a => a.Key == "IsRequired") + .Select(a => a.Value.Value) + .FirstOrDefault() as bool?; var converter = attribute .NamedArguments @@ -63,17 +73,16 @@ internal partial class CommandParameterSymbol .Cast() .ToArray(); - return new CommandParameterSymbol(order, name, converter, validators); + return new CommandParameterSymbol(order, name, isRequired, converter, validators); } public static CommandParameterSymbol? TryResolve(IPropertySymbol property) { var attribute = TryGetParameterAttribute(property); - if (attribute is null) - return null; - - return FromAttribute(attribute); + return attribute is not null + ? FromAttribute(attribute) + : null; } public static bool IsParameterProperty(IPropertySymbol property) => diff --git a/CliFx.Analyzers/ParameterMustBeLastIfNotRequiredAnalyzer.cs b/CliFx.Analyzers/ParameterMustBeLastIfNotRequiredAnalyzer.cs new file mode 100644 index 0000000..19a9153 --- /dev/null +++ b/CliFx.Analyzers/ParameterMustBeLastIfNotRequiredAnalyzer.cs @@ -0,0 +1,59 @@ +using System; +using System.Linq; +using CliFx.Analyzers.ObjectModel; +using CliFx.Analyzers.Utils.Extensions; +using Microsoft.CodeAnalysis; +using Microsoft.CodeAnalysis.CSharp.Syntax; +using Microsoft.CodeAnalysis.Diagnostics; + +namespace CliFx.Analyzers; + +[DiagnosticAnalyzer(LanguageNames.CSharp)] +public class ParameterMustBeLastIfNotRequiredAnalyzer : AnalyzerBase +{ + public ParameterMustBeLastIfNotRequiredAnalyzer() + : base( + "Only last parameter can be optional", + "IsRequired can only be set to false on the last parameter.") + { + } + + private void Analyze( + SyntaxNodeAnalysisContext context, + PropertyDeclarationSyntax propertyDeclaration, + IPropertySymbol property) + { + if (property.ContainingType is null) + return; + + var parameter = CommandParameterSymbol.TryResolve(property); + + if (parameter is null || parameter.IsRequired != false) + return; + + var otherProperties = property + .ContainingType + .GetMembers() + .OfType() + .Where(m => !m.Equals(property, SymbolEqualityComparer.Default)) + .ToArray(); + + foreach (var otherProperty in otherProperties) + { + var otherParameter = CommandParameterSymbol.TryResolve(otherProperty); + if (otherParameter is null) + continue; + + if (parameter.IsRequired is false && parameter.Order < otherParameter.Order) + { + context.ReportDiagnostic(CreateDiagnostic(propertyDeclaration.GetLocation())); + } + } + } + + public override void Initialize(AnalysisContext context) + { + base.Initialize(context); + context.HandlePropertyDeclaration(Analyze); + } +} \ No newline at end of file diff --git a/CliFx.Tests/ParameterBindingSpecs.cs b/CliFx.Tests/ParameterBindingSpecs.cs index 272237c..8298f3d 100644 --- a/CliFx.Tests/ParameterBindingSpecs.cs +++ b/CliFx.Tests/ParameterBindingSpecs.cs @@ -153,7 +153,7 @@ public class Command : ICommand // Assert exitCode.Should().NotBe(0); - stdErr.Should().Contain("Missing parameter(s)"); + stdErr.Should().Contain("Missing required parameter(s)"); } [Fact] @@ -190,7 +190,7 @@ public class Command : ICommand // Assert exitCode.Should().NotBe(0); - stdErr.Should().Contain("Missing parameter(s)"); + stdErr.Should().Contain("Missing required parameter(s)"); } [Fact] diff --git a/CliFx/Attributes/CommandParameterAttribute.cs b/CliFx/Attributes/CommandParameterAttribute.cs index 94e1345..11ac318 100644 --- a/CliFx/Attributes/CommandParameterAttribute.cs +++ b/CliFx/Attributes/CommandParameterAttribute.cs @@ -22,6 +22,13 @@ public sealed class CommandParameterAttribute : Attribute /// Only one non-scalar parameter is allowed in a command. /// public int Order { get; } + + /// + /// Whether this parameter is required. (default: true) + /// If a parameter is required, the user will get an error if they don't set it. + /// Only the last parameter in order can be optional. + /// + public bool IsRequired { get; set; } = true; /// /// Parameter name. diff --git a/CliFx/CommandBinder.cs b/CliFx/CommandBinder.cs index 27740f4..4f6ddac 100644 --- a/CliFx/CommandBinder.cs +++ b/CliFx/CommandBinder.cs @@ -223,7 +223,7 @@ internal class CommandBinder { // Ensure there are no unexpected parameters and that all parameters are provided var remainingParameterInputs = commandInput.Parameters.ToList(); - var remainingParameterSchemas = commandSchema.Parameters.ToList(); + var remainingRequiredParameterSchemas = commandSchema.Parameters.Where(p => p.IsRequired).ToList(); var position = 0; @@ -258,7 +258,7 @@ internal class CommandBinder remainingParameterInputs.RemoveRange(parameterInputs); } - remainingParameterSchemas.Remove(parameterSchema); + remainingRequiredParameterSchemas.Remove(parameterSchema); } if (remainingParameterInputs.Any()) @@ -272,12 +272,12 @@ internal class CommandBinder ); } - if (remainingParameterSchemas.Any()) + if (remainingRequiredParameterSchemas.Any()) { throw CliFxException.UserError( - "Missing parameter(s):" + + "Missing required parameter(s):" + Environment.NewLine + - remainingParameterSchemas + remainingRequiredParameterSchemas .Select(o => o.GetFormattedIdentifier()) .JoinToString(" ") ); diff --git a/CliFx/Formatting/HelpConsoleFormatter.cs b/CliFx/Formatting/HelpConsoleFormatter.cs index 5e10e0d..df0f5c9 100644 --- a/CliFx/Formatting/HelpConsoleFormatter.cs +++ b/CliFx/Formatting/HelpConsoleFormatter.cs @@ -157,7 +157,15 @@ internal class HelpConsoleFormatter : ConsoleFormatter foreach (var parameterSchema in _context.CommandSchema.Parameters.OrderBy(p => p.Order)) { - Write(ConsoleColor.Red, "* "); + if (parameterSchema.IsRequired) + { + Write(ConsoleColor.Red, "* "); + } + else + { + WriteHorizontalMargin(); + } + Write(ConsoleColor.DarkCyan, $"{parameterSchema.Name}"); WriteColumnMargin(); diff --git a/CliFx/Schema/ParameterSchema.cs b/CliFx/Schema/ParameterSchema.cs index e8ec7c6..a2f4601 100644 --- a/CliFx/Schema/ParameterSchema.cs +++ b/CliFx/Schema/ParameterSchema.cs @@ -14,16 +14,18 @@ internal partial class ParameterSchema : IMemberSchema public string Name { get; } public string? Description { get; } - + + public bool IsRequired { get; } + public Type? ConverterType { get; } public IReadOnlyList ValidatorTypes { get; } - public ParameterSchema( - IPropertyDescriptor property, + public ParameterSchema(IPropertyDescriptor property, int order, string name, string? description, + bool isRequired, Type? converterType, IReadOnlyList validatorTypes) { @@ -31,6 +33,7 @@ internal partial class ParameterSchema : IMemberSchema Order = order; Name = name; Description = description; + IsRequired = isRequired; ConverterType = converterType; ValidatorTypes = validatorTypes; } @@ -56,6 +59,7 @@ internal partial class ParameterSchema attribute.Order, name, description, + attribute.IsRequired, attribute.Converter, attribute.Validators );