provide analyser corresponding to the GD0001 and GD0002, add ClassPar…

…tialModifierAnalyzerFix, and tests Co-authored-by: Raul Santos <[email protected]> Co-authored-by: A Thousand Ships <[email protected]>
godotengine · Feb 21, 2024 · 00dc195 · 00dc195
1 parent 0246230
commit 00dc195
Show file tree

Hide file tree

Showing 15 changed files with 231 additions and 77 deletions.
diff --git a/modules/mono/editor/Godot.NET.Sdk/Godot.SourceGenerators.Tests/CSharpCodeFixVerifier.cs b/modules/mono/editor/Godot.NET.Sdk/Godot.SourceGenerators.Tests/CSharpCodeFixVerifier.cs
@@ -0,0 +1,49 @@
+using System.IO;
+using System.Threading.Tasks;
+using Microsoft.CodeAnalysis;
+using Microsoft.CodeAnalysis.CodeFixes;
+using Microsoft.CodeAnalysis.CSharp.Testing;
+using Microsoft.CodeAnalysis.Diagnostics;
+using Microsoft.CodeAnalysis.Testing;
+using Microsoft.CodeAnalysis.Testing.Verifiers;
+
+namespace Godot.SourceGenerators.Tests;
+
+public static class CSharpCodeFixVerifier<TCodeFix, TAnalyzer>
+    where TCodeFix : CodeFixProvider, new()
+    where TAnalyzer : DiagnosticAnalyzer, new()
+{
+    public class Test : CSharpCodeFixTest<TAnalyzer, TCodeFix, XUnitVerifier>
+    {
+        public Test()
+        {
+            ReferenceAssemblies = ReferenceAssemblies.Net.Net60;
+            SolutionTransforms.Add((Solution solution, ProjectId projectId) =>
+            {
+                Project project = solution.GetProject(projectId)!
+                    .AddMetadataReference(Constants.GodotSharpAssembly.CreateMetadataReference());
+                return project.Solution;
+            });
+        }
+    }
+
+    public static Task Verify(string sources, string fixedSources)
+    {
+        return MakeVerifier(sources, fixedSources).RunAsync();
+    }
+
+    public static Test MakeVerifier(string source, string results)
+    {
+        var verifier = new Test();
+
+        verifier.TestCode = File.ReadAllText(Path.Combine(Constants.SourceFolderPath, source));
+        verifier.FixedCode = File.ReadAllText(Path.Combine(Constants.GeneratedSourceFolderPath, results));
+
+        verifier.TestState.AnalyzerConfigFiles.Add(("/.globalconfig", $"""
+        is_global = true
+        build_property.GodotProjectDir = {Constants.ExecutingAssemblyPath}
+        """));
+
+        return verifier;
+    }
+}
diff --git a/...ono/editor/Godot.NET.Sdk/Godot.SourceGenerators.Tests/ClassPartialModifierAnalyzerTest.cs b/...ono/editor/Godot.NET.Sdk/Godot.SourceGenerators.Tests/ClassPartialModifierAnalyzerTest.cs
@@ -0,0 +1,20 @@
+using System.Threading.Tasks;
+using Xunit;
+
+namespace Godot.SourceGenerators.Tests;
+
+public class ClassPartialModifierTest
+{
+    [Fact]
+    public async Task ClassPartialModifierCodeFixTest()
+    {
+        await CSharpCodeFixVerifier<ClassPartialModifierCodeFixProvider, ClassPartialModifierAnalyzer>
+            .Verify("ClassPartialModifier.GD0001.cs", "ClassPartialModifier.GD0001.fixed.cs");
+    }
+
+    [Fact]
+    public async void OuterClassPartialModifierAnalyzerTest()
+    {
+        await CSharpAnalyzerVerifier<ClassPartialModifierAnalyzer>.Verify("OuterClassPartialModifierAnalyzer.GD0002.cs");
+    }
+}
diff --git a/...ono/editor/Godot.NET.Sdk/Godot.SourceGenerators.Tests/Godot.SourceGenerators.Tests.csproj b/...ono/editor/Godot.NET.Sdk/Godot.SourceGenerators.Tests/Godot.SourceGenerators.Tests.csproj
@@ -15,6 +15,8 @@
   </PropertyGroup>
 
   <ItemGroup>
+    <PackageReference Include="Microsoft.CodeAnalysis.CSharp.CodeFix.Testing" Version="1.1.1" />
+    <PackageReference Include="Microsoft.CodeAnalysis.CSharp.CodeFix.Testing.XUnit" Version="1.1.1" />
     <PackageReference Include="Microsoft.CodeAnalysis.CSharp.Workspaces" Version="4.8.0" />
     <PackageReference Include="Microsoft.CodeAnalysis.CSharp.SourceGenerators.Testing.XUnit" Version="1.1.1" />
     <PackageReference Include="Microsoft.NET.Test.Sdk" Version="17.7.1" />

diff --git a/...dot.SourceGenerators.Tests/TestData/GeneratedSources/ClassPartialModifier.GD0001.fixed.cs b/...dot.SourceGenerators.Tests/TestData/GeneratedSources/ClassPartialModifier.GD0001.fixed.cs
@@ -0,0 +1,6 @@
+using Godot;
+
+public partial class ClassPartialModifier : Node
+{
+
+}
diff --git a/...odot.NET.Sdk/Godot.SourceGenerators.Tests/TestData/Sources/ClassPartialModifier.GD0001.cs b/...odot.NET.Sdk/Godot.SourceGenerators.Tests/TestData/Sources/ClassPartialModifier.GD0001.cs
@@ -0,0 +1,6 @@
+using Godot;
+
+public class {|GD0001:ClassPartialModifier|} : Node
+{
+
+}
diff --git a/...Godot.SourceGenerators.Tests/TestData/Sources/OuterClassPartialModifierAnalyzer.GD0002.cs b/...Godot.SourceGenerators.Tests/TestData/Sources/OuterClassPartialModifierAnalyzer.GD0002.cs
@@ -0,0 +1,11 @@
+using Godot;
+
+public class {|GD0002:OuterOuterClassPartialModifierAnalyzer|}
+{
+    public class {|GD0002:OuterClassPartialModifierAnalyzer|}
+    {
+        // MyNode is contained in a non-partial type so the source generators
+        // can't enhance this type to work with Godot.
+        public partial class MyNode : Node { }
+    }
+}
diff --git a/modules/mono/editor/Godot.NET.Sdk/Godot.SourceGenerators/ClassPartialModifierAnalyzer.cs b/modules/mono/editor/Godot.NET.Sdk/Godot.SourceGenerators/ClassPartialModifierAnalyzer.cs
@@ -0,0 +1,112 @@
+using System.Collections.Immutable;
+using System.Linq;
+using System.Threading;
+using System.Threading.Tasks;
+using Microsoft.CodeAnalysis;
+using Microsoft.CodeAnalysis.CodeActions;
+using Microsoft.CodeAnalysis.CodeFixes;
+using Microsoft.CodeAnalysis.CSharp;
+using Microsoft.CodeAnalysis.CSharp.Syntax;
+using Microsoft.CodeAnalysis.Diagnostics;
+
+namespace Godot.SourceGenerators
+{
+    [DiagnosticAnalyzer(LanguageNames.CSharp)]
+    public sealed class ClassPartialModifierAnalyzer : DiagnosticAnalyzer
+    {
+        public override ImmutableArray<DiagnosticDescriptor> SupportedDiagnostics =>
+            ImmutableArray.Create(Common.ClassPartialModifierRule, Common.OuterClassPartialModifierRule);
+
+        public override void Initialize(AnalysisContext context)
+        {
+            context.ConfigureGeneratedCodeAnalysis(GeneratedCodeAnalysisFlags.None);
+            context.EnableConcurrentExecution();
+            context.RegisterSyntaxNodeAction(AnalyzeNode, SyntaxKind.ClassDeclaration);
+        }
+
+        private void AnalyzeNode(SyntaxNodeAnalysisContext context)
+        {
+            if (context.Node is not ClassDeclarationSyntax classDeclaration)
+                return;
+
+            if (context.ContainingSymbol is not INamedTypeSymbol typeSymbol)
+                return;
+
+            if (!typeSymbol.InheritsFrom("GodotSharp", GodotClasses.GodotObject))
+                return;
+
+            if (!classDeclaration.IsPartial())
+                context.ReportDiagnostic(Diagnostic.Create(
+                    Common.ClassPartialModifierRule,
+                    classDeclaration.Identifier.GetLocation(),
+                    typeSymbol.ToDisplayString()));
+
+            var outerClassDeclaration = context.Node.Parent as ClassDeclarationSyntax;
+            while (outerClassDeclaration is not null)
+            {
+                var outerClassTypeSymbol = context.SemanticModel.GetDeclaredSymbol(outerClassDeclaration);
+                if (outerClassTypeSymbol == null)
+                    return;
+
+                if (!outerClassDeclaration.IsPartial())
+                    context.ReportDiagnostic(Diagnostic.Create(
+                        Common.OuterClassPartialModifierRule,
+                        outerClassDeclaration.Identifier.GetLocation(),
+                        outerClassTypeSymbol.ToDisplayString()));
+
+                outerClassDeclaration = outerClassDeclaration.Parent as ClassDeclarationSyntax;
+            }
+        }
+    }
+
+    [ExportCodeFixProvider(LanguageNames.CSharp)]
+    public sealed class ClassPartialModifierCodeFixProvider : CodeFixProvider
+    {
+        public override ImmutableArray<string> FixableDiagnosticIds =>
+            ImmutableArray.Create(Common.ClassPartialModifierRule.Id);
+
+        public override FixAllProvider GetFixAllProvider() => WellKnownFixAllProviders.BatchFixer;
+
+        public override async Task RegisterCodeFixesAsync(CodeFixContext context)
+        {
+            // Get the syntax root of the document.
+            var root = await context.Document.GetSyntaxRootAsync(context.CancellationToken).ConfigureAwait(false);
+
+            // Get the diagnostic to fix.
+            var diagnostic = context.Diagnostics.First();
+
+            // Get the location of code issue.
+            var diagnosticSpan = diagnostic.Location.SourceSpan;
+
+            // Use that location to find the containing class declaration.
+            var classDeclaration = root?.FindToken(diagnosticSpan.Start)
+                .Parent?
+                .AncestorsAndSelf()
+                .OfType<ClassDeclarationSyntax>()
+                .First();
+
+            if (classDeclaration == null)
+                return;
+
+            context.RegisterCodeFix(
+                CodeAction.Create(
+                    "Add partial modifier",
+                    cancellationToken => AddPartialModifierAsync(context.Document, classDeclaration, cancellationToken),
+                    classDeclaration.ToFullString()),
+                context.Diagnostics);
+        }
+
+        private static async Task<Document> AddPartialModifierAsync(Document document,
+            ClassDeclarationSyntax classDeclaration, CancellationToken cancellationToken)
+        {
+            // Create a new partial modifier.
+            var partialModifier = SyntaxFactory.Token(SyntaxKind.PartialKeyword);
+            var modifiedClassDeclaration = classDeclaration.AddModifiers(partialModifier);
+            var root = await document.GetSyntaxRootAsync(cancellationToken).ConfigureAwait(false);
+            // Replace the old class declaration with the modified one in the syntax root.
+            var newRoot = root!.ReplaceNode(classDeclaration, modifiedClassDeclaration);
+            var newDocument = document.WithSyntaxRoot(newRoot);
+            return newDocument;
+        }
+    }
+}
diff --git a/modules/mono/editor/Godot.NET.Sdk/Godot.SourceGenerators/Common.cs b/modules/mono/editor/Godot.NET.Sdk/Godot.SourceGenerators/Common.cs
@@ -7,63 +7,25 @@ public static partial class Common
     {
         private static readonly string _helpLinkFormat = $"{VersionDocsUrl}/tutorials/scripting/c_sharp/diagnostics/{{0}}.html";
 
-        public static void ReportNonPartialGodotScriptClass(
-            GeneratorExecutionContext context,
-            ClassDeclarationSyntax cds, INamedTypeSymbol symbol
-        )
-        {
-            string message =
-                "Missing partial modifier on declaration of type '" +
-                $"{symbol.FullQualifiedNameOmitGlobal()}' that derives from '{GodotClasses.GodotObject}'";
-
-            string description = $"{message}. Classes that derive from '{GodotClasses.GodotObject}' " +
-                                 "must be declared with the partial modifier.";
-
-            context.ReportDiagnostic(Diagnostic.Create(
-                new DiagnosticDescriptor(id: "GD0001",
-                    title: message,
-                    messageFormat: message,
-                    category: "Usage",
-                    DiagnosticSeverity.Error,
-                    isEnabledByDefault: true,
-                    description,
-                    helpLinkUri: string.Format(_helpLinkFormat, "GD0001")),
-                cds.GetLocation(),
-                cds.SyntaxTree.FilePath));
-        }
-
-        public static void ReportNonPartialGodotScriptOuterClass(
-            GeneratorExecutionContext context,
-            TypeDeclarationSyntax outerTypeDeclSyntax
-        )
-        {
-            var outerSymbol = context.Compilation
-                .GetSemanticModel(outerTypeDeclSyntax.SyntaxTree)
-                .GetDeclaredSymbol(outerTypeDeclSyntax);
-
-            string fullQualifiedName = outerSymbol is INamedTypeSymbol namedTypeSymbol ?
-                namedTypeSymbol.FullQualifiedNameOmitGlobal() :
-                "type not found";
-
-            string message =
-                $"Missing partial modifier on declaration of type '{fullQualifiedName}', " +
-                $"which contains nested classes that derive from '{GodotClasses.GodotObject}'";
-
-            string description = $"{message}. Classes that derive from '{GodotClasses.GodotObject}' and their " +
-                                 "containing types must be declared with the partial modifier.";
-
-            context.ReportDiagnostic(Diagnostic.Create(
-                new DiagnosticDescriptor(id: "GD0002",
-                    title: message,
-                    messageFormat: message,
-                    category: "Usage",
-                    DiagnosticSeverity.Error,
-                    isEnabledByDefault: true,
-                    description,
-                    helpLinkUri: string.Format(_helpLinkFormat, "GD0002")),
-                outerTypeDeclSyntax.GetLocation(),
-                outerTypeDeclSyntax.SyntaxTree.FilePath));
-        }
+        internal static readonly DiagnosticDescriptor ClassPartialModifierRule =
+            new DiagnosticDescriptor(id: "GD0001",
+                title: $"Missing partial modifier on declaration of type that derives from '{GodotClasses.GodotObject}'",
+                messageFormat: $"Missing partial modifier on declaration of type '{{0}}' that derives from '{GodotClasses.GodotObject}'",
+                category: "Usage",
+                DiagnosticSeverity.Error,
+                isEnabledByDefault: true,
+                $"Classes that derive from '{GodotClasses.GodotObject}' must be declared with the partial modifier.",
+                helpLinkUri: string.Format(_helpLinkFormat, "GD0001"));
+
+        internal static readonly DiagnosticDescriptor OuterClassPartialModifierRule =
+            new DiagnosticDescriptor(id: "GD0002",
+                title: $"Missing partial modifier on declaration of type which contains nested classes that derive from '{GodotClasses.GodotObject}'",
+                messageFormat: $"Missing partial modifier on declaration of type '{{0}}' which contains nested classes that derive from '{GodotClasses.GodotObject}'",
+                category: "Usage",
+                DiagnosticSeverity.Error,
+                isEnabledByDefault: true,
+                $"Classes that derive from '{GodotClasses.GodotObject}' and their containing types must be declared with the partial modifier.",
+                helpLinkUri: string.Format(_helpLinkFormat, "GD0002"));
 
         public static readonly DiagnosticDescriptor MultipleClassesInGodotScriptRule =
             new DiagnosticDescriptor(id: "GD0003",

diff --git a/modules/mono/editor/Godot.NET.Sdk/Godot.SourceGenerators/Godot.SourceGenerators.csproj b/modules/mono/editor/Godot.NET.Sdk/Godot.SourceGenerators/Godot.SourceGenerators.csproj
@@ -21,8 +21,7 @@
     <SuppressDependenciesWhenPacking>true</SuppressDependenciesWhenPacking>
   </PropertyGroup>
   <ItemGroup>
-    <PackageReference Include="Microsoft.CodeAnalysis.CSharp" Version="3.8.0" PrivateAssets="all" />
-    <PackageReference Include="Microsoft.CodeAnalysis.Analyzers" Version="3.3.3" PrivateAssets="all" />
+    <PackageReference Include="Microsoft.CodeAnalysis.CSharp.Workspaces" Version="3.11.0" PrivateAssets="all" />
   </ItemGroup>
   <ItemGroup>
     <!-- Package the generator in the analyzer directory of the nuget package -->

diff --git a/modules/mono/editor/Godot.NET.Sdk/Godot.SourceGenerators/ScriptMethodsGenerator.cs b/modules/mono/editor/Godot.NET.Sdk/Godot.SourceGenerators/ScriptMethodsGenerator.cs
@@ -30,16 +30,13 @@ public void Execute(GeneratorExecutionContext context)
                         {
                             if (x.cds.IsPartial())
                             {
-                                if (x.cds.IsNested() && !x.cds.AreAllOuterTypesPartial(out var typeMissingPartial))
+                                if (x.cds.IsNested() && !x.cds.AreAllOuterTypesPartial(out _))
                                 {
-                                    Common.ReportNonPartialGodotScriptOuterClass(context, typeMissingPartial!);
                                     return false;
                                 }
 
                                 return true;
                             }
-
-                            Common.ReportNonPartialGodotScriptClass(context, x.cds, x.symbol);
                             return false;
                         })
                         .Select(x => x.symbol)

diff --git a/modules/mono/editor/Godot.NET.Sdk/Godot.SourceGenerators/ScriptPathAttributeGenerator.cs b/modules/mono/editor/Godot.NET.Sdk/Godot.SourceGenerators/ScriptPathAttributeGenerator.cs
@@ -48,7 +48,6 @@ public void Execute(GeneratorExecutionContext context)
                         {
                             if (x.cds.IsPartial())
                                 return true;
-                            Common.ReportNonPartialGodotScriptClass(context, x.cds, x.symbol);
                             return false;
                         })
                 )

diff --git a/modules/mono/editor/Godot.NET.Sdk/Godot.SourceGenerators/ScriptPropertiesGenerator.cs b/modules/mono/editor/Godot.NET.Sdk/Godot.SourceGenerators/ScriptPropertiesGenerator.cs
@@ -30,16 +30,13 @@ public void Execute(GeneratorExecutionContext context)
                         {
                             if (x.cds.IsPartial())
                             {
-                                if (x.cds.IsNested() && !x.cds.AreAllOuterTypesPartial(out var typeMissingPartial))
+                                if (x.cds.IsNested() && !x.cds.AreAllOuterTypesPartial(out _))
                                 {
-                                    Common.ReportNonPartialGodotScriptOuterClass(context, typeMissingPartial!);
                                     return false;
                                 }
 
                                 return true;
                             }
-
-                            Common.ReportNonPartialGodotScriptClass(context, x.cds, x.symbol);
                             return false;
                         })
                         .Select(x => x.symbol)

diff --git a/modules/mono/editor/Godot.NET.Sdk/Godot.SourceGenerators/ScriptPropertyDefValGenerator.cs b/modules/mono/editor/Godot.NET.Sdk/Godot.SourceGenerators/ScriptPropertyDefValGenerator.cs
@@ -31,16 +31,14 @@ public void Execute(GeneratorExecutionContext context)
                         {
                             if (x.cds.IsPartial())
                             {
-                                if (x.cds.IsNested() && !x.cds.AreAllOuterTypesPartial(out var typeMissingPartial))
+                                if (x.cds.IsNested() && !x.cds.AreAllOuterTypesPartial(out _))
                                 {
-                                    Common.ReportNonPartialGodotScriptOuterClass(context, typeMissingPartial!);
                                     return false;
                                 }
 
                                 return true;
                             }
 
-                            Common.ReportNonPartialGodotScriptClass(context, x.cds, x.symbol);
                             return false;
                         })
                         .Select(x => x.symbol)

diff --git a/modules/mono/editor/Godot.NET.Sdk/Godot.SourceGenerators/ScriptSerializationGenerator.cs b/modules/mono/editor/Godot.NET.Sdk/Godot.SourceGenerators/ScriptSerializationGenerator.cs
@@ -30,16 +30,14 @@ public void Execute(GeneratorExecutionContext context)
                         {
                             if (x.cds.IsPartial())
                             {
-                                if (x.cds.IsNested() && !x.cds.AreAllOuterTypesPartial(out var typeMissingPartial))
+                                if (x.cds.IsNested() && !x.cds.AreAllOuterTypesPartial(out _))
                                 {
-                                    Common.ReportNonPartialGodotScriptOuterClass(context, typeMissingPartial!);
                                     return false;
                                 }
 
                                 return true;
                             }
 
-                            Common.ReportNonPartialGodotScriptClass(context, x.cds, x.symbol);
                             return false;
                         })
                         .Select(x => x.symbol)