Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
8 changes: 3 additions & 5 deletions src/Core/gen/Eventuous.Shared.Generators/Constants.cs
Original file line number Diff line number Diff line change
Expand Up @@ -4,11 +4,9 @@
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.
/// Constants used for type and member lookups via Compilation.GetTypeByMetadataName().
/// The full list of metadata names the analyzers depend on lives in <see cref="WellKnownTypeNames"/>,
/// which is pinned by tests against the real Eventuous assemblies.
/// </summary>
internal static class Constants {
/// <summary>Base namespace for Eventuous types.</summary>
Expand Down
210 changes: 93 additions & 117 deletions src/Core/gen/Eventuous.Shared.Generators/EventUsageAnalyzer.cs
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@
using Microsoft.CodeAnalysis.Diagnostics;
using Microsoft.CodeAnalysis.Operations;
using Microsoft.CodeAnalysis.CSharp.Syntax;
using static Eventuous.Shared.Generators.Constants;

// ReSharper disable CognitiveComplexity

Expand Down Expand Up @@ -37,48 +36,66 @@ public override void Initialize(AnalysisContext context) {
var compilation = compilationContext.Compilation;
var knownTypes = new KnownTypeSymbols(compilation);

// Without these two, annotations and explicit registrations cannot be checked,
// so any diagnostic would be an unverifiable false positive — stay silent instead
if (knownTypes.EventTypeAttribute == null || knownTypes.TypeMapper == null) return;

compilationContext.RegisterOperationAction(ctx => AnalyzeInvocation(ctx, knownTypes), OperationKind.Invocation);
compilationContext.RegisterOperationAction(ctx => AnalyzeObjectCreation(ctx, knownTypes), OperationKind.ObjectCreation);
});
}

/// <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.
/// Symbol comparison against these is the only matching mechanism; if a symbol doesn't resolve,
/// the corresponding check simply doesn't apply. The metadata names in <see cref="WellKnownTypeNames"/>
/// are pinned by tests against the real Eventuous assemblies.
/// </summary>
sealed class KnownTypeSymbols(Compilation compilation) {
public INamedTypeSymbol? EventTypeAttribute { get; } = compilation.GetTypeByMetadataName(EventTypeAttrFqcn);
public INamedTypeSymbol? TypeMapper { get; } = compilation.GetTypeByMetadataName($"{BaseNamespace}.TypeMapper");
public INamedTypeSymbol? Aggregate { get; } = compilation.GetTypeByMetadataName($"{BaseNamespace}.Aggregate`1");
public INamedTypeSymbol? State { get; } = compilation.GetTypeByMetadataName($"{BaseNamespace}.State`1");
public INamedTypeSymbol? CommandHandlerBuilder { get; } = compilation.GetTypeByMetadataName($"{BaseNamespace}.CommandHandlerBuilder");
public INamedTypeSymbol? IDefineExecution { get; } = compilation.GetTypeByMetadataName($"{BaseNamespace}.IDefineExecution");
public INamedTypeSymbol? ICommandHandlerBuilder { get; } = compilation.GetTypeByMetadataName($"{BaseNamespace}.ICommandHandlerBuilder");
public INamedTypeSymbol? IDefineStoreOrExecution { get; } = compilation.GetTypeByMetadataName($"{BaseNamespace}.IDefineStoreOrExecution");
public INamedTypeSymbol? BaseEventHandler { get; } = compilation.GetTypeByMetadataName("Eventuous.Subscriptions.BaseEventHandler");
public INamedTypeSymbol? EventTypeAttribute { get; } = GetBestTypeByMetadataName(compilation, WellKnownTypeNames.EventTypeAttribute);
public INamedTypeSymbol? TypeMapper { get; } = GetBestTypeByMetadataName(compilation, WellKnownTypeNames.TypeMapper);
public INamedTypeSymbol? Aggregate { get; } = GetBestTypeByMetadataName(compilation, WellKnownTypeNames.Aggregate);
public INamedTypeSymbol? State { get; } = GetBestTypeByMetadataName(compilation, WellKnownTypeNames.State);
public INamedTypeSymbol? CommandHandlerBuilder { get; } = GetBestTypeByMetadataName(compilation, WellKnownTypeNames.CommandHandlerBuilder);
public INamedTypeSymbol? IDefineExecution { get; } = GetBestTypeByMetadataName(compilation, WellKnownTypeNames.IDefineExecution);
public INamedTypeSymbol? ICommandHandlerBuilder { get; } = GetBestTypeByMetadataName(compilation, WellKnownTypeNames.ICommandHandlerBuilder);
public INamedTypeSymbol? IDefineStoreOrExecution { get; } = GetBestTypeByMetadataName(compilation, WellKnownTypeNames.IDefineStoreOrExecution);
public INamedTypeSymbol? BaseEventHandler { get; } = GetBestTypeByMetadataName(compilation, WellKnownTypeNames.BaseEventHandler);

// GetTypeByMetadataName returns null not only when the type is missing but also when more than
// one referenced assembly defines it; in the ambiguous case pick the single accessible candidate
static INamedTypeSymbol? GetBestTypeByMetadataName(Compilation compilation, string metadataName) {
var type = compilation.GetTypeByMetadataName(metadataName);

if (type != null) return type;

INamedTypeSymbol? best = null;

foreach (var candidate in compilation.GetTypesByMetadataName(metadataName)) {
if (candidate.DeclaredAccessibility != Accessibility.Public
&& !SymbolEqualityComparer.Default.Equals(candidate.ContainingAssembly, compilation.Assembly)) continue;

if (best != null) return null;

Comment on lines +74 to +79

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remediation recommended

1. Registered events may warn 🐞 Bug ≡ Correctness

If TypeMapper cannot be uniquely resolved (e.g., multiple accessible candidates),
KnownTypeSymbols.TypeMapper becomes null and GetExplicitRegistrations returns an empty set, so
IsExplicitlyRegistered will never suppress EVTC001 for TypeMap.Instance.AddType registrations. This
can reintroduce false positives for explicitly-registered events in projects with ambiguous
references.
Agent Prompt
## Issue description
`GetBestTypeByMetadataName` intentionally returns `null` when more than one accessible candidate exists. For `TypeMapper`, that cascades into `GetExplicitRegistrations` returning an empty set, which disables the explicit-registration suppression and can produce EVTC001 for events that are actually registered via `TypeMap.Instance.AddType(...)`.

## Issue Context
This PR removed all string-name fallbacks and made explicit-registration detection depend on successfully resolving `Eventuous.TypeMapper`. In ambiguous-reference scenarios (or other resolution failures), suppression now breaks.

## Fix Focus Areas
- src/Core/gen/Eventuous.Shared.Generators/EventUsageAnalyzer.cs[61-107]

## Suggested fix approach
- Keep the symbol-first approach, but add a narrowly-scoped fallback for explicit registration detection only:
  - If `knownTypes.TypeMapper == null`, treat invocations as registrations when:
    - method name is `AddType`, and
    - containing type metadata name matches `WellKnownTypeNames.TypeMapper` (or containing type name/namespace match), and
    - the invocation target is reachable via `TypeMap.Instance` (optional).
- Alternatively: change `GetBestTypeByMetadataName` for `TypeMapper` to select a deterministic candidate instead of returning null when multiple accessible candidates exist (e.g., prefer the one whose containing assembly name matches the Eventuous assembly reference set, or prefer the one that also contains `EventTypeAttribute`).

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Addressed in f28b6bf, with a stronger invariant than a fallback: if either EventTypeAttribute or TypeMapper fails to resolve, the analyzer registers no actions at all — no diagnostic can be verified without them, so silence beats unverifiable warnings. Note the ambiguity scenario itself (two accessible Eventuous.TypeMapper definitions) means the user's own TypeMap.Instance.AddType calls fail with CS0433, so their build is broken regardless; the ILMerge-style case (one public + internalized copies) already resolves to the public candidate. Pinned by Should_stay_silent_when_event_type_attribute_cannot_be_resolved.

best = candidate;
}
Comment on lines +74 to +81

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Keeping the explicit loop: the logic is 'return null when a second accessible candidate appears', and moving the filter into .Where(...) still leaves the two-match early exit in the loop body, so it doesn't simplify anything — it just splits one condition across two places.


return best;
}
}

static ImmutableHashSet<ITypeSymbol> GetExplicitRegistrations(OperationAnalysisContext ctx, KnownTypeSymbols knownTypes) {
var model = ctx.Operation.SemanticModel;
if (model == null) return ImmutableHashSet<ITypeSymbol>.Empty;
if (model == null || knownTypes.TypeMapper == null) return ImmutableHashSet<ITypeSymbol>.Empty;
var root = ctx.Operation.Syntax.SyntaxTree.GetRoot();
var set = ImmutableHashSet.CreateBuilder<ITypeSymbol>(SymbolEqualityComparer.Default);

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;

// 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 (!SymbolEqualityComparer.Default.Equals(m.ContainingType, knownTypes.TypeMapper)) continue;

if (m.TypeArguments.Length == 1) {
set.Add(m.TypeArguments[0]);
Expand Down Expand Up @@ -159,20 +176,18 @@ static void AnalyzeInvocation(OperationAnalysisContext ctx, KnownTypeSymbols kno
// Heuristic: only consider the overloads that accept a delegate and are defined in CommandHandlerBuilder interfaces/classes
if (!IsFunctionalServiceAct(method, knownTypes)) return;

// If the argument is a lambda, analyze its body for created event instances.
// Lambdas passed as delegate arguments surface as IDelegateCreationOperation, possibly wrapped in a conversion.
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, knownTypes);

break;
case IConversionOperation { Operand: IAnonymousFunctionOperation lambdaConv }:
AnalyzeDelegateBodyForEventCreations(ctx, lambdaConv.Body, knownTypes);

break;
}
var lambda = value switch {
IAnonymousFunctionOperation anon => anon,
IDelegateCreationOperation { Target: IAnonymousFunctionOperation anon } => anon,
IConversionOperation { Operand: IAnonymousFunctionOperation anon } => anon,
IConversionOperation { Operand: IDelegateCreationOperation { Target: IAnonymousFunctionOperation anon } } => anon,
Comment on lines +184 to +186

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Restrict Act analysis to emitted event values

When an Act/ActAsync lambda builds its result with an intermediate allocation, such as var events = new List<object>();, this newly enabled traversal sends the entire lambda body to a scanner that treats every class or struct construction except object as a domain event. It therefore emits EVTC001 for List<object> and other helper objects that are never events; trace only values returned or added to NewEvents instead of scanning every creation in the body.

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in 88c22b8. IsConcreteEvent now excludes the entire System namespace, so List<object> and other BCL allocations inside handlers are no longer flagged (pinned by Should_not_warn_for_helper_allocations_in_act_handlers). User-defined non-event types constructed inside a handler (e.g. value objects) can still be flagged — full emitted-value tracing would trade that for false negatives on indirect flows like var e = new MyEvent(); return [e];, which this analyzer intentionally keeps catching.

_ => null
};

if (lambda != null) AnalyzeDelegateBodyForEventCreations(ctx, lambda.Body, knownTypes);

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Avoid reporting synchronous Act creations twice

For a synchronous handler such as Act(_ => new object[] { new UnannotatedEvent() }), this traversal reports the event construction, and the separately registered OperationKind.ObjectCreation callback analyzes the same node again. Because the lambda symbol returns NewEvents as IEnumerable<object>, ReturnsNewEvents accepts it and emits an identical diagnostic at the same location; reserve that fallback for method groups or otherwise deduplicate these reports.

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in 88c22b8. The object-creation safety net now skips creations located inside a lambda passed to a functional-service Act/ActAsync invocation — the invocation traversal owns those, so the safety net remains for method groups only. Pinned by Should_report_sync_act_event_exactly_once, which failed with 2 identical diagnostics before the change.

}
}
}
Expand Down Expand Up @@ -204,13 +219,22 @@ static void AnalyzeObjectCreation(OperationAnalysisContext ctx, KnownTypeSymbols

if (method == null) return;

if (ReturnsNewEvents(method)) {
// Creations inside lambdas passed to Act/ActAsync are reported by the invocation traversal; skip them here
if (ReturnsNewEvents(method) && !IsWithinFunctionalActInvocation(create, knownTypes)) {
if (!HasEventTypeAttribute(created, knownTypes) && !IsExplicitlyRegistered(created, ctx, knownTypes)) {
ctx.ReportDiagnostic(Diagnostic.Create(MissingEventTypeAttribute, create.Syntax.GetLocation(), created.ToDisplayString()));
}
}
}

static bool IsWithinFunctionalActInvocation(IOperation op, KnownTypeSymbols knownTypes) {
for (var p = op.Parent; p != null; p = p.Parent) {
if (p is IInvocationOperation inv && IsFunctionalServiceAct(inv.TargetMethod, knownTypes)) return true;
}

return false;
}

static IMethodSymbol? GetEnclosingMethod(IOperation op) {
for (var p = op.Parent; p != null; p = p.Parent) {
switch (p) {
Expand Down Expand Up @@ -248,105 +272,57 @@ static bool IsIEnumerableOfObject(INamedTypeSymbol type) {
return false;
}

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) {
// 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, KnownTypeSymbols knownTypes) {
if (type == null) return false;
// Walk base types to check if the type derives from Eventuous.Aggregate<>
static bool IsAggregate(INamedTypeSymbol? type, KnownTypeSymbols knownTypes) => DerivesFrom(type, knownTypes.Aggregate);

// Walk base types to check if it derives from Eventuous.State<>
for (var t = type; t != null; t = t.BaseType) {
// 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;
}
}
}
// Walk base types to check if the type derives from Eventuous.State<>
static bool IsState(INamedTypeSymbol? type, KnownTypeSymbols knownTypes) => DerivesFrom(type, knownTypes.State);

return false;
}
static bool IsEventHandler(INamedTypeSymbol? type, KnownTypeSymbols knownTypes) => DerivesFrom(type, knownTypes.BaseEventHandler);

static bool IsEventHandler(INamedTypeSymbol? type, KnownTypeSymbols knownTypes) {
if (type == null) return false;
static bool DerivesFrom(INamedTypeSymbol? type, INamedTypeSymbol? baseDefinition) {
if (baseDefinition == null) return false;

for (var t = type; t != null; t = t.BaseType) {
if (knownTypes.BaseEventHandler != null) {
if (SymbolEqualityComparer.Default.Equals(t.OriginalDefinition, knownTypes.BaseEventHandler)) {
return true;
}
}
else {
if (t is { Name: "BaseEventHandler", Arity: 0 } && t.ContainingNamespace?.ToDisplayString() == "Eventuous.Subscriptions") {
return true;
}
}
if (SymbolEqualityComparer.Default.Equals(t.OriginalDefinition, baseDefinition)) return true;
}
Comment thread
qodo-free-for-open-source-projects[bot] marked this conversation as resolved.

return false;
}

static bool IsFunctionalServiceAct(IMethodSymbol method, KnownTypeSymbols knownTypes) {
// We only care about the Act methods from CommandHandlerBuilder and the related interfaces in Eventuous namespace
// We only care about the Act methods from CommandHandlerBuilder and the related interfaces in Eventuous namespace.
// The containing type at the call site is a constructed generic, so compare its original definition.
if (method.Name is not ("Act" or "ActAsync")) return false;

var containing = method.ContainingType;
var definition = method.ContainingType?.OriginalDefinition;

if (containing == null) return false;
if (definition == null) return false;

// 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;

return containing.Name is "CommandHandlerBuilder" or "IDefineExecution" or "ICommandHandlerBuilder" or "IDefineStoreOrExecution";
return SymbolEqualityComparer.Default.Equals(definition, knownTypes.CommandHandlerBuilder)
|| SymbolEqualityComparer.Default.Equals(definition, knownTypes.IDefineExecution)
|| SymbolEqualityComparer.Default.Equals(definition, knownTypes.ICommandHandlerBuilder)
|| SymbolEqualityComparer.Default.Equals(definition, knownTypes.IDefineStoreOrExecution);
}

static bool IsConcreteEvent(ITypeSymbol type) => type.TypeKind is TypeKind.Class or TypeKind.Struct;

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));
// System.Object is excluded: an object-typed value (e.g. StreamEvent.Payload or IMessageConsumeContext.Message)
// carries a runtime-resolved event type, so there is nothing to annotate at the call site.
// The System namespace is excluded as a whole: framework types constructed inside handlers
// (List<object>, DateTime, ...) are never domain events.
static bool IsConcreteEvent(ITypeSymbol type)
=> type.SpecialType is not SpecialType.System_Object
&& type.TypeKind is TypeKind.Class or TypeKind.Struct
&& !IsInSystemNamespace(type);

static bool IsInSystemNamespace(ITypeSymbol type) {
for (var ns = type.ContainingNamespace; ns is { IsGlobalNamespace: false }; ns = ns.ContainingNamespace) {
if (ns.ContainingNamespace is { IsGlobalNamespace: true }) return ns.Name == "System";
}

// 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();
return false;
}

static bool HasEventTypeAttribute(ITypeSymbol type, KnownTypeSymbols knownTypes)
=> knownTypes.EventTypeAttribute != null
&& type.GetAttributes().Any(a => SymbolEqualityComparer.Default.Equals(a.AttributeClass, knownTypes.EventTypeAttribute));
}
37 changes: 37 additions & 0 deletions src/Core/gen/Eventuous.Shared.Generators/WellKnownTypeNames.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
// Copyright (C) Eventuous HQ OÜ. All rights reserved
// Licensed under the Apache License, Version 2.0.

using System.Collections.Immutable;

namespace Eventuous.Shared.Generators;

/// <summary>
/// Fully qualified metadata names of the Eventuous types the analyzers resolve via
/// <see cref="Microsoft.CodeAnalysis.Compilation.GetTypeByMetadataName(string)"/>.
/// The analyzer cannot reference the runtime assemblies (it targets netstandard2.0, they don't),
/// so these strings are the single source of truth. Every name is pinned by a test that resolves
/// it against the current Eventuous assemblies, so renaming a type fails CI in the same change.
/// </summary>
public static class WellKnownTypeNames {
public const string EventTypeAttribute = Constants.EventTypeAttrFqcn;
public const string TypeMapper = $"{Constants.BaseNamespace}.TypeMapper";
public const string Aggregate = $"{Constants.BaseNamespace}.Aggregate`1";
public const string State = $"{Constants.BaseNamespace}.State`1";
public const string CommandHandlerBuilder = $"{Constants.BaseNamespace}.CommandHandlerBuilder`2";
public const string IDefineExecution = $"{Constants.BaseNamespace}.IDefineExecution`2";
public const string ICommandHandlerBuilder = $"{Constants.BaseNamespace}.ICommandHandlerBuilder`2";
public const string IDefineStoreOrExecution = $"{Constants.BaseNamespace}.IDefineStoreOrExecution`2";
public const string BaseEventHandler = "Eventuous.Subscriptions.BaseEventHandler";

public static readonly ImmutableArray<string> All = [
EventTypeAttribute,
TypeMapper,
Aggregate,
State,
CommandHandlerBuilder,
IDefineExecution,
ICommandHandlerBuilder,
IDefineStoreOrExecution,
BaseEventHandler
];
}
Loading
Loading