From bb0c4e7355b33bd74dea774ffc19f174a1c6c954 Mon Sep 17 00:00:00 2001 From: Alexey Zimarev Date: Sun, 20 Sep 2026 14:33:17 +0200 Subject: [PATCH 1/2] fix(subscriptions): emit converter switch arms most-specific-first (#589) ConsumeContextConverterGenerator emitted one type-pattern switch arm per discovered message type in discovery order. When one type was a supertype of another (a base [EventType] record, or an interface), its arm could precede and subsume the more specific one, failing the build with CS8510. Where it did compile, a derived message was wrapped in the base type's context, unlike the reflection fallback which uses the exact runtime type. Rank each candidate by its number of supertypes (base classes plus all interfaces), which is strictly greater for a subtype than for any of its supertypes, and emit arms by that rank descending, then by name for deterministic output. Base-class depth alone is not enough: all interfaces have depth zero, so a base interface could still precede a derived one. Add generator tests covering class and interface inheritance. Co-Authored-By: Claude Fable 5.1 --- .../ConsumeContextConverterGenerator.cs | 47 +++++++--- .../ConsumeContextConverterGeneratorTests.cs | 92 +++++++++++++++++++ .../Eventuous.Tests.Subscriptions.csproj | 3 + 3 files changed, 129 insertions(+), 13 deletions(-) create mode 100644 src/Core/test/Eventuous.Tests.Subscriptions/ConsumeContextConverterGeneratorTests.cs diff --git a/src/Core/gen/Eventuous.Subscriptions.Generators/ConsumeContextConverterGenerator.cs b/src/Core/gen/Eventuous.Subscriptions.Generators/ConsumeContextConverterGenerator.cs index ff7f37c03..07d5ec49a 100644 --- a/src/Core/gen/Eventuous.Subscriptions.Generators/ConsumeContextConverterGenerator.cs +++ b/src/Core/gen/Eventuous.Subscriptions.Generators/ConsumeContextConverterGenerator.cs @@ -40,7 +40,7 @@ public void Initialize(IncrementalGeneratorInitializationContext context) { .Combine(knownSymbols) .Select(static (pair, _) => TransformWithSymbol(pair.Left, pair.Right)) .Where(static t => t is not null) - .Select(static (t, _) => t!) + .Select(static (t, _) => t!.Value) .Collect(); var eventTypeCandidates = eventTypeAttributeSymbol @@ -71,7 +71,7 @@ static bool IsPotentialUsage(SyntaxNode node, CancellationToken _) { return ctx; } - static string? TransformWithSymbol(GeneratorSyntaxContext? ctx, KnownSymbols known) { + static (string Name, int Specificity)? TransformWithSymbol(GeneratorSyntaxContext? ctx, KnownSymbols known) { if (ctx is not { } context) return null; // Explicit generic type usage: IMessageConsumeContext @@ -84,7 +84,7 @@ static bool IsPotentialUsage(SyntaxNode node, CancellationToken _) { var def = symbol.OriginalDefinition; if (IsTargetInterface(def, known.MessageConsumeContext) && symbol.TypeArguments.Length == 1) { var arg = symbol.TypeArguments[0]; - return GetTypeSyntax(arg); + return GetCandidate(arg); } } @@ -97,7 +97,7 @@ static bool IsPotentialUsage(SyntaxNode node, CancellationToken _) { var method = symbolInfo as IMethodSymbol; if (method?.TypeArguments.Length == 1 && IsEventHandlerOnMethod(method, known.BaseEventHandler)) { var tArg = method.TypeArguments[0]; - if (tArg.IsReferenceType) return GetTypeSyntax(tArg); + if (tArg.IsReferenceType) return GetCandidate(tArg); } } // If we cannot resolve the method symbol reliably, skip to avoid false positives @@ -113,7 +113,7 @@ static bool IsPotentialUsage(SyntaxNode node, CancellationToken _) { var def = symbol.OriginalDefinition; if (IsTargetInterface(def, known.MessageConsumeContext) && symbol.TypeArguments.Length == 1) { var arg = symbol.TypeArguments[0]; - return GetTypeSyntax(arg); + return GetCandidate(arg); } } } @@ -126,7 +126,7 @@ static bool IsPotentialUsage(SyntaxNode node, CancellationToken _) { if (invoke is not null) { foreach (var p in invoke.Parameters) { if (TryExtractTypeArgFromIMessageConsumeContext(p.Type, known.MessageConsumeContext, out var typeArg)) { - return GetTypeSyntax(typeArg); + return GetCandidate(typeArg); } } } @@ -135,6 +135,19 @@ static bool IsPotentialUsage(SyntaxNode node, CancellationToken _) { return null; } + static (string Name, int Specificity)? GetCandidate(ITypeSymbol symbol) + => GetTypeSyntax(symbol) is { } name ? (name, GetSpecificity(symbol)) : null; + + // A type always has strictly more supertypes (base classes and interfaces) than any of its supertypes has, + // so emitting switch arms by this count in descending order guarantees that no arm is subsumed by an earlier one + static int GetSpecificity(ITypeSymbol symbol) { + var count = symbol.AllInterfaces.Length; + + for (var baseType = symbol.BaseType; baseType != null; baseType = baseType.BaseType) count++; + + return count; + } + static string? GetTypeSyntax(ITypeSymbol symbol) { // Skip unresolved generic type parameters (e.g. T in IMessageConsumeContext) if (symbol.TypeKind == TypeKind.TypeParameter) return null; @@ -210,8 +223,17 @@ static bool TryExtractTypeArgFromIMessageConsumeContext( return false; } - static void Generate(SourceProductionContext context, ImmutableArray typeNames) { - var distinct = typeNames.Where(static t => !string.IsNullOrWhiteSpace(t)).Distinct().ToArray(); + static void Generate(SourceProductionContext context, ImmutableArray<(string Name, int Specificity)> candidates) { + // Most specific types go first, otherwise the arm of a base type or an interface makes the arms of its subtypes + // unreachable (CS8510). Ordering by name keeps the output deterministic. + var distinct = candidates + .Where(static c => !string.IsNullOrWhiteSpace(c.Name)) + .GroupBy(static c => c.Name, StringComparer.Ordinal) + .Select(static g => (Name: g.Key, Specificity: g.Max(static c => c.Specificity))) + .OrderByDescending(static c => c.Specificity) + .ThenBy(static c => c.Name, StringComparer.Ordinal) + .Select(static c => c.Name) + .ToArray(); if (distinct.Length == 0) { // Always emit a marker file so users can verify the generator ran @@ -249,10 +271,10 @@ static void Generate(SourceProductionContext context, ImmutableArray typ context.AddSource("MessageConsumeContext_Converters.g.cs", sb.ToString()); } - static ImmutableArray DiscoverEventTypes(Compilation compilation, INamedTypeSymbol? eventTypeAttributeSymbol) { - if (eventTypeAttributeSymbol is null) return ImmutableArray.Empty; + static ImmutableArray<(string Name, int Specificity)> DiscoverEventTypes(Compilation compilation, INamedTypeSymbol? eventTypeAttributeSymbol) { + if (eventTypeAttributeSymbol is null) return ImmutableArray<(string Name, int Specificity)>.Empty; - var builder = ImmutableArray.CreateBuilder(); + var builder = ImmutableArray.CreateBuilder<(string Name, int Specificity)>(); ProcessNamespace(compilation.Assembly.GlobalNamespace, isReferenced: false); @@ -264,8 +286,7 @@ static ImmutableArray DiscoverEventTypes(Compilation compilation, INamed void ProcessType(INamedTypeSymbol type, bool isReferenced) { if (HasEventTypeAttribute(type) && (!isReferenced || IsPublicType(type))) { - var name = GetTypeSyntax(type); - if (name is not null) builder.Add(name); + if (GetCandidate(type) is { } candidate) builder.Add(candidate); } foreach (var nt in type.GetTypeMembers()) { diff --git a/src/Core/test/Eventuous.Tests.Subscriptions/ConsumeContextConverterGeneratorTests.cs b/src/Core/test/Eventuous.Tests.Subscriptions/ConsumeContextConverterGeneratorTests.cs new file mode 100644 index 000000000..a543cdacd --- /dev/null +++ b/src/Core/test/Eventuous.Tests.Subscriptions/ConsumeContextConverterGeneratorTests.cs @@ -0,0 +1,92 @@ +using Eventuous.Subscriptions.Generators; +using Microsoft.CodeAnalysis; +using Microsoft.CodeAnalysis.CSharp; + +namespace Eventuous.Tests.Subscriptions; + +public class ConsumeContextConverterGeneratorTests { + [Test] + public async Task Should_emit_derived_event_type_before_its_base() { + // Issue #589: the base type lives in an outer namespace, so discovery finds it first + const string source = """ + using Eventuous; + + namespace Foo { + [EventType("V1.Base")] + public record BaseEvent; + } + + namespace Foo.Bar { + [EventType("V1.Derived")] + public sealed record DerivedEvent : Foo.BaseEvent; + } + """; + + var (generated, errors) = RunGenerator(source); + + await Assert.That(errors).IsEmpty(); + await Assert.That(ArmIndex(generated, "global::Foo.Bar.DerivedEvent")).IsLessThan(ArmIndex(generated, "global::Foo.BaseEvent")); + } + + [Test] + public async Task Should_emit_interfaces_after_their_implementations_and_derived_interfaces() { + // Names are chosen so that alphabetical order is the opposite of the required one + const string source = """ + using Eventuous; + using Eventuous.Subscriptions.Context; + + namespace Foo; + + public interface IAnyEvent; + + public interface IBookingEvent : IAnyEvent; + + [EventType("V1.RoomBooked")] + public record RoomBooked : IBookingEvent; + + public static class Usages { + public static void Any(IMessageConsumeContext ctx) { } + + public static void Booking(IMessageConsumeContext ctx) { } + } + """; + + var (generated, errors) = RunGenerator(source); + + await Assert.That(errors).IsEmpty(); + await Assert.That(ArmIndex(generated, "global::Foo.RoomBooked")).IsLessThan(ArmIndex(generated, "global::Foo.IBookingEvent")); + await Assert.That(ArmIndex(generated, "global::Foo.IBookingEvent")).IsLessThan(ArmIndex(generated, "global::Foo.IAnyEvent")); + } + + static int ArmIndex(string generated, string typeName) { + var index = generated.IndexOf($"{typeName} =>", StringComparison.Ordinal); + + return index >= 0 ? index : throw new InvalidOperationException($"No switch arm generated for {typeName}"); + } + + static (string Generated, Diagnostic[] Errors) RunGenerator(string source) { + var parseOptions = new CSharpParseOptions(LanguageVersion.Preview); + var runtimeDir = Path.GetDirectoryName(typeof(object).Assembly.Location)!; + + var refs = Directory.GetFiles(runtimeDir, "System.*.dll") + .Append(typeof(EventTypeAttribute).Assembly.Location) + .Append(typeof(Eventuous.Subscriptions.EventHandler).Assembly.Location) + .Select(path => (MetadataReference)MetadataReference.CreateFromFile(path)); + + var compilation = CSharpCompilation.Create( + assemblyName: "ConsumeContextConverterGeneratorTestAssembly", + syntaxTrees: [CSharpSyntaxTree.ParseText(source, parseOptions)], + references: refs, + options: new(OutputKind.DynamicallyLinkedLibrary, specificDiagnosticOptions: [new("CS1701", ReportDiagnostic.Suppress)]) + ); + + var driver = CSharpGeneratorDriver.Create([new ConsumeContextConverterGenerator().AsSourceGenerator()], parseOptions: parseOptions); + + driver.RunGeneratorsAndUpdateCompilation(compilation, out var output, out _); + + var generated = output.SyntaxTrees.Single(t => t.FilePath.EndsWith("MessageConsumeContext_Converters.g.cs")); + var errors = output.GetDiagnostics().Where(d => d.Severity == DiagnosticSeverity.Error).ToArray(); + + return (generated.GetText().ToString(), errors); + } +} diff --git a/src/Core/test/Eventuous.Tests.Subscriptions/Eventuous.Tests.Subscriptions.csproj b/src/Core/test/Eventuous.Tests.Subscriptions/Eventuous.Tests.Subscriptions.csproj index 673f6ca15..a3d909674 100644 --- a/src/Core/test/Eventuous.Tests.Subscriptions/Eventuous.Tests.Subscriptions.csproj +++ b/src/Core/test/Eventuous.Tests.Subscriptions/Eventuous.Tests.Subscriptions.csproj @@ -10,8 +10,11 @@ + + + all From a965b3ed2b561bc2d0eecbe03a0429087e7df0e4 Mon Sep 17 00:00:00 2001 From: Alexey Zimarev Date: Sun, 20 Sep 2026 15:06:57 +0200 Subject: [PATCH 2/2] fix(subscriptions): keep discovery order for equally specific converter arms Types related only through generic variance (IEnvelope and IEnvelope with IEnvelope) have the same number of supertypes, so the specificity rank cannot order them. Breaking such ties by name could emit the broader arm first and fail with CS8510, including in projects that compiled before, when arms followed declaration order. Drop the name tie-break and rely on the stable sort, so equally specific types keep their discovery order. That is still deterministic, and anything that compiled before the arms were ranked still compiles. Reword the comments that claimed the rank alone guarantees no arm is subsumed, and add a regression test. Co-Authored-By: Claude Fable 5.1 --- .../ConsumeContextConverterGenerator.cs | 7 +++-- .../ConsumeContextConverterGeneratorTests.cs | 28 +++++++++++++++++++ 2 files changed, 32 insertions(+), 3 deletions(-) diff --git a/src/Core/gen/Eventuous.Subscriptions.Generators/ConsumeContextConverterGenerator.cs b/src/Core/gen/Eventuous.Subscriptions.Generators/ConsumeContextConverterGenerator.cs index 07d5ec49a..898c3ed5c 100644 --- a/src/Core/gen/Eventuous.Subscriptions.Generators/ConsumeContextConverterGenerator.cs +++ b/src/Core/gen/Eventuous.Subscriptions.Generators/ConsumeContextConverterGenerator.cs @@ -139,7 +139,8 @@ static bool IsPotentialUsage(SyntaxNode node, CancellationToken _) { => GetTypeSyntax(symbol) is { } name ? (name, GetSpecificity(symbol)) : null; // A type always has strictly more supertypes (base classes and interfaces) than any of its supertypes has, - // so emitting switch arms by this count in descending order guarantees that no arm is subsumed by an earlier one + // so emitting switch arms by this count in descending order keeps the arm of a type ahead of the arms of its + // supertypes. Types related only through generic variance or array covariance get the same count. static int GetSpecificity(ITypeSymbol symbol) { var count = symbol.AllInterfaces.Length; @@ -225,13 +226,13 @@ static bool TryExtractTypeArgFromIMessageConsumeContext( static void Generate(SourceProductionContext context, ImmutableArray<(string Name, int Specificity)> candidates) { // Most specific types go first, otherwise the arm of a base type or an interface makes the arms of its subtypes - // unreachable (CS8510). Ordering by name keeps the output deterministic. + // unreachable (CS8510). The sort is stable, so types of equal specificity keep their discovery order: that is + // deterministic, and it keeps arms that are related only through variance in the order they were declared. var distinct = candidates .Where(static c => !string.IsNullOrWhiteSpace(c.Name)) .GroupBy(static c => c.Name, StringComparer.Ordinal) .Select(static g => (Name: g.Key, Specificity: g.Max(static c => c.Specificity))) .OrderByDescending(static c => c.Specificity) - .ThenBy(static c => c.Name, StringComparer.Ordinal) .Select(static c => c.Name) .ToArray(); diff --git a/src/Core/test/Eventuous.Tests.Subscriptions/ConsumeContextConverterGeneratorTests.cs b/src/Core/test/Eventuous.Tests.Subscriptions/ConsumeContextConverterGeneratorTests.cs index a543cdacd..924f45255 100644 --- a/src/Core/test/Eventuous.Tests.Subscriptions/ConsumeContextConverterGeneratorTests.cs +++ b/src/Core/test/Eventuous.Tests.Subscriptions/ConsumeContextConverterGeneratorTests.cs @@ -58,6 +58,34 @@ public static void Booking(IMessageConsumeContext ctx) { } await Assert.That(ArmIndex(generated, "global::Foo.IBookingEvent")).IsLessThan(ArmIndex(generated, "global::Foo.IAnyEvent")); } + [Test] + public async Task Should_keep_discovery_order_for_types_of_equal_specificity() { + // Types related only through generic variance have the same number of supertypes, so their relative order + // must stay as declared. Names are chosen so that alphabetical order would put the broader type first. + const string source = """ + using Eventuous.Subscriptions.Context; + + namespace Foo; + + public class Animal; + + public class Zebra : Animal; + + public interface IEnvelope; + + public static class Usages { + public static void Zebras(IMessageConsumeContext> ctx) { } + + public static void Animals(IMessageConsumeContext> ctx) { } + } + """; + + var (generated, errors) = RunGenerator(source); + + await Assert.That(errors).IsEmpty(); + await Assert.That(ArmIndex(generated, "global::Foo.IEnvelope")).IsLessThan(ArmIndex(generated, "global::Foo.IEnvelope")); + } + static int ArmIndex(string generated, string typeName) { var index = generated.IndexOf($"{typeName} =>", StringComparison.Ordinal);