From bbfff4632ea2807d565857ba517633d2227deec2 Mon Sep 17 00:00:00 2001 From: Daniel Cazzulino Date: Mon, 29 Jul 2024 18:22:37 -0300 Subject: [PATCH] Improve support for validating indirectly used types Detect lacking partial type in indirect types (i.e. parameters to actor messages). We cannot provide code fixers for these because they have to be reported for an entire compilation (since we need to scan types and usages), which makes the experience suboptimal. But at least we don't fail at run-time. --- .../ActorMessageGenerator.cs | 49 +++++++++++--- src/CloudActors.CodeAnaysis/Diagnostics.cs | 13 +++- .../OrleansGenerator.cs | 2 - .../PartialAnalyzer.cs | 55 +++++++++++++-- src/Tests/CodeFixers.cs | 67 ++++++++++++++----- src/Tests/Customer.cs | 2 +- 6 files changed, 153 insertions(+), 35 deletions(-) diff --git a/src/CloudActors.CodeAnaysis/ActorMessageGenerator.cs b/src/CloudActors.CodeAnaysis/ActorMessageGenerator.cs index dcaf8d5..49e0cd5 100644 --- a/src/CloudActors.CodeAnaysis/ActorMessageGenerator.cs +++ b/src/CloudActors.CodeAnaysis/ActorMessageGenerator.cs @@ -1,3 +1,4 @@ +using System.Collections.Generic; using System.Linq; using Microsoft.CodeAnalysis; using static Devlooped.CloudActors.Diagnostics; @@ -15,13 +16,42 @@ public void Initialize(IncrementalGeneratorInitializationContext context) .Where(IsActorMessage) .Where(t => t.IsPartial()); - context.RegisterImplementationSourceOutput(messages.Combine(options), (ctx, source) => + var additionalTypes = messages.SelectMany((x, _) => + x.GetMembers().OfType() + // Generated serializers only expose public members. + .Where(p => p.DeclaredAccessibility == Accessibility.Public) + .Select(p => p.Type) + .OfType() + .Where(t => t.IsPartial()) + .Concat(x.GetMembers() + .OfType() + // Generated serializers only expose public members. + .Where(m => m.DeclaredAccessibility == Accessibility.Public) + .SelectMany(m => m.Parameters) + .Select(p => p.Type) + .OfType())) + // We already generate separately for actor messages. + .Where(t => !IsActorMessage(t) && t.IsPartial()) + .Collect(); + + context.RegisterImplementationSourceOutput(messages.Combine(options), GenerateCode); + context.RegisterImplementationSourceOutput(additionalTypes.Combine(options), (ctx, source) => { - var (message, options) = source; - var ns = message.ContainingNamespace.ToDisplayString(); - var kind = message.IsRecord ? "record" : "class"; - var output = - $$""" + var (messages, options) = source; + var distinct = new HashSet(messages, SymbolEqualityComparer.Default); + foreach (var message in distinct) + GenerateCode(ctx, (message, options)); + }); + } + + static void GenerateCode(SourceProductionContext ctx, (INamedTypeSymbol, OrleansGeneratorOptions) source) + { + var (message, options) = source; + + var ns = message.ContainingNamespace.ToDisplayString(); + var kind = message.IsRecord ? "record" : "class"; + var output = + $$""" // using System.CodeDom.Compiler; @@ -35,10 +65,9 @@ namespace {{ns}} } """; - var orleans = OrleansGenerator.GenerateCode(options, output, message.Name, ctx.CancellationToken); + var orleans = OrleansGenerator.GenerateCode(options, output, message.Name, ctx.CancellationToken); - ctx.AddSource($"{message.ToFileName()}.Serializable.cs", output); - ctx.AddSource($"{message.ToFileName()}.Serializable.orleans.cs", orleans); - }); + ctx.AddSource($"{message.ToFileName()}.Serializable.cs", output); + ctx.AddSource($"{message.ToFileName()}.Serializable.orleans.cs", orleans); } } \ No newline at end of file diff --git a/src/CloudActors.CodeAnaysis/Diagnostics.cs b/src/CloudActors.CodeAnaysis/Diagnostics.cs index 190e988..3f933c1 100644 --- a/src/CloudActors.CodeAnaysis/Diagnostics.cs +++ b/src/CloudActors.CodeAnaysis/Diagnostics.cs @@ -13,7 +13,7 @@ public static class Diagnostics public static DiagnosticDescriptor MustBePartial { get; } = new( "DCA001", "Actors and messages must be partial", - "Cloud Actors require partial actor and message types.", + "Add the partial keyword to '{0}' as required for types used by actors.", "Build", DiagnosticSeverity.Error, isEnabledByDefault: true); @@ -40,6 +40,17 @@ public static class Diagnostics DiagnosticSeverity.Error, isEnabledByDefault: true); + /// + /// DCA004: Indirectly used types must be serializable + /// + public static DiagnosticDescriptor MustBeSerializable { get; } = new( + "DCA004", + "Types used in actor messages must have a [GenerateSerializer] attribute,", + "Annotate '{0}' with [GenerateSerializer] as it is used by at least one actor message.", + "Build", + DiagnosticSeverity.Error, + isEnabledByDefault: true); + public static SymbolDisplayFormat FullName { get; } = new( typeQualificationStyle: SymbolDisplayTypeQualificationStyle.NameAndContainingTypesAndNamespaces, genericsOptions: SymbolDisplayGenericsOptions.IncludeTypeParameters, diff --git a/src/CloudActors.CodeAnaysis/OrleansGenerator.cs b/src/CloudActors.CodeAnaysis/OrleansGenerator.cs index 971137f..e344e9e 100644 --- a/src/CloudActors.CodeAnaysis/OrleansGenerator.cs +++ b/src/CloudActors.CodeAnaysis/OrleansGenerator.cs @@ -1,10 +1,8 @@ using System; -using System.Collections.Generic; using System.Diagnostics; using System.Linq; using System.Text; using System.Threading; -using System.Xml.Linq; using Microsoft.CodeAnalysis; using Microsoft.CodeAnalysis.CSharp; using Microsoft.CodeAnalysis.CSharp.Syntax; diff --git a/src/CloudActors.CodeAnaysis/PartialAnalyzer.cs b/src/CloudActors.CodeAnaysis/PartialAnalyzer.cs index ccfb6f1..3a38f73 100644 --- a/src/CloudActors.CodeAnaysis/PartialAnalyzer.cs +++ b/src/CloudActors.CodeAnaysis/PartialAnalyzer.cs @@ -1,4 +1,5 @@ -using System.Collections.Immutable; +using System.Collections.Generic; +using System.Collections.Immutable; using System.Linq; using Microsoft.CodeAnalysis; using Microsoft.CodeAnalysis.CSharp; @@ -17,18 +18,16 @@ public override void Initialize(AnalysisContext context) { context.ConfigureGeneratedCodeAnalysis(GeneratedCodeAnalysisFlags.None); context.EnableConcurrentExecution(); + context.RegisterCompilationAction(Analyze); context.RegisterSyntaxNodeAction(Analyze, SyntaxKind.ClassDeclaration, SyntaxKind.RecordDeclaration); } - void Analyze(SyntaxNodeAnalysisContext context) + static void Analyze(SyntaxNodeAnalysisContext context) { if (context.Node is not TypeDeclarationSyntax typeDeclaration || context.Compilation.GetTypeByMetadataName("Devlooped.CloudActors.IActorMessage") is not { } messageType) return; - if (typeDeclaration.Modifiers.Any(SyntaxKind.PartialKeyword)) - return; - var symbol = context.SemanticModel.GetDeclaredSymbol(typeDeclaration); if (symbol is null) return; @@ -39,6 +38,50 @@ void Analyze(SyntaxNodeAnalysisContext context) !symbol.AllInterfaces.Contains(messageType, SymbolEqualityComparer.Default)) return; - context.ReportDiagnostic(Diagnostic.Create(MustBePartial, typeDeclaration.Identifier.GetLocation())); + if (!typeDeclaration.Modifiers.Any(SyntaxKind.PartialKeyword)) + context.ReportDiagnostic(Diagnostic.Create(MustBePartial, typeDeclaration.Identifier.GetLocation(), symbol.Name)); + } + + static void Analyze(CompilationAnalysisContext context) + { + if (context.Compilation.GetTypeByMetadataName("Devlooped.CloudActors.IActorMessage") is not { } messageType) + return; + + var messageTypes = context.Compilation + .Assembly.GetAllTypes() + .OfType() + .Where(x => x.AllInterfaces.Contains(messageType, SymbolEqualityComparer.Default)); + + // Report also for all source-declared custom types used in the message, as constructors or properties + var indirect = new HashSet(messageTypes + .SelectMany(x => x.GetMembers()) + .OfType() + // Generated serializers only expose public members. + .Where(p => p.DeclaredAccessibility == Accessibility.Public) + .Select(p => p.Type) + .Concat(messageTypes.SelectMany(x => x.GetMembers() + .OfType() + // Generated serializers only expose public members. + .Where(m => m.DeclaredAccessibility == Accessibility.Public) + .SelectMany(m => m.Parameters) + .Select(p => p.Type))) + .OfType() + // where the type is not partial + //.Where(t => !t.GetAttributes().Any(a => generateAttr.Equals(a.AttributeClass, SymbolEqualityComparer.Default))), + .Where(t => + !t.GetAttributes().Any(IsActor) && + !t.AllInterfaces.Contains(messageType, SymbolEqualityComparer.Default) && + !t.IsPartial() && + t.Locations.Any(l => l.IsInSource)), + SymbolEqualityComparer.Default); + + foreach (var type in indirect) + { + // select the type declarations + if (type.DeclaringSyntaxReferences.FirstOrDefault()?.GetSyntax() is not TypeDeclarationSyntax declaration) + continue; + + context.ReportDiagnostic(Diagnostic.Create(MustBePartial, declaration!.Identifier.GetLocation(), type.Name)); + } } } diff --git a/src/Tests/CodeFixers.cs b/src/Tests/CodeFixers.cs index 41ad4fd..3b95d2e 100644 --- a/src/Tests/CodeFixers.cs +++ b/src/Tests/CodeFixers.cs @@ -70,34 +70,41 @@ public partial class MyActor { } } [Fact] - public async Task NoGenerateSerializer() + public async Task AddPartialMessage() { - var context = new CSharpAnalyzerTest(); + var context = new CSharpCodeFixTest(); context.ReferenceAssemblies = ReferenceAssemblies.Net.Net80 - .AddPackages([ - new PackageIdentity("Devlooped.CloudActors", "0.4.0"), - new PackageIdentity("Microsoft.Orleans.Serialization.Abstractions", "8.2.0"), - ]); + .AddPackages([new PackageIdentity("Devlooped.CloudActors", "0.4.0")]); context.TestCode = /* lang=c#-test */ """ using Devlooped.CloudActors; - using Orleans; namespace Tests; - [GenerateSerializer] - public record {|DCA002:GetBalance|}() : IActorQuery; + public record {|DCA001:GetBalance|}() : IActorQuery; + """; + + context.FixedCode = + /* lang=c#-test */ + """ + using Devlooped.CloudActors; + + namespace Tests; + + public partial record GetBalance() : IActorQuery; """; await context.RunAsync(); } + [Fact] - public async Task AddPartialMessage() + public async Task ReportPartialIndirectMessage() { - var context = new CSharpCodeFixTest(); + // Can't verify the codefix due to being reported for another node. + var context = new CSharpAnalyzerTest(); context.ReferenceAssemblies = ReferenceAssemblies.Net.Net80 .AddPackages([new PackageIdentity("Devlooped.CloudActors", "0.4.0")]); @@ -108,19 +115,49 @@ public async Task AddPartialMessage() namespace Tests; - public record {|DCA001:GetBalance|}() : IActorQuery; + public record {|DCA001:Address|}(string Street, string City, string State, string Zip); + + public partial record SetAddress(Address Address) : IActorCommand; """; - context.FixedCode = + //context.FixedCode = + // /* lang=c#-test */ + // """ + // using Devlooped.CloudActors; + + // namespace Tests; + + // public partial record Address(string Street, string City, string State, string Zip); + + // public partial record SetAddress(Address Address) : IActorCommand; + // """; + + await context.RunAsync(); + } + + [Fact] + public async Task NoGenerateSerializer() + { + var context = new CSharpAnalyzerTest(); + context.ReferenceAssemblies = ReferenceAssemblies.Net.Net80 + .AddPackages([ + new PackageIdentity("Devlooped.CloudActors", "0.4.0"), + new PackageIdentity("Microsoft.Orleans.Serialization.Abstractions", "8.2.0"), + ]); + + context.TestCode = /* lang=c#-test */ """ using Devlooped.CloudActors; - + using Orleans; + namespace Tests; - public partial record GetBalance() : IActorQuery; + [GenerateSerializer] + public record {|DCA002:GetBalance|}() : IActorQuery; """; await context.RunAsync(); } + } diff --git a/src/Tests/Customer.cs b/src/Tests/Customer.cs index 44bda72..d35b9d2 100644 --- a/src/Tests/Customer.cs +++ b/src/Tests/Customer.cs @@ -34,7 +34,7 @@ await CloudStorageAccount.DevelopmentStorageAccount } } -public record Address(string Street, string City, string State, string Zip); +public partial record Address(string Street, string City, string State, string Zip); public partial record SetAddress(Address Address) : IActorCommand;