Add support for optional parameters (#119)

This commit is contained in:
AliReZa Sabouri
2022-01-11 00:41:04 +03:30
committed by GitHub
parent 8e307df231
commit 8e96d2701d
8 changed files with 180 additions and 17 deletions

View File

@@ -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);
}
}

View File

@@ -11,6 +11,8 @@ internal partial class CommandParameterSymbol
public string? Name { get; } public string? Name { get; }
public bool? IsRequired { get; }
public ITypeSymbol? ConverterType { get; } public ITypeSymbol? ConverterType { get; }
public IReadOnlyList<ITypeSymbol> ValidatorTypes { get; } public IReadOnlyList<ITypeSymbol> ValidatorTypes { get; }
@@ -18,11 +20,13 @@ internal partial class CommandParameterSymbol
public CommandParameterSymbol( public CommandParameterSymbol(
int order, int order,
string? name, string? name,
bool? isRequired,
ITypeSymbol? converterType, ITypeSymbol? converterType,
IReadOnlyList<ITypeSymbol> validatorTypes) IReadOnlyList<ITypeSymbol> validatorTypes)
{ {
Order = order; Order = order;
Name = name; Name = name;
IsRequired = isRequired;
ConverterType = converterType; ConverterType = converterType;
ValidatorTypes = validatorTypes; ValidatorTypes = validatorTypes;
} }
@@ -48,6 +52,12 @@ internal partial class CommandParameterSymbol
.Select(a => a.Value.Value) .Select(a => a.Value.Value)
.FirstOrDefault() as string; .FirstOrDefault() as string;
var isRequired = attribute
.NamedArguments
.Where(a => a.Key == "IsRequired")
.Select(a => a.Value.Value)
.FirstOrDefault() as bool?;
var converter = attribute var converter = attribute
.NamedArguments .NamedArguments
.Where(a => a.Key == "Converter") .Where(a => a.Key == "Converter")
@@ -63,17 +73,16 @@ internal partial class CommandParameterSymbol
.Cast<ITypeSymbol>() .Cast<ITypeSymbol>()
.ToArray(); .ToArray();
return new CommandParameterSymbol(order, name, converter, validators); return new CommandParameterSymbol(order, name, isRequired, converter, validators);
} }
public static CommandParameterSymbol? TryResolve(IPropertySymbol property) public static CommandParameterSymbol? TryResolve(IPropertySymbol property)
{ {
var attribute = TryGetParameterAttribute(property); var attribute = TryGetParameterAttribute(property);
if (attribute is null) return attribute is not null
return null; ? FromAttribute(attribute)
: null;
return FromAttribute(attribute);
} }
public static bool IsParameterProperty(IPropertySymbol property) => public static bool IsParameterProperty(IPropertySymbol property) =>

View File

@@ -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<IPropertySymbol>()
.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);
}
}

View File

@@ -153,7 +153,7 @@ public class Command : ICommand
// Assert // Assert
exitCode.Should().NotBe(0); exitCode.Should().NotBe(0);
stdErr.Should().Contain("Missing parameter(s)"); stdErr.Should().Contain("Missing required parameter(s)");
} }
[Fact] [Fact]
@@ -190,7 +190,7 @@ public class Command : ICommand
// Assert // Assert
exitCode.Should().NotBe(0); exitCode.Should().NotBe(0);
stdErr.Should().Contain("Missing parameter(s)"); stdErr.Should().Contain("Missing required parameter(s)");
} }
[Fact] [Fact]

View File

@@ -23,6 +23,13 @@ public sealed class CommandParameterAttribute : Attribute
/// </remarks> /// </remarks>
public int Order { get; } public int Order { get; }
/// <summary>
/// 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.
/// </summary>
public bool IsRequired { get; set; } = true;
/// <summary> /// <summary>
/// Parameter name. /// Parameter name.
/// This is shown to the user in the help text. /// This is shown to the user in the help text.

View File

@@ -223,7 +223,7 @@ internal class CommandBinder
{ {
// Ensure there are no unexpected parameters and that all parameters are provided // Ensure there are no unexpected parameters and that all parameters are provided
var remainingParameterInputs = commandInput.Parameters.ToList(); var remainingParameterInputs = commandInput.Parameters.ToList();
var remainingParameterSchemas = commandSchema.Parameters.ToList(); var remainingRequiredParameterSchemas = commandSchema.Parameters.Where(p => p.IsRequired).ToList();
var position = 0; var position = 0;
@@ -258,7 +258,7 @@ internal class CommandBinder
remainingParameterInputs.RemoveRange(parameterInputs); remainingParameterInputs.RemoveRange(parameterInputs);
} }
remainingParameterSchemas.Remove(parameterSchema); remainingRequiredParameterSchemas.Remove(parameterSchema);
} }
if (remainingParameterInputs.Any()) if (remainingParameterInputs.Any())
@@ -272,12 +272,12 @@ internal class CommandBinder
); );
} }
if (remainingParameterSchemas.Any()) if (remainingRequiredParameterSchemas.Any())
{ {
throw CliFxException.UserError( throw CliFxException.UserError(
"Missing parameter(s):" + "Missing required parameter(s):" +
Environment.NewLine + Environment.NewLine +
remainingParameterSchemas remainingRequiredParameterSchemas
.Select(o => o.GetFormattedIdentifier()) .Select(o => o.GetFormattedIdentifier())
.JoinToString(" ") .JoinToString(" ")
); );

View File

@@ -156,8 +156,16 @@ internal class HelpConsoleFormatter : ConsoleFormatter
WriteHeader("Parameters"); WriteHeader("Parameters");
foreach (var parameterSchema in _context.CommandSchema.Parameters.OrderBy(p => p.Order)) foreach (var parameterSchema in _context.CommandSchema.Parameters.OrderBy(p => p.Order))
{
if (parameterSchema.IsRequired)
{ {
Write(ConsoleColor.Red, "* "); Write(ConsoleColor.Red, "* ");
}
else
{
WriteHorizontalMargin();
}
Write(ConsoleColor.DarkCyan, $"{parameterSchema.Name}"); Write(ConsoleColor.DarkCyan, $"{parameterSchema.Name}");
WriteColumnMargin(); WriteColumnMargin();

View File

@@ -15,15 +15,17 @@ internal partial class ParameterSchema : IMemberSchema
public string? Description { get; } public string? Description { get; }
public bool IsRequired { get; }
public Type? ConverterType { get; } public Type? ConverterType { get; }
public IReadOnlyList<Type> ValidatorTypes { get; } public IReadOnlyList<Type> ValidatorTypes { get; }
public ParameterSchema( public ParameterSchema(IPropertyDescriptor property,
IPropertyDescriptor property,
int order, int order,
string name, string name,
string? description, string? description,
bool isRequired,
Type? converterType, Type? converterType,
IReadOnlyList<Type> validatorTypes) IReadOnlyList<Type> validatorTypes)
{ {
@@ -31,6 +33,7 @@ internal partial class ParameterSchema : IMemberSchema
Order = order; Order = order;
Name = name; Name = name;
Description = description; Description = description;
IsRequired = isRequired;
ConverterType = converterType; ConverterType = converterType;
ValidatorTypes = validatorTypes; ValidatorTypes = validatorTypes;
} }
@@ -56,6 +59,7 @@ internal partial class ParameterSchema
attribute.Order, attribute.Order,
name, name,
description, description,
attribute.IsRequired,
attribute.Converter, attribute.Converter,
attribute.Validators attribute.Validators
); );