Skip to content

Avoid using magic strings #488

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
alexeyzimarev wants to merge 1 commit into
base: dev
Choose a base branch
from
Open

Avoid using magic strings #488

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 14 additions & 2 deletions src/Core/gen/Eventuous.Shared.Generators/Constants.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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}";
}
143 changes: 109 additions & 34 deletions src/Core/gen/Eventuous.Shared.Generators/EventUsageAnalyzer.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand All @@ -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]);
Expand All @@ -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;
Expand All @@ -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;
Expand All @@ -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()));
}

Expand All @@ -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;
Expand All @@ -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()));
}
}
Expand Down Expand Up @@ -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();
}
}
Loading
Loading