Add code fix for ARGP0005 and modify analyzer to report hidden diag…

…nostic in case of an error type, so we can offer the fix in all scenarios
DoctorKrolic · Mar 16, 2024 · 1a3762e · 1a3762e
1 parent 544e951
commit 1a3762e
Show file tree

Hide file tree

Showing 4 changed files with 148 additions and 31 deletions.
diff --git a/src/ArgumentParsing.CodeFixes/WrapReturnTypeIntoParseResultCodeFixProvider.cs b/src/ArgumentParsing.CodeFixes/WrapReturnTypeIntoParseResultCodeFixProvider.cs
@@ -0,0 +1,66 @@
+using System.Collections.Immutable;
+using System.Composition;
+using System.Diagnostics;
+using ArgumentParsing.Generators.Diagnostics;
+using ArgumentParsing.Generators.Extensions;
+using Microsoft.CodeAnalysis;
+using Microsoft.CodeAnalysis.CodeActions;
+using Microsoft.CodeAnalysis.CodeFixes;
+using Microsoft.CodeAnalysis.CSharp;
+using Microsoft.CodeAnalysis.CSharp.Syntax;
+using Microsoft.CodeAnalysis.Editing;
+
+namespace ArgumentParsing.CodeFixes;
+
+[ExportCodeFixProvider(LanguageNames.CSharp), Shared]
+public sealed class WrapReturnTypeIntoParseResultCodeFixProvider : CodeFixProvider
+{
+    public override ImmutableArray<string> FixableDiagnosticIds { get; } = ImmutableArray.Create(DiagnosticDescriptors.ReturnTypeMustBeParseResult.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) is TypeSyntax returnTypeSyntax)
+        {
+            context.RegisterCodeFix(
+                CodeAction.Create(
+                    "Wrap return type into ParseResult<T>",
+                    ct => WrapReturnTypeIntoParseResult(document, root, returnTypeSyntax, ct),
+                    nameof(WrapReturnTypeIntoParseResultCodeFixProvider)),
+                context.Diagnostics[0]);
+        }
+    }
+
+    private static async Task<Document> WrapReturnTypeIntoParseResult(Document document, SyntaxNode root, TypeSyntax returnTypeSyntax, CancellationToken cancellationToken)
+    {
+        var semanticModel = await document.GetSemanticModelAsync(cancellationToken).ConfigureAwait(false);
+        if (semanticModel is null)
+        {
+            Debug.Fail("Shouldn't really happen");
+            return document;
+        }
+
+        var parseResultOfTType = semanticModel.Compilation.ParseResultOfTType();
+        if (parseResultOfTType is null)
+        {
+            Debug.Fail("Shouldn't really happen");
+            return document;
+        }
+
+        var returnType = semanticModel.GetTypeInfo(returnTypeSyntax, cancellationToken).Type!;
+        var parseResultOfReturnType = parseResultOfTType.Construct(returnType);
+
+        var generator = SyntaxGenerator.GetGenerator(document);
+        var wrappedReturnTypeSyntax = generator.TypeExpression(parseResultOfReturnType, addImport: true);
+
+        var changedRoot = root.ReplaceNode(returnTypeSyntax, wrappedReturnTypeSyntax);
+        return document.WithSyntaxRoot(changedRoot);
+    }
+}
diff --git a/src/ArgumentParsing.Generators/Diagnostics/Analyzers/ParserSignatureAnalyzer.cs b/src/ArgumentParsing.Generators/Diagnostics/Analyzers/ParserSignatureAnalyzer.cs
@@ -96,34 +96,35 @@ private static void AnalyzeParserSignature(SymbolAnalysisContext context, KnownT
 
         var returnType = method.ReturnType;
 
-        if (returnType.TypeKind != TypeKind.Error)
-        {
-            var returnTypeSyntax = ((MethodDeclarationSyntax)method.DeclaringSyntaxReferences.First().GetSyntax(context.CancellationToken)).ReturnType;
-            var genericArgumentErrorSyntax = returnTypeSyntax is GenericNameSyntax { TypeArgumentList.Arguments: [var genericArgument] } ? genericArgument : returnTypeSyntax;
+        var returnTypeSyntax = ((MethodDeclarationSyntax)method.DeclaringSyntaxReferences.First().GetSyntax(context.CancellationToken)).ReturnType;
+        var genericArgumentErrorSyntax = returnTypeSyntax is GenericNameSyntax { TypeArgumentList.Arguments: [var genericArgument] } ? genericArgument : returnTypeSyntax;
 
-            if (returnType is not INamedTypeSymbol { TypeArguments: [var optionsType] } namedReturnType ||
-                !namedReturnType.ConstructedFrom.Equals(knownTypes.ParseResultOfTType, SymbolEqualityComparer.Default))
-            {
-                context.ReportDiagnostic(
-                    Diagnostic.Create(
-                        DiagnosticDescriptors.ReturnTypeMustBeParseResult, returnTypeSyntax.GetLocation()));
-            }
-            else if (optionsType is not INamedTypeSymbol { SpecialType: SpecialType.None, TypeKind: TypeKind.Class or TypeKind.Struct } namedOptionsType || !namedOptionsType.Constructors.Any(c => c.Parameters.Length == 0))
-            {
-                if (optionsType.TypeKind != TypeKind.Error)
-                {
-                    context.ReportDiagnostic(
-                        Diagnostic.Create(
-                            DiagnosticDescriptors.InvalidOptionsType, genericArgumentErrorSyntax.GetLocation()));
-                }
-            }
-            else if (!namedOptionsType.GetAttributes().Any(a => a.AttributeClass?.Equals(knownTypes.OptionsTypeAttributeType, SymbolEqualityComparer.Default) == true))
+        if (returnType is not INamedTypeSymbol { TypeArguments: [var optionsType] } namedReturnType ||
+            !namedReturnType.ConstructedFrom.Equals(knownTypes.ParseResultOfTType, SymbolEqualityComparer.Default))
+        {
+            context.ReportDiagnostic(
+                Diagnostic.Create(
+                    DiagnosticDescriptors.ReturnTypeMustBeParseResult,
+                    returnTypeSyntax.GetLocation(),
+                    effectiveSeverity: returnType.TypeKind == TypeKind.Error ? DiagnosticSeverity.Hidden : DiagnosticDescriptors.ReturnTypeMustBeParseResult.DefaultSeverity,
+                    additionalLocations: null,
+                    properties: null));
+        }
+        else if (optionsType is not INamedTypeSymbol { SpecialType: SpecialType.None, TypeKind: TypeKind.Class or TypeKind.Struct } namedOptionsType || !namedOptionsType.Constructors.Any(c => c.Parameters.Length == 0))
+        {
+            if (optionsType.TypeKind != TypeKind.Error)
             {
                 context.ReportDiagnostic(
                     Diagnostic.Create(
-                        DiagnosticDescriptors.OptionsTypeMustBeAnnotatedWithAttribute, genericArgumentErrorSyntax.GetLocation()));
+                        DiagnosticDescriptors.InvalidOptionsType, genericArgumentErrorSyntax.GetLocation()));
             }
         }
+        else if (!namedOptionsType.GetAttributes().Any(a => a.AttributeClass?.Equals(knownTypes.OptionsTypeAttributeType, SymbolEqualityComparer.Default) == true))
+        {
+            context.ReportDiagnostic(
+                Diagnostic.Create(
+                    DiagnosticDescriptors.OptionsTypeMustBeAnnotatedWithAttribute, genericArgumentErrorSyntax.GetLocation()));
+        }
     }
 
     private readonly struct KnownTypes

diff --git a/src/ArgumentParsing.Generators/Extensions/CommonTypesCompilationExtensions.cs b/src/ArgumentParsing.Generators/Extensions/CommonTypesCompilationExtensions.cs
@@ -2,7 +2,7 @@
 
 namespace ArgumentParsing.Generators.Extensions;
 
-internal static class CommonTypesCompilationExtensions
+public static class CommonTypesCompilationExtensions
 {
     public static INamedTypeSymbol? ParseResultOfTType(this Compilation compilation) => compilation.GetTypeByMetadataName("ArgumentParsing.Results.ParseResult`1");
 

diff --git a/tests/ArgumentParsing.Tests.Unit/ParserSignatureAnalyzerTests.cs b/tests/ArgumentParsing.Tests.Unit/ParserSignatureAnalyzerTests.cs
@@ -1,6 +1,8 @@
 using ArgumentParsing.CodeFixes;
 using ArgumentParsing.Generators.Diagnostics.Analyzers;
 using ArgumentParsing.Tests.Unit.Utilities;
+using Microsoft.CodeAnalysis;
+using Microsoft.CodeAnalysis.Testing;
 
 namespace ArgumentParsing.Tests.Unit;
 
@@ -161,27 +163,63 @@ partial class C
     }
 
     [Theory]
-    [InlineData("C")]
-    [InlineData("int")]
-    [InlineData("string")]
     [InlineData("MyOptions")]
     [InlineData("EmptyOptions")]
-    public async Task InvalidReturnType(string invalidType)
+    public async Task InvalidReturnType_ResultsInValidOptionsTypeAfterFix(string returnType)
     {
         var source = $$"""
             partial class C
             {
                 [GeneratedArgumentParser]
-                public static partial {|ARGP0005:{{invalidType}}|} {|CS8795:ParseArguments|}(string[] args);
+                public static partial {|ARGP0005:{{returnType}}|} {|CS8795:ParseArguments|}(string[] args);
             }
 
+            [OptionsType]
             class MyOptions
             {
                 public string S { get; set; }
             }
             """;
 
-        await VerifyAnalyzerAsync(source);
+        var fixedSource = $$"""
+            partial class C
+            {
+                [GeneratedArgumentParser]
+                public static partial ParseResult<{{returnType}}> {|CS8795:ParseArguments|}(string[] args);
+            }
+            
+            [OptionsType]
+            class MyOptions
+            {
+                public string S { get; set; }
+            }
+            """;
+
+        await VerifyAnalyzerWithCodeFixAsync<WrapReturnTypeIntoParseResultCodeFixProvider>(source, fixedSource);
+    }
+
+    [Theory]
+    [InlineData("int")]
+    [InlineData("string")]
+    public async Task InvalidReturnType_ResultsInInvalidOptionsTypeAfterFix(string returnType)
+    {
+        var source = $$"""
+            partial class C
+            {
+                [GeneratedArgumentParser]
+                public static partial {|ARGP0005:{{returnType}}|} {|CS8795:ParseArguments|}(string[] args);
+            }
+            """;
+
+        var fixedSource = $$"""
+            partial class C
+            {
+                [GeneratedArgumentParser]
+                public static partial ParseResult<{|ARGP0006:{{returnType}}|}> {|CS8795:ParseArguments|}(string[] args);
+            }
+            """;
+
+        await VerifyAnalyzerWithCodeFixAsync<WrapReturnTypeIntoParseResultCodeFixProvider>(source, fixedSource);
     }
 
     [Fact]
@@ -191,11 +229,23 @@ public async Task InvalidReturnType_ErrorType()
             partial class C
             {
                 [GeneratedArgumentParser]
-                public static partial {|CS0246:ErrorType|} {|CS8795:ParseArguments|}(string[] args);
+                public static partial {|#0:{|CS0246:ErrorType|}|} {|CS8795:ParseArguments|}(string[] args);
             }
             """;
 
-        await VerifyAnalyzerAsync(source);
+        var fixedSource = """
+            partial class C
+            {
+                [GeneratedArgumentParser]
+                public static partial ParseResult<{|CS0246:ErrorType|}> {|CS8795:ParseArguments|}(string[] args);
+            }
+            """;
+
+        await VerifyAnalyzerWithCodeFixAsync<WrapReturnTypeIntoParseResultCodeFixProvider>(source, fixedSource,
+        [
+            new DiagnosticResult("ARGP0005", DiagnosticSeverity.Hidden)
+                .WithLocation(0)
+        ]);
     }
 
     [Theory]