From f29d7e49ef1305da597aceeb6cc9da2579a5988c Mon Sep 17 00:00:00 2001 From: Ledjon Behluli <46324828+ledjon-behluli@users.noreply.github.com> Date: Sun, 12 Nov 2023 02:53:55 +0100 Subject: [PATCH] ORLEANS0010 (#8643) * Interface & interface methods analyzer working so far * changed logic for attribute finding to be consistent with the other analyzers * Analyzer working correctly * Code fixer working * removed sandbox for testing analyzers * Add generic arity and namespace/nesting --------- Co-authored-by: Ledjon Behluli Co-authored-by: ReubenBond --- src/Orleans.Analyzers/Constants.cs | 13 +- .../GenerateAliasAttributesAnalyzer.cs | 156 ++++++++++++++++++ .../GenerateAliasAttributesCodeFix.cs | 81 +++++++++ src/Orleans.Analyzers/Resources.Designer.cs | 27 +++ src/Orleans.Analyzers/Resources.resx | 9 + src/Orleans.Analyzers/SyntaxHelpers.cs | 16 +- .../GenerateAliasAttributesAnalyzerTest.cs | 146 ++++++++++++++++ .../GeneratedSerializerTests.cs | 14 ++ .../Orleans.Serialization.UnitTests/Models.cs | 16 ++ 9 files changed, 475 insertions(+), 3 deletions(-) create mode 100644 src/Orleans.Analyzers/GenerateAliasAttributesAnalyzer.cs create mode 100644 src/Orleans.Analyzers/GenerateAliasAttributesCodeFix.cs create mode 100644 test/Analyzers.Tests/GenerateAliasAttributesAnalyzerTest.cs diff --git a/src/Orleans.Analyzers/Constants.cs b/src/Orleans.Analyzers/Constants.cs index 3d408d5316..b092cdc1e2 100644 --- a/src/Orleans.Analyzers/Constants.cs +++ b/src/Orleans.Analyzers/Constants.cs @@ -2,13 +2,22 @@ namespace Orleans.Analyzers { internal static class Constants { + public const string SystemNamespace = "System"; + + public const string IAddressibleFullyQualifiedName = "Orleans.Runtime.IAddressable"; + public const string IdAttributeName = "Id"; public const string IdAttributeFullyQualifiedName = "global::Orleans.IdAttribute"; + public const string GenerateSerializerAttributeName = "GenerateSerializer"; public const string GenerateSerializerAttributeFullyQualifiedName = "global::Orleans.GenerateSerializerAttribute"; + public const string SerializableAttributeName = "Serializable"; + public const string NonSerializedAttribute = "NonSerialized"; public const string NonSerializedAttributeFullyQualifiedName = "global::System.NonSerializedAttribute"; - public const string SystemNamespace = "System"; + + public const string AliasAttributeName = "Alias"; + public const string AliasAttributeFullyQualifiedName = "global::Orleans.AliasAttribute"; } -} +} \ No newline at end of file diff --git a/src/Orleans.Analyzers/GenerateAliasAttributesAnalyzer.cs b/src/Orleans.Analyzers/GenerateAliasAttributesAnalyzer.cs new file mode 100644 index 0000000000..a5f5bb6101 --- /dev/null +++ b/src/Orleans.Analyzers/GenerateAliasAttributesAnalyzer.cs @@ -0,0 +1,156 @@ +using Microsoft.CodeAnalysis; +using Microsoft.CodeAnalysis.CSharp; +using Microsoft.CodeAnalysis.CSharp.Syntax; +using Microsoft.CodeAnalysis.Diagnostics; +using System.Collections.Generic; +using System.Collections.Immutable; +using System.Linq; +using System.Net.NetworkInformation; +using System.Text; + +namespace Orleans.Analyzers; + +[DiagnosticAnalyzer(LanguageNames.CSharp)] +public class GenerateAliasAttributesAnalyzer : DiagnosticAnalyzer +{ + public const string RuleId = "ORLEANS0010"; + private const string Category = "Usage"; + private static readonly LocalizableString Title = new LocalizableResourceString(nameof(Resources.AddAliasAttributesTitle), Resources.ResourceManager, typeof(Resources)); + private static readonly LocalizableString MessageFormat = new LocalizableResourceString(nameof(Resources.AddAliasMessageFormat), Resources.ResourceManager, typeof(Resources)); + private static readonly LocalizableString Description = new LocalizableResourceString(nameof(Resources.AddAliasAttributesDescription), Resources.ResourceManager, typeof(Resources)); + + private static readonly DiagnosticDescriptor Rule = new(RuleId, Title, MessageFormat, Category, DiagnosticSeverity.Info, isEnabledByDefault: true, description: Description); + + public override ImmutableArray SupportedDiagnostics => ImmutableArray.Create(Rule); + + public override void Initialize(AnalysisContext context) + { + context.EnableConcurrentExecution(); + context.ConfigureGeneratedCodeAnalysis(GeneratedCodeAnalysisFlags.Analyze); + context.RegisterSyntaxNodeAction(CheckSyntaxNode, + SyntaxKind.InterfaceDeclaration, + SyntaxKind.ClassDeclaration, + SyntaxKind.StructDeclaration, + SyntaxKind.RecordDeclaration, + SyntaxKind.RecordStructDeclaration); + } + + private void CheckSyntaxNode(SyntaxNodeAnalysisContext context) + { + // Interface types and their methods + if (context.Node is InterfaceDeclarationSyntax { } interfaceDeclaration) + { + if (!context.SemanticModel + .GetDeclaredSymbol(interfaceDeclaration, context.CancellationToken) + .ExtendsGrainInterface()) + { + return; + } + + if (!interfaceDeclaration.HasAttribute(Constants.AliasAttributeName)) + { + ReportFor( + context, + interfaceDeclaration.GetLocation(), + interfaceDeclaration.Identifier.ToString(), + GetArity(interfaceDeclaration), + GetNamespaceAndNesting(interfaceDeclaration)); + } + + foreach (var methodDeclaration in interfaceDeclaration.Members.OfType()) + { + if (methodDeclaration.IsStatic()) + { + continue; + } + + if (!methodDeclaration.HasAttribute(Constants.AliasAttributeName)) + { + ReportFor(context, methodDeclaration.GetLocation(), methodDeclaration.Identifier.ToString(), arity: 0, namespaceAndNesting: null); + } + } + + return; + } + + // Rest of types: class, struct, record + if (context.Node is TypeDeclarationSyntax { } typeDeclaration) + { + if (!typeDeclaration.HasAttribute(Constants.GenerateSerializerAttributeName)) + { + return; + } + + if (typeDeclaration.HasAttribute(Constants.AliasAttributeName)) + { + return; + } + + ReportFor( + context, + typeDeclaration.GetLocation(), + typeDeclaration.Identifier.ToString(), + GetArity(typeDeclaration), + GetNamespaceAndNesting(typeDeclaration)); + } + } + + private static int GetArity(TypeDeclarationSyntax typeDeclarationSyntax) + { + var node = typeDeclarationSyntax; + int arity = 0; + while (node is TypeDeclarationSyntax type) + { + arity += type.Arity; + node = type.Parent as TypeDeclarationSyntax; + } + + return arity; + } + + private static string GetNamespaceAndNesting(TypeDeclarationSyntax typeDeclarationSyntax) + { + SyntaxNode node = typeDeclarationSyntax.Parent; + StringBuilder sb = new(); + Stack segments = new(); + while (node is not null) + { + if (node is TypeDeclarationSyntax type) + { + segments.Push(type.Identifier.ToString()); + } + else if (node is NamespaceDeclarationSyntax ns) + { + segments.Push(ns.Name.ToString()); + } + + node = node.Parent; + } + + foreach (var segment in segments) + { + if (sb.Length > 0) + { + sb.Append('.'); + } + + sb.Append(segment); + } + + return sb.ToString(); + } + + private static void ReportFor(SyntaxNodeAnalysisContext context, Location location, string typeName, int arity, string namespaceAndNesting) + { + var builder = ImmutableDictionary.CreateBuilder(); + + builder.Add("TypeName", typeName); + builder.Add("NamespaceAndNesting", namespaceAndNesting); + builder.Add("Arity", arity.ToString(System.Globalization.CultureInfo.InvariantCulture)); + + context.ReportDiagnostic(Diagnostic.Create( + descriptor: Rule, + location: location, + properties: builder.ToImmutable())); + } +} diff --git a/src/Orleans.Analyzers/GenerateAliasAttributesCodeFix.cs b/src/Orleans.Analyzers/GenerateAliasAttributesCodeFix.cs new file mode 100644 index 0000000000..cb12de93e0 --- /dev/null +++ b/src/Orleans.Analyzers/GenerateAliasAttributesCodeFix.cs @@ -0,0 +1,81 @@ +using System.Collections.Immutable; +using System.Threading.Tasks; +using Microsoft.CodeAnalysis.CodeActions; +using Microsoft.CodeAnalysis; +using Microsoft.CodeAnalysis.CodeFixes; +using System.Composition; +using Microsoft.CodeAnalysis.Editing; +using Microsoft.CodeAnalysis.CSharp.Syntax; +using Microsoft.CodeAnalysis.Simplification; +using static Microsoft.CodeAnalysis.CSharp.SyntaxFactory; + +namespace Orleans.Analyzers; + +[ExportCodeFixProvider(LanguageNames.CSharp, Name = nameof(GenerateAliasAttributesCodeFix)), Shared] +public class GenerateAliasAttributesCodeFix : CodeFixProvider +{ + public override ImmutableArray FixableDiagnosticIds => ImmutableArray.Create(GenerateAliasAttributesAnalyzer.RuleId); + public override FixAllProvider GetFixAllProvider() => WellKnownFixAllProviders.BatchFixer; + + public override async Task RegisterCodeFixesAsync(CodeFixContext context) + { + var root = await context.Document.GetSyntaxRootAsync(context.CancellationToken).ConfigureAwait(false); + + foreach (var diagnostic in context.Diagnostics) + { + var node = root.FindNode(diagnostic.Location.SourceSpan); + if (node != null) + { + // Check if its an interface method + var methodDeclaration = node.FirstAncestorOrSelf(); + if (methodDeclaration != null) + { + await FixFor(context, diagnostic, methodDeclaration); + continue; + } + + // Check if its a type declaration (interface itself, class, struct, record) + var typeDeclaration = node.FirstAncestorOrSelf(); + if (typeDeclaration != null) + { + await FixFor(context, diagnostic, typeDeclaration); + continue; + } + } + } + } + + private static async Task FixFor(CodeFixContext context, Diagnostic diagnostic, SyntaxNode declaration) + { + var documentEditor = await DocumentEditor.CreateAsync(context.Document, context.CancellationToken); + + var arityString = diagnostic.Properties["Arity"] switch + { + null or "0" => "", + string value => $"`{value}" + }; + var typeName = diagnostic.Properties["TypeName"]; + var ns = diagnostic.Properties["NamespaceAndNesting"] switch + { + { Length: > 0 } value => $"{value}.", + _ => "" + }; + + var aliasAttribute = + Attribute( + ParseName(Constants.AliasAttributeFullyQualifiedName)) + .WithArgumentList( + ParseAttributeArgumentList($"(\"{ns}{typeName}{arityString}\")")) + .WithAdditionalAnnotations(Simplifier.Annotation); + + documentEditor.AddAttribute(declaration, aliasAttribute); + var updatedDocument = documentEditor.GetChangedDocument(); + + context.RegisterCodeFix( + action: CodeAction.Create( + Resources.AddAliasAttributesTitle, + createChangedDocument: ct => Task.FromResult(updatedDocument), + equivalenceKey: GenerateAliasAttributesAnalyzer.RuleId), + diagnostic: diagnostic); + } +} diff --git a/src/Orleans.Analyzers/Resources.Designer.cs b/src/Orleans.Analyzers/Resources.Designer.cs index e07f159362..608bc77468 100644 --- a/src/Orleans.Analyzers/Resources.Designer.cs +++ b/src/Orleans.Analyzers/Resources.Designer.cs @@ -78,6 +78,33 @@ internal static string AbstractOrStaticMembersCannotBeSerializedTitle { } } + /// + /// Looks up a localized string similar to Add [Alias] attributes to specify well-known names that can be used to identify types or methods. + /// + internal static string AddAliasAttributesDescription { + get { + return ResourceManager.GetString("AddAliasAttributesDescription", resourceCulture); + } + } + + /// + /// Looks up a localized string similar to Add missing alias attributes. + /// + internal static string AddAliasAttributesTitle { + get { + return ResourceManager.GetString("AddAliasAttributesTitle", resourceCulture); + } + } + + /// + /// Looks up a localized string similar to Add missing alias attributes. + /// + internal static string AddAliasMessageFormat { + get { + return ResourceManager.GetString("AddAliasMessageFormat", resourceCulture); + } + } + /// /// Looks up a localized string similar to Add the [GenerateSerializer] attribute to serializable types in your application. /// diff --git a/src/Orleans.Analyzers/Resources.resx b/src/Orleans.Analyzers/Resources.resx index b910957748..206f9b9730 100644 --- a/src/Orleans.Analyzers/Resources.resx +++ b/src/Orleans.Analyzers/Resources.resx @@ -147,4 +147,13 @@ A single type is not allowed to have multiple constructors annotated with the [OrleansConstructor] attribute + + Add [Alias] attributes to specify well-known names that can be used to identify types or methods + + + Add missing alias attributes + + + Add missing alias attributes + \ No newline at end of file diff --git a/src/Orleans.Analyzers/SyntaxHelpers.cs b/src/Orleans.Analyzers/SyntaxHelpers.cs index 4bdf1a7098..89718480b9 100644 --- a/src/Orleans.Analyzers/SyntaxHelpers.cs +++ b/src/Orleans.Analyzers/SyntaxHelpers.cs @@ -144,5 +144,19 @@ public static bool IsFieldOrAutoProperty(this MemberDeclarationSyntax member) return isFieldOrAutoProperty; } + public static bool ExtendsGrainInterface(this INamedTypeSymbol symbol) + { + if (symbol.TypeKind != TypeKind.Interface) return false; + + foreach (var interfaceSymbol in symbol.AllInterfaces) + { + if (Constants.IAddressibleFullyQualifiedName.Equals(interfaceSymbol.ToDisplayString(NullableFlowState.None), StringComparison.Ordinal)) + { + return true; + } + } + + return false; + } } -} +} \ No newline at end of file diff --git a/test/Analyzers.Tests/GenerateAliasAttributesAnalyzerTest.cs b/test/Analyzers.Tests/GenerateAliasAttributesAnalyzerTest.cs new file mode 100644 index 0000000000..9d6f83a67d --- /dev/null +++ b/test/Analyzers.Tests/GenerateAliasAttributesAnalyzerTest.cs @@ -0,0 +1,146 @@ +using Microsoft.CodeAnalysis; +using Orleans.Analyzers; +using Xunit; + +namespace Analyzers.Tests; + +[TestCategory("BVT"), TestCategory("Analyzer")] +public class GenerateAliasAttributesAnalyzerTest : DiagnosticAnalyzerTestBase +{ + private async Task VerifyHasDiagnostic(string code, int diagnosticsCount = 1) + { + var (diagnostics, _) = await GetDiagnosticsAsync(code, Array.Empty()); + + Assert.NotEmpty(diagnostics); + Assert.Equal(diagnosticsCount, diagnostics.Length); + + var diagnostic = diagnostics.First(); + + Assert.Equal(GenerateAliasAttributesAnalyzer.RuleId, diagnostic.Id); + Assert.Equal(DiagnosticSeverity.Info, diagnostic.Severity); + } + + private async Task VerifyHasNoDiagnostic(string code) + { + var (diagnostics, _) = await GetDiagnosticsAsync(code, Array.Empty()); + Assert.Empty(diagnostics); + } + + #region Interfaces & Methods + + [Theory] + [MemberData(nameof(GrainInterfaces))] + public Task GrainInterfaceWithoutAliasAttribute_ShouldTriggerDiagnostic(string grainInterface) + { + var code = $$""" + public interface I : {{grainInterface}} + { + Task M1(); + Task M2(); + + static Task M3() => Task.FromResult(0); + } + """; + + return VerifyHasDiagnostic(code, 3); // 3 diagnostics, because 1 for interface, and 2 for the non-static methods + } + + [Theory] + [MemberData(nameof(GrainInterfaces))] + public Task GrainInterfaceWithAliasAttribute_ShouldNotTriggerDiagnostic(string grainInterface) + { + var code = $$""" + [Alias("I")] + public interface I : {{grainInterface}} + { + [Alias("M1")] Task M1(); + [Alias("M2")] Task M2(); + + static Task M3() => Task.FromResult(0); + } + """; + + return VerifyHasNoDiagnostic(code); + } + + [Fact] + public Task NonGrainInterfaceWithoutAliasAttribute_ShouldNotTriggerDiagnostic() + { + var code = """ + public interface I + { + Task M1(); + Task M2(); + + static Task M3() => Task.FromResult(0); + } + """; + + return VerifyHasNoDiagnostic(code); + } + + public static IEnumerable GrainInterfaces => + new List + { + new object[] { "Orleans.IGrain" }, + new object[] { "Orleans.IGrainWithStringKey" }, + new object[] { "Orleans.IGrainWithGuidKey" }, + new object[] { "Orleans.IGrainWithGuidCompoundKey" }, + new object[] { "Orleans.IGrainWithIntegerKey" }, + new object[] { "Orleans.IGrainWithIntegerCompoundKey" } + }; + + #endregion + + #region Classes, Structs, Records + + [Fact] + public Task ClassWithoutAliasAttribute_AndWithGenerateSerializerAttribute_ShouldTriggerDiagnostic() + => VerifyHasDiagnostic("[GenerateSerializer] public class C {}"); + + [Fact] + public Task StructWithoutAliasAttribute_AndWithGenerateSerializerAttribute_ShouldTriggerDiagnostic() + => VerifyHasDiagnostic("[GenerateSerializer] public struct S {}"); + + [Fact] + public Task RecordClassWithoutAliasAttribute_AndWithGenerateSerializerAttribute_ShouldTriggerDiagnostic() + => VerifyHasDiagnostic("[GenerateSerializer] public record R {}"); + + [Fact] + public Task RecordStructWithoutAliasAttribute_AndWithGenerateSerializerAttribute_ShouldTriggerDiagnostic() + => VerifyHasDiagnostic("[GenerateSerializer] public record struct RS {}"); + + [Fact] + public Task ClassWithAliasAttribute_AndWithGenerateSerializerAttribute_ShouldNotTriggerDiagnostic() + => VerifyHasNoDiagnostic("[GenerateSerializer, Alias(\"C\")] public class C {}"); + + [Fact] + public Task StructWithAliasAttribute_AndWithGenerateSerializerAttribute_ShouldNotTriggerDiagnostic() + => VerifyHasNoDiagnostic("[GenerateSerializer, Alias(\"S\")] public struct S {}"); + + [Fact] + public Task RecordClassWithAliasAttribute_AndWithGenerateSerializerAttribute_ShouldNotTriggerDiagnostic() + => VerifyHasNoDiagnostic("[GenerateSerializer, Alias(\"R\")] public record R {}"); + + [Fact] + public Task RecordStructWithAliasAttribute_AndWithGenerateSerializerAttribute_ShouldNotTriggerDiagnostic() + => VerifyHasNoDiagnostic("[GenerateSerializer, Alias(\"RS\")] public record struct RS {}"); + + [Fact] + public Task ClassWithoutAliasAttribute_AndWithoutGenerateSerializerAttribute_ShouldNotTriggerDiagnostic() + => VerifyHasNoDiagnostic("public class C {}"); + + [Fact] + public Task StructWithoutAliasAttribute_AndWithoutGenerateSerializerAttribute_ShouldNotTriggerDiagnostic() + => VerifyHasNoDiagnostic("public struct S {}"); + + [Fact] + public Task RecordClassWithoutAliasAttribute_AndWithoutGenerateSerializerAttribute_ShouldNotTriggerDiagnostic() + => VerifyHasNoDiagnostic("public record R {}"); + + [Fact] + public Task RecordStructWithoutAliasAttribute_AndWithoutGenerateSerializerAttribute_ShouldNotTriggerDiagnostic() + => VerifyHasNoDiagnostic("public record struct RS {}"); + + #endregion +} diff --git a/test/Orleans.Serialization.UnitTests/GeneratedSerializerTests.cs b/test/Orleans.Serialization.UnitTests/GeneratedSerializerTests.cs index e267646f22..332bd4c935 100644 --- a/test/Orleans.Serialization.UnitTests/GeneratedSerializerTests.cs +++ b/test/Orleans.Serialization.UnitTests/GeneratedSerializerTests.cs @@ -278,6 +278,20 @@ public void NestedGenericPocoWithTypeAlias() Assert.Contains("gpoco`1[[gpoco`1[[string]]]]", formattedBitStream); } + [Fact] + public void GenericArityAliasTest() + { + { + var original = new Outer.InnerGen(); + RoundTripThroughUntypedSerializer(original, out var formattedBitStream); + } + + { + var original = new Outer.InnerNonGen(); + RoundTripThroughUntypedSerializer(original, out var formattedBitStream); + } + } + [Fact] public void ArraysAreSupported() { diff --git a/test/Orleans.Serialization.UnitTests/Models.cs b/test/Orleans.Serialization.UnitTests/Models.cs index d6ca2e7600..17fd42935a 100644 --- a/test/Orleans.Serialization.UnitTests/Models.cs +++ b/test/Orleans.Serialization.UnitTests/Models.cs @@ -628,6 +628,7 @@ public class GenericPoco } [GenerateSerializer] + [Alias("GenericPocoWithConstraint`2")] public class GenericPocoWithConstraint : GenericPoco where TClass : List, new() where TStruct : struct { @@ -638,6 +639,21 @@ public class GenericPocoWithConstraint public TStruct ValueField { get; set; } } + public sealed class Outer + { + [GenerateSerializer] + [Alias("Orleans.Serialization.UnitTests.Outer.InnerNonGen`1")] + public class InnerNonGen + { + } + + [GenerateSerializer] + [Alias("Orleans.Serialization.UnitTests.Outer.InnerGen`2")] + public class InnerGen + { + } + } + [GenerateSerializer] public class ArrayPoco {