Avoid using magic strings (#488)

Author: alexeyzimarev

src/Core/gen/Eventuous.Shared.Generators/Constants.cs

@@ -3,8 +3,20 @@
 
 namespace Eventuous.Shared.Generators;
 
+/// <summary>
+/// Constants used for type and member lookups.
+/// These are primarily used for symbol resolution via Compilation.GetTypeByMetadataName()
+/// and as fallback when symbol-based comparison is not available.
+/// The generators now prefer symbol-based comparisons using SymbolEqualityComparer,
+/// which are refactoring-safe and won't break when types are renamed.
+/// </summary>
 internal static class Constants {
-    public const string BaseNamespace      = "Eventuous";
+    /// <summary>Base namespace for Eventuous types.</summary>
+    public const string BaseNamespace = "Eventuous";
+
+    /// <summary>Name of the EventType attribute class (without namespace).</summary>
     public const string EventTypeAttribute = "EventTypeAttribute";
-    public const string EventTypeAttrFqcn  = $"{BaseNamespace}.{EventTypeAttribute}";
+
+    /// <summary>Fully qualified name of the EventType attribute for GetTypeByMetadataName().</summary>
+    public const string EventTypeAttrFqcn = $"{BaseNamespace}.{EventTypeAttribute}";
 }

src/Core/gen/Eventuous.Shared.Generators/EventUsageAnalyzer.cs

@@ -32,11 +32,43 @@ public override void Initialize(AnalysisContext context) {
         context.ConfigureGeneratedCodeAnalysis(GeneratedCodeAnalysisFlags.None);
         context.EnableConcurrentExecution();
 
-        context.RegisterOperationAction(AnalyzeInvocation, OperationKind.Invocation);
-        context.RegisterOperationAction(AnalyzeObjectCreation, OperationKind.ObjectCreation);
+        context.RegisterCompilationStartAction(compilationContext => {
+            // Resolve well-known type symbols once per compilation
+            var compilation = compilationContext.Compilation;
+            var knownTypes = new KnownTypeSymbols(compilation);
+
+            compilationContext.RegisterOperationAction(ctx => AnalyzeInvocation(ctx, knownTypes), OperationKind.Invocation);
+            compilationContext.RegisterOperationAction(ctx => AnalyzeObjectCreation(ctx, knownTypes), OperationKind.ObjectCreation);
+        });
     }
 
-    static ImmutableHashSet<ITypeSymbol> GetExplicitRegistrations(OperationAnalysisContext ctx) {
+    /// <summary>
+    /// Cache of well-known type symbols resolved from the compilation.
+    /// This makes the analyzer refactoring-safe by using symbol comparison instead of string matching.
+    /// </summary>
+    sealed class KnownTypeSymbols {
+        public INamedTypeSymbol? EventTypeAttribute { get; }
+        public INamedTypeSymbol? TypeMapper { get; }
+        public INamedTypeSymbol? Aggregate { get; }
+        public INamedTypeSymbol? State { get; }
+        public INamedTypeSymbol? CommandHandlerBuilder { get; }
+        public INamedTypeSymbol? IDefineExecution { get; }
+        public INamedTypeSymbol? ICommandHandlerBuilder { get; }
+        public INamedTypeSymbol? IDefineStoreOrExecution { get; }
+
+        public KnownTypeSymbols(Compilation compilation) {
+            EventTypeAttribute = compilation.GetTypeByMetadataName(EventTypeAttrFqcn);
+            TypeMapper = compilation.GetTypeByMetadataName($"{BaseNamespace}.TypeMapper");
+            Aggregate = compilation.GetTypeByMetadataName($"{BaseNamespace}.Aggregate`1");
+            State = compilation.GetTypeByMetadataName($"{BaseNamespace}.State`1");
+            CommandHandlerBuilder = compilation.GetTypeByMetadataName($"{BaseNamespace}.CommandHandlerBuilder");
+            IDefineExecution = compilation.GetTypeByMetadataName($"{BaseNamespace}.IDefineExecution");
+            ICommandHandlerBuilder = compilation.GetTypeByMetadataName($"{BaseNamespace}.ICommandHandlerBuilder");
+            IDefineStoreOrExecution = compilation.GetTypeByMetadataName($"{BaseNamespace}.IDefineStoreOrExecution");
+        }
+    }
+
+    static ImmutableHashSet<ITypeSymbol> GetExplicitRegistrations(OperationAnalysisContext ctx, KnownTypeSymbols knownTypes) {
         var model = ctx.Operation.SemanticModel;
         if (model == null) return ImmutableHashSet<ITypeSymbol>.Empty;
         var root = ctx.Operation.Syntax.SyntaxTree.GetRoot();
@@ -45,12 +77,18 @@ static ImmutableHashSet<ITypeSymbol> GetExplicitRegistrations(OperationAnalysisC
         foreach (var invSyntax in root.DescendantNodes().OfType<InvocationExpressionSyntax>()) {
             if (model.GetOperation(invSyntax) is not IInvocationOperation op) continue;
             var m = op.TargetMethod;
+
+            // Use symbol comparison when available, fall back to string comparison
             if (m.Name != "AddType") continue;
             var ct = m.ContainingType;
             if (ct == null) continue;
-            if (ct.Name != "TypeMapper") continue;
-            var ns = ct.ContainingNamespace?.ToDisplayString();
-            if (ns != BaseNamespace) continue;
+
+            // Prefer symbol comparison (refactoring-safe)
+            var isTypeMapper = knownTypes.TypeMapper != null
+                ? SymbolEqualityComparer.Default.Equals(ct, knownTypes.TypeMapper)
+                : ct.Name == "TypeMapper" && ct.ContainingNamespace?.ToDisplayString() == BaseNamespace;
+
+            if (!isTypeMapper) continue;
 
             if (m.TypeArguments.Length == 1) {
                 set.Add(m.TypeArguments[0]);
@@ -65,12 +103,12 @@ static ImmutableHashSet<ITypeSymbol> GetExplicitRegistrations(OperationAnalysisC
         return set.ToImmutable();
     }
 
-    static bool IsExplicitlyRegistered(ITypeSymbol type, OperationAnalysisContext ctx) {
-        var set = GetExplicitRegistrations(ctx);
+    static bool IsExplicitlyRegistered(ITypeSymbol type, OperationAnalysisContext ctx, KnownTypeSymbols knownTypes) {
+        var set = GetExplicitRegistrations(ctx, knownTypes);
         return set.Contains(type);
     }
 
-    static void AnalyzeInvocation(OperationAnalysisContext ctx) {
+    static void AnalyzeInvocation(OperationAnalysisContext ctx, KnownTypeSymbols knownTypes) {
         if (ctx.Operation is not IInvocationOperation inv) return;
 
         var method = inv.TargetMethod;
@@ -80,18 +118,18 @@ static void AnalyzeInvocation(OperationAnalysisContext ctx) {
             case { Name: "Apply", TypeArguments.Length: 1, Parameters.Length: 1 }: {
                 var containing = method.ContainingType;
 
-                if (IsAggregate(containing)) {
+                if (IsAggregate(containing, knownTypes)) {
                     var eventType = method.TypeArguments[0];
 
-                    if (IsConcreteEvent(eventType) && !HasEventTypeAttribute(eventType) && !IsExplicitlyRegistered(eventType, ctx)) {
+                    if (IsConcreteEvent(eventType) && !HasEventTypeAttribute(eventType, knownTypes) && !IsExplicitlyRegistered(eventType, ctx, knownTypes)) {
                         ctx.ReportDiagnostic(Diagnostic.Create(MissingEventTypeAttribute, inv.Syntax.GetLocation(), eventType.ToDisplayString()));
                     }
                 }
 
                 return;
             }
             // Case 1b: State<T>.When(...) invocations where an event instance is passed
-            case { Name: "When", Parameters.Length: 1 } when IsState(method.ContainingType): {
+            case { Name: "When", Parameters.Length: 1 } when IsState(method.ContainingType, knownTypes): {
                 var arg = inv.Arguments.Length > 0 ? inv.Arguments[0].Value : null;
 
                 ITypeSymbol? eventType = null;
@@ -105,18 +143,18 @@ static void AnalyzeInvocation(OperationAnalysisContext ctx) {
                     _                                                    => arg?.Type
                 };
 
-                if (eventType != null && IsConcreteEvent(eventType) && !HasEventTypeAttribute(eventType) && !IsExplicitlyRegistered(eventType, ctx)) {
+                if (eventType != null && IsConcreteEvent(eventType) && !HasEventTypeAttribute(eventType, knownTypes) && !IsExplicitlyRegistered(eventType, ctx, knownTypes)) {
                     var location = arg?.Syntax.GetLocation() ?? inv.Syntax.GetLocation();
                     ctx.ReportDiagnostic(Diagnostic.Create(MissingEventTypeAttribute, location, eventType.ToDisplayString()));
                 }
 
                 return;
             }
             // Case 1c: State<T>.On<TEvent>(...) handler registrations
-            case { Name: "On", TypeArguments.Length: 1 } when IsState(method.ContainingType): {
+            case { Name: "On", TypeArguments.Length: 1 } when IsState(method.ContainingType, knownTypes): {
                 var eventType = method.TypeArguments[0];
 
-                if (IsConcreteEvent(eventType) && !HasEventTypeAttribute(eventType) && !IsExplicitlyRegistered(eventType, ctx)) {
+                if (IsConcreteEvent(eventType) && !HasEventTypeAttribute(eventType, knownTypes) && !IsExplicitlyRegistered(eventType, ctx, knownTypes)) {
                     ctx.ReportDiagnostic(Diagnostic.Create(MissingEventTypeAttribute, inv.Syntax.GetLocation(), eventType.ToDisplayString()));
                 }
 
@@ -127,41 +165,41 @@ static void AnalyzeInvocation(OperationAnalysisContext ctx) {
         // Case 2: Functional service: Act/ActAsync handlers
         if (method.Name is "Act" or "ActAsync") {
             // Heuristic: only consider the overloads that accept a delegate and are defined in CommandHandlerBuilder interfaces/classes
-            if (!IsFunctionalServiceAct(method)) return;
+            if (!IsFunctionalServiceAct(method, knownTypes)) return;
 
             foreach (var value in inv.Arguments.Select(arg => arg.Value)) {
                 switch (value) {
                     case null:
                         continue;
                     // If the argument is a lambda, analyze its body for created event instances
                     case IAnonymousFunctionOperation lambda:
-                        AnalyzeDelegateBodyForEventCreations(ctx, lambda.Body);
+                        AnalyzeDelegateBodyForEventCreations(ctx, lambda.Body, knownTypes);
 
                         break;
                     case IConversionOperation { Operand: IAnonymousFunctionOperation lambdaConv }:
-                        AnalyzeDelegateBodyForEventCreations(ctx, lambdaConv.Body);
+                        AnalyzeDelegateBodyForEventCreations(ctx, lambdaConv.Body, knownTypes);
 
                         break;
                 }
             }
         }
     }
 
-    static void AnalyzeDelegateBodyForEventCreations(OperationAnalysisContext ctx, IBlockOperation? body) {
+    static void AnalyzeDelegateBodyForEventCreations(OperationAnalysisContext ctx, IBlockOperation? body, KnownTypeSymbols knownTypes) {
         if (body is null) return;
 
         foreach (var op in body.Descendants()) {
             if (op is IObjectCreationOperation create) {
                 var created = create.Type;
 
-                if (created != null && IsConcreteEvent(created) && !HasEventTypeAttribute(created) && !IsExplicitlyRegistered(created, ctx)) {
+                if (created != null && IsConcreteEvent(created) && !HasEventTypeAttribute(created, knownTypes) && !IsExplicitlyRegistered(created, ctx, knownTypes)) {
                     ctx.ReportDiagnostic(Diagnostic.Create(MissingEventTypeAttribute, create.Syntax.GetLocation(), created.ToDisplayString()));
                 }
             }
         }
     }
 
-    static void AnalyzeObjectCreation(OperationAnalysisContext ctx) {
+    static void AnalyzeObjectCreation(OperationAnalysisContext ctx, KnownTypeSymbols knownTypes) {
         // Global safety net for method groups passed into Act where we couldn't traverse the body via the invocation site.
         // If the object creation is within a method that appears to be an Act handler (returns NewEvents/ IEnumerable<object>), warn.
         if (ctx.Operation is not IObjectCreationOperation create) return;
@@ -175,7 +213,7 @@ static void AnalyzeObjectCreation(OperationAnalysisContext ctx) {
         if (method == null) return;
 
         if (ReturnsNewEvents(method)) {
-            if (!HasEventTypeAttribute(created) && !IsExplicitlyRegistered(created, ctx)) {
+            if (!HasEventTypeAttribute(created, knownTypes) && !IsExplicitlyRegistered(created, ctx, knownTypes)) {
                 ctx.ReportDiagnostic(Diagnostic.Create(MissingEventTypeAttribute, create.Syntax.GetLocation(), created.ToDisplayString()));
             }
         }
@@ -218,49 +256,86 @@ static bool IsIEnumerableOfObject(INamedTypeSymbol type) {
         return false;
     }
 
-    static bool IsAggregate(INamedTypeSymbol? type) {
+    static bool IsAggregate(INamedTypeSymbol? type, KnownTypeSymbols knownTypes) {
         if (type == null) return false;
 
         // Walk base types to check if it derives from Eventuous.Aggregate<>
         for (var t = type; t != null; t = t.BaseType) {
-            if (t is { Name: "Aggregate", Arity: 1 } && t.ContainingNamespace.ToDisplayString() == BaseNamespace) return true;
+            // Prefer symbol comparison (refactoring-safe)
+            if (knownTypes.Aggregate != null) {
+                if (SymbolEqualityComparer.Default.Equals(t.OriginalDefinition, knownTypes.Aggregate)) {
+                    return true;
+                }
+            }
+            else {
+                // Fallback to string comparison
+                if (t is { Name: "Aggregate", Arity: 1 } && t.ContainingNamespace.ToDisplayString() == BaseNamespace) {
+                    return true;
+                }
+            }
         }
 
         return false;
     }
 
-    static bool IsState(INamedTypeSymbol? type) {
+    static bool IsState(INamedTypeSymbol? type, KnownTypeSymbols knownTypes) {
         if (type == null) return false;
 
         // Walk base types to check if it derives from Eventuous.State<>
         for (var t = type; t != null; t = t.BaseType) {
-            if (t is { Name: "State", Arity: 1 } && t.ContainingNamespace.ToDisplayString() == BaseNamespace) return true;
+            // Prefer symbol comparison (refactoring-safe)
+            if (knownTypes.State != null) {
+                if (SymbolEqualityComparer.Default.Equals(t.OriginalDefinition, knownTypes.State)) {
+                    return true;
+                }
+            }
+            else {
+                // Fallback to string comparison
+                if (t is { Name: "State", Arity: 1 } && t.ContainingNamespace.ToDisplayString() == BaseNamespace) {
+                    return true;
+                }
+            }
         }
 
         return false;
     }
 
-    static bool IsFunctionalServiceAct(IMethodSymbol method) {
+    static bool IsFunctionalServiceAct(IMethodSymbol method, KnownTypeSymbols knownTypes) {
         // We only care about the Act methods from CommandHandlerBuilder and the related interfaces in Eventuous namespace
         if (method.Name is not ("Act" or "ActAsync")) return false;
 
         var containing = method.ContainingType;
 
         if (containing == null) return false;
 
-        var ns = containing.ContainingNamespace?.ToDisplayString();
+        // Prefer symbol comparison (refactoring-safe)
+        if (knownTypes.CommandHandlerBuilder != null || knownTypes.IDefineExecution != null ||
+            knownTypes.ICommandHandlerBuilder != null || knownTypes.IDefineStoreOrExecution != null) {
+            return SymbolEqualityComparer.Default.Equals(containing, knownTypes.CommandHandlerBuilder) ||
+                   SymbolEqualityComparer.Default.Equals(containing, knownTypes.IDefineExecution) ||
+                   SymbolEqualityComparer.Default.Equals(containing, knownTypes.ICommandHandlerBuilder) ||
+                   SymbolEqualityComparer.Default.Equals(containing, knownTypes.IDefineStoreOrExecution);
+        }
 
+        // Fallback to string comparison
+        var ns = containing.ContainingNamespace?.ToDisplayString();
         if (ns != BaseNamespace) return false;
 
-        // Simple name checks
         return containing.Name is "CommandHandlerBuilder" or "IDefineExecution" or "ICommandHandlerBuilder" or "IDefineStoreOrExecution";
     }
 
     static bool IsConcreteEvent(ITypeSymbol type) => type.TypeKind is TypeKind.Class or TypeKind.Struct;
 
-    static bool HasEventTypeAttribute(ITypeSymbol type)
-        => (from attrClass in type.GetAttributes().Select(a => a.AttributeClass).OfType<INamedTypeSymbol>()
-            let name = attrClass.ToDisplayString()
-            where name == EventTypeAttrFqcn || attrClass.Name is EventTypeAttribute
-            select attrClass).Any();
+    static bool HasEventTypeAttribute(ITypeSymbol type, KnownTypeSymbols knownTypes) {
+        // Prefer symbol comparison (refactoring-safe)
+        if (knownTypes.EventTypeAttribute != null) {
+            return type.GetAttributes().Any(a => SymbolEqualityComparer.Default.Equals(a.AttributeClass, knownTypes.EventTypeAttribute));
+        }
+
+        // Fallback to string comparison
+        return (from attrClass in type.GetAttributes().Select(a => a.AttributeClass).OfType<INamedTypeSymbol>()
+                let name = attrClass.ToDisplayString()
+                where name == EventTypeAttrFqcn || attrClass.Name is EventTypeAttribute
+                select attrClass).Any();
+    }
 }