From 00dc19585b9136644db850f372c0c8ad0daed189 Mon Sep 17 00:00:00 2001 From: Ivan Shakhov Date: Tue, 16 Jan 2024 15:30:45 +0100 Subject: [PATCH] provide analyser corresponding to the GD0001 and GD0002, add ClassPartialModifierAnalyzerFix, and tests Co-authored-by: Raul Santos Co-authored-by: A Thousand Ships <96648715+AThousandShips@users.noreply.github.com> --- .../CSharpCodeFixVerifier.cs | 49 ++++++++ .../ClassPartialModifierAnalyzerTest.cs | 20 ++++ .../Godot.SourceGenerators.Tests.csproj | 2 + .../ClassPartialModifier.GD0001.fixed.cs | 6 + .../Sources/ClassPartialModifier.GD0001.cs | 6 + ...uterClassPartialModifierAnalyzer.GD0002.cs | 11 ++ .../ClassPartialModifierAnalyzer.cs | 112 ++++++++++++++++++ .../Godot.SourceGenerators/Common.cs | 74 +++--------- .../Godot.SourceGenerators.csproj | 3 +- .../ScriptMethodsGenerator.cs | 5 +- .../ScriptPathAttributeGenerator.cs | 1 - .../ScriptPropertiesGenerator.cs | 5 +- .../ScriptPropertyDefValGenerator.cs | 4 +- .../ScriptSerializationGenerator.cs | 4 +- .../ScriptSignalsGenerator.cs | 4 +- 15 files changed, 230 insertions(+), 76 deletions(-) create mode 100644 modules/mono/editor/Godot.NET.Sdk/Godot.SourceGenerators.Tests/CSharpCodeFixVerifier.cs create mode 100644 modules/mono/editor/Godot.NET.Sdk/Godot.SourceGenerators.Tests/ClassPartialModifierAnalyzerTest.cs create mode 100644 modules/mono/editor/Godot.NET.Sdk/Godot.SourceGenerators.Tests/TestData/GeneratedSources/ClassPartialModifier.GD0001.fixed.cs create mode 100644 modules/mono/editor/Godot.NET.Sdk/Godot.SourceGenerators.Tests/TestData/Sources/ClassPartialModifier.GD0001.cs create mode 100644 modules/mono/editor/Godot.NET.Sdk/Godot.SourceGenerators.Tests/TestData/Sources/OuterClassPartialModifierAnalyzer.GD0002.cs create mode 100644 modules/mono/editor/Godot.NET.Sdk/Godot.SourceGenerators/ClassPartialModifierAnalyzer.cs 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 new file mode 100644 index 00000000000..51e215a17b2 --- /dev/null +++ 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 + where TCodeFix : CodeFixProvider, new() + where TAnalyzer : DiagnosticAnalyzer, new() +{ + public class Test : CSharpCodeFixTest + { + 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/modules/mono/editor/Godot.NET.Sdk/Godot.SourceGenerators.Tests/ClassPartialModifierAnalyzerTest.cs b/modules/mono/editor/Godot.NET.Sdk/Godot.SourceGenerators.Tests/ClassPartialModifierAnalyzerTest.cs new file mode 100644 index 00000000000..19f05a2f027 --- /dev/null +++ b/modules/mono/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 + .Verify("ClassPartialModifier.GD0001.cs", "ClassPartialModifier.GD0001.fixed.cs"); + } + + [Fact] + public async void OuterClassPartialModifierAnalyzerTest() + { + await CSharpAnalyzerVerifier.Verify("OuterClassPartialModifierAnalyzer.GD0002.cs"); + } +} diff --git a/modules/mono/editor/Godot.NET.Sdk/Godot.SourceGenerators.Tests/Godot.SourceGenerators.Tests.csproj b/modules/mono/editor/Godot.NET.Sdk/Godot.SourceGenerators.Tests/Godot.SourceGenerators.Tests.csproj index 32d7ca74867..e5a81c0e1c1 100644 --- a/modules/mono/editor/Godot.NET.Sdk/Godot.SourceGenerators.Tests/Godot.SourceGenerators.Tests.csproj +++ b/modules/mono/editor/Godot.NET.Sdk/Godot.SourceGenerators.Tests/Godot.SourceGenerators.Tests.csproj @@ -15,6 +15,8 @@ + + diff --git a/modules/mono/editor/Godot.NET.Sdk/Godot.SourceGenerators.Tests/TestData/GeneratedSources/ClassPartialModifier.GD0001.fixed.cs b/modules/mono/editor/Godot.NET.Sdk/Godot.SourceGenerators.Tests/TestData/GeneratedSources/ClassPartialModifier.GD0001.fixed.cs new file mode 100644 index 00000000000..6ba6439d70d --- /dev/null +++ b/modules/mono/editor/Godot.NET.Sdk/Godot.SourceGenerators.Tests/TestData/GeneratedSources/ClassPartialModifier.GD0001.fixed.cs @@ -0,0 +1,6 @@ +using Godot; + +public partial class ClassPartialModifier : Node +{ + +} diff --git a/modules/mono/editor/Godot.NET.Sdk/Godot.SourceGenerators.Tests/TestData/Sources/ClassPartialModifier.GD0001.cs b/modules/mono/editor/Godot.NET.Sdk/Godot.SourceGenerators.Tests/TestData/Sources/ClassPartialModifier.GD0001.cs new file mode 100644 index 00000000000..c8ff673b764 --- /dev/null +++ b/modules/mono/editor/Godot.NET.Sdk/Godot.SourceGenerators.Tests/TestData/Sources/ClassPartialModifier.GD0001.cs @@ -0,0 +1,6 @@ +using Godot; + +public class {|GD0001:ClassPartialModifier|} : Node +{ + +} diff --git a/modules/mono/editor/Godot.NET.Sdk/Godot.SourceGenerators.Tests/TestData/Sources/OuterClassPartialModifierAnalyzer.GD0002.cs b/modules/mono/editor/Godot.NET.Sdk/Godot.SourceGenerators.Tests/TestData/Sources/OuterClassPartialModifierAnalyzer.GD0002.cs new file mode 100644 index 00000000000..49c898e3592 --- /dev/null +++ b/modules/mono/editor/Godot.NET.Sdk/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 new file mode 100644 index 00000000000..e4bdb8db847 --- /dev/null +++ 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 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 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() + .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 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 index 6cd5ddb42f2..ad7962e7df1 100644 --- 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 @@ namespace Godot.SourceGenerators { 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}'"; + 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")); - 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 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 index b1578fb5f44..684ba34012e 100644 --- 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 @@ true - - + diff --git a/modules/mono/editor/Godot.NET.Sdk/Godot.SourceGenerators/ScriptMethodsGenerator.cs b/modules/mono/editor/Godot.NET.Sdk/Godot.SourceGenerators/ScriptMethodsGenerator.cs index 2aa0519269e..f837dcd810a 100644 --- 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 @@ namespace Godot.SourceGenerators { 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 index 6dc541cc596..59cf9946c97 100644 --- 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 @@ namespace Godot.SourceGenerators { 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 index 9c883ffbf00..ecd208e38e7 100644 --- 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 @@ namespace Godot.SourceGenerators { 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 index 27e7632ff7b..e6725cd4a96 100644 --- 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 @@ namespace Godot.SourceGenerators { 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 index 0bc58c2b47f..df0484333aa 100644 --- 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 @@ namespace Godot.SourceGenerators { 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/ScriptSignalsGenerator.cs b/modules/mono/editor/Godot.NET.Sdk/Godot.SourceGenerators/ScriptSignalsGenerator.cs index 323af85d3bf..ff8422ea099 100644 --- a/modules/mono/editor/Godot.NET.Sdk/Godot.SourceGenerators/ScriptSignalsGenerator.cs +++ b/modules/mono/editor/Godot.NET.Sdk/Godot.SourceGenerators/ScriptSignalsGenerator.cs @@ -37,16 +37,14 @@ namespace Godot.SourceGenerators { 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)