Report diagnostics about command aliases

DoctorKrolic · Apr 14, 2024 · 4355009 · 4355009
1 parent b46aa0f
commit 4355009
Show file tree

Hide file tree

Showing 7 changed files with 202 additions and 8 deletions.
diff --git a/src/ArgumentParsing.CodeFixes/PrefixAliasWithDashesCodeFixProvider.cs b/src/ArgumentParsing.CodeFixes/PrefixAliasWithDashesCodeFixProvider.cs
@@ -0,0 +1,56 @@
+using System.Collections.Immutable;
+using System.Composition;
+using ArgumentParsing.Generators.Diagnostics;
+using Microsoft.CodeAnalysis;
+using Microsoft.CodeAnalysis.CodeActions;
+using Microsoft.CodeAnalysis.CodeFixes;
+using Microsoft.CodeAnalysis.CSharp;
+using Microsoft.CodeAnalysis.CSharp.Syntax;
+
+namespace ArgumentParsing.CodeFixes;
+
+[ExportCodeFixProvider(LanguageNames.CSharp), Shared]
+public sealed class PrefixAliasWithDashesCodeFixProvider : CodeFixProvider
+{
+    public override ImmutableArray<string> FixableDiagnosticIds { get; } = ImmutableArray.Create(DiagnosticDescriptors.AliasShouldStartWithDash.Id);
+
+    public override FixAllProvider? GetFixAllProvider()
+        => WellKnownFixAllProviders.BatchFixer;
+
+    public override async Task RegisterCodeFixesAsync(CodeFixContext context)
+    {
+        var document = context.Document;
+        var cancellationToken = context.CancellationToken;
+
+        var root = await document.GetSyntaxRootAsync(cancellationToken).ConfigureAwait(false);
+        if (root?.FindNode(context.Span, getInnermostNodeForTie: true) is LiteralExpressionSyntax { RawKind: (int)SyntaxKind.StringLiteralExpression } aliasLiteral)
+        {
+            var diagnostic = context.Diagnostics[0];
+
+            context.RegisterCodeFix(
+                CodeAction.Create(
+                    "Prefix alias with '--'",
+                    _ => PrefixAlias(document, root, aliasLiteral, "--"),
+                    $"--{nameof(PrefixAliasWithDashesCodeFixProvider)}"),
+                diagnostic);
+
+            context.RegisterCodeFix(
+                CodeAction.Create(
+                    "Prefix alias with '-'",
+                    _ => PrefixAlias(document, root, aliasLiteral, "-"),
+                    $"-{nameof(PrefixAliasWithDashesCodeFixProvider)}"),
+                diagnostic);
+        }
+    }
+
+    private static Task<Document> PrefixAlias(Document document, SyntaxNode root, LiteralExpressionSyntax aliasLiteral, string prefix)
+    {
+        var token = aliasLiteral.Token;
+        var fixedAliasText = prefix + token.ValueText;
+        var fixedLiteral = aliasLiteral
+            .WithToken(
+                SyntaxFactory.Token(token.LeadingTrivia, SyntaxKind.StringLiteralToken, "\"" + fixedAliasText + "\"", fixedAliasText, token.TrailingTrivia));
+        var fixedRoot = root.ReplaceNode(aliasLiteral, fixedLiteral);
+        return Task.FromResult(document.WithSyntaxRoot(fixedRoot));
+    }
+}
diff --git a/src/ArgumentParsing.Generators/AnalyzerReleases.Unshipped.md b/src/ArgumentParsing.Generators/AnalyzerReleases.Unshipped.md
@@ -4,3 +4,5 @@ Rule ID | Category | Severity | Notes
 --------|----------|----------|-------
 ARGP0038 | ArgumentParsing | Warning |
 ARGP0039 | ArgumentParsing | Error |
+ARGP0040 | ArgumentParsing | Error |
+ARGP0041 | ArgumentParsing | Warning |
diff --git a/src/ArgumentParsing.Generators/Diagnostics/Analyzers/SpecialCommandHandlerAnalyzer.cs b/src/ArgumentParsing.Generators/Diagnostics/Analyzers/SpecialCommandHandlerAnalyzer.cs
@@ -12,7 +12,9 @@ public sealed class SpecialCommandHandlerAnalyzer : DiagnosticAnalyzer
     public override ImmutableArray<DiagnosticDescriptor> SupportedDiagnostics { get; } =
         ImmutableArray.Create(
             DiagnosticDescriptors.SpecialCommandHandlerShouldBeClass,
-            DiagnosticDescriptors.SpecialCommandHandlerMustHaveAliases);
+            DiagnosticDescriptors.SpecialCommandHandlerMustHaveAliases,
+            DiagnosticDescriptors.InvalidSpecialCommandAlias,
+            DiagnosticDescriptors.AliasShouldStartWithDash);
 
     public override void Initialize(AnalysisContext context)
     {
@@ -54,10 +56,10 @@ private static void AnalyzeSpecialCommandHandler(SymbolAnalysisContext context,
         }
 
         var aliasesAttribute = type.GetAttributes().FirstOrDefault(a => a.AttributeClass?.Equals(knownTypes.SpecialCommandAliasesAttributeType, SymbolEqualityComparer.Default) == true);
-        var constructorArg = aliasesAttribute?.ConstructorArguments.First();
+        var constructorArg = aliasesAttribute?.ConstructorArguments[0];
 
         if (aliasesAttribute is null ||
-            constructorArg is { IsNull: true } or { Values.IsEmpty: true })
+            constructorArg is null or { IsNull: true } or { Values.IsEmpty: true })
         {
             var diagType = aliasesAttribute is null
                 ? SpecialCommandHandlerMustHaveAliasesDiagnosticTypes.NoAttribute
@@ -71,6 +73,30 @@ private static void AnalyzeSpecialCommandHandler(SymbolAnalysisContext context,
                     diagnosticLocation,
                     properties: ImmutableDictionary.CreateRange([new KeyValuePair<string, string?>("Type", diagType)])));
         }
+        else
+        {
+            var aliasSyntaxes = ((AttributeSyntax?)aliasesAttribute.ApplicationSyntaxReference?.GetSyntax())?.ArgumentList?.Arguments;
+
+            for (var i = 0; i < constructorArg.Value.Values.Length; i++)
+            {
+                var aliasValue = (string?)constructorArg.Value.Values[i].Value;
+                if (!aliasValue.IsValidName(allowDashPrefix: true))
+                {
+                    context.ReportDiagnostic(
+                        Diagnostic.Create(
+                            DiagnosticDescriptors.InvalidSpecialCommandAlias,
+                            aliasSyntaxes?[i].GetLocation() ?? diagnosticLocation,
+                            aliasValue));
+                }
+                else if (aliasValue[0] != '-')
+                {
+                    context.ReportDiagnostic(
+                        Diagnostic.Create(
+                            DiagnosticDescriptors.AliasShouldStartWithDash,
+                            aliasSyntaxes?[i].GetLocation() ?? diagnosticLocation));
+                }
+            }
+        }
     }
 
     public static class SpecialCommandHandlerMustHaveAliasesDiagnosticTypes

diff --git a/src/ArgumentParsing.Generators/Diagnostics/DiagnosticDescriptors.cs b/src/ArgumentParsing.Generators/Diagnostics/DiagnosticDescriptors.cs
@@ -326,4 +326,22 @@ public static class DiagnosticDescriptors
         category: ArgumentParsingCategoryName,
         defaultSeverity: DiagnosticSeverity.Error,
         isEnabledByDefault: true);
+
+    public static readonly DiagnosticDescriptor InvalidSpecialCommandAlias = new(
+        id: "ARGP0040",
+        title: "Invalid alias of a special command",
+        messageFormat: "Special command alias '{0}' is invalid",
+        description: "Alias of a special command must contain only letters, digits and dashes.",
+        category: ArgumentParsingCategoryName,
+        defaultSeverity: DiagnosticSeverity.Error,
+        isEnabledByDefault: true);
+
+    public static readonly DiagnosticDescriptor AliasShouldStartWithDash = new(
+        id: "ARGP0041",
+        title: "Alias of a special command should start with a dash",
+        messageFormat: "Alias of a special command should start with a dash or 2 dashes",
+        description: "Special command aliases should start with dashes. If you want a normal command, you should use a verb.",
+        category: ArgumentParsingCategoryName,
+        defaultSeverity: DiagnosticSeverity.Warning,
+        isEnabledByDefault: true);
 }
diff --git a/src/ArgumentParsing.Generators/Extensions/StringExtensions.cs b/src/ArgumentParsing.Generators/Extensions/StringExtensions.cs
@@ -1,3 +1,5 @@
+using System.Diagnostics.CodeAnalysis;
+
 namespace ArgumentParsing.Generators.Extensions;
 
 public static class StringExtensions
@@ -27,13 +29,22 @@ public static string ToKebabCase(this string s)
         return new string([.. buffer]);
     }
 
-    public static bool IsValidName(this string s)
+    public static bool IsValidName([NotNullWhen(true)] this string? s, bool allowDashPrefix = false)
     {
-        if (!char.IsLetter(s[0]))
+        if (s is null)
         {
             return false;
         }
 
+        var s0 = s[0];
+        if (!char.IsLetter(s0))
+        {
+            if (s0 != '-' || !allowDashPrefix)
+            {
+                return false;
+            }
+        }
+
         foreach (var ch in s)
         {
             if (ch != '-' && !char.IsLetterOrDigit(ch))

diff --git a/tests/ArgumentParsing.Tests.Unit/SpecialCommandHandlerAnalyzerTests.cs b/tests/ArgumentParsing.Tests.Unit/SpecialCommandHandlerAnalyzerTests.cs
@@ -525,4 +525,71 @@ partial class {{handlerName}}
 
         await VerifyAnalyzerWithCodeFixAsync<DeclareSpecialCommandAliasCodeFixProvider>(source, fixedSource);
     }
+
+    [Theory]
+    [InlineData("%")]
+    [InlineData("--my$name")]
+    [InlineData("2me")]
+    [InlineData("my command")]
+    public async Task InvalidAlias(string invalidName)
+    {
+        var source = $$"""
+            [SpecialCommandAliases({|ARGP0040:"{{invalidName}}"|})]
+            class InfoCommandHandler : ISpecialCommandHandler
+            {
+                public int HandleCommand() => 0;
+            }
+            """;
+
+        await VerifyAnalyzerAsync(source);
+    }
+
+    [Theory]
+    [InlineData("%")]
+    [InlineData("--my$name")]
+    [InlineData("2me")]
+    [InlineData("my command")]
+    public async Task InvalidAlias_TwoAliases(string invalidName)
+    {
+        var source = $$"""
+            [SpecialCommandAliases("--info", {|ARGP0040:"{{invalidName}}"|})]
+            class InfoCommandHandler : ISpecialCommandHandler
+            {
+                public int HandleCommand() => 0;
+            }
+            """;
+
+        await VerifyAnalyzerAsync(source);
+    }
+
+    [Fact]
+    public async Task NameStartsWithLetter()
+    {
+        var source = """
+            [SpecialCommandAliases({|ARGP0041:"info"|})]
+            class InfoCommandHandler : ISpecialCommandHandler
+            {
+                public int HandleCommand() => 0;
+            }
+            """;
+
+        var fixedSource1 = """
+            [SpecialCommandAliases("--info")]
+            class InfoCommandHandler : ISpecialCommandHandler
+            {
+                public int HandleCommand() => 0;
+            }
+            """;
+
+        var fixedSource2 = """
+            [SpecialCommandAliases("-info")]
+            class InfoCommandHandler : ISpecialCommandHandler
+            {
+                public int HandleCommand() => 0;
+            }
+            """;
+
+        await VerifyAnalyzerWithCodeFixAsync<PrefixAliasWithDashesCodeFixProvider>(source, fixedSource1, codeActionIndex: 0);
+        await VerifyAnalyzerWithCodeFixAsync<PrefixAliasWithDashesCodeFixProvider>(source, fixedSource2, codeActionIndex: 1);
+    }
 }
diff --git a/tests/ArgumentParsing.Tests.Unit/Utilities/AnalyzerTestBase.cs b/tests/ArgumentParsing.Tests.Unit/Utilities/AnalyzerTestBase.cs
@@ -16,13 +16,26 @@ protected static Task VerifyAnalyzerAsync(string source, LanguageVersion languag
     protected static Task VerifyAnalyzerAsync(string source, DiagnosticResult[] diagnostics, LanguageVersion languageVersion = LanguageVersion.Latest, ReferenceAssemblies referenceAssemblies = null, CompilerDiagnostics compilerDiagnostics = CompilerDiagnostics.Errors)
         => VerifyAnalyzerWithCodeFixAsync<EmptyCodeFixProvider>(source, fixedSource: null, diagnostics, languageVersion, referenceAssemblies, compilerDiagnostics);
 
-    protected static Task VerifyAnalyzerWithCodeFixAsync<TCodeFix>(string source, string fixedSource, LanguageVersion languageVersion = LanguageVersion.Latest, ReferenceAssemblies referenceAssemblies = null, CompilerDiagnostics compilerDiagnostics = CompilerDiagnostics.Errors)
+    protected static Task VerifyAnalyzerWithCodeFixAsync<TCodeFix>(
+        string source,
+        string fixedSource,
+        LanguageVersion languageVersion = LanguageVersion.Latest,
+        ReferenceAssemblies referenceAssemblies = null,
+        CompilerDiagnostics compilerDiagnostics = CompilerDiagnostics.Errors,
+        int codeActionIndex = 0)
         where TCodeFix : CodeFixProvider, new()
     {
-        return VerifyAnalyzerWithCodeFixAsync<TCodeFix>(source, fixedSource, [], languageVersion, referenceAssemblies, compilerDiagnostics);
+        return VerifyAnalyzerWithCodeFixAsync<TCodeFix>(source, fixedSource, [], languageVersion, referenceAssemblies, compilerDiagnostics, codeActionIndex);
     }
 
-    protected static async Task VerifyAnalyzerWithCodeFixAsync<TCodeFix>(string source, string fixedSource, DiagnosticResult[] diagnostics, LanguageVersion languageVersion = LanguageVersion.Latest, ReferenceAssemblies referenceAssemblies = null, CompilerDiagnostics compilerDiagnostics = CompilerDiagnostics.Errors)
+    protected static async Task VerifyAnalyzerWithCodeFixAsync<TCodeFix>(
+        string source,
+        string fixedSource,
+        DiagnosticResult[] diagnostics,
+        LanguageVersion languageVersion = LanguageVersion.Latest,
+        ReferenceAssemblies referenceAssemblies = null,
+        CompilerDiagnostics compilerDiagnostics = CompilerDiagnostics.Errors,
+        int codeActionIndex = 0)
         where TCodeFix : CodeFixProvider, new()
     {
         var usings = """
@@ -57,6 +70,7 @@ class EmptyOptions { }
                     systemComponentModelDataAnnotationsLibraryReference,
                 }
             },
+            CodeActionIndex = codeActionIndex,
             CompilerDiagnostics = compilerDiagnostics,
             LanguageVersion = languageVersion,
             ReferenceAssemblies = referenceAssemblies ?? ReferenceAssemblies.Net.Net80,