Skip to content

Commit 04818d3

Browse files
Fix source generator event type detection (#538) (#542)
* docs: add design spec for source generator event detection fixes (#538) Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * docs: remove type parameter name heuristic from spec The containing-type check replaces all name-based detection entirely. The type parameter name (T, TEvent, TC, etc.) is irrelevant. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * docs: add implementation plan for source generator event detection fixes Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix(analyzers): detect EventHandler.On<T>() for missing EventType warning Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * test(analyzers): add EventHandler.On<T>() detection test Add test fixture for EventHandler.On<T>() to verify the analyzer detects unannotated event types in subscription handlers. Also improved test compilation's metadata references to use runtime directory scanning for more robust type resolution. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix(generators): replace name heuristic with containing-type check for On<T> Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix(generators): add EventType attribute discovery for consume context converters Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix(generators): skip inaccessible types in consume context converter generation Private/protected nested event types (e.g. in benchmarks) cannot be referenced from the module-level generated converter code. Add an accessibility check to filter them out and prevent CS0122 build errors. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * chore: fix stale comment in ConsumeContextConverterGenerator Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix(generators): require public visibility for referenced assembly event types Internal/protected-or-internal types from referenced assemblies are not accessible from generated code. Only require public for cross-assembly types; internal types from the current assembly remain valid. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix(tests): replace HttpWaitStrategy with log-based wait for Service Bus emulator The default HttpWaitStrategy creates an HttpClient with the default 100-second timeout. The emulator can take longer to respond on CI, and the TaskCanceledException is not retried by the wait loop. Use log message detection instead to avoid the HttpClient timeout entirely. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
1 parent 2ac1902 commit 04818d3

8 files changed

Lines changed: 618 additions & 21 deletions

File tree

Lines changed: 385 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,385 @@
1+
# Source Generator Event Type Detection Fixes
2+
3+
> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking.
4+
5+
**Goal:** Fix two source generators so they detect event types registered via `EventHandler.On<T>()` and similar patterns, and add `[EventType]` attribute discovery as a fallback for indirect registration patterns.
6+
7+
**Architecture:** Three changes across two generator files. Change 1 adds `EventHandler.On<T>()` detection to the `EventUsageAnalyzer` diagnostic. Change 2 replaces the name-based heuristic in `ConsumeContextConverterGenerator` with a containing-type check. Change 3 adds `[EventType]` attribute discovery from current and referenced assemblies to the converter generator.
8+
9+
**Tech Stack:** Roslyn source generators (IIncrementalGenerator), Roslyn analyzers (DiagnosticAnalyzer), Microsoft.CodeAnalysis.CSharp, netstandard2.0
10+
11+
---
12+
13+
## File Map
14+
15+
| File | Action | Responsibility |
16+
|------|--------|----------------|
17+
| `src/Core/gen/Eventuous.Shared.Generators/EventUsageAnalyzer.cs` | Modify | Add `BaseEventHandler` to `KnownTypeSymbols`, add `IsEventHandler()`, add case 1d |
18+
| `src/Core/gen/Eventuous.Subscriptions.Generators/ConsumeContextConverterGenerator.cs` | Modify | Replace name heuristic with containing-type check, add `[EventType]` discovery path |
19+
| `src/Core/test/Eventuous.Tests.Shared.Analyzers/Analyzed.cs` | Modify | Add `EventHandler.On<T>()` test fixture |
20+
| `src/Core/test/Eventuous.Tests.Shared.Analyzers/Analyzer_Ev001_Tests.cs` | Modify | Add test for EventHandler case |
21+
| `src/Core/test/Eventuous.Tests.Shared.Analyzers/Eventuous.Tests.Shared.Analyzers.csproj` | Modify | Add Subscriptions project reference |
22+
23+
---
24+
25+
### Task 1: Add `EventHandler.On<T>()` detection to `EventUsageAnalyzer`
26+
27+
**Files:**
28+
- Modify: `src/Core/gen/Eventuous.Shared.Generators/EventUsageAnalyzer.cs:49-58` (KnownTypeSymbols)
29+
- Modify: `src/Core/gen/Eventuous.Shared.Generators/EventUsageAnalyzer.cs:142-151` (add case 1d)
30+
- Modify: `src/Core/gen/Eventuous.Shared.Generators/EventUsageAnalyzer.cs:270-290` (add IsEventHandler)
31+
32+
- [ ] **Step 1: Add `BaseEventHandler` to `KnownTypeSymbols`**
33+
34+
In `EventUsageAnalyzer.cs`, add to the `KnownTypeSymbols` class (after line 57):
35+
36+
```csharp
37+
public INamedTypeSymbol? BaseEventHandler { get; } = compilation.GetTypeByMetadataName("Eventuous.Subscriptions.BaseEventHandler");
38+
```
39+
40+
- [ ] **Step 2: Add `IsEventHandler()` helper**
41+
42+
After the `IsState()` method (after line 290), add:
43+
44+
```csharp
45+
static bool IsEventHandler(INamedTypeSymbol? type, KnownTypeSymbols knownTypes) {
46+
if (type == null) return false;
47+
48+
for (var t = type; t != null; t = t.BaseType) {
49+
if (knownTypes.BaseEventHandler != null) {
50+
if (SymbolEqualityComparer.Default.Equals(t.OriginalDefinition, knownTypes.BaseEventHandler)) {
51+
return true;
52+
}
53+
}
54+
else {
55+
if (t is { Name: "BaseEventHandler", Arity: 0 } && t.ContainingNamespace?.ToDisplayString() == "Eventuous.Subscriptions") {
56+
return true;
57+
}
58+
}
59+
}
60+
61+
return false;
62+
}
63+
```
64+
65+
- [ ] **Step 3: Add case 1d in the method switch**
66+
67+
In `AnalyzeInvocation`, after case 1c (after line 151, before the closing `}` of the switch on line 152), add:
68+
69+
```csharp
70+
// Case 1d: EventHandler.On<T>(...) handler registrations
71+
case { Name: "On", TypeArguments.Length: 1 } when IsEventHandler(method.ContainingType, knownTypes): {
72+
var eventType = method.TypeArguments[0];
73+
74+
if (IsConcreteEvent(eventType) && !HasEventTypeAttribute(eventType, knownTypes) && !IsExplicitlyRegistered(eventType, ctx, knownTypes)) {
75+
ctx.ReportDiagnostic(Diagnostic.Create(MissingEventTypeAttribute, inv.Syntax.GetLocation(), eventType.ToDisplayString()));
76+
}
77+
78+
return;
79+
}
80+
```
81+
82+
- [ ] **Step 4: Build the generator project**
83+
84+
Run: `dotnet build src/Core/gen/Eventuous.Shared.Generators/Eventuous.Shared.Generators.csproj`
85+
Expected: Build succeeded
86+
87+
- [ ] **Step 5: Commit**
88+
89+
```bash
90+
git add src/Core/gen/Eventuous.Shared.Generators/EventUsageAnalyzer.cs
91+
git commit -m "fix(analyzers): detect EventHandler.On<T>() for missing EventType warning"
92+
```
93+
94+
---
95+
96+
### Task 2: Add analyzer test for `EventHandler.On<T>()` detection
97+
98+
**Files:**
99+
- Modify: `src/Core/test/Eventuous.Tests.Shared.Analyzers/Eventuous.Tests.Shared.Analyzers.csproj`
100+
- Modify: `src/Core/test/Eventuous.Tests.Shared.Analyzers/Analyzed.cs`
101+
- Modify: `src/Core/test/Eventuous.Tests.Shared.Analyzers/Analyzer_Ev001_Tests.cs`
102+
103+
- [ ] **Step 1: Add Subscriptions project reference**
104+
105+
In `Eventuous.Tests.Shared.Analyzers.csproj`, add after the existing `ProjectReference` entries (after line 21):
106+
107+
```xml
108+
<ProjectReference Include="$(LocalRoot)\Eventuous.Subscriptions\Eventuous.Subscriptions.csproj"/>
109+
```
110+
111+
- [ ] **Step 2: Add EventHandler fixture to `Analyzed.cs`**
112+
113+
Add the following after the existing `Events` class (after line 23):
114+
115+
```csharp
116+
file class TestEventHandler : Eventuous.Subscriptions.EventHandler {
117+
public TestEventHandler() {
118+
On<Events.RoomBooked>(ctx => new ValueTask());
119+
}
120+
}
121+
```
122+
123+
- [ ] **Step 3: Add metadata reference in `Analyzer_Ev001_Tests.cs`**
124+
125+
In the `CreateCompilation` method, add to the `refs` list (after line 56):
126+
127+
```csharp
128+
MetadataReference.CreateFromFile(typeof(Eventuous.Subscriptions.EventHandler).Assembly.Location),
129+
```
130+
131+
- [ ] **Step 4: Update test assertion to expect 3 diagnostics**
132+
133+
In `Should_warn_for_unannotated_events_in_state_and_aggregate`, update the assertion at line 29:
134+
135+
```csharp
136+
await Assert.That(ev001.Length).IsGreaterThanOrEqualTo(3);
137+
```
138+
139+
- [ ] **Step 5: Run the test**
140+
141+
Run: `dotnet test src/Core/test/Eventuous.Tests.Shared.Analyzers/Eventuous.Tests.Shared.Analyzers.csproj --filter "FullyQualifiedName~Analyzer_Ev001_Tests" -f net10.0`
142+
Expected: PASS — 3 EV001 diagnostics including one for `EventHandler.On<RoomBooked>`
143+
144+
- [ ] **Step 6: Commit**
145+
146+
```bash
147+
git add src/Core/test/Eventuous.Tests.Shared.Analyzers/
148+
git commit -m "test(analyzers): add EventHandler.On<T>() detection test"
149+
```
150+
151+
---
152+
153+
### Task 3: Replace name heuristic in `ConsumeContextConverterGenerator`
154+
155+
**Files:**
156+
- Modify: `src/Core/gen/Eventuous.Subscriptions.Generators/ConsumeContextConverterGenerator.cs`
157+
158+
- [ ] **Step 1: Add `BaseEventHandler` symbol resolution to the pipeline**
159+
160+
In `Initialize()`, add a second symbol resolution alongside the existing `messageConsumeContextSymbol` (after line 20):
161+
162+
```csharp
163+
var baseEventHandlerSymbol = context.CompilationProvider
164+
.Select(static (c, _) => c.GetTypeByMetadataName("Eventuous.Subscriptions.BaseEventHandler"));
165+
```
166+
167+
- [ ] **Step 2: Combine both symbols into the pipeline**
168+
169+
Replace the existing pipeline (lines 22-29) to thread both symbols through. Change the `.Combine(messageConsumeContextSymbol)` to combine both:
170+
171+
```csharp
172+
var knownSymbols = messageConsumeContextSymbol
173+
.Combine(baseEventHandlerSymbol)
174+
.Select(static (pair, _) => new KnownSymbols(pair.Left, pair.Right));
175+
176+
var candidateTypes = context.SyntaxProvider
177+
.CreateSyntaxProvider(IsPotentialUsage, Transform)
178+
.Where(static t => t is not null)
179+
.Combine(knownSymbols)
180+
.Select(static (pair, _) => TransformWithSymbol(pair.Left, pair.Right))
181+
.Where(static t => t is not null)
182+
.Select(static (t, _) => t!)
183+
.Collect();
184+
```
185+
186+
- [ ] **Step 3: Add `KnownSymbols` record**
187+
188+
Add inside the class (e.g., after line 15):
189+
190+
```csharp
191+
sealed record KnownSymbols(INamedTypeSymbol? MessageConsumeContext, INamedTypeSymbol? BaseEventHandler);
192+
```
193+
194+
- [ ] **Step 4: Update `TransformWithSymbol` signature**
195+
196+
Change the signature from:
197+
198+
```csharp
199+
static string? TransformWithSymbol(GeneratorSyntaxContext? ctx, INamedTypeSymbol? messageConsumeContextSymbol)
200+
```
201+
202+
to:
203+
204+
```csharp
205+
static string? TransformWithSymbol(GeneratorSyntaxContext? ctx, KnownSymbols known)
206+
```
207+
208+
Update all references to `messageConsumeContextSymbol` inside the method to `known.MessageConsumeContext`. Pass `known.BaseEventHandler` to the `On<T>` check.
209+
210+
- [ ] **Step 5: Replace `ShouldTreatGenericOnAsEvent`**
211+
212+
Replace the existing method (lines 135-143) with:
213+
214+
```csharp
215+
static bool IsEventHandlerOnMethod(IMethodSymbol method, INamedTypeSymbol? baseEventHandlerSymbol) {
216+
if (method is not { Name: "On" }) return false;
217+
var def = method.OriginalDefinition;
218+
if (def.TypeParameters.Length != 1) return false;
219+
220+
var containingType = def.ContainingType;
221+
222+
for (var t = containingType; t != null; t = t.BaseType) {
223+
if (baseEventHandlerSymbol != null) {
224+
if (SymbolEqualityComparer.Default.Equals(t.OriginalDefinition, baseEventHandlerSymbol)) {
225+
return true;
226+
}
227+
}
228+
else {
229+
if (t is { Name: "BaseEventHandler", Arity: 0 } && t.ContainingNamespace?.ToDisplayString() == "Eventuous.Subscriptions") {
230+
return true;
231+
}
232+
}
233+
}
234+
235+
return false;
236+
}
237+
```
238+
239+
- [ ] **Step 6: Update the call site in `TransformWithSymbol`**
240+
241+
In Case 2 (around line 75), change:
242+
243+
```csharp
244+
if (method?.TypeArguments.Length == 1 && ShouldTreatGenericOnAsEvent(method)) {
245+
```
246+
247+
to:
248+
249+
```csharp
250+
if (method?.TypeArguments.Length == 1 && IsEventHandlerOnMethod(method, known.BaseEventHandler)) {
251+
```
252+
253+
- [ ] **Step 7: Build the generator project**
254+
255+
Run: `dotnet build src/Core/gen/Eventuous.Subscriptions.Generators/Eventuous.Subscriptions.Generators.csproj`
256+
Expected: Build succeeded
257+
258+
- [ ] **Step 8: Commit**
259+
260+
```bash
261+
git add src/Core/gen/Eventuous.Subscriptions.Generators/ConsumeContextConverterGenerator.cs
262+
git commit -m "fix(generators): replace name heuristic with containing-type check for On<T>"
263+
```
264+
265+
---
266+
267+
### Task 4: Add `[EventType]` discovery path to `ConsumeContextConverterGenerator`
268+
269+
**Files:**
270+
- Modify: `src/Core/gen/Eventuous.Subscriptions.Generators/ConsumeContextConverterGenerator.cs`
271+
272+
- [ ] **Step 1: Add `EventTypeAttribute` symbol resolution**
273+
274+
In `Initialize()`, add after the `baseEventHandlerSymbol` resolution:
275+
276+
```csharp
277+
var eventTypeAttributeSymbol = context.CompilationProvider
278+
.Select(static (c, _) => c.GetTypeByMetadataName("Eventuous.EventTypeAttribute"));
279+
```
280+
281+
- [ ] **Step 2: Add the `[EventType]` discovery pipeline**
282+
283+
After the existing `candidateTypes` pipeline, add:
284+
285+
```csharp
286+
var eventTypeCandidates = eventTypeAttributeSymbol
287+
.Combine(context.CompilationProvider)
288+
.Select(static (pair, _) => DiscoverEventTypes(pair.Right, pair.Left));
289+
```
290+
291+
- [ ] **Step 3: Merge both candidate sources**
292+
293+
Replace the `context.RegisterSourceOutput(candidateTypes, Generate);` line with:
294+
295+
```csharp
296+
var mergedCandidates = candidateTypes
297+
.Combine(eventTypeCandidates)
298+
.Select(static (pair, _) => pair.Left.AddRange(pair.Right));
299+
300+
context.RegisterSourceOutput(mergedCandidates, Generate);
301+
```
302+
303+
- [ ] **Step 4: Add `DiscoverEventTypes` method**
304+
305+
Add after the `Generate` method:
306+
307+
```csharp
308+
static ImmutableArray<string> DiscoverEventTypes(Compilation compilation, INamedTypeSymbol? eventTypeAttributeSymbol) {
309+
if (eventTypeAttributeSymbol is null) return ImmutableArray<string>.Empty;
310+
311+
var builder = ImmutableArray.CreateBuilder<string>();
312+
313+
ProcessNamespace(compilation.Assembly.GlobalNamespace);
314+
315+
foreach (var ra in compilation.SourceModule.ReferencedAssemblySymbols) {
316+
ProcessNamespace(ra.GlobalNamespace);
317+
}
318+
319+
return builder.ToImmutable();
320+
321+
void ProcessType(INamedTypeSymbol type) {
322+
if (HasEventTypeAttribute(type)) {
323+
var name = GetTypeSyntax(type);
324+
if (name is not null) builder.Add(name);
325+
}
326+
327+
foreach (var nt in type.GetTypeMembers()) {
328+
ProcessType(nt);
329+
}
330+
}
331+
332+
void ProcessNamespace(INamespaceSymbol ns) {
333+
foreach (var member in ns.GetMembers()) {
334+
switch (member) {
335+
case INamespaceSymbol cns:
336+
ProcessNamespace(cns);
337+
break;
338+
case INamedTypeSymbol type:
339+
ProcessType(type);
340+
break;
341+
}
342+
}
343+
}
344+
345+
bool HasEventTypeAttribute(INamedTypeSymbol type) =>
346+
type.GetAttributes().Any(a => SymbolEqualityComparer.Default.Equals(a.AttributeClass, eventTypeAttributeSymbol));
347+
}
348+
```
349+
350+
- [ ] **Step 5: Build the generator project**
351+
352+
Run: `dotnet build src/Core/gen/Eventuous.Subscriptions.Generators/Eventuous.Subscriptions.Generators.csproj`
353+
Expected: Build succeeded
354+
355+
- [ ] **Step 6: Commit**
356+
357+
```bash
358+
git add src/Core/gen/Eventuous.Subscriptions.Generators/ConsumeContextConverterGenerator.cs
359+
git commit -m "fix(generators): add EventType attribute discovery for consume context converters"
360+
```
361+
362+
---
363+
364+
### Task 5: Verify full solution builds and existing tests pass
365+
366+
**Files:** None (verification only)
367+
368+
- [ ] **Step 1: Build full solution**
369+
370+
Run: `dotnet build Eventuous.slnx`
371+
Expected: Build succeeded
372+
373+
- [ ] **Step 2: Run analyzer tests**
374+
375+
Run: `dotnet test src/Core/test/Eventuous.Tests.Shared.Analyzers/Eventuous.Tests.Shared.Analyzers.csproj -f net10.0`
376+
Expected: All tests pass
377+
378+
- [ ] **Step 3: Run existing context conversion tests**
379+
380+
Run: `dotnet test src/Mongo/test/Eventuous.Tests.Projections.MongoDB/Eventuous.Tests.Projections.MongoDB.csproj --filter "FullyQualifiedName~ContextConversions" -f net10.0`
381+
Expected: All tests pass
382+
383+
- [ ] **Step 4: Commit (if any fixups needed)**
384+
385+
Only if build or tests required adjustments.

0 commit comments

Comments
 (0)