Convention selection and shape gaps - #25
Merged
Merged
Conversation
Working notes for picking up in a fresh session rather than project documentation, so they stay on disk and out of the repository. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01C56x6Vv6HJ6ArfqwKsuSb9
GetConstructorInfo walked DescendantNodes to find constructor declarations, which visits the entire subtree of a class — every method body, every statement, every expression — to find nodes that can only ever be direct children. It runs for every candidate, so what it cost tracked how much code a class contained. Measured on 2,000 ordinary classes, the second generator run after editing one file spent 73 ms there; reading the type's own members takes 12 ms. Half of that gain lands on the convention path that already shipped. It was also wrong. DescendantNodes reaches into nested declarations, so a service containing a nested class with a parameterised constructor was registered against that constructor's parameters. Invisible unless factory generation is on, because otherwise the container picks the constructor at run time and covers the mistake — which is why the test that pins this sets DependencyModules_GenerateFactories and fails against the old walk. The benchmark that found it is kept, in the solution so it cannot rot and out of the coverage gate. Four things in it are load-bearing, and each produced a believable but wrong number first: time only RunGeneratorsAndUpdateCompilation, one syntax tree per class, real class bodies, and measure the run after an edit rather than the cold one. The design doc's earlier performance numbers were taken with something that was not retained. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01C56x6Vv6HJ6ArfqwKsuSb9
Six changes to what a convention can select and what it registers matches as. **A partial class is one candidate.** The candidate provider runs per declaration and reads the base list in front of it, so `partial class Foo : IFoo` in one part and `partial class Foo : FooBase` in another produced two candidates that each saw half the picture — and the ambiguity check, grouping by implementation type, read the second as a competing match and refused both with "matched by more than one convention — as 'IFoo' and as 'IFoo'". Declarations are merged before matching, so a type that declares an interface in any part declares it, and the greediest constructor across parts wins. **A type filling two roles registers twice.** DM0004 now keys on the pair of implementation and service type rather than on the implementation alone. Two conventions reaching a type through different interfaces is the ordinary shape of a MediatR handler, not an ambiguity; one service type claimed twice still fails, because one lifetime has to win and the source does not say which. Equal lifetimes fail too — the outcome is predictable but the declaration is redundant. BuildServiceModels emits one ServiceModel per implementation carrying N registrations, which is what the attribute path builds and what keeps per-implementation state from being duplicated across models. **One convention registers every closing a candidate implements.** A handler covering two messages registered only the first, silently: green build, no diagnostic, an event that never fires. FirstMatchingInterface is now AllMatchingInterfaces. **AsSelf and AsSelfWithInterfaces.** The latter is the existing cross-wire emission rather than a second mechanism, so one instance is reachable through the type and each of its interfaces. Plain multi-interface registration stays N instances, matching Scrutor and MediatR; sharing is opt-in. **Namespace filters, and selection without assignability.** InNamespaceOf<T>, InNamespaces, InExactNamespaces and the NotIn forms, with inclusions combining as or and exclusions applied after. RegisterAll() with no service type selects by filter alone, which is how a concrete class implementing no interface gets registered by convention — the largest single hole against Scrutor. It requires a shape and a filter: with neither there is nothing to register matches as, and without a filter it would match every class in the compilation. That widened the candidate population to every class, since the predicate cannot know whether any convention selects by filter. Measured at 2,000 classes, the run after an edit went from 9 ms to 73 ms. A syntax-only path for declarations with no base list — nothing they need is a semantic question, so no symbol is bound — brings it to 12 ms. **Using and WithKey.** Both fields already existed on ServiceRegistrationModel and were being passed null. Covers RegistrationStrategy and WithServiceKey. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01C56x6Vv6HJ6ArfqwKsuSb9
Finishes the filter and shape axes of the Scrutor parity table.
conventions.RegisterAll<IFoo>().WithAttribute<HandlerAttribute>().AsSingleton();
conventions.RegisterAll<IFoo>().WithoutAttribute<LegacyAttribute>().AsSingleton();
conventions.RegisterAll().WithName("*Repository").AsSelf().AsScoped();
conventions.RegisterAll<IFoo>().As<IMarker>().AsSingleton();
conventions.RegisterAll<IMarker>().AsMatchingInterface().AsSingleton();
Attribute filters resolve the attribute type rather than matching how it was
written, which is how a namespace-qualified usage came to be silently ignored in
this generator once already. They combine with and: a type carries every
attribute asked for and none of the excluded ones. The keys are collected only
for declarations that carry attributes at all, so this stays off the cost of the
wider candidate population, and the partial-class merge unions them because
attributes on partial parts combine.
Name globs follow the semantics the design doc specified: * for zero or more
characters, ? for exactly one, a dot meaning the pattern is matched against the
qualified name, ordinal and case-sensitive. Everything else is escaped, so a
pattern cannot smuggle in a regular expression. The doc asked for the Regex to
be built once per pattern rather than once per candidate; a Regex is not
equatable and holding one would break the incremental cache, so the model keeps
the pattern string and the matcher compiles one per convention — the matcher
runs at output time and does not have to be cacheable.
AsMatchingInterface skips a match that implements no correspondingly named
interface rather than registering it some other way, because the convention
asked for one shape.
RegisterAll() with no service type now accepts a name or attribute filter as its
required inclusion, not only a namespace.
579 tests pass, 0 warnings, 87.9% coverage, verify-packages.sh clean.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01C56x6Vv6HJ6ArfqwKsuSb9
Ian's design note, committed separately rather than folded into the parity work it critiques. Records two changes measured against FluentValidation: a shape between Interfaces and SelfAndInterfaces, and a System.* exclusion for the AsSelfWithInterfaces expansion. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01C56x6Vv6HJ6ArfqwKsuSb9
ipjohnson
force-pushed
the
feature/convention-gaps
branch
from
August 9, 2026 14:17
f3dca66 to
f7ebb20
Compare
The expansion looped every interface a type could reach, so anything whose base implements IDisposable became resolvable as IDisposable, and a FluentValidation validator became resolvable as IEnumerable<IValidationRule> and IEnumerable. Neither is what "register this as its interfaces" means. Skips interfaces declared in System or a namespace beginning "System.". One rule rather than a list: Autofac excludes IDisposable and not IEnumerable, Scrutor excludes IEnumerable and not IDisposable, and both are patches added after users hit them — the list they imply keeps growing through IEquatable<T>, IComparable and ICloneable. Not extended to Microsoft.Extensions.*, because a BackgroundService reaching IHostedService is something a developer could legitimately want cross-wired. Scoped to the expansion only. A service type the developer named is honoured whatever namespace it lives in, so RegisterAll<IDisposable>() still registers IDisposable — refusing to honour a named service type is a different kind of wrong. Filtering everything away falls back to registering the implementation type, so it degrades to AsSelf rather than to nothing. Implements item 2 of docs/design/convention-self-registration.md. Item 1, AlsoAsSelf, is not done. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01C56x6Vv6HJ6ArfqwKsuSb9
The last step of the Scrutor parity plan, and the only one needing a new
pipeline shape. A convention can be anchored to one named assembly and match its
public types out of metadata:
conventions.RegisterAll(typeof(IHandler<,>))
.InAssemblyOf<SomeTypeInThatPackage>()
.AsScoped();
Entirely compile time. The types are read as Roslyn symbols through
compilation.GetAssemblyOrModuleSymbol, and each match is emitted as a literal
typeof() into the consumer's assembly — a static reference the trimmer roots,
which also lets the DynamicallyAccessedMembers annotation on ServiceDescriptor's
implementation-type parameter flow to a known type so the constructor survives.
Nothing is loaded at run time. That is the difference from Scrutor, whose scan
is reflection over assemblies and finds nothing once the trimmer has run.
The assembly is always named, and named by a type rather than a string, so an
assembly that is not referenced cannot be asked for. There is no way to scan
everything depended on and there should not be one: walking every reference
visits thousands of types on every keystroke where one named assembly visits its
own, so a reference is rejected on its name before any symbol is touched.
A convention sees one source or the other. A scan of the project being built
does not pick up a type from a package, and a scan of a package does not pick up
a local one. Absent the call, behaviour is unchanged.
Constructors come from IMethodSymbol.InstanceConstructors, the symbol-driven
path the design doc called the one genuinely new piece of code — and now
definitely separate, since the syntax path reads a declaration's own members.
Only public types are visible across the boundary, where the in-compilation path
also takes internal ones; nothing can report the type it cannot see, so that is
documented rather than diagnosed.
Diagnostics about a metadata match report at the convention that asked for it.
DM0010's affordance is naming the service at the class, and there is no class to
squiggle inside a DLL, so the convention line is the only place a developer can
act on it. The same fallback applies to DM0004 and DM0006.
The metadata provider combines the conventions with the compilation, so it
re-runs on every keystroke by construction; its result is an equatable list, so
the emission downstream stays cached unless the scanned assembly's public
surface actually differs. Measured, no regression: 2,000 classes, second run
after an edit, 18.6 ms against 20.3 ms before.
The test harness gained the ability to compile a library to real metadata and
reference it, which is the only honest way to test this — the types have to
exist with no syntax tree in the consuming compilation. Loading it is test
infrastructure, so the behavioural tests can resolve what the generated code
registers; no assembly loading ships.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01C56x6Vv6HJ6ArfqwKsuSb9
AlsoAsSelf registers each match as the service type the convention matched and
also as its own concrete type, sharing one instance. Additive where AsSelf
replaces: AsSelf means "instead of the interface", AlsoAsSelf means "as well as
it", and only the interfaces the convention matched are registered rather than
every interface the type can reach.
conventions.RegisterAll(typeof(IValidator<>))
.IncludeBaseClasses()
.AlsoAsSelf()
.AsScoped();
This is the FluentValidation shape, which registers each validator as
IValidator<T> and as the concrete type. FluentValidation registers the pair
independently, which hands you two instances per scope; cross-wiring gives one,
which is the better behaviour and a deliberate difference.
It emits only the cross-wired interface registration. The writer already adds the
implementation registration once per service model whenever anything is
cross-wired, so emitting one here as well produced the type twice — caught by the
test for a handler closing two messages. What separates AlsoAsSelf from
AsSelfWithInterfaces is which interfaces are expanded, not what self costs.
The ambiguity check now runs over emitted registrations rather than over matches.
A shape can produce several registrations from one match, and keying the check on
the match meant a duplicate on one of them dropped the others with it. Keyed on
the implementation and the service type that actually reaches the container, two
conventions using AsSelfWithInterfaces on one type now collide per interface
rather than once, and a registration is only ever dropped for its own collision.
Implements docs/design/convention-self-registration.md, whose status is updated
with the two places the plan and the outcome differ.
595 tests pass, 0 warnings, 87.6% coverage, verify-packages.sh clean.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01C56x6Vv6HJ6ArfqwKsuSb9
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The whole Scrutor parity plan from
docs/design/convention-registration-and-decorators.md— steps 1 through 8 — plus two performance fixes and one defect the work exposed.Silent or wrong behaviours, fixed
A partial class was two candidates. The provider runs per declaration and reads the base list in front of it, so
partial class Foo : IFooin one part andpartial class Foo : FooBasein another each saw half the picture — and the ambiguity check refused both withmatched by more than one convention — as 'IFoo' and as 'IFoo'. One convention, named twice.A handler covering two messages registered one. Silently — green build, no diagnostic, an event that never fires.
FirstMatchingInterfaceis nowAllMatchingInterfaces.A nested type's constructor was the outer type's.
GetConstructorInfowalkedDescendantNodes, which reaches into nested declarations. Only visible withGenerateFactorieson, which is why the test that pins it sets the property and fails against the old walk.AsSelfWithInterfacescross-wired BCL interfaces. Anything whose base implementsIDisposablebecame resolvable asIDisposable; a FluentValidation validator became resolvable asIEnumerable<IValidationRule>. Now skipsSystem.*— one rule rather than the growing blocklist Autofac and Scrutor each maintain half of.DM0004 now means what it says
Keyed on
(implementation, service type)rather than the implementation alone. A type filling two roles is the ordinary shape of a MediatR handler and registers twice; one service type claimed by two conventions still fails, because one lifetime has to win and the source does not say which.The API
Multi-interface registration is N instances, matching Scrutor and MediatR; one shared instance is
AsSelfWithInterfaces(), opt-in. Attributes and service types are resolved, not matched on how they were written — name-matching is how a namespace-qualified usage came to be silently ignored here once already.Referenced assemblies, and why it is AOT-safe
InAssemblyOf<T>()reads types as Roslyn symbols at compile time and emits a literaltypeof()per match — a static reference the trimmer roots, which also lets theDynamicallyAccessedMembersannotation onServiceDescriptorflow to a known type so the constructor survives. Nothing is loaded at run time. That is the difference from Scrutor, whose scan is reflection and finds nothing once the trimmer has run.The assembly is always named, by a type rather than a string, so an unreferenced assembly cannot be asked for. There is no scan-everything: walking every reference visits thousands of types per keystroke where one named assembly visits its own.
Performance
Admitting concrete types with no interface meant the predicate could no longer reject declarations with no base list, and providers cannot see each other, so the cost is unconditional. Measured, 2,000 ordinary classes, second generator run after editing one file:
The constructor fix applies to the convention path that already shipped — 29 ms to 17 ms on the same workload. The metadata provider adds nothing when no convention names an assembly.
The benchmark is kept, in the solution so it cannot rot and out of the coverage gate. Four things in it are load-bearing and each produced a believable but wrong number first: time only
RunGeneratorsAndUpdateCompilation, one syntax tree per class, real class bodies, and measure the run after an edit rather than the cold one.Verification
dotnet build -c Release --no-incremental— 0 warnings./scripts/coverage.sh 85— 590 tests pass, 87.8% coverage./scripts/verify-packages.sh— cleanBehaviour is asserted by compiling and executing generated assemblies. Every defect here was reproduced before being fixed.
Not in this PR
AlsoAsSelf— item 1 ofdocs/design/convention-self-registration.md, the shape betweenInterfacesandSelfAndInterfacesthat FluentValidation wants. Item 2 of that doc is done.The lambda-taking Scrutor overloads have no compile-time equivalent;
IServiceCollectionConfiguration.ConfigureServicesis the escape hatch.FromApplicationDependenciesand friends load assemblies by name at run time and are deliberately out.🤖 Generated with Claude Code
https://claude.ai/code/session_01C56x6Vv6HJ6ArfqwKsuSb9