Skip to content

fix(subscriptions): emit converter switch arms most-specific-first #592

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

Merged
alexeyzimarev merged 2 commits into from
Sep 20, 2026
Merged

fix(subscriptions): emit converter switch arms most-specific-first #592

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
bb0c4e7
fix(subscriptions): emit converter switch arms most-specific-first (#…
alexeyzimarev Sep 20, 2026
a965b3e
fix(subscriptions): keep discovery order for equally specific convert…
alexeyzimarev Sep 20, 2026
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
48 changes: 35 additions & 13 deletions src/Core/gen/Eventuous.Subscriptions.Generators/ConsumeContextConverterGenerator.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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<T>
Expand All @@ -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);
}
}

Expand All @@ -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
Expand All @@ -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);
}
}
}
Expand All @@ -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);
}
}
}
Expand All @@ -135,6 +135,20 @@ 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 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;

for (var baseType = symbol.BaseType; baseType != null; baseType = baseType.BaseType) count++;
Comment thread
qodo-free-for-open-source-projects[bot] marked this conversation as resolved.
Comment on lines +144 to +147

@chatgpt-codex-connector chatgpt-codex-connector Bot Sep 20, 2026

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 Order variant constructed types by conversion, not rank

When candidates are related through generic variance—for example, IEvent<out T> used as both IMessageConsumeContext<IEvent<Base>> and IMessageConsumeContext<IEvent<Derived>>—the two constructed interfaces have identical base/interface counts. The name tiebreak can therefore emit IEvent<Base> first, which subsumes the IEvent<Derived> pattern and still produces CS8510. Determine ordering from actual implicit reference conversions/subtyping rather than relying solely on the supertype count.

Useful? React with 👍 / 👎.

@alexeyzimarev alexeyzimarev Sep 20, 2026

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.

Verified with a generator test: IEnvelope<Zebra> and IEnvelope<Animal> (with IEnvelope<out T>) get the same supertype count, and the name tie-break emitted the broader arm first, which fails with CS8510. The name tie-break could also break a project that compiled before this PR, when arms simply followed declaration order.

Fixed in a965b3e: the name tie-break is gone and the sort is stable, so equally specific types keep their discovery order. That is still deterministic, and anything that compiled before still compiles. Added Should_keep_discovery_order_for_types_of_equal_specificity as a regression test, and reworded the comment that claimed the rank alone guarantees no arm is subsumed.

Not doing the conversion-based topological ordering here. Variance-related message types declared broader-first failed before this PR too, so it is not a regression. Ordering them correctly needs implicit-conversion checks across all candidate pairs, on every generator run, over every [EventType] type in all referenced assemblies, and nobody has reported hitting this. It can be revisited if it comes up.


return count;
}

static string? GetTypeSyntax(ITypeSymbol symbol) {
// Skip unresolved generic type parameters (e.g. T in IMessageConsumeContext<T>)
if (symbol.TypeKind == TypeKind.TypeParameter) return null;
Expand Down Expand Up @@ -210,8 +224,17 @@ static bool TryExtractTypeArgFromIMessageConsumeContext(
return false;
}

static void Generate(SourceProductionContext context, ImmutableArray<string> 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). 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)
.Select(static c => c.Name)
.ToArray();

if (distinct.Length == 0) {
// Always emit a marker file so users can verify the generator ran
Expand Down Expand Up @@ -249,10 +272,10 @@ static void Generate(SourceProductionContext context, ImmutableArray<string> typ
context.AddSource("MessageConsumeContext_Converters.g.cs", sb.ToString());
}

static ImmutableArray<string> DiscoverEventTypes(Compilation compilation, INamedTypeSymbol? eventTypeAttributeSymbol) {
if (eventTypeAttributeSymbol is null) return ImmutableArray<string>.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<string>();
var builder = ImmutableArray.CreateBuilder<(string Name, int Specificity)>();

ProcessNamespace(compilation.Assembly.GlobalNamespace, isReferenced: false);

Expand All @@ -264,8 +287,7 @@ static ImmutableArray<string> 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()) {
Expand Down
120 changes: 120 additions & 0 deletions src/Core/test/Eventuous.Tests.Subscriptions/ConsumeContextConverterGeneratorTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,120 @@
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<IAnyEvent> ctx) { }

public static void Booking(IMessageConsumeContext<IBookingEvent> 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"));
}

[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<out T>;

public static class Usages {
public static void Zebras(IMessageConsumeContext<IEnvelope<Zebra>> ctx) { }

public static void Animals(IMessageConsumeContext<IEnvelope<Animal>> ctx) { }
}
""";

var (generated, errors) = RunGenerator(source);

await Assert.That(errors).IsEmpty();
await Assert.That(ArmIndex(generated, "global::Foo.IEnvelope<global::Foo.Zebra>")).IsLessThan(ArmIndex(generated, "global::Foo.IEnvelope<global::Foo.Animal>"));
}

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);
}
}
3 changes: 3 additions & 0 deletions src/Core/test/Eventuous.Tests.Subscriptions/Eventuous.Tests.Subscriptions.csproj
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,11 @@
<ProjectReference Include="$(CoreRoot)\Eventuous.Serialization.Json.Dynamic\Eventuous.Serialization.Json.Dynamic.csproj"/>
<ProjectReference Include="$(ExtRoot)\Eventuous.Extensions.DependencyInjection\Eventuous.Extensions.DependencyInjection.csproj"/>
<ProjectReference Include="$(RepoRoot)\test\Eventuous.TestHelpers.TUnit\Eventuous.TestHelpers.TUnit.csproj"/>
<ProjectReference Include="$(LocalGenRoot)\Eventuous.Subscriptions.Generators\Eventuous.Subscriptions.Generators.csproj"/>
</ItemGroup>
<ItemGroup>
<PackageReference Include="Microsoft.CodeAnalysis" PrivateAssets="all" />
<PackageReference Include="Microsoft.CodeAnalysis.CSharp" PrivateAssets="all" />
<PackageReference Include="Shouldly" />
<PackageReference Update="coverlet.collector">
<PrivateAssets>all</PrivateAssets>
Expand Down
Loading