Skip to content

Fix four silent-failure findings and give diagnostics source locations - #40

Merged
ipjohnson merged 1 commit into
mainfrom
fix/field-report-findings
Aug 26, 2026
Merged

Fix four silent-failure findings and give diagnostics source locations#40
ipjohnson merged 1 commit into
mainfrom
fix/field-report-findings

Conversation

@ipjohnson

Copy link
Copy Markdown
Owner

Fixes four findings from the four-application evaluation, plus source locations
for the diagnostics that had none. Every claim below was reproduced against the
published 1.0.0 packages before being fixed, and re-verified after.

main + 1 commit. Full suite green: 1,812 tests, 0 warnings.

What was wrong, and what changed

DM-F01 — a qualified or aliased service attribute silently registered as Transient

Every legal spelling of [SingletonService] was discovered, but the lifetime
was re-derived from attributeSyntax.Name.ToString().StartsWith("Singleton")
so only spellings whose written text began with Singleton/Scoped got that
lifetime. A qualified name, a global:: prefix and any using alias not named
after its target fell through to Transient. Registered, wrong lifetime, clean
build, no diagnostic.

The sharpest form: two using aliases of the same attribute type behaved
differently depending on how the alias was spelled.

Spelling Before After
[SingletonService] Singleton Singleton
[DependencyModules.Runtime.Attributes.SingletonService] Transient Singleton
[SingletonServiceAttribute] Singleton Singleton
using SingletonAlias = …SingletonServiceAttribute Singleton Singleton
using ZebraAlias = …SingletonServiceAttribute Transient Singleton
[DmAttrs.SingletonService] Transient Singleton

This costs nothing at build time. AttributeTypeMatcher already resolves the
usage through the semantic model to decide the attribute matched at all; the
lifetime branch was throwing that answer away and re-deriving it from source
text. The caller now passes it on.

DM-F03 — OnlyRealm = true did not filter interceptors

Services and decorators filtered by realm correctly. Interception did not: every
module received every interception in the compilation. An OnlyRealm module
emitted applicators for interceptions that named no realm and had nothing to do
with it — wrapping an unrelated service, or throwing while building the provider
when the leaked interceptor needed a dependency the isolated container had never
heard of.

[Intercept] gains a Realm property to match [Decorator], and the
interceptor pass now follows the same rule the other two already did: a
realm-scoped interception belongs only to the module it names, an unscoped one
to every module that is not realm-only.

DM-F07 — DM0005 recommended a call you had already made

The advice was a fixed tail on the message format, appended whatever the cause.
Naming a class as the service type can never match — conventions match through
declared interfaces — so "call IncludeBaseClasses()" sent readers hunting for a
typo they had not made. That is the shape dotnet new avalonia.mvvm ships, so
it is the first thing a UI developer types.

The advice is now a parameter chosen by cause: a class service type is told
conventions match interfaces; a convention that already calls
IncludeBaseClasses() is pointed at its filters instead.

DM-F14 — a nested module was silent (new DM0017, Error)

Documented as always wrong in README.md, the modules guide and the
troubleshooting guide — and reported by none of them. The generator emitted the
completed half at namespace level, producing a second, unrelated type while the
nested declaration never implemented IDependencyModule. [assembly: Nested]
then bound to the detached type's attribute and built green, registering
nothing.

Generation now stops rather than emitting the detached type.

New DM0018 (Info) — a module with properties relying on generated equality

Modules dedupe by type, so two instances carrying different parameter values are
the same module and the first one reached wins. Declaring your own Equals
already suppresses the generated one — GetEqualsFlag has always honoured that.

Info, not Warning, deliberately. Built first as a Warning, it fired on
eleven modules in this repository's own integration tests, none of which are
doing anything wrong: dedupe-by-type is correct for a module composed once, and
only a module reached twice with differing values is actually bitten. Promote it
per project with dotnet_diagnostic.DM0018.severity = warning.

DM-F13 — diagnostics reported at the project, not the declaration

Thirteen sites passed Location.None. Three were correct (the DM0001
generator-failure handlers — an exception escaped, there is no location) and one
was not a report site. The rest held a model, not a symbol, and Roslyn
Locations cannot ride an incremental pipeline — which is exactly why
LocationModel exists, and why the convention diagnostics had locations and
these did not.

LocationModel moves from the Conventions subtree into .Impl so the service,
interceptor, decorator and module models can carry one. Now reporting at the
declaration: DM0002, DM0003, DM0007, DM0008, DM0009, DM0011, DM0012, DM0013,
DM0014, DM0015
, plus the two new codes.

Program.cs(7,42):  warning DM0002: 'AbstractThing' is abstract and cannot be instantiated…
Program.cs(11,33): warning DM0014: 'Ledger' is generic, so [CrossWireService] cannot register it…
Program.cs(19,14): warning DM0008: This service cannot be intercepted…
Program.cs(23,49): warning DM0012: NoCondition names no environment name to test…

One tradeoff worth knowing: location is deliberately excluded from the
incremental cache keys.
It shifts whenever anything above a declaration is
edited — including a comment — and including it regenerated every model on a
keystroke that changed nothing (AddingAComment_ReusesCachedOutput covers
exactly that). The cost is that a diagnostic replayed from cache can sit a line
or two off until the next semantic edit, which is the cheaper of the two
mistakes.

API surface

Two additive changes, both semver-safe:

  • InterceptAttribute.Realm — new property on the runtime package.
  • InterceptorModel.Realm / .Location, ServiceModel.Location,
    DecoratorModel.Location, ModuleEntryPointModel.Location, and
    LocationModel's namespace — all in DependencyModules.SourceGenerator.Impl,
    documented as outside the versioning promise.

Snapshots updated; AnalyzerReleases.Unshipped.md records DM0017 and DM0018.

Verification

  • dotnet build DependencyModules.sln -c Release — 0 warnings
  • dotnet test DependencyModules.sln -c Release — 1,812 passed, 0 failed
  • Each fix additionally checked from a fresh dotnet new console consuming the
    packed generator, not just from the unit tests

Not in this PR

  • DM-F02 — an interceptor wrapper is applied to every registration of the
    interface, so an implementation carrying no [Intercept] comes back wrapped
    and interceptors run twice when two implementations are marked. The fix is the
    one InterceptOpenGeneric already uses (DecoratorHelper.cs:77-81 — match
    ImplementationOf(descriptor) != implementationType, register the
    implementation type, then wrap). Left out because it changes Decorate's
    contract and needs care around the Applied guard and lifetime carry-over.
  • DM-F04 — a non-nullable reference-typed module parameter has its
    initialiser overwritten with null; dropping the IsNullable gate fixes it.
    Value-typed parameters cannot express "unset" and that is accepted as-is.
  • DM-F05 — a diagnostic for an assembly-level module attribute outside the
    entry-point file.

🤖 Generated with Claude Code

https://claude.ai/code/session_01R9DWh8FWTib6hRYURbypWU

DM-F01: read a service attribute's lifetime from the attribute type the usage
resolved to, not from how it was spelled. AttributeTypeMatcher already resolves
the usage through the semantic model to decide the attribute matched at all;
the lifetime branch re-derived it from attributeSyntax.Name and so only a
spelling literally starting with "Singleton"/"Scoped" produced that lifetime.
A qualified name, a global:: prefix and any using alias not named after its
target fell through to Transient - registered, wrong lifetime, no diagnostic.
Costs nothing extra: the caller now passes on the answer instead of discarding it.

DM-F03: apply realm filtering to the interceptor pass, and give [Intercept] a
Realm property to match [Decorator]. Every module previously received every
interception in the compilation, so an OnlyRealm module emitted applicators for
interceptions that named no realm - wrapping unrelated services, or throwing
while building the provider when the leaked interceptor needed a dependency the
isolated container did not have.

DM-F07: DM0005's advice is now chosen by cause rather than being a fixed tail.
Naming a class as the service type can never match, so recommending
IncludeBaseClasses() sent readers hunting for a typo; a convention that already
calls it gets filter advice instead.

DM-F14: DM0017 reports a module nested inside another type - documented as
always wrong in the README and two guides, and until now reported by none of
them. Generation stops rather than emitting a detached same-named type.

DM0018 reports a module with settable properties relying on generated equality.
Info, not Warning: dedupe-by-type is correct for a module composed once, and
this repo's own integration tests carry eleven such modules doing nothing wrong.

DM-F13: LocationModel moves from the Conventions subtree into .Impl so service,
interceptor, decorator and module models can carry one. DM0002, DM0003, DM0007,
DM0008, DM0009, DM0011, DM0012, DM0013, DM0014 and DM0015 now report at the
declaration instead of at the project. The two remaining Location.None sites are
the DM0001 generator-failure handlers, where there is genuinely no location.
Location is deliberately excluded from the incremental cache keys: it shifts
when anything above a declaration is edited, and including it regenerated every
model on a comment keystroke.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01R9DWh8FWTib6hRYURbypWU
@ipjohnson
ipjohnson merged commit dd7a0b2 into main Aug 26, 2026
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants