Scope an interceptor to its own implementation, keep module defaults, diagnose a stranded assembly attribute - #41
Merged
Conversation
… diagnose a stranded assembly attribute
DM-F02: an interceptor's wrapper is generated from one class and forwards that
class's members, but it was applied to every registration of the service type -
because interception reuses the decorator rewrite, and wrapping everything
behind an interface is correct for a decorator and wrong for this. A sibling
implementation carrying no [Intercept] came back wrapped in the first class's
wrapper; with both marked, each registration was wrapped once per generated
wrapper and every interceptor ran twice per call. Nothing threw.
Decorate<TService> gains an overload taking the implementation type, and the
generator names it. Two details this needed:
* The filter is skipped when the descriptor's origin cannot be known - an
instance, a hand-written factory, or the factories
DependencyModules_GenerateFactories emits. Refusing to wrap there would
silently stop intercepting a service that had asked for it, which is a worse
failure than the one being fixed.
* Decoration rewrites an implementation-type descriptor into a factory, which
erases the implementation. An interceptor ordered outside a decorator would
then fail to recognise its own registration, so the origin is carried across
the replacement the way Applied already is.
It is a separate overload rather than an optional parameter: generated code from
an earlier version is compiled against the three-argument signature, and
widening that one would leave it unable to bind.
DM-F04: the generated module attribute assigned every property unconditionally
unless it was declared nullable, so `public string Label { get; set; } = "x";`
had its initialiser overwritten with null by a composition that never mentioned
it. Nullability is an annotation rather than a runtime fact - the attribute
property is null until assigned either way - so the guard is now emitted for
every property. A value-typed property compares as always-true and is assigned
unconditionally, which is unchanged and is the documented limit: 0 is a
legitimate value, so null cannot express "unset" for one.
DM-F05: DM0019 reports an assembly-level module attribute outside the entry
point file. Those attributes are composed into the generated ApplicationModule,
which is built from one compilation unit, so written anywhere else they were
read by nobody - a clean build and an InvalidOperationException at the first
resolve. It stays quiet when nothing generated an ApplicationModule, which is
the test-project case the testing guide shows: there the attributes are read at
run time, and there is no entry point to be in the wrong file relative to.
Nine tests added. Two of the three interception ones fail without the fix; the
third guards the decorator-interleaving path the fix could have broken. The
value-typed parameter test pins the accepted limit rather than a fix.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01R9DWh8FWTib6hRYURbypWU
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 three findings left out of #40, with the reasoning for each judgement call
written down.
main+ 1 commit, 1,830 tests green, 0 warnings.DM-F02 — an interceptor wrapped every implementation of the interface
An interceptor's wrapper is generated from one class and forwards that
class's members. It was applied to every registration of the service type,
because interception reuses the decorator rewrite — and wrapping everything
behind an interface is exactly right for a decorator and wrong for this.
LoudGreeterresolves asLoudGreeter_InterceptedLoudGreeter_InterceptedQuietGreeterresolves asLoudGreeter_InterceptedQuietGreeterGreet()Greet()No exception either way — just doubled metrics, audit rows and retries.
Decorate<TService>gains an overload taking the implementation type, and thegenerator names it. Two things this needed that are worth review:
The filter is skipped when the origin cannot be known. An instance
registration, a hand-written factory, or the factories
DependencyModules_GenerateFactoriesemits all leave the descriptor with noimplementation type. Refusing to wrap there would silently stop intercepting a
service that had asked for it — a worse failure than the one being fixed — so
the rule is "narrow when we can tell, behave as before when we cannot". The
residual case is
GenerateFactoriesplus two implementations of one interceptedinterface, which is no worse than today.
Origin is carried across rewrites. Decoration replaces an
implementation-type descriptor with a factory, erasing the implementation. An
interceptor ordered outside a decorator would then fail to recognise its own
registration and silently stop applying. The origin now follows the replacement
the way
Appliedalready does, and there is a test for that ordering.A separate overload, not an optional parameter. Generated code from an
earlier version is compiled against the three-argument signature; widening that
one would leave it unable to bind against a newer runtime. The Runtime API diff
is therefore purely additive.
The decorator contract is unchanged — a decorator still wraps every registration
of its service, which
DecoratorHelperTests.DecorateOfT_WrapsEveryRegistrationOfTheServicealready covers and which I re-checked end to end.
DM-F04 — a module parameter's initialiser was overwritten with null
The generated attribute assigned every property unconditionally unless it was
declared nullable:
Nullability is an annotation, not a runtime fact — the attribute property is
null until somebody assigns it whichever way it is written — so the guard is now
emitted for every property. This is your call from the review, and it is the
smaller change: the gate came out rather than going in.
A value-typed property compares as always-true and is still assigned
unconditionally. That is unchanged and is the accepted limit:
intis 0 untilassigned and 0 is a legitimate value, so null cannot express "unset" for one.
There is a test pinning that so it reads as a decision rather than an oversight.
Visible in this repo's own snapshot:
ModuleWithConstructorParametersAndPropertiesdeclares
public string OptionalString { get; set; } = "";, and its generatedGetModulenow guards the assignment.DM-F05 —
[assembly: SomeModule]outside the entry point (newDM0019, Error)Assembly-level module attributes are composed into the generated
ApplicationModule, and that module is built from one compilation unit. Writtenanywhere else they were read by nobody: a clean build, no diagnostic, and an
InvalidOperationExceptionat the first resolve — the failure the library existsto prevent.
The false positive this had to avoid is the one you raised: a unit-test
project applying a module across all its tests is a real use case, and the
testing guide shows exactly that shape in its own file. It works there because
the test integration reads assembly attributes at run time. So the check keys
off the entry-point file the generator actually used, and stays silent when
nothing generated an
ApplicationModule— a class library or a test project hasno entry point to be in the wrong file relative to. There is a test for that
case specifically.
Reported at Error, per "either support it or add a diagnostic error". Supporting
it properly would mean deciding what an assembly-scoped attribute means when
there are several candidate module roots, which is the ambiguity you flagged and
I don't think is worth buying.
Tests
Nine new, across three files:
InterceptionScopeTests— sibling not wrapped, two marked implementations gettheir own wrappers, interceptor ordered outside a decorator still applies.
The first two fail without the fix (verified by reverting the filter); the
third guards the path the fix could have broken.
DefaultedParameterModuleTests(integ) — reference-typed default survives,value-typed one documented as not surviving, named parameters still carried.
AssemblyModuleAttributeDiagnosticsTests— DM0019 fires outside the entrypoint, silent inside it, silent with no generated
ApplicationModule.API surface
Additive only:
DecoratorHelper.Decorate<TService>(…, Type? implementationType)— newoverload; the three-argument one is untouched.
DependencyModuleDiagnostics.AssemblyModuleAttributeNotComposed— in.Impl.AnalyzerReleases.Unshipped.mdrecords DM0019.Verification
dotnet build DependencyModules.sln -c Release— 0 warningsdotnet test DependencyModules.sln -c Release— 1,830 passed, 0 faileddotnet new consoleconsuming thepacked generator, including the decorator-contract regression and the
interceptor-after-decorator ordering
🤖 Generated with Claude Code
https://claude.ai/code/session_01R9DWh8FWTib6hRYURbypWU