From 5981886fb798aba459bf93f78176e7aee1aaaf86 Mon Sep 17 00:00:00 2001 From: Paul Joannon Date: Sat, 17 Feb 2024 21:12:06 +0100 Subject: [PATCH] Clean diagnostic rules Move the following diagnostics into static readonly fields: GD0101, GD0102, GD0103, GD0104, GD0105, GD0106, GD0107, GD0201, GD0202, GD0203, GD0301, GD0302, GD0303, GD0401, GD0402. To be more consistent, the titles for the following diagnostics were modified: GD0101, GD0105, GD0106, GD0302, GD0303, GD0401, GD0402. A subsequent update of the documentation repo is needed. Tests for the following diagnostics were created: GD0201, GD0202, GD0203. --- .../CSharpSourceGeneratorVerifier.cs | 16 +- .../TestData/Sources/EventSignals.cs | 11 + .../TestData/Sources/GlobalClass.GD0401.cs | 6 +- .../TestData/Sources/GlobalClass.GD0402.cs | 6 +- .../Godot.SourceGenerators/Common.cs | 486 ++++-------------- .../ExtensionMethods.cs | 5 + .../GlobalClassAnalyzer.cs | 18 +- .../MustBeVariantAnalyzer.cs | 40 +- .../ScriptPropertiesGenerator.cs | 12 +- .../ScriptPropertyDefValGenerator.cs | 72 ++- .../ScriptSignalsGenerator.cs | 25 +- 11 files changed, 267 insertions(+), 430 deletions(-) diff --git a/modules/mono/editor/Godot.NET.Sdk/Godot.SourceGenerators.Tests/CSharpSourceGeneratorVerifier.cs b/modules/mono/editor/Godot.NET.Sdk/Godot.SourceGenerators.Tests/CSharpSourceGeneratorVerifier.cs index d75922481cb..84e319352d9 100644 --- a/modules/mono/editor/Godot.NET.Sdk/Godot.SourceGenerators.Tests/CSharpSourceGeneratorVerifier.cs +++ b/modules/mono/editor/Godot.NET.Sdk/Godot.SourceGenerators.Tests/CSharpSourceGeneratorVerifier.cs @@ -61,15 +61,15 @@ where TSourceGenerator : ISourceGenerator, new() build_property.GodotProjectDir = {Constants.ExecutingAssemblyPath} """)); - verifier.TestState.Sources.AddRange(sources.Select(source => - { - return (source, SourceText.From(File.ReadAllText(Path.Combine(Constants.SourceFolderPath, source)))); - })); + verifier.TestState.Sources.AddRange(sources.Select(source => ( + source, + SourceText.From(File.ReadAllText(Path.Combine(Constants.SourceFolderPath, source))) + ))); - verifier.TestState.GeneratedSources.AddRange(generatedSources.Select(generatedSource => - { - return (FullGeneratedSourceName(generatedSource), SourceText.From(File.ReadAllText(Path.Combine(Constants.GeneratedSourceFolderPath, generatedSource)), Encoding.UTF8)); - })); + verifier.TestState.GeneratedSources.AddRange(generatedSources.Select(generatedSource => ( + FullGeneratedSourceName(generatedSource), + SourceText.From(File.ReadAllText(Path.Combine(Constants.GeneratedSourceFolderPath, generatedSource)), Encoding.UTF8) + ))); return verifier; } diff --git a/modules/mono/editor/Godot.NET.Sdk/Godot.SourceGenerators.Tests/TestData/Sources/EventSignals.cs b/modules/mono/editor/Godot.NET.Sdk/Godot.SourceGenerators.Tests/TestData/Sources/EventSignals.cs index 160c5d193de..51dc3591574 100644 --- a/modules/mono/editor/Godot.NET.Sdk/Godot.SourceGenerators.Tests/TestData/Sources/EventSignals.cs +++ b/modules/mono/editor/Godot.NET.Sdk/Godot.SourceGenerators.Tests/TestData/Sources/EventSignals.cs @@ -4,4 +4,15 @@ public partial class EventSignals : GodotObject { [Signal] public delegate void MySignalEventHandler(string str, int num); + + private struct MyStruct { } + + [Signal] + private delegate void {|GD0201:MyInvalidSignal|}(); + + [Signal] + private delegate void MyInvalidParameterTypeSignalEventHandler(MyStruct {|GD0202:myStruct|}); + + [Signal] + private delegate MyStruct {|GD0203:MyInvalidReturnTypeSignalEventHandler|}(); } diff --git a/modules/mono/editor/Godot.NET.Sdk/Godot.SourceGenerators.Tests/TestData/Sources/GlobalClass.GD0401.cs b/modules/mono/editor/Godot.NET.Sdk/Godot.SourceGenerators.Tests/TestData/Sources/GlobalClass.GD0401.cs index 6e6d3a6f39d..1908703a712 100644 --- a/modules/mono/editor/Godot.NET.Sdk/Godot.SourceGenerators.Tests/TestData/Sources/GlobalClass.GD0401.cs +++ b/modules/mono/editor/Godot.NET.Sdk/Godot.SourceGenerators.Tests/TestData/Sources/GlobalClass.GD0401.cs @@ -15,8 +15,8 @@ public partial class CustomGlobalClass2 : Node } // This raises a GD0401 diagnostic error: global classes must inherit from GodotObject -{|GD0401:[GlobalClass] -public partial class CustomGlobalClass3 +[GlobalClass] +public partial class {|GD0401:CustomGlobalClass3|} { -}|} +} diff --git a/modules/mono/editor/Godot.NET.Sdk/Godot.SourceGenerators.Tests/TestData/Sources/GlobalClass.GD0402.cs b/modules/mono/editor/Godot.NET.Sdk/Godot.SourceGenerators.Tests/TestData/Sources/GlobalClass.GD0402.cs index 1c0a169841d..4f7885cf37f 100644 --- a/modules/mono/editor/Godot.NET.Sdk/Godot.SourceGenerators.Tests/TestData/Sources/GlobalClass.GD0402.cs +++ b/modules/mono/editor/Godot.NET.Sdk/Godot.SourceGenerators.Tests/TestData/Sources/GlobalClass.GD0402.cs @@ -8,8 +8,8 @@ public partial class CustomGlobalClass : GodotObject } // This raises a GD0402 diagnostic error: global classes can't have any generic type parameter -{|GD0402:[GlobalClass] -public partial class CustomGlobalClass : GodotObject +[GlobalClass] +public partial class {|GD0402:CustomGlobalClass|} : GodotObject { -}|} +} 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 1113629fefc..a47e2e170f5 100644 --- a/modules/mono/editor/Godot.NET.Sdk/Godot.SourceGenerators/Common.cs +++ b/modules/mono/editor/Godot.NET.Sdk/Godot.SourceGenerators/Common.cs @@ -1,7 +1,5 @@ -using System.Linq; using Microsoft.CodeAnalysis; using Microsoft.CodeAnalysis.CSharp.Syntax; -using Microsoft.CodeAnalysis.Diagnostics; namespace Godot.SourceGenerators { @@ -67,430 +65,154 @@ namespace Godot.SourceGenerators outerTypeDeclSyntax.SyntaxTree.FilePath)); } - public static void ReportExportedMemberIsStatic( - GeneratorExecutionContext context, - ISymbol exportedMemberSymbol - ) - { - var locations = exportedMemberSymbol.Locations; - var location = locations.FirstOrDefault(l => l.SourceTree != null) ?? locations.FirstOrDefault(); - bool isField = exportedMemberSymbol is IFieldSymbol; + public static readonly DiagnosticDescriptor ExportedMemberIsStaticRule = + new DiagnosticDescriptor(id: "GD0101", + title: "The exported member is static", + messageFormat: "The exported member '{0}' is static", + category: "Usage", + DiagnosticSeverity.Error, + isEnabledByDefault: true, + "The exported member is static. Only instance fields and properties can be exported. Remove the 'static' modifier, or the '[Export]' attribute.", + helpLinkUri: string.Format(_helpLinkFormat, "GD0101")); - string message = $"Attempted to export static {(isField ? "field" : "property")}: " + - $"'{exportedMemberSymbol.ToDisplayString()}'"; + public static readonly DiagnosticDescriptor ExportedMemberTypeIsNotSupportedRule = + new DiagnosticDescriptor(id: "GD0102", + title: "The type of the exported member is not supported", + messageFormat: "The type of the exported member '{0}' is not supported", + category: "Usage", + DiagnosticSeverity.Error, + isEnabledByDefault: true, + "The type of the exported member is not supported. Use a supported type, or remove the '[Export]' attribute.", + helpLinkUri: string.Format(_helpLinkFormat, "GD0102")); - string description = $"{message}. Only instance fields and properties can be exported." + - " Remove the 'static' modifier or the '[Export]' attribute."; + public static readonly DiagnosticDescriptor ExportedMemberIsReadOnlyRule = + new DiagnosticDescriptor(id: "GD0103", + title: "The exported member is read-only", + messageFormat: "The exported member '{0}' is read-only", + category: "Usage", + DiagnosticSeverity.Error, + isEnabledByDefault: true, + "The exported member is read-only. Exported member must be writable.", + helpLinkUri: string.Format(_helpLinkFormat, "GD0103")); - context.ReportDiagnostic(Diagnostic.Create( - new DiagnosticDescriptor(id: "GD0101", - title: message, - messageFormat: message, - category: "Usage", - DiagnosticSeverity.Error, - isEnabledByDefault: true, - description, - helpLinkUri: string.Format(_helpLinkFormat, "GD0101")), - location, - location?.SourceTree?.FilePath)); - } + public static readonly DiagnosticDescriptor ExportedPropertyIsWriteOnlyRule = + new DiagnosticDescriptor(id: "GD0104", + title: "The exported property is write-only", + messageFormat: "The exported property '{0}' is write-only", + category: "Usage", + DiagnosticSeverity.Error, + isEnabledByDefault: true, + "The exported property is write-only. Exported properties must be readable.", + helpLinkUri: string.Format(_helpLinkFormat, "GD0104")); - public static void ReportExportedMemberTypeNotSupported( - GeneratorExecutionContext context, - ISymbol exportedMemberSymbol - ) - { - var locations = exportedMemberSymbol.Locations; - var location = locations.FirstOrDefault(l => l.SourceTree != null) ?? locations.FirstOrDefault(); - bool isField = exportedMemberSymbol is IFieldSymbol; + public static readonly DiagnosticDescriptor ExportedMemberIsIndexerRule = + new DiagnosticDescriptor(id: "GD0105", + title: "The exported property is an indexer", + messageFormat: "The exported property '{0}' is an indexer", + category: "Usage", + DiagnosticSeverity.Error, + isEnabledByDefault: true, + "The exported property is an indexer. Remove the '[Export]' attribute.", + helpLinkUri: string.Format(_helpLinkFormat, "GD0105")); - string message = $"The type of the exported {(isField ? "field" : "property")} " + - $"is not supported: '{exportedMemberSymbol.ToDisplayString()}'"; + public static readonly DiagnosticDescriptor ExportedMemberIsExplicitInterfaceImplementationRule = + new DiagnosticDescriptor(id: "GD0106", + title: "The exported property is an explicit interface implementation", + messageFormat: "The exported property '{0}' is an explicit interface implementation", + category: "Usage", + DiagnosticSeverity.Error, + isEnabledByDefault: true, + "The exported property is an explicit interface implementation. Remove the '[Export]' attribute.", + helpLinkUri: string.Format(_helpLinkFormat, "GD0106")); - string description = $"{message}. Use a supported type or remove the '[Export]' attribute."; + public static readonly DiagnosticDescriptor OnlyNodesShouldExportNodesRule = + new DiagnosticDescriptor(id: "GD0107", + title: "Types not derived from Node should not export Node members", + messageFormat: "Types not derived from Node should not export Node members", + category: "Usage", + DiagnosticSeverity.Error, + isEnabledByDefault: true, + "Types not derived from Node should not export Node members. Node export is only supported in Node-derived classes.", + helpLinkUri: string.Format(_helpLinkFormat, "GD0107")); - context.ReportDiagnostic(Diagnostic.Create( - new DiagnosticDescriptor(id: "GD0102", - title: message, - messageFormat: message, - category: "Usage", - DiagnosticSeverity.Error, - isEnabledByDefault: true, - description, - helpLinkUri: string.Format(_helpLinkFormat, "GD0102")), - location, - location?.SourceTree?.FilePath)); - } + public static readonly DiagnosticDescriptor SignalDelegateMissingSuffixRule = + new DiagnosticDescriptor(id: "GD0201", + title: "The name of the delegate must end with 'EventHandler'", + messageFormat: "The name of the delegate '{0}' must end with 'EventHandler'", + category: "Usage", + DiagnosticSeverity.Error, + isEnabledByDefault: true, + "The name of the delegate must end with 'EventHandler'. Rename the delegate accordingly, or remove the '[Signal]' attribute.", + helpLinkUri: string.Format(_helpLinkFormat, "GD0201")); - public static void ReportExportedMemberIsReadOnly( - GeneratorExecutionContext context, - ISymbol exportedMemberSymbol - ) - { - var locations = exportedMemberSymbol.Locations; - var location = locations.FirstOrDefault(l => l.SourceTree != null) ?? locations.FirstOrDefault(); - bool isField = exportedMemberSymbol is IFieldSymbol; + public static readonly DiagnosticDescriptor SignalParameterTypeNotSupportedRule = + new DiagnosticDescriptor(id: "GD0202", + title: "The parameter of the delegate signature of the signal is not supported", + messageFormat: "The parameter of the delegate signature of the signal '{0}' is not supported", + category: "Usage", + DiagnosticSeverity.Error, + isEnabledByDefault: true, + "The parameter of the delegate signature of the signal is not supported. Use supported types only, or remove the '[Signal]' attribute.", + helpLinkUri: string.Format(_helpLinkFormat, "GD0202")); - string message = $"The exported {(isField ? "field" : "property")} " + - $"is read-only: '{exportedMemberSymbol.ToDisplayString()}'"; - - string description = isField ? - $"{message}. Exported fields cannot be read-only." : - $"{message}. Exported properties must be writable."; - - context.ReportDiagnostic(Diagnostic.Create( - new DiagnosticDescriptor(id: "GD0103", - title: message, - messageFormat: message, - category: "Usage", - DiagnosticSeverity.Error, - isEnabledByDefault: true, - description, - helpLinkUri: string.Format(_helpLinkFormat, "GD0103")), - location, - location?.SourceTree?.FilePath)); - } - - public static void ReportExportedMemberIsWriteOnly( - GeneratorExecutionContext context, - ISymbol exportedMemberSymbol - ) - { - var locations = exportedMemberSymbol.Locations; - var location = locations.FirstOrDefault(l => l.SourceTree != null) ?? locations.FirstOrDefault(); - - string message = $"The exported property is write-only: '{exportedMemberSymbol.ToDisplayString()}'"; - - string description = $"{message}. Exported properties must be readable."; - - context.ReportDiagnostic(Diagnostic.Create( - new DiagnosticDescriptor(id: "GD0104", - title: message, - messageFormat: message, - category: "Usage", - DiagnosticSeverity.Error, - isEnabledByDefault: true, - description, - helpLinkUri: string.Format(_helpLinkFormat, "GD0104")), - location, - location?.SourceTree?.FilePath)); - } - - public static void ReportExportedMemberIsIndexer( - GeneratorExecutionContext context, - ISymbol exportedMemberSymbol - ) - { - var locations = exportedMemberSymbol.Locations; - var location = locations.FirstOrDefault(l => l.SourceTree != null) ?? locations.FirstOrDefault(); - - string message = $"Attempted to export indexer property: " + - $"'{exportedMemberSymbol.ToDisplayString()}'"; - - string description = $"{message}. Indexer properties can't be exported." + - " Remove the '[Export]' attribute."; - - context.ReportDiagnostic(Diagnostic.Create( - new DiagnosticDescriptor(id: "GD0105", - title: message, - messageFormat: message, - category: "Usage", - DiagnosticSeverity.Error, - isEnabledByDefault: true, - description, - helpLinkUri: string.Format(_helpLinkFormat, "GD0105")), - location, - location?.SourceTree?.FilePath)); - } - - public static void ReportExportedMemberIsExplicitInterfaceImplementation( - GeneratorExecutionContext context, - ISymbol exportedMemberSymbol - ) - { - var locations = exportedMemberSymbol.Locations; - var location = locations.FirstOrDefault(l => l.SourceTree != null) ?? locations.FirstOrDefault(); - - string message = $"Attempted to export explicit interface property implementation: " + - $"'{exportedMemberSymbol.ToDisplayString()}'"; - - string description = $"{message}. Explicit interface implementations can't be exported." + - " Remove the '[Export]' attribute."; - - context.ReportDiagnostic(Diagnostic.Create( - new DiagnosticDescriptor(id: "GD0106", - title: message, - messageFormat: message, - category: "Usage", - DiagnosticSeverity.Error, - isEnabledByDefault: true, - description, - helpLinkUri: string.Format(_helpLinkFormat, "GD0106")), - location, - location?.SourceTree?.FilePath)); - } - - public static void ReportOnlyNodesShouldExportNodes( - GeneratorExecutionContext context, - ISymbol exportedMemberSymbol - ) - { - var locations = exportedMemberSymbol.Locations; - var location = locations.FirstOrDefault(l => l.SourceTree != null) ?? locations.FirstOrDefault(); - bool isField = exportedMemberSymbol is IFieldSymbol; - - string message = $"Types not derived from Node should not export Node {(isField ? "fields" : "properties")}"; - - string description = $"{message}. Node export is only supported in Node-derived classes."; - - context.ReportDiagnostic(Diagnostic.Create( - new DiagnosticDescriptor(id: "GD0107", - title: message, - messageFormat: message, - category: "Usage", - DiagnosticSeverity.Error, - isEnabledByDefault: true, - description), - location, - location?.SourceTree?.FilePath)); - } - - public static void ReportSignalDelegateMissingSuffix( - GeneratorExecutionContext context, - INamedTypeSymbol delegateSymbol) - { - var locations = delegateSymbol.Locations; - var location = locations.FirstOrDefault(l => l.SourceTree != null) ?? locations.FirstOrDefault(); - - string message = "The name of the delegate must end with 'EventHandler': " + - delegateSymbol.ToDisplayString() + - $". Did you mean '{delegateSymbol.Name}EventHandler'?"; - - string description = $"{message}. Rename the delegate accordingly or remove the '[Signal]' attribute."; - - context.ReportDiagnostic(Diagnostic.Create( - new DiagnosticDescriptor(id: "GD0201", - title: message, - messageFormat: message, - category: "Usage", - DiagnosticSeverity.Error, - isEnabledByDefault: true, - description, - helpLinkUri: string.Format(_helpLinkFormat, "GD0201")), - location, - location?.SourceTree?.FilePath)); - } - - public static void ReportSignalParameterTypeNotSupported( - GeneratorExecutionContext context, - IParameterSymbol parameterSymbol) - { - var locations = parameterSymbol.Locations; - var location = locations.FirstOrDefault(l => l.SourceTree != null) ?? locations.FirstOrDefault(); - - string message = "The parameter of the delegate signature of the signal " + - $"is not supported: '{parameterSymbol.ToDisplayString()}'"; - - string description = $"{message}. Use supported types only or remove the '[Signal]' attribute."; - - context.ReportDiagnostic(Diagnostic.Create( - new DiagnosticDescriptor(id: "GD0202", - title: message, - messageFormat: message, - category: "Usage", - DiagnosticSeverity.Error, - isEnabledByDefault: true, - description, - helpLinkUri: string.Format(_helpLinkFormat, "GD0202")), - location, - location?.SourceTree?.FilePath)); - } - - public static void ReportSignalDelegateSignatureMustReturnVoid( - GeneratorExecutionContext context, - INamedTypeSymbol delegateSymbol) - { - var locations = delegateSymbol.Locations; - var location = locations.FirstOrDefault(l => l.SourceTree != null) ?? locations.FirstOrDefault(); - - string message = "The delegate signature of the signal " + - $"must return void: '{delegateSymbol.ToDisplayString()}'"; - - string description = $"{message}. Return void or remove the '[Signal]' attribute."; - - context.ReportDiagnostic(Diagnostic.Create( - new DiagnosticDescriptor(id: "GD0203", - title: message, - messageFormat: message, - category: "Usage", - DiagnosticSeverity.Error, - isEnabledByDefault: true, - description, - helpLinkUri: string.Format(_helpLinkFormat, "GD0203")), - location, - location?.SourceTree?.FilePath)); - } + public static readonly DiagnosticDescriptor SignalDelegateSignatureMustReturnVoidRule = + new DiagnosticDescriptor(id: "GD0203", + title: "The delegate signature of the signal must return void", + messageFormat: "The delegate signature of the signal '{0}' must return void", + category: "Usage", + DiagnosticSeverity.Error, + isEnabledByDefault: true, + "The delegate signature of the signal must return void. Return void, or remove the '[Signal]' attribute.", + helpLinkUri: string.Format(_helpLinkFormat, "GD0203")); public static readonly DiagnosticDescriptor GenericTypeArgumentMustBeVariantRule = new DiagnosticDescriptor(id: "GD0301", title: "The generic type argument must be a Variant compatible type", - messageFormat: "The generic type argument must be a Variant compatible type: {0}", + messageFormat: "The generic type argument '{0}' must be a Variant compatible type", category: "Usage", DiagnosticSeverity.Error, isEnabledByDefault: true, "The generic type argument must be a Variant compatible type. Use a Variant compatible type as the generic type argument.", helpLinkUri: string.Format(_helpLinkFormat, "GD0301")); - public static void ReportGenericTypeArgumentMustBeVariant( - SyntaxNodeAnalysisContext context, - SyntaxNode typeArgumentSyntax, - ISymbol typeArgumentSymbol) - { - string message = "The generic type argument " + - $"must be a Variant compatible type: '{typeArgumentSymbol.ToDisplayString()}'"; - - string description = $"{message}. Use a Variant compatible type as the generic type argument."; - - context.ReportDiagnostic(Diagnostic.Create( - new DiagnosticDescriptor(id: "GD0301", - title: message, - messageFormat: message, - category: "Usage", - DiagnosticSeverity.Error, - isEnabledByDefault: true, - description, - helpLinkUri: string.Format(_helpLinkFormat, "GD0301")), - typeArgumentSyntax.GetLocation(), - typeArgumentSyntax.SyntaxTree.FilePath)); - } - public static readonly DiagnosticDescriptor GenericTypeParameterMustBeVariantAnnotatedRule = new DiagnosticDescriptor(id: "GD0302", - title: "The generic type parameter must be annotated with the MustBeVariant attribute", - messageFormat: "The generic type argument must be a Variant type: {0}", + title: "The generic type parameter must be annotated with the '[MustBeVariant]' attribute", + messageFormat: "The generic type parameter '{0}' must be annotated with the '[MustBeVariant]' attribute", category: "Usage", DiagnosticSeverity.Error, isEnabledByDefault: true, - "The generic type argument must be a Variant type. Use a Variant type as the generic type argument.", + "The generic type parameter must be annotated with the '[MustBeVariant]' attribute. Add the '[MustBeVariant]' attribute to the generic type parameter.", helpLinkUri: string.Format(_helpLinkFormat, "GD0302")); - public static void ReportGenericTypeParameterMustBeVariantAnnotated( - SyntaxNodeAnalysisContext context, - SyntaxNode typeArgumentSyntax, - ISymbol typeArgumentSymbol) - { - string message = "The generic type parameter must be annotated with the MustBeVariant attribute"; - - string description = $"{message}. Add the MustBeVariant attribute to the generic type parameter."; - - context.ReportDiagnostic(Diagnostic.Create( - new DiagnosticDescriptor(id: "GD0302", - title: message, - messageFormat: message, - category: "Usage", - DiagnosticSeverity.Error, - isEnabledByDefault: true, - description, - helpLinkUri: string.Format(_helpLinkFormat, "GD0302")), - typeArgumentSyntax.GetLocation(), - typeArgumentSyntax.SyntaxTree.FilePath)); - } - public static readonly DiagnosticDescriptor TypeArgumentParentSymbolUnhandledRule = new DiagnosticDescriptor(id: "GD0303", - title: "The generic type parameter must be annotated with the MustBeVariant attribute", - messageFormat: "The generic type argument must be a Variant type: {0}", + title: "The parent symbol of a type argument that must be Variant compatible was not handled", + messageFormat: "The parent symbol '{0}' of a type argument that must be Variant compatible was not handled", category: "Usage", DiagnosticSeverity.Error, isEnabledByDefault: true, - "The generic type argument must be a Variant type. Use a Variant type as the generic type argument.", + "The parent symbol of a type argument that must be Variant compatible was not handled. This is an issue in the engine, and should be reported.", helpLinkUri: string.Format(_helpLinkFormat, "GD0303")); - public static void ReportTypeArgumentParentSymbolUnhandled( - SyntaxNodeAnalysisContext context, - SyntaxNode typeArgumentSyntax, - ISymbol parentSymbol) - { - string message = $"Symbol '{parentSymbol.ToDisplayString()}' parent of a type argument " + - "that must be Variant compatible was not handled."; - - string description = $"{message}. Handle type arguments that are children of the unhandled symbol type."; - - context.ReportDiagnostic(Diagnostic.Create( - new DiagnosticDescriptor(id: "GD0303", - title: message, - messageFormat: message, - category: "Usage", - DiagnosticSeverity.Error, - isEnabledByDefault: true, - description, - helpLinkUri: string.Format(_helpLinkFormat, "GD0303")), - typeArgumentSyntax.GetLocation(), - typeArgumentSyntax.SyntaxTree.FilePath)); - } - public static readonly DiagnosticDescriptor GlobalClassMustDeriveFromGodotObjectRule = new DiagnosticDescriptor(id: "GD0401", - title: "The class must derive from GodotObject or a derived class", - messageFormat: "The class '{0}' must derive from GodotObject or a derived class", + title: $"The class must derive from {GodotClasses.GodotObject} or a derived class", + messageFormat: $"The class '{{0}}' must derive from {GodotClasses.GodotObject} or a derived class", category: "Usage", DiagnosticSeverity.Error, isEnabledByDefault: true, - "The class must derive from GodotObject or a derived class. Change the base class or remove the '[GlobalClass]' attribute.", + $"The class must derive from {GodotClasses.GodotObject} or a derived class. Change the base type, or remove the '[GlobalClass]' attribute.", helpLinkUri: string.Format(_helpLinkFormat, "GD0401")); - public static void ReportGlobalClassMustDeriveFromGodotObject( - SyntaxNodeAnalysisContext context, - SyntaxNode classSyntax, - ISymbol typeSymbol) - { - string message = $"The class '{typeSymbol.ToDisplayString()}' must derive from GodotObject or a derived class"; - - string description = $"{message}. Change the base class or remove the '[GlobalClass]' attribute."; - - context.ReportDiagnostic(Diagnostic.Create( - new DiagnosticDescriptor(id: "GD0401", - title: message, - messageFormat: message, - category: "Usage", - DiagnosticSeverity.Error, - isEnabledByDefault: true, - description, - helpLinkUri: string.Format(_helpLinkFormat, "GD0401")), - classSyntax.GetLocation(), - classSyntax.SyntaxTree.FilePath)); - } - public static readonly DiagnosticDescriptor GlobalClassMustNotBeGenericRule = new DiagnosticDescriptor(id: "GD0402", - title: "The class must not contain generic arguments", - messageFormat: "The class '{0}' must not contain generic arguments", + title: "The class must not be generic", + messageFormat: "The class '{0}' must not be generic", category: "Usage", DiagnosticSeverity.Error, isEnabledByDefault: true, - "The class must be a non-generic type. Remove the generic arguments or the '[GlobalClass]' attribute.", - helpLinkUri: string.Format(_helpLinkFormat, "GD0401")); - - public static void ReportGlobalClassMustNotBeGeneric( - SyntaxNodeAnalysisContext context, - SyntaxNode classSyntax, - ISymbol typeSymbol) - { - string message = $"The class '{typeSymbol.ToDisplayString()}' must not contain generic arguments"; - - string description = $"{message}. Remove the generic arguments or the '[GlobalClass]' attribute."; - - context.ReportDiagnostic(Diagnostic.Create( - new DiagnosticDescriptor(id: "GD0402", - title: message, - messageFormat: message, - category: "Usage", - DiagnosticSeverity.Error, - isEnabledByDefault: true, - description, - helpLinkUri: string.Format(_helpLinkFormat, "GD0402")), - classSyntax.GetLocation(), - classSyntax.SyntaxTree.FilePath)); - } + "The class must not be generic. Make the class non-generic, or remove the '[GlobalClass]' attribute.", + helpLinkUri: string.Format(_helpLinkFormat, "GD0402")); } } diff --git a/modules/mono/editor/Godot.NET.Sdk/Godot.SourceGenerators/ExtensionMethods.cs b/modules/mono/editor/Godot.NET.Sdk/Godot.SourceGenerators/ExtensionMethods.cs index c7fd45238db..35db0d6f10a 100644 --- a/modules/mono/editor/Godot.NET.Sdk/Godot.SourceGenerators/ExtensionMethods.cs +++ b/modules/mono/editor/Godot.NET.Sdk/Godot.SourceGenerators/ExtensionMethods.cs @@ -329,6 +329,11 @@ namespace Godot.SourceGenerators } } + public static Location? FirstLocationWithSourceTreeOrDefault(this IEnumerable locations) + { + return locations.FirstOrDefault(location => location.SourceTree != null) ?? locations.FirstOrDefault(); + } + public static string Path(this Location location) => location.SourceTree?.GetLineSpan(location.SourceSpan).Path ?? location.GetLineSpan().Path; diff --git a/modules/mono/editor/Godot.NET.Sdk/Godot.SourceGenerators/GlobalClassAnalyzer.cs b/modules/mono/editor/Godot.NET.Sdk/Godot.SourceGenerators/GlobalClassAnalyzer.cs index bcb35dae8a2..77530ea0493 100644 --- a/modules/mono/editor/Godot.NET.Sdk/Godot.SourceGenerators/GlobalClassAnalyzer.cs +++ b/modules/mono/editor/Godot.NET.Sdk/Godot.SourceGenerators/GlobalClassAnalyzer.cs @@ -9,7 +9,7 @@ using Microsoft.CodeAnalysis.Diagnostics; namespace Godot.SourceGenerators { [DiagnosticAnalyzer(LanguageNames.CSharp)] - public class GlobalClassAnalyzer : DiagnosticAnalyzer + public sealed class GlobalClassAnalyzer : DiagnosticAnalyzer { public override ImmutableArray SupportedDiagnostics => ImmutableArray.Create( @@ -33,10 +33,22 @@ namespace Godot.SourceGenerators return; if (typeSymbol.IsGenericType) - Common.ReportGlobalClassMustNotBeGeneric(context, typeClassDecl, typeSymbol); + { + context.ReportDiagnostic(Diagnostic.Create( + Common.GlobalClassMustNotBeGenericRule, + typeSymbol.Locations.FirstLocationWithSourceTreeOrDefault(), + typeSymbol.ToDisplayString() + )); + } if (!typeSymbol.InheritsFrom("GodotSharp", GodotClasses.GodotObject)) - Common.ReportGlobalClassMustDeriveFromGodotObject(context, typeClassDecl, typeSymbol); + { + context.ReportDiagnostic(Diagnostic.Create( + Common.GlobalClassMustDeriveFromGodotObjectRule, + typeSymbol.Locations.FirstLocationWithSourceTreeOrDefault(), + typeSymbol.ToDisplayString() + )); + } } } } diff --git a/modules/mono/editor/Godot.NET.Sdk/Godot.SourceGenerators/MustBeVariantAnalyzer.cs b/modules/mono/editor/Godot.NET.Sdk/Godot.SourceGenerators/MustBeVariantAnalyzer.cs index b4f78fd218f..95eaca4d3d3 100644 --- a/modules/mono/editor/Godot.NET.Sdk/Godot.SourceGenerators/MustBeVariantAnalyzer.cs +++ b/modules/mono/editor/Godot.NET.Sdk/Godot.SourceGenerators/MustBeVariantAnalyzer.cs @@ -8,7 +8,7 @@ using Microsoft.CodeAnalysis.Diagnostics; namespace Godot.SourceGenerators { [DiagnosticAnalyzer(LanguageNames.CSharp)] - public class MustBeVariantAnalyzer : DiagnosticAnalyzer + public sealed class MustBeVariantAnalyzer : DiagnosticAnalyzer { public override ImmutableArray SupportedDiagnostics => ImmutableArray.Create( @@ -62,7 +62,11 @@ namespace Godot.SourceGenerators { if (!typeParamSymbol.GetAttributes().Any(a => a.AttributeClass?.IsGodotMustBeVariantAttribute() ?? false)) { - Common.ReportGenericTypeParameterMustBeVariantAnnotated(context, typeSyntax, typeSymbol); + context.ReportDiagnostic(Diagnostic.Create( + Common.GenericTypeParameterMustBeVariantAnnotatedRule, + typeSyntax.GetLocation(), + typeSymbol.ToDisplayString() + )); } continue; } @@ -71,8 +75,11 @@ namespace Godot.SourceGenerators if (marshalType is null) { - Common.ReportGenericTypeArgumentMustBeVariant(context, typeSyntax, typeSymbol); - continue; + context.ReportDiagnostic(Diagnostic.Create( + Common.GenericTypeArgumentMustBeVariantRule, + typeSyntax.GetLocation(), + typeSymbol.ToDisplayString() + )); } } } @@ -106,8 +113,15 @@ namespace Godot.SourceGenerators /// The symbol retrieved for the parent node syntax. /// The type node syntax of the argument type to check. /// The symbol retrieved for the type node syntax. + /// /// if the type must be variant and must be analyzed. - private bool ShouldCheckTypeArgument(SyntaxNodeAnalysisContext context, SyntaxNode parentSyntax, ISymbol parentSymbol, TypeSyntax typeArgumentSyntax, ITypeSymbol typeArgumentSymbol, int typeArgumentIndex) + private bool ShouldCheckTypeArgument( + SyntaxNodeAnalysisContext context, + SyntaxNode parentSyntax, + ISymbol parentSymbol, + TypeSyntax typeArgumentSyntax, + ITypeSymbol typeArgumentSymbol, + int typeArgumentIndex) { ITypeParameterSymbol? typeParamSymbol = parentSymbol switch { @@ -120,18 +134,24 @@ namespace Godot.SourceGenerators INamedTypeSymbol { TypeParameters.Length: > 0 } typeSymbol => typeSymbol.TypeParameters[typeArgumentIndex], + _ => null }; - if (typeParamSymbol == null) + if (typeParamSymbol != null) { - Common.ReportTypeArgumentParentSymbolUnhandled(context, typeArgumentSyntax, parentSymbol); - return false; + return typeParamSymbol.GetAttributes() + .Any(a => a.AttributeClass?.IsGodotMustBeVariantAttribute() ?? false); } - return typeParamSymbol.GetAttributes() - .Any(a => a.AttributeClass?.IsGodotMustBeVariantAttribute() ?? false); + context.ReportDiagnostic(Diagnostic.Create( + Common.TypeArgumentParentSymbolUnhandledRule, + typeArgumentSyntax.GetLocation(), + parentSymbol.ToDisplayString() + )); + + 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 6e034c6e720..9c883ffbf00 100644 --- a/modules/mono/editor/Godot.NET.Sdk/Godot.SourceGenerators/ScriptPropertiesGenerator.cs +++ b/modules/mono/editor/Godot.NET.Sdk/Godot.SourceGenerators/ScriptPropertiesGenerator.cs @@ -443,14 +443,22 @@ namespace Godot.SourceGenerators if (propertySymbol.GetMethod == null) { // This should never happen, as we filtered WriteOnly properties, but just in case. - Common.ReportExportedMemberIsWriteOnly(context, propertySymbol); + context.ReportDiagnostic(Diagnostic.Create( + Common.ExportedPropertyIsWriteOnlyRule, + propertySymbol.Locations.FirstLocationWithSourceTreeOrDefault(), + propertySymbol.ToDisplayString() + )); return null; } if (propertySymbol.SetMethod == null || propertySymbol.SetMethod.IsInitOnly) { // This should never happen, as we filtered ReadOnly properties, but just in case. - Common.ReportExportedMemberIsReadOnly(context, propertySymbol); + context.ReportDiagnostic(Diagnostic.Create( + Common.ExportedMemberIsReadOnlyRule, + propertySymbol.Locations.FirstLocationWithSourceTreeOrDefault(), + propertySymbol.ToDisplayString() + )); return null; } } 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 253e24f092e..27e7632ff7b 100644 --- a/modules/mono/editor/Godot.NET.Sdk/Godot.SourceGenerators/ScriptPropertyDefValGenerator.cs +++ b/modules/mono/editor/Godot.NET.Sdk/Godot.SourceGenerators/ScriptPropertyDefValGenerator.cs @@ -131,33 +131,55 @@ namespace Godot.SourceGenerators { if (property.IsStatic) { - Common.ReportExportedMemberIsStatic(context, property); + context.ReportDiagnostic(Diagnostic.Create( + Common.ExportedMemberIsStaticRule, + property.Locations.FirstLocationWithSourceTreeOrDefault(), + property.ToDisplayString() + )); continue; } if (property.IsIndexer) { - Common.ReportExportedMemberIsIndexer(context, property); + context.ReportDiagnostic(Diagnostic.Create( + Common.ExportedMemberIsIndexerRule, + property.Locations.FirstLocationWithSourceTreeOrDefault(), + property.ToDisplayString() + )); continue; } - // TODO: We should still restore read-only properties after reloading assembly. Two possible ways: reflection or turn RestoreGodotObjectData into a constructor overload. - // Ignore properties without a getter, without a setter or with an init-only setter. Godot properties must be both readable and writable. + // TODO: We should still restore read-only properties after reloading assembly. + // Two possible ways: reflection or turn RestoreGodotObjectData into a constructor overload. + // Ignore properties without a getter, without a setter or with an init-only setter. + // Godot properties must be both readable and writable. if (property.IsWriteOnly) { - Common.ReportExportedMemberIsWriteOnly(context, property); + context.ReportDiagnostic(Diagnostic.Create( + Common.ExportedPropertyIsWriteOnlyRule, + property.Locations.FirstLocationWithSourceTreeOrDefault(), + property.ToDisplayString() + )); continue; } if (property.IsReadOnly || property.SetMethod!.IsInitOnly) { - Common.ReportExportedMemberIsReadOnly(context, property); + context.ReportDiagnostic(Diagnostic.Create( + Common.ExportedMemberIsReadOnlyRule, + property.Locations.FirstLocationWithSourceTreeOrDefault(), + property.ToDisplayString() + )); continue; } if (property.ExplicitInterfaceImplementations.Length > 0) { - Common.ReportExportedMemberIsExplicitInterfaceImplementation(context, property); + context.ReportDiagnostic(Diagnostic.Create( + Common.ExportedMemberIsExplicitInterfaceImplementationRule, + property.Locations.FirstLocationWithSourceTreeOrDefault(), + property.ToDisplayString() + )); continue; } @@ -166,7 +188,11 @@ namespace Godot.SourceGenerators if (marshalType == null) { - Common.ReportExportedMemberTypeNotSupported(context, property); + context.ReportDiagnostic(Diagnostic.Create( + Common.ExportedMemberTypeIsNotSupportedRule, + property.Locations.FirstLocationWithSourceTreeOrDefault(), + property.ToDisplayString() + )); continue; } @@ -175,7 +201,10 @@ namespace Godot.SourceGenerators if (!symbol.InheritsFrom("GodotSharp", "Godot.Node") && propertyType.InheritsFrom("GodotSharp", "Godot.Node")) { - Common.ReportOnlyNodesShouldExportNodes(context, property); + context.ReportDiagnostic(Diagnostic.Create( + Common.OnlyNodesShouldExportNodesRule, + property.Locations.FirstLocationWithSourceTreeOrDefault() + )); } } @@ -194,7 +223,7 @@ namespace Godot.SourceGenerators else { var propertyGet = propertyDeclarationSyntax.AccessorList?.Accessors - .Where(a => a.Keyword.IsKind(SyntaxKind.GetKeyword)).FirstOrDefault(); + .FirstOrDefault(a => a.Keyword.IsKind(SyntaxKind.GetKeyword)); if (propertyGet != null) { if (propertyGet.ExpressionBody != null) @@ -253,7 +282,11 @@ namespace Godot.SourceGenerators { if (field.IsStatic) { - Common.ReportExportedMemberIsStatic(context, field); + context.ReportDiagnostic(Diagnostic.Create( + Common.ExportedMemberIsStaticRule, + field.Locations.FirstLocationWithSourceTreeOrDefault(), + field.ToDisplayString() + )); continue; } @@ -261,7 +294,11 @@ namespace Godot.SourceGenerators // Ignore properties without a getter or without a setter. Godot properties must be both readable and writable. if (field.IsReadOnly) { - Common.ReportExportedMemberIsReadOnly(context, field); + context.ReportDiagnostic(Diagnostic.Create( + Common.ExportedMemberIsReadOnlyRule, + field.Locations.FirstLocationWithSourceTreeOrDefault(), + field.ToDisplayString() + )); continue; } @@ -270,7 +307,11 @@ namespace Godot.SourceGenerators if (marshalType == null) { - Common.ReportExportedMemberTypeNotSupported(context, field); + context.ReportDiagnostic(Diagnostic.Create( + Common.ExportedMemberTypeIsNotSupportedRule, + field.Locations.FirstLocationWithSourceTreeOrDefault(), + field.ToDisplayString() + )); continue; } @@ -279,7 +320,10 @@ namespace Godot.SourceGenerators if (!symbol.InheritsFrom("GodotSharp", "Godot.Node") && fieldType.InheritsFrom("GodotSharp", "Godot.Node")) { - Common.ReportOnlyNodesShouldExportNodes(context, field); + context.ReportDiagnostic(Diagnostic.Create( + Common.OnlyNodesShouldExportNodesRule, + field.Locations.FirstLocationWithSourceTreeOrDefault() + )); } } 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 5246cc57809..323af85d3bf 100644 --- a/modules/mono/editor/Godot.NET.Sdk/Godot.SourceGenerators/ScriptSignalsGenerator.cs +++ b/modules/mono/editor/Godot.NET.Sdk/Godot.SourceGenerators/ScriptSignalsGenerator.cs @@ -136,7 +136,11 @@ namespace Godot.SourceGenerators { if (!signalDelegateSymbol.Name.EndsWith(SignalDelegateSuffix)) { - Common.ReportSignalDelegateMissingSuffix(context, signalDelegateSymbol); + context.ReportDiagnostic(Diagnostic.Create( + Common.SignalDelegateMissingSuffixRule, + signalDelegateSymbol.Locations.FirstLocationWithSourceTreeOrDefault(), + signalDelegateSymbol.ToDisplayString() + )); continue; } @@ -154,21 +158,32 @@ namespace Godot.SourceGenerators { if (parameter.RefKind != RefKind.None) { - Common.ReportSignalParameterTypeNotSupported(context, parameter); + context.ReportDiagnostic(Diagnostic.Create( + Common.SignalParameterTypeNotSupportedRule, + parameter.Locations.FirstLocationWithSourceTreeOrDefault(), + parameter.ToDisplayString() + )); continue; } var marshalType = MarshalUtils.ConvertManagedTypeToMarshalType(parameter.Type, typeCache); - if (marshalType == null) { - Common.ReportSignalParameterTypeNotSupported(context, parameter); + context.ReportDiagnostic(Diagnostic.Create( + Common.SignalParameterTypeNotSupportedRule, + parameter.Locations.FirstLocationWithSourceTreeOrDefault(), + parameter.ToDisplayString() + )); } } if (!methodSymbol.ReturnsVoid) { - Common.ReportSignalDelegateSignatureMustReturnVoid(context, signalDelegateSymbol); + context.ReportDiagnostic(Diagnostic.Create( + Common.SignalDelegateSignatureMustReturnVoidRule, + signalDelegateSymbol.Locations.FirstLocationWithSourceTreeOrDefault(), + signalDelegateSymbol.ToDisplayString() + )); } }