Add integration with the new required keyword

Closes #132
This commit is contained in:
Oleksii Holub
2022-12-08 21:46:14 +02:00
parent af96d0d31d
commit b10577fec5
19 changed files with 430 additions and 32 deletions

View File

@@ -9,7 +9,7 @@
</ItemGroup>
<ItemGroup>
<PackageReference Include="Basic.Reference.Assemblies" Version="1.4.1" />
<PackageReference Include="Basic.Reference.Assemblies.Net70" Version="1.4.1" />
<PackageReference Include="GitHubActionsTestLogger" Version="2.0.1" PrivateAssets="all" />
<PackageReference Include="FluentAssertions" Version="6.8.0" />
<PackageReference Include="Microsoft.NET.Test.Sdk" Version="17.4.0" />

View File

@@ -0,0 +1,114 @@
using CliFx.Analyzers.Tests.Utils;
using Microsoft.CodeAnalysis.Diagnostics;
using Xunit;
namespace CliFx.Analyzers.Tests;
public class OptionMustBeRequiredIfPropertyRequiredAnalyzerSpecs
{
private static DiagnosticAnalyzer Analyzer { get; } = new OptionMustBeRequiredIfPropertyRequiredAnalyzer();
[Fact]
public void Analyzer_reports_an_error_if_a_non_required_option_is_bound_to_a_required_property()
{
// Arrange
// language=cs
const string code =
"""
[Command]
public class MyCommand : ICommand
{
[CommandOption('f', IsRequired = false)]
public required string Foo { get; set; }
public ValueTask ExecuteAsync(IConsole console) => default;
}
""";
// Act & assert
Analyzer.Should().ProduceDiagnostics(code);
}
[Fact]
public void Analyzer_does_not_report_an_error_if_a_required_option_is_bound_to_a_required_property()
{
// Arrange
// language=cs
const string code =
"""
[Command]
public class MyCommand : ICommand
{
[CommandOption('f', IsRequired = true)]
public required string Foo { get; set; }
public ValueTask ExecuteAsync(IConsole console) => default;
}
""";
// Act & assert
Analyzer.Should().NotProduceDiagnostics(code);
}
[Fact]
public void Analyzer_does_not_report_an_error_if_a_non_required_option_is_bound_to_an_unannotated_property()
{
// Arrange
// language=cs
const string code =
"""
[Command]
public class MyCommand : ICommand
{
[CommandOption('f', IsRequired = false)]
public string Foo { get; set; }
public ValueTask ExecuteAsync(IConsole console) => default;
}
""";
// Act & assert
Analyzer.Should().NotProduceDiagnostics(code);
}
[Fact]
public void Analyzer_does_not_report_an_error_if_a_required_option_is_bound_to_an_unannotated_property()
{
// Arrange
// language=cs
const string code =
"""
[Command]
public class MyCommand : ICommand
{
[CommandOption('f', IsRequired = true)]
public string Foo { 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_that_is_not_an_option()
{
// Arrange
// language=cs
const string code =
"""
[Command]
public class MyCommand : ICommand
{
public required string Foo { get; set; }
public ValueTask ExecuteAsync(IConsole console) => default;
}
""";
// Act & assert
Analyzer.Should().NotProduceDiagnostics(code);
}
}

View File

@@ -18,10 +18,10 @@ public class ParameterMustBeLastIfNonRequiredAnalyzerSpecs
[Command]
public class MyCommand : ICommand
{
[CommandParameter(0, Name = "foo", IsRequired = false)]
[CommandParameter(0, IsRequired = false)]
public string Foo { get; set; }
[CommandParameter(1, Name = "bar")]
[CommandParameter(1)]
public string Bar { get; set; }
public ValueTask ExecuteAsync(IConsole console) => default;
@@ -42,10 +42,10 @@ public class ParameterMustBeLastIfNonRequiredAnalyzerSpecs
[Command]
public class MyCommand : ICommand
{
[CommandParameter(0, Name = "foo")]
[CommandParameter(0)]
public string Foo { get; set; }
[CommandParameter(1, Name = "bar", IsRequired = false)]
[CommandParameter(1, IsRequired = false)]
public string Bar { get; set; }
public ValueTask ExecuteAsync(IConsole console) => default;
@@ -66,10 +66,10 @@ public class ParameterMustBeLastIfNonRequiredAnalyzerSpecs
[Command]
public class MyCommand : ICommand
{
[CommandParameter(0, Name = "foo")]
[CommandParameter(0)]
public string Foo { get; set; }
[CommandParameter(1, Name = "bar", IsRequired = true)]
[CommandParameter(1, IsRequired = true)]
public string Bar { get; set; }
public ValueTask ExecuteAsync(IConsole console) => default;

View File

@@ -0,0 +1,114 @@
using CliFx.Analyzers.Tests.Utils;
using Microsoft.CodeAnalysis.Diagnostics;
using Xunit;
namespace CliFx.Analyzers.Tests;
public class ParameterMustBeRequiredIfPropertyRequiredAnalyzerSpecs
{
private static DiagnosticAnalyzer Analyzer { get; } = new ParameterMustBeRequiredIfPropertyRequiredAnalyzer();
[Fact]
public void Analyzer_reports_an_error_if_a_non_required_parameter_is_bound_to_a_required_property()
{
// Arrange
// language=cs
const string code =
"""
[Command]
public class MyCommand : ICommand
{
[CommandParameter(0, IsRequired = false)]
public required string Foo { get; set; }
public ValueTask ExecuteAsync(IConsole console) => default;
}
""";
// Act & assert
Analyzer.Should().ProduceDiagnostics(code);
}
[Fact]
public void Analyzer_does_not_report_an_error_if_a_required_parameter_is_bound_to_a_required_property()
{
// Arrange
// language=cs
const string code =
"""
[Command]
public class MyCommand : ICommand
{
[CommandParameter(0, IsRequired = true)]
public required string Foo { get; set; }
public ValueTask ExecuteAsync(IConsole console) => default;
}
""";
// Act & assert
Analyzer.Should().NotProduceDiagnostics(code);
}
[Fact]
public void Analyzer_does_not_report_an_error_if_a_non_required_parameter_is_bound_to_an_unannotated_property()
{
// Arrange
// language=cs
const string code =
"""
[Command]
public class MyCommand : ICommand
{
[CommandParameter(0, IsRequired = false)]
public string Foo { get; set; }
public ValueTask ExecuteAsync(IConsole console) => default;
}
""";
// Act & assert
Analyzer.Should().NotProduceDiagnostics(code);
}
[Fact]
public void Analyzer_does_not_report_an_error_if_a_required_parameter_is_bound_to_an_unannotated_property()
{
// Arrange
// language=cs
const string code =
"""
[Command]
public class MyCommand : ICommand
{
[CommandParameter(0, IsRequired = true)]
public string Foo { 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_that_is_not_a_parameter()
{
// Arrange
// language=cs
const string code =
"""
[Command]
public class MyCommand : ICommand
{
public required string Foo { get; set; }
public ValueTask ExecuteAsync(IConsole console) => default;
}
""";
// Act & assert
Analyzer.Should().NotProduceDiagnostics(code);
}
}

View File

@@ -58,8 +58,7 @@ internal class AnalyzerAssertions : ReferenceTypeAssertions<DiagnosticAnalyzer,
var compilation = CSharpCompilation.Create(
"CliFxTests_DynamicAssembly_" + Guid.NewGuid(),
new[] { ast },
ReferenceAssemblies
.Net60
Net70.References.All
.Append(MetadataReference.CreateFromFile(typeof(ICommand).Assembly.Location)),
// DLL to avoid having to define the Main() method
new CSharpCompilationOptions(OutputKind.DynamicallyLinkedLibrary)

View File

@@ -13,6 +13,8 @@ internal partial class CommandOptionSymbol : ICommandMemberSymbol
public char? ShortName { get; }
public bool? IsRequired { get; }
public ITypeSymbol? ConverterType { get; }
public IReadOnlyList<ITypeSymbol> ValidatorTypes { get; }
@@ -21,12 +23,14 @@ internal partial class CommandOptionSymbol : ICommandMemberSymbol
IPropertySymbol property,
string? name,
char? shortName,
bool? isRequired,
ITypeSymbol? converterType,
IReadOnlyList<ITypeSymbol> validatorTypes)
{
Property = property;
Name = name;
ShortName = shortName;
IsRequired = isRequired;
ConverterType = converterType;
ValidatorTypes = validatorTypes;
}
@@ -56,6 +60,12 @@ internal partial class CommandOptionSymbol
.Select(a => a.Value)
.FirstOrDefault() as char?;
var isRequired = attribute
.NamedArguments
.Where(a => a.Key == "IsRequired")
.Select(a => a.Value.Value)
.FirstOrDefault() as bool?;
var converter = attribute
.NamedArguments
.Where(a => a.Key == "Converter")
@@ -71,7 +81,7 @@ internal partial class CommandOptionSymbol
.Cast<ITypeSymbol>()
.ToArray();
return new CommandOptionSymbol(property, name, shortName, converter, validators);
return new CommandOptionSymbol(property, name, shortName, isRequired, converter, validators);
}
public static bool IsOptionProperty(IPropertySymbol property) =>

View File

@@ -0,0 +1,49 @@
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 OptionMustBeRequiredIfPropertyRequiredAnalyzer : AnalyzerBase
{
public OptionMustBeRequiredIfPropertyRequiredAnalyzer()
: base(
"Options bound to required properties cannot be marked as non-required",
"This option cannot be marked as non-required because it's bound to a required property.")
{
}
private void Analyze(
SyntaxNodeAnalysisContext context,
PropertyDeclarationSyntax propertyDeclaration,
IPropertySymbol property)
{
if (property.ContainingType is null)
return;
if (!property.IsRequired)
return;
var option = CommandOptionSymbol.TryResolve(property);
if (option is null)
return;
if (option.IsRequired != false)
return;
context.ReportDiagnostic(
CreateDiagnostic(
propertyDeclaration.Identifier.GetLocation()
)
);
}
public override void Initialize(AnalysisContext context)
{
base.Initialize(context);
context.HandlePropertyDeclaration(Analyze);
}
}

View File

@@ -0,0 +1,49 @@
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 ParameterMustBeRequiredIfPropertyRequiredAnalyzer : AnalyzerBase
{
public ParameterMustBeRequiredIfPropertyRequiredAnalyzer()
: base(
"Parameters bound to required properties cannot be marked as non-required",
"This parameter cannot be marked as non-required because it's bound to a required property.")
{
}
private void Analyze(
SyntaxNodeAnalysisContext context,
PropertyDeclarationSyntax propertyDeclaration,
IPropertySymbol property)
{
if (property.ContainingType is null)
return;
if (!property.IsRequired)
return;
var parameter = CommandParameterSymbol.TryResolve(property);
if (parameter is null)
return;
if (parameter.IsRequired != false)
return;
context.ReportDiagnostic(
CreateDiagnostic(
propertyDeclaration.Identifier.GetLocation()
)
);
}
public override void Initialize(AnalysisContext context)
{
base.Initialize(context);
context.HandlePropertyDeclaration(Analyze);
}
}

View File

@@ -2,4 +2,4 @@
Sample command line interface for managing a library of books.
This demo project showcases basic CliFx functionality such as command routing, argument parsing, autogenerated help text.
This demo project showcases basic CliFx functionality such as command routing, argument parsing, and autogenerated help text.

View File

@@ -9,7 +9,7 @@
</ItemGroup>
<ItemGroup>
<PackageReference Include="Basic.Reference.Assemblies" Version="1.4.1" />
<PackageReference Include="Basic.Reference.Assemblies.Net70" Version="1.4.1" />
<PackageReference Include="CliWrap" Version="3.5.0" />
<PackageReference Include="FluentAssertions" Version="6.8.0" />
<PackageReference Include="GitHubActionsTestLogger" Version="2.0.1" PrivateAssets="all" />

View File

@@ -816,4 +816,40 @@ public class OptionBindingSpecs : SpecsBase
exitCode.Should().NotBe(0);
stdErr.Should().Contain("expects a single argument, but provided with multiple");
}
[Fact]
public async Task Option_binding_fails_if_a_required_property_option_has_not_been_provided()
{
// Arrange
var commandType = DynamicCommandBuilder.Compile(
// language=cs
"""
[Command]
public class Command : ICommand
{
[CommandOption("foo")]
public required string Foo { get; set; }
public ValueTask ExecuteAsync(IConsole console) => default;
}
"""
);
var application = new CliApplicationBuilder()
.AddCommand(commandType)
.UseConsole(FakeConsole)
.Build();
// Act
var exitCode = await application.RunAsync(
Array.Empty<string>(),
new Dictionary<string, string>()
);
var stdErr = FakeConsole.ReadErrorString();
// Assert
exitCode.Should().NotBe(0);
stdErr.Should().Contain("Missing required option(s)");
}
}

View File

@@ -61,8 +61,7 @@ internal static class DynamicCommandBuilder
var compilation = CSharpCompilation.Create(
"CliFxTests_DynamicAssembly_" + Guid.NewGuid(),
new[] {ast},
ReferenceAssemblies
.Net60
Net70.References.All
.Append(MetadataReference.CreateFromFile(typeof(ICommand).Assembly.Location))
.Append(MetadataReference.CreateFromFile(typeof(DynamicCommandBuilder).Assembly.Location)),
// DLL to avoid having to define the Main() method

View File

@@ -32,6 +32,10 @@ public sealed class CommandOptionAttribute : Attribute
/// Whether this option is required (default: <c>false</c>).
/// If an option is required, the user will get an error if they don't set it.
/// </summary>
/// <remarks>
/// You can use the <c>required</c> keyword on the property (introduced in C# 11) to implicitly
/// set <see cref="IsRequired" /> to <c>true</c>.
/// </remarks>
public bool IsRequired { get; set; }
/// <summary>

View File

@@ -3,6 +3,7 @@ using System.Collections.Generic;
using System.Reflection;
using System.Text;
using CliFx.Attributes;
using CliFx.Utils.Extensions;
namespace CliFx.Schema;
@@ -101,6 +102,7 @@ internal partial class OptionSchema
// The user may mistakenly specify dashes, thinking it's required, so trim them
var name = attribute.Name?.TrimStart('-').Trim();
var environmentVariable = attribute.EnvironmentVariable?.Trim();
var isRequired = attribute.IsRequired || property.IsRequired();
var description = attribute.Description?.Trim();
return new OptionSchema(
@@ -108,7 +110,7 @@ internal partial class OptionSchema
name,
attribute.ShortName,
environmentVariable,
attribute.IsRequired,
isRequired,
description,
attribute.Converter,
attribute.Validators

View File

@@ -2,6 +2,7 @@
using System.Collections.Generic;
using System.Reflection;
using CliFx.Attributes;
using CliFx.Utils.Extensions;
namespace CliFx.Schema;
@@ -53,13 +54,14 @@ internal partial class ParameterSchema
return null;
var name = attribute.Name?.Trim() ?? property.Name.ToLowerInvariant();
var isRequired = attribute.IsRequired || property.IsRequired();
var description = attribute.Description?.Trim();
return new ParameterSchema(
new BindablePropertyDescriptor(property),
attribute.Order,
name,
attribute.IsRequired,
isRequired,
description,
attribute.Converter,
attribute.Validators

View File

@@ -1,4 +1,5 @@
using System.Collections;
using System;
using System.Collections;
using System.Collections.Generic;
using System.Linq;
@@ -37,4 +38,14 @@ internal static class CollectionExtensions
dictionary
.Cast<DictionaryEntry>()
.ToDictionary(entry => (TKey) entry.Key, entry => (TValue) entry.Value, comparer);
public static Array ToNonGenericArray<T>(this IEnumerable<T> source, Type elementType)
{
var sourceAsCollection = source as ICollection ?? source.ToArray();
var array = Array.CreateInstance(elementType, sourceAsCollection.Count);
sourceAsCollection.CopyTo(array, 0);
return array;
}
}

View File

@@ -0,0 +1,18 @@
using System;
using System.Linq;
using System.Reflection;
namespace CliFx.Utils.Extensions;
internal static class PropertyExtensions
{
public static bool IsRequired(this PropertyInfo propertyInfo) =>
// Match attribute by name to avoid depending on .NET 7.0+ and to allow polyfilling
propertyInfo.GetCustomAttributes().Any(a =>
string.Equals(
a.GetType().FullName,
"System.Runtime.CompilerServices.RequiredMemberAttribute",
StringComparison.Ordinal
)
);
}

View File

@@ -11,7 +11,8 @@ internal static class TypeExtensions
public static bool Implements(this Type type, Type interfaceType) =>
type.GetInterfaces().Contains(interfaceType);
public static Type? TryGetNullableUnderlyingType(this Type type) => Nullable.GetUnderlyingType(type);
public static Type? TryGetNullableUnderlyingType(this Type type) =>
Nullable.GetUnderlyingType(type);
public static Type? TryGetEnumerableUnderlyingType(this Type type)
{
@@ -44,16 +45,6 @@ internal static class TypeExtensions
);
}
public static Array ToNonGenericArray<T>(this IEnumerable<T> source, Type elementType)
{
var sourceAsCollection = source as ICollection ?? source.ToArray();
var array = Array.CreateInstance(elementType, sourceAsCollection.Count);
sourceAsCollection.CopyTo(array, 0);
return array;
}
public static bool IsToStringOverriden(this Type type)
{
var toStringMethod = type.GetMethod(nameof(ToString), Type.EmptyTypes);

View File

@@ -144,7 +144,7 @@ public class LogCommand : ICommand
{
// Order: 0
[CommandParameter(0, Description = "Value whose logarithm is to be found.")]
public double Value { get; init; }
public required double Value { get; init; }
// Name: --base
// Short name: -b
@@ -382,7 +382,7 @@ If the user does not provide value for such option through command line argument
[Command]
public class AuthCommand : ICommand
{
[CommandOption("token", IsRequired = true, EnvironmentVariable = "AUTH_TOKEN")]
[CommandOption("token", EnvironmentVariable = "AUTH_TOKEN")]
public required string AuthToken { get; init; }
public ValueTask ExecuteAsync(IConsole console)
@@ -499,10 +499,10 @@ This special exception can be used to print an error message to the console, ret
[Command]
public class DivideCommand : ICommand
{
[CommandOption("dividend", IsRequired = true)]
[CommandOption("dividend")]
public required double Dividend { get; init; }
[CommandOption("divisor", IsRequired = true)]
[CommandOption("divisor")]
public required double Divisor { get; init; }
public ValueTask ExecuteAsync(IConsole console)