diff --git a/CHANGELOG.md b/CHANGELOG.md index b670754..742e695 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,199 @@ All notable changes to this project are documented in this file. The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/), and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). +## [1.0.0-rc9330] - 2026-08-14 + +Everything since `1.0.0-rc9230`. The theme is registrations that were silently not happening: an +attribute the generator declined to recognise, a `Replace` that depended on how a class was named, a +mock that replaced the wrong slot, and three shapes it refused without saying so. One of those +refusals turned out to be unnecessary, and generic services can now be intercepted. + +Still a release candidate. `DecoratorExpansion.Expand` changed shape, but only on the generator +extension points, which are documented as unversioned. + +**Upgrade note:** three new warnings. A project building with `TreatWarningsAsErrors` may go red on +work that was previously green and quietly not doing anything — which is the point of them, but it is +a build break rather than a nudge. `NoWarn` takes them per-project; note that `.editorconfig` does not +(see below). + +### Added + +- **Three diagnostics for shapes the generator used to refuse without saying so.** The first two share + a constraint: decoration and cross-wiring replace a registration with a factory, and the container + refuses a factory for an open generic service type — ``Open generic service type 'IRepository`1[T]' + requires registering an open generic implementation type``. The third is about an interceptor that + does not run. + + **`DM0013`** reports a decorator whose service is registered as an open generic. It covers all + three shapes of the mistake, which until now failed in two different ways. A *generic* decorator + is expanded against the closed constructions a compilation registers; an open generic registration + closes nothing, so the expansion produced no decorations and the declaration was dropped in + silence — a decorator sitting in the source, a green build, and nothing wrapping anything. A + *non-generic* decorator named against an unbound service needed no expansion at all, so nothing + caught it: it reached emission carrying `IStore<>` and produced `Decorate>`, which is + CS7003 inside generated code. Reported whichever way the decorator was declared, on the class or + on the module with `[Decorate]`. + + A decorator naming a service the compilation does **not** register stays quiet. Naming a service + someone else registers is what `[Decorate]` exists for, so reporting there would fire on the + feature's primary use. + + **`DM0014`** reports `[CrossWireService]` on a generic type. Cross-wiring shares one instance + across the implementation and every interface it declares, which is emitted as a factory per + interface. Registering each interface to the same open generic implementation type would compile + and is a different contract — one instance per service type, the opposite of what the attribute + promises — so it is refused rather than quietly substituted. The whole registration is dropped + rather than the cross-wired half, because keeping the implementation's own registration would + leave the instance unreachable through any of its interfaces. + + **`DM0015`** reports an interceptor that is quietly absent from some of the members it was applied + to. Three interfaces cover the member shapes and the generator picks per member, so an interceptor + implementing none of the one a member needs was simply left out of that member's chain. An + argument-rewriting interceptor stopped rewriting; read as an authorisation or audit gate, it was a + service that quietly was not gated. The sharpest form — an `IInterceptor` applied to a service whose + members are all async, where it never ran at all — was invisible even to the generator, which + discarded the model before anything could report on it. Reported once per interceptor and member + shape, so a wide interface produces one line rather than forty. + +- **A generic service can be intercepted.** A generic implementation registers as an open generic, and + it was refused outright — because *decoration* cannot touch one, and interception inherited that + constraint without needing it. Decoration rewrites a registration into a factory, which an open + generic service type cannot carry; interception generates a type, and an open generic implementation + type is what the container does accept. + + ```csharp + [SingletonService] + [Intercept(typeof(TracingInterceptor))] + public class Repository : IRepository { … } + ``` + + The wrapper is generic over the same parameters — `Repository_Intercepted : IRepository` — and + takes `Repository` by its own type rather than the service, which would resolve back to the + wrapper and recurse. `DecoratorHelper.InterceptOpenGeneric` swaps the registration and registers the + implementation alongside it, carrying the lifetime and the service key across. + + Constraints come along with the parameters: `Repository where T : class, IEntity, new()` is + wrapped by `Repository_Intercepted : IRepository where T : class, IEntity, new()`, without + which the wrapper could not reference what it wraps. `struct` and `unmanaged` already guarantee a + default constructor and Roslyn reports one for them, so `new()` is dropped rather than repeated — + writing it out is CS0451. + + Worth knowing before relying on it under Native AOT, and true of open generic registrations + generally rather than of interception: a published binary closes them over reference types only. + Measured on `osx-arm64`, `IRepository` resolves and `IRepository` throws + `Unable to create a generic service … because 'System.Int32' is a ValueType`. A plain + `[SingletonService]` on a generic class behaves identically, which the AOT guide now says. + +### Fixed + +- **An attribute the generator declined to recognise, depending on how it was spelled.** Attribute + usages were compared as written — the type's simple name, and that name with `Attribute` appended — + so every other legal spelling missed, and missing meant the registration was silently absent: no + diagnostic, a green build, and a failure at the first resolve. + + | Written as | Before | Now | + |---|---|---| + | `[SingletonService]` | registered | registered | + | `[DependencyModules.Runtime.Attributes.SingletonService]` | **skipped** | registered | + | `[global::DependencyModules.Runtime.Attributes.SingletonServiceAttribute]` | **skipped** | registered | + | `[DmAttrs.SingletonService]` (namespace alias) | **skipped** | registered | + | `[DmSingleton]` (type alias) | **skipped** | registered | + + Service attributes are now resolved through the semantic model rather than string-matched, which is + what makes an alias and a qualified name mean the same thing. Module attributes are matched on the + name the usage ends in, so every qualified form works there too; a `using` alias of a *module* + attribute is still not seen, because a predicate that must stay syntax-only cannot resolve one — and + that case fails as a `CS0311` at `AddModule()` rather than silently. + +- **`Using = Replace` and `Using = Try` decided by the alphabet.** Registrations within a module are + emitted sorted by implementation type name, and both act *on* a registration that has to already be + there. Named so that the sort put them first, they ran before their target existed: `Replace` + replaced nothing, added itself, and was then beaten by the very registration it meant to displace. + + ```csharp + [SingletonService(Using = RegistrationType.Replace)] public class AaaThing : IThing; + [SingletonService] public class ZzzThing : IThing; + // asked for AaaThing; got [AaaThing, ZzzThing], and ZzzThing won + ``` + + They are now emitted after the plain `Add` registrations in their group, the same rule that already + put conditional registrations last so the override pattern works. Renaming the class was the + previous workaround, and nothing said you needed it. + +- **`[Mock]` ignored `[FromKeyedServices]` on the same parameter.** The double was registered + unkeyed, leaving the keyed registration — the one a consumer injects — untouched. The service under + test kept the real implementation while the test held a double it believed was wired in: the + arrangement ran, the double recorded nothing, and the assertion failed somewhere else entirely. The + key on the parameter is now the key the double is registered under, and read back from. Identical + under NSubstitute, Moq and FakeItEasy. + +- **The generator copied 50 of its own source files into consuming projects' output.** + `CopyToOutputDirectory` on a `Compile` item copies the *source*, and the metadata flowed to every + project referencing the analyzer — 428KB of generator internals in `bin` and in `publish`. It + affected `ProjectReference` consumers and this repository's own `benchmarks/` and test output; the + NuGet package, which ships only `analyzers/dotnet/cs` and `build/`, was never affected. + +- **Generated code that did not compile.** `[CrossWireService]` on a generic type leaked the type + parameter into the registration as `typeof(ILedger)`, with no `T` in scope, beside + `GetRequiredService>()` — CS0246 and CS7003, in a file the developer did not write. A + non-generic decorator named against an open generic service produced CS7003 and CS1503 the same + way. Both are now `DM0014` and `DM0013`. + +### Changed + +- **`CSharpAuthor` 1.1.1010**, for `AddConstraint`. A `where` clause was previously assembled as a + string and assigned to `WhereStatement`, which put C#'s ordering rules — one primary constraint + first, `new()` last — in this generator. Two places needed them once a class could carry constraints + as well as a method, and two copies of a subtle rule drift. `TypeParameterReader` reads a parameter + once for both, and the library puts the parts in order. + +- **`DM0008` now says what it costs.** One member the wrapper cannot override means *no* wrapper is + generated, so every other member on the interface goes uninterceped too — and the guide read as + though only the offending member did. A reader who fixed the named member and rebuilt would then + meet the next one. The message and the interception guide both say so now. + +- **Documentation corrections, each with a reproduction behind it.** The README's duplicate-module + example compared `module.someString` against a primary constructor parameter, which is captured + rather than a member — `CS1061`, and it was the only documented way to load a module more than + once. `ExcludeGeneratedCodeFromCoverage` was documented with a `DependencyModules_` prefix it does + not have. Getting started did not mention that a console app or class library needs + `Microsoft.Extensions.DependencyInjection` for `ServiceCollection` and `BuildServiceProvider`. + `[Decorator]`'s `Realm` property was undocumented. + +- **`DM####` diagnostics cannot be tuned through `.editorconfig`, and the reference said they could.** + They are reported by a source generator rather than an analyzer, and Roslyn's `.editorconfig` + severity mapping applies to analyzer diagnostics — so `dotnet_diagnostic.DM0005.severity = none` had + no effect. `NoWarn`, `WarningsAsErrors` and `#pragma warning disable` are applied at the compilation + level and do work; the reference now says that instead. + +- **Two traps are written down rather than left to be discovered.** + `DependencyModules_GenerateFactories` emits a factory per registration, which + `Microsoft.Extensions.DependencyInjection` cannot see inside — so it silently disables + `ValidateOnBuild` and `ValidateScopes` for the whole project, measured on the same captive + dependency with only that property differing. And an assembly declaring two modules that neither set + `OnlyRealm` puts the whole registration list in both, so loading both in one `AddModules` call + registers everything twice. + +- **`DependencyModules_*` properties are invisible over a `ProjectReference`.** They reach the + generator through `build/DependencyModules.SourceGenerator.targets`, which ships inside the NuGet + package, so a project referencing the analyzer as a project never imports it and every property + silently takes its default — `DependencyModules_LogOutputDirectory` included, producing no log and + no message. Troubleshooting now says so and gives the `CompilerVisibleProperty` block, and this + repository's own integration projects declare it. + +- **`DecorateAttribute`'s documentation said the opposite of the README.** Its `service` parameter + was documented as "may be an open generic", which reads as a service *registered* as one. It means + a generic service named unbound, expanded across the closed constructions the compilation + registers. The decorators guide also described the failure as an `InvalidOperationException` from + `DecoratorHelper`; that guard is unreachable from generated code, because the expansion drops the + decorator before anything is emitted, so the guide now points at `DM0013`. + +- **Breaking, for anyone building on the generator extension points:** + `DecoratorExpansion.Expand` takes an additional `out IReadOnlyList` parameter, + carrying the decorators it refused so the caller can report them. These are the extension points + the convention generator uses and are not a versioned API — see the + [generator guide](https://ipjohnson.github.io/DependencyModules/guide/extending). + ## [1.0.0-rc9230] - 2026-08-12 Everything since `1.0.0-rc9210`. Still a release candidate: convention registration and the NUnit @@ -522,5 +715,6 @@ The entries below were written for a 1.0.0 that was not cut. They describe the s Enable it with ``. - A tag-driven release workflow publishing to nuget.org and GitHub Packages. +[1.0.0-rc9330]: https://github.com/ipjohnson/DependencyModules/releases/tag/v1.0.0-rc9330 [1.0.0-rc9230]: https://github.com/ipjohnson/DependencyModules/releases/tag/v1.0.0-rc9230 [1.0.0-rc9210]: https://github.com/ipjohnson/DependencyModules/releases/tag/v1.0.0-rc9210 diff --git a/Directory.Build.props b/Directory.Build.props index 2f99ec5..98a1848 100644 --- a/Directory.Build.props +++ b/Directory.Build.props @@ -11,7 +11,7 @@ The release candidate label. Assembly and file versions carry no prerelease part, so they stay put until the version they do carry changes. --> - rc9230 + rc9330 1.0.0.0 1.0.0.0 diff --git a/README.md b/README.md index 94a2e4c..9033ec0 100644 --- a/README.md +++ b/README.md @@ -174,11 +174,15 @@ public partial class CustomModule(string someString) : IServiceCollectionConfigu // custom logic } - public override bool Equals(object obj) + // Re-exposed as a member: a primary constructor parameter is captured, not a property, so + // module.someString would not compile. + private string Key => someString; + + public override bool Equals(object? obj) { if (obj is CustomModule module) { - return someString.Equals(module.someString); + return someString.Equals(module.Key); } return false; diff --git a/integ-tests/Directory.Build.props b/integ-tests/Directory.Build.props index c378683..2bd6a43 100644 --- a/integ-tests/Directory.Build.props +++ b/integ-tests/Directory.Build.props @@ -7,4 +7,20 @@ false + + + + + + + + + + diff --git a/src/DependencyModules.Runtime/Attributes/DecorateAttribute.cs b/src/DependencyModules.Runtime/Attributes/DecorateAttribute.cs index 5f296f7..6d4313f 100644 --- a/src/DependencyModules.Runtime/Attributes/DecorateAttribute.cs +++ b/src/DependencyModules.Runtime/Attributes/DecorateAttribute.cs @@ -13,7 +13,12 @@ namespace DependencyModules.Runtime.Attributes; /// public partial class DataModule; /// /// -/// The service being decorated. May be an open generic. +/// +/// The service being decorated. A generic service is named unbound — typeof(IHandler<,>) +/// — and the decoration is expanded across the closed constructions the compilation registers. A +/// service registered as an open generic cannot be decorated at all, and is reported as +/// DM0013. +/// /// The decorator, which must implement . [AttributeUsage(AttributeTargets.Class, AllowMultiple = true, Inherited = false)] public class DecorateAttribute(Type service, Type decorator) : Attribute { diff --git a/src/DependencyModules.Runtime/Helpers/DecoratorHelper.cs b/src/DependencyModules.Runtime/Helpers/DecoratorHelper.cs index 6f45824..7cfb4ed 100644 --- a/src/DependencyModules.Runtime/Helpers/DecoratorHelper.cs +++ b/src/DependencyModules.Runtime/Helpers/DecoratorHelper.cs @@ -26,6 +26,76 @@ namespace DependencyModules.Runtime.Helpers; /// public static class DecoratorHelper { + /// + /// Swaps an open generic registration for a generated wrapper that implements the same + /// open generic service. + /// + /// The collection to rewrite. + /// The open generic service, such as IRepository<>. + /// + /// The open generic implementation currently registered for it, such as Repository<>. + /// + /// + /// The generated wrapper, such as Repository_Intercepted<>. It implements the service + /// and takes the implementation as a constructor parameter. + /// + /// + /// + /// cannot + /// serve this: it rewrites a registration into a factory, and the container rejects a factory for + /// an open generic service type — "requires registering an open generic implementation type". An + /// open generic implementation type is exactly what it does accept, and a generated wrapper is + /// one. + /// + /// + /// The implementation is additionally registered under its own concrete type, which is how the + /// wrapper receives it without asking for the service it is itself registered as — that would + /// resolve to the wrapper and recurse. Lifetime is carried across to both, so wrapping does not + /// change how long anything lives. + /// + /// + /// Idempotent: a second pass finds the wrapper in the slot rather than the implementation and + /// leaves it alone, which is what keeps two modules carrying the same registration from + /// double-wrapping. + /// + /// + public static void InterceptOpenGeneric( + IServiceCollection services, + Type serviceType, + [DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.PublicConstructors)] + Type implementationType, + [DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.PublicConstructors)] + Type wrapperType) { + + // Snapshotted: the loop appends the implementation's own registration, and re-reading Count + // would walk into what it just added. + var count = services.Count; + + for (var i = 0; i < count; i++) { + var descriptor = services[i]; + + if (descriptor.ServiceType != serviceType || ImplementationOf(descriptor) != implementationType) { + continue; + } + + services.Add(new ServiceDescriptor(implementationType, implementationType, descriptor.Lifetime)); + + services[i] = descriptor.IsKeyedService + ? new ServiceDescriptor(serviceType, descriptor.ServiceKey, wrapperType, descriptor.Lifetime) + : new ServiceDescriptor(serviceType, wrapperType, descriptor.Lifetime); + } + } + + /// + /// The implementation type of a descriptor, keyed or not. + /// + /// + /// A keyed descriptor throws from ImplementationType rather than returning null, so the two + /// cannot be read through one property. + /// + private static Type? ImplementationOf(ServiceDescriptor descriptor) => + descriptor.IsKeyedService ? descriptor.KeyedImplementationType : descriptor.ImplementationType; + /// /// Wraps every registration of using . /// diff --git a/src/DependencyModules.SourceGenerator.Impl/AnalyzerReleases.Unshipped.md b/src/DependencyModules.SourceGenerator.Impl/AnalyzerReleases.Unshipped.md index d60d403..e62eb69 100644 --- a/src/DependencyModules.SourceGenerator.Impl/AnalyzerReleases.Unshipped.md +++ b/src/DependencyModules.SourceGenerator.Impl/AnalyzerReleases.Unshipped.md @@ -17,3 +17,6 @@ DM0009 | DependencyModules | Error | A convention declaration could not be read. DM0010 | DependencyModules | Info | A service is registered by convention. DM0011 | DependencyModules | Info | A service is registered only when an environment condition holds. DM0012 | DependencyModules | Warning | An environment condition names nothing to test. +DM0013 | DependencyModules | Warning | A service registered as an open generic cannot be decorated. +DM0014 | DependencyModules | Warning | A generic type cannot be cross-wired. +DM0015 | DependencyModules | Warning | An interceptor does not apply to every member it was applied to. diff --git a/src/DependencyModules.SourceGenerator.Impl/DependencyFileWriter.cs b/src/DependencyModules.SourceGenerator.Impl/DependencyFileWriter.cs index 4c0bec2..1bf3fb4 100644 --- a/src/DependencyModules.SourceGenerator.Impl/DependencyFileWriter.cs +++ b/src/DependencyModules.SourceGenerator.Impl/DependencyFileWriter.cs @@ -125,7 +125,7 @@ private string GenerateDependencyMethod(ModuleEntryPointModel entryPointModel, var stringBuilder = new StringBuilder(); - var sortedServiceModels = GetSortedServiceModels(serviceModels); + var sortedServiceModels = GetSortedServiceModels(serviceModels, configurationModel); var autoRegisterGenerators = entryPointModel.RegisterJsonSerializers ?? configurationModel.RegisterSourceGenerator; @@ -570,17 +570,34 @@ private static RegistrationType GetRegistrationType(ModuleEntryPointModel entryP /// because by the time it runs the service type is already registered. That is what Try /// means, and the override pattern wants the default Add. /// + /// + /// Try and Replace are ordered after plain Add within each group for the same + /// reason the conditional key exists: both act on a registration that has to already be + /// there. Ordered by name alone, whether they worked depended on how the two classes happened to + /// be named — a Replace emitted before its target replaced nothing, added itself, and was + /// then beaten by the very registration it meant to displace. Renaming the class fixed it, and + /// nothing said so. + /// /// - private List GetSortedServiceModels(IEnumerable serviceModels) { + private List GetSortedServiceModels( + IEnumerable serviceModels, DependencyModuleConfigurationModel configurationModel) { + var list = new List(serviceModels); list.Sort((x, y) => { var byCondition = IsConditional(x).CompareTo(IsConditional(y)); + if (byCondition != 0) { + return byCondition; + } + + var byStrategy = ActsOnExistingRegistration(x, configurationModel) + .CompareTo(ActsOnExistingRegistration(y, configurationModel)); + // Name is the tie-break rather than the only key, so the order stays total and the // output stays deterministic under List.Sort, which is not stable. - return byCondition != 0 - ? byCondition + return byStrategy != 0 + ? byStrategy : string.Compare(x.ImplementationType.Name, y.ImplementationType.Name, StringComparison.Ordinal); }); @@ -589,4 +606,28 @@ private List GetSortedServiceModels(IEnumerable serv private static bool IsConditional(ServiceModel serviceModel) => serviceModel.Conditions is { Count: > 0 }; + + /// + /// Whether any of a service's registrations only makes sense once its service type is registered. + /// + /// + /// TryEnumerable is deliberately not here. It skips only an identical service-and- + /// implementation pair, so several implementations of one service all register whatever order + /// they arrive in, and deferring it would change nothing. + /// + private static bool ActsOnExistingRegistration( + ServiceModel serviceModel, DependencyModuleConfigurationModel configurationModel) { + + foreach (var registration in serviceModel.Registrations) { + // Null means the registration took the project-wide default, which is what + // DependencyModules_RegistrationType sets. + var registrationType = registration.RegistrationType ?? configurationModel.RegistrationType; + + if (registrationType is RegistrationType.Try or RegistrationType.Replace) { + return true; + } + } + + return false; + } } \ No newline at end of file diff --git a/src/DependencyModules.SourceGenerator.Impl/DependencyModuleDiagnostics.cs b/src/DependencyModules.SourceGenerator.Impl/DependencyModuleDiagnostics.cs index fd5d73e..a003b2b 100644 --- a/src/DependencyModules.SourceGenerator.Impl/DependencyModuleDiagnostics.cs +++ b/src/DependencyModules.SourceGenerator.Impl/DependencyModuleDiagnostics.cs @@ -131,10 +131,19 @@ public static class DependencyModuleDiagnostics { /// interface, and a few member shapes cannot be forwarded; saying so beats emitting a wrapper /// that does not compile. /// + /// + /// The message names the consequence as well as the cause. One member the wrapper cannot override + /// means no wrapper is generated, so every other member on the interface goes uninterceped + /// too — and the guide read as though only the offending member did. A reader who fixed the named + /// member and rebuilt would then meet the next one. + /// public static readonly DiagnosticDescriptor CannotIntercept = new( id: "DM0008", title: "Service cannot be intercepted", - messageFormat: "This service cannot be intercepted: {0}", + messageFormat: + "This service cannot be intercepted, so no wrapper was generated and none of its members are " + + "intercepted: {0}. Other members may be unsupported for the same reason. Write a decorator " + + "instead, or move the member to an interface that is not intercepted.", category: Category, defaultSeverity: DiagnosticSeverity.Warning, isEnabledByDefault: true); @@ -213,4 +222,91 @@ public static class DependencyModuleDiagnostics { defaultSeverity: DiagnosticSeverity.Warning, isEnabledByDefault: true); + /// + /// Raised for an interceptor that cannot serve some of the members it was applied to. + /// + /// + /// Three interfaces cover the member shapes — IInterceptor for a direct return, + /// IAsyncInterceptor for a task, IAsyncEnumerableInterceptor for a stream — and the + /// generator picks per member. An interceptor that implements none of the one a member needs was + /// simply left out of that member's chain, with nothing said. + /// + /// That is the interceptor silently not running. An argument-rewriting interceptor stops + /// rewriting; read as an authorisation or audit gate, it is a service that quietly is not gated. + /// The sharpest form is an interceptor implementing only IInterceptor applied to a service + /// whose members are all async, where it never runs at all and the build is green. + /// + /// Reported once per interceptor and member shape rather than once per member, so a wide + /// interface produces one line rather than forty. + /// + public static readonly DiagnosticDescriptor InterceptorCannotServeMembers = new( + id: "DM0015", + title: "Interceptor does not apply to every member", + messageFormat: + "'{0}' does not implement '{1}', so it is not applied to {2} on '{3}': {4}. Those members run " + + "without it. Implement '{1}' on the interceptor, or apply it to a service that has no such member.", + category: Category, + defaultSeverity: DiagnosticSeverity.Warning, + isEnabledByDefault: true); + + /// + /// Raised for a decorator whose service is registered as an open generic. + /// + /// + /// Decoration replaces a registration with a factory, and the container refuses a factory for an + /// open generic service type — "requires registering an open generic implementation type". So + /// there is nothing to emit, and both shapes of the mistake failed badly until this existed. + /// + /// A generic decorator is expanded against the closed constructions a compilation + /// registers. An open generic registration closes nothing, so the expansion produced no + /// decorations and the declaration was dropped in silence — a build with a decorator in it that + /// never runs. + /// + /// A non-generic decorator named against an unbound service is worse: it needs no + /// expansion, so it reached emission carrying IHolder<> and produced + /// Decorate<IHolder<>> — CS7003 inside generated code, which is the one failure + /// mode this generator exists to avoid. + /// + /// Registering closed constructions is the way through, and the message says so. + /// + public static readonly DiagnosticDescriptor OpenGenericCannotBeDecorated = new( + id: "DM0013", + title: "Open generic registration cannot be decorated", + messageFormat: + "'{0}' is registered as an open generic, so '{1}' cannot decorate it. Decoration replaces a " + + "registration with a factory, and the container does not allow one for an open generic " + + "service type. Register closed constructions of '{0}' instead — a convention over the open " + + "generic registers one per implementation, and a generic decorator is then expanded across them.", + category: Category, + defaultSeverity: DiagnosticSeverity.Warning, + isEnabledByDefault: true); + + /// + /// Raised for [CrossWireService] on a generic type. + /// + /// + /// Cross-wiring means one instance shared across the implementation and every interface it + /// declares, which is emitted as s => s.GetRequiredService<T>() per interface — a + /// factory, and a factory is what an open generic registration cannot have. + /// + /// Registering each interface to the same open generic implementation type compiles, and is a + /// different contract: the container builds one instance per service type, which is the opposite + /// of what the attribute promises. Silently substituting that would be worse than refusing. + /// + /// Until this existed the generated code did not compile at all — the type parameter leaked into + /// the registration as typeof(ILedger<T>) (CS0246, no T in scope) beside + /// GetRequiredService<Ledger<>>() (CS7003). + /// + public static readonly DiagnosticDescriptor CrossWireCannotBeGeneric = new( + id: "DM0014", + title: "Generic type cannot be cross-wired", + messageFormat: + "'{0}' is generic, so [CrossWireService] cannot register it. Cross-wiring shares one instance " + + "across every service type, which needs a factory, and the container does not allow one for an " + + "open generic registration. Use [SingletonService], [ScopedService] or [TransientService] to " + + "register it, applying one per interface if it needs to answer to more than one.", + category: Category, + defaultSeverity: DiagnosticSeverity.Warning, + isEnabledByDefault: true); + } diff --git a/src/DependencyModules.SourceGenerator.Impl/DependencyModules.SourceGenerator.Impl.csproj b/src/DependencyModules.SourceGenerator.Impl/DependencyModules.SourceGenerator.Impl.csproj index b59cd20..34e7c57 100644 --- a/src/DependencyModules.SourceGenerator.Impl/DependencyModules.SourceGenerator.Impl.csproj +++ b/src/DependencyModules.SourceGenerator.Impl/DependencyModules.SourceGenerator.Impl.csproj @@ -65,7 +65,7 @@ all runtime; build; native; contentfiles; analyzers; buildtransitive - + all build diff --git a/src/DependencyModules.SourceGenerator.Impl/InterceptorFileWriter.cs b/src/DependencyModules.SourceGenerator.Impl/InterceptorFileWriter.cs index a546701..e0de6f5 100644 --- a/src/DependencyModules.SourceGenerator.Impl/InterceptorFileWriter.cs +++ b/src/DependencyModules.SourceGenerator.Impl/InterceptorFileWriter.cs @@ -33,6 +33,19 @@ public string Write(InterceptorModel model, string wrapperName, string namespace var wrapper = csharpFile.AddClass(wrapperName); wrapper.Modifiers |= ComponentModifier.Internal; + + // A generic service is wrapped by a generic type closed over the same parameters, which is + // what lets the container register it as an open generic implementation. Only constraint-free + // parameters reach here; a constrained one is refused upstream, because the wrapper would have + // to repeat the constraint and there is no way to emit one. + if (model.IsOpenGeneric) { + foreach (var typeParameter in model.TypeParameters!) { + wrapper.AddGenericParameter(typeParameter.Name); + + WriteConstraint(wrapper.AddConstraint(typeParameter.Name), typeParameter); + } + } + wrapper.AddBaseType(model.ServiceType); wrapper.AddAttribute(TypeDefinition.Get("System.Diagnostics.CodeAnalysis", "ExcludeFromCodeCoverage")); @@ -62,8 +75,100 @@ public string Write(InterceptorModel model, string wrapperName, string namespace return output.Output(); } + /// + /// What the wrapper holds and is handed as the instance it wraps. + /// + /// + /// The service interface for an ordinary wrapper, which the container hands over because + /// decoration captured the original registration first. + /// + /// For an open generic there is no factory to capture anything, so the wrapper is the + /// registration for the service — and asking for the service would resolve the wrapper itself and + /// recurse. It takes the implementation by its own concrete type instead, which + /// DecoratorHelper.InterceptOpenGeneric registers alongside it. + /// + private static ITypeDefinition InnerType(InterceptorModel model) => + model.IsOpenGeneric ? Closed(model.ImplementationType, model.TypeParameters!) : model.ServiceType; + + /// + /// A type closed over the wrapper's own type parameters — Repository becomes + /// Repository<T>. + /// + private static ITypeDefinition Closed( + ITypeDefinition type, IReadOnlyList typeParameters) { + + var arguments = new ITypeDefinition[typeParameters.Count]; + + for (var i = 0; i < arguments.Length; i++) { + arguments[i] = TypeDefinition.Get("", typeParameters[i].Name); + } + + return new GenericTypeDefinition( + TypeDefinitionEnum.ClassDefinition, type.Namespace, type.Name, arguments); + } + + /// + /// How a nested state class names the wrapper that owns it. + /// + /// + /// A nested type inherits its outer type's parameters but still has to write them: inside + /// Repository_Intercepted<T> the name is Repository_Intercepted<T>, and + /// the bare name is CS0305. + /// + private static ITypeDefinition SelfType(InterceptorModel model, string wrapperName) => + model.IsOpenGeneric + ? Closed(TypeDefinition.Get("", wrapperName), model.TypeParameters!) + : TypeDefinition.Get("", wrapperName); + + /// + /// The constraints a member declares, which both the forwarding member and its state class have + /// to repeat or the call they forward will not satisfy them. + /// + private static void WriteConstraints( + InterceptedMemberModel member, Func addConstraint) { + + foreach (var typeParameter in member.TypeParameters) { + WriteConstraint(addConstraint(typeParameter.Name), typeParameter); + } + } + + /// + /// Repeats one type parameter's constraints. + /// + /// + /// The parts go in as the symbol reported them and come out in the order C# requires, which is + /// ConstraintDefinition's job rather than this writer's. + /// + private static void WriteConstraint(ConstraintDefinition constraint, TypeParameterModel typeParameter) { + switch (typeParameter.Primary) { + case "class": + constraint.Class(); + break; + case "class?": + constraint.Class(nullable: true); + break; + case "struct": + constraint.Struct(); + break; + case "unmanaged": + constraint.Unmanaged(); + break; + case "notnull": + constraint.NotNull(); + break; + } + + foreach (var constraintType in typeParameter.ConstraintTypes) { + constraint.Implements(constraintType); + } + + if (typeParameter.DefaultConstructor) { + constraint.DefaultConstructor(); + } + } + private static void WriteFields(ClassDefinition wrapper, InterceptorModel model) { - var inner = wrapper.AddField(model.ServiceType, InnerField); + var inner = wrapper.AddField(InnerType(model), InnerField); inner.Modifiers |= ComponentModifier.Private | ComponentModifier.Readonly; for (var index = 0; index < model.Interceptors.Count; index++) { @@ -93,7 +198,7 @@ private static void WriteFields(ClassDefinition wrapper, InterceptorModel model) private static void WriteConstructor(ClassDefinition wrapper, InterceptorModel model) { var constructor = wrapper.AddConstructor(); - constructor.AddParameter(model.ServiceType, "inner"); + constructor.AddParameter(InnerType(model), "inner"); constructor.AddIndentedStatement($"{InnerField} = inner"); for (var index = 0; index < model.Interceptors.Count; index++) { @@ -246,7 +351,7 @@ private static void WriteForwardingMethod( method.AddGenericParameter(new TypeParameterDefinition(typeParameter.Name)); } - method.WhereStatement = Constraints(member); + WriteConstraints(member, method.AddConstraint); var arguments = new List { "this" }; @@ -331,10 +436,12 @@ private static void WriteState( state.AddGenericParameter(typeParameter.Name); } - state.WhereStatement = Constraints(member); + WriteConstraints(member, state.AddConstraint); + + var selfType = SelfType(model, wrapperName); - WriteStateFields(state, member, wrapperName); - WriteStateConstructor(state, member, index, wrapperName); + WriteStateFields(state, member, selfType); + WriteStateConstructor(state, member, index, selfType); WriteCallerAndCount(state, member, index); WriteArgumentsIndexer(state, member); WriteNameAt(state, member); @@ -342,9 +449,9 @@ private static void WriteState( } private static void WriteStateFields( - ClassDefinition state, InterceptedMemberModel member, string wrapperName) { + ClassDefinition state, InterceptedMemberModel member, ITypeDefinition selfType) { - var self = state.AddField(TypeDefinition.Get("", wrapperName), "_self"); + var self = state.AddField(selfType, "_self"); self.Modifiers |= ComponentModifier.Private | ComponentModifier.Readonly; for (var index = 0; index < member.Parameters.Count; index++) { @@ -358,11 +465,11 @@ private static void WriteStateFields( /// non-nullable reference is definitely assigned and the wrapper needs no nullable suppression. /// private static void WriteStateConstructor( - ClassDefinition state, InterceptedMemberModel member, int index, string wrapperName) { + ClassDefinition state, InterceptedMemberModel member, int index, ITypeDefinition selfType) { var constructor = state.AddConstructor(); - constructor.AddParameter(TypeDefinition.Get("", wrapperName), "self"); + constructor.AddParameter(selfType, "self"); constructor.AddIndentedStatement("_self = self"); for (var argument = 0; argument < member.Parameters.Count; argument++) { @@ -614,20 +721,6 @@ private static string ClosedStateName(InterceptedMemberModel member, int index) ">"; } - /// - /// The constraints the member declares, which both the forwarding member and the state class - /// have to repeat or the call they forward will not satisfy them. - /// - private static IOutputComponent? Constraints(InterceptedMemberModel member) { - var clauses = member.TypeParameters - .Where(parameter => parameter.Constraints.Length > 0) - .Select(parameter => $"where {parameter.Name} : {parameter.Constraints}") - .ToList(); - - return clauses.Count == 0 - ? null - : new CodeOutputComponent(" " + string.Join(" ", clauses)) { Indented = false }; - } private static string InterceptorField(int index) => $"_dmInterceptor{index}"; diff --git a/src/DependencyModules.SourceGenerator.Impl/InterceptorRegistrationWriter.cs b/src/DependencyModules.SourceGenerator.Impl/InterceptorRegistrationWriter.cs index 9f1b4b2..0e86219 100644 --- a/src/DependencyModules.SourceGenerator.Impl/InterceptorRegistrationWriter.cs +++ b/src/DependencyModules.SourceGenerator.Impl/InterceptorRegistrationWriter.cs @@ -34,6 +34,26 @@ public string Write( return EntryModelUtil.ApplyRecordDeclaration(outputContext.Output(), entryPointModel); } + /// + /// A type written as its unbound generic form — IVault<> rather than + /// IVault<T>. + /// + /// + /// The only form a typeof can carry outside the type's own declaration. Writing the + /// parameter names instead is CS0246, because no T is in scope at the registration. + /// Blank-named arguments are how this codebase represents unbound throughout. + /// + private static ITypeDefinition Unbound(ITypeDefinition type, int arity) { + var arguments = new ITypeDefinition[arity]; + + for (var i = 0; i < arity; i++) { + arguments[i] = TypeDefinition.Get("", ""); + } + + return new GenericTypeDefinition( + TypeDefinitionEnum.ClassDefinition, type.Namespace, type.Name, arguments); + } + private static void WriteInterceptor( ModuleEntryPointModel entryPointModel, ClassDefinition classDefinition, @@ -80,30 +100,52 @@ private static void WriteInterceptor( var wrapperName = $"{model.ImplementationType.Name.Replace(".", "_")}_Intercepted"; var wrapperType = TypeDefinition.Get(model.ImplementationType.Namespace, wrapperName); - // The wrapper is generated right here, so its constructor is known exactly: the intercepted - // instance, then one parameter per interceptor. Emitting the `new` rather than handing the - // type to ActivatorUtilities is what keeps interception working in a published Native AOT - // application — the same reason decorators are emitted closed. - var arguments = new List { CodeOutputComponent.Get("inner") }; + method.NewLine(); - for (var i = 0; i < model.Interceptors.Count; i++) { - arguments.Add( - new InvokeGenericDefinition( - "provider", "GetRequiredService", new[] { model.Interceptors[i].Type })); - } + if (model.IsOpenGeneric) { + // An open generic service cannot be decorated: decoration rewrites the registration into + // a factory, and the container refuses a factory for one. It does accept an open generic + // implementation type, and the wrapper is one — so the registration is swapped for the + // wrapper and the implementation is registered under its own type for the wrapper to take. + // + // Nothing is closed here. The container closes the wrapper per requested construction, and + // every type it names exists in the assembly, so this survives publishing as the closed + // path does. + method.AddIndentedStatement( + new StaticInvokeStatement( + KnownTypes.DependencyModules.Helpers.DecoratorHelper, + "InterceptOpenGeneric", + new List { + CodeOutputComponent.Get(services.Name), + TypeOf(Unbound(model.ServiceType, model.TypeParameters!.Count)), + TypeOf(Unbound(model.ImplementationType, model.TypeParameters!.Count)), + TypeOf(Unbound(wrapperType, model.TypeParameters!.Count)) + })); + } else { + // The wrapper is generated right here, so its constructor is known exactly: the + // intercepted instance, then one parameter per interceptor. Emitting the `new` rather than + // handing the type to ActivatorUtilities is what keeps interception working in a published + // Native AOT application — the same reason decorators are emitted closed. + var arguments = new List { CodeOutputComponent.Get("inner") }; + + for (var i = 0; i < model.Interceptors.Count; i++) { + arguments.Add( + new InvokeGenericDefinition( + "provider", "GetRequiredService", new[] { model.Interceptors[i].Type })); + } - method.NewLine(); - method.AddIndentedStatement( - SyntaxHelpers.InvokeGeneric( - KnownTypes.DependencyModules.Helpers.DecoratorHelper, - "Decorate", - new[] { model.ServiceType }, - CodeOutputComponent.Get(services.Name), - TypeOf(wrapperType), - new WrapStatement( - CodeOutputComponent.Get(" => "), - CodeOutputComponent.Get("(provider, inner)"), - New(wrapperType, arguments.ToArray())))); + method.AddIndentedStatement( + SyntaxHelpers.InvokeGeneric( + KnownTypes.DependencyModules.Helpers.DecoratorHelper, + "Decorate", + new[] { model.ServiceType }, + CodeOutputComponent.Get(services.Name), + TypeOf(wrapperType), + new WrapStatement( + CodeOutputComponent.Get(" => "), + CodeOutputComponent.Get("(provider, inner)"), + New(wrapperType, arguments.ToArray())))); + } // A field initializer registers the method, matching how decorator registrations are hooked // up. DynamicDependency keeps the trimmer from removing a method only referenced this way. diff --git a/src/DependencyModules.SourceGenerator.Impl/Models/InterceptorModel.cs b/src/DependencyModules.SourceGenerator.Impl/Models/InterceptorModel.cs index 04805df..08e8669 100644 --- a/src/DependencyModules.SourceGenerator.Impl/Models/InterceptorModel.cs +++ b/src/DependencyModules.SourceGenerator.Impl/Models/InterceptorModel.cs @@ -92,7 +92,54 @@ public record InterceptedParameterModel( /// /// The constraints without the where T : prefix, such as class, new(), or empty. /// -public record InterceptedTypeParameterModel(string Name, string Constraints); +/// +/// A type parameter of the intercepted class, with its constraints held as parts rather than +/// rendered. +/// +/// +/// The wrapper is declared over the same parameters and has to repeat their constraints, or it cannot +/// reference the implementation it wraps. Parts rather than a string because the writer decides how a +/// type name is written and the reader does not know: rendering here would bake one output mode into +/// the model, and CSharpAuthor's AddConstraint takes the pieces and puts them in the +/// order C# requires. +/// +/// The parameter name, repeated verbatim on the wrapper. +/// +/// The primary constraint keyword — class, struct, unmanaged, notnull — +/// or null when there is none. At most one is legal. +/// +/// Base class and interface constraints, in declaration order. +/// Whether new() was declared. +public record TypeParameterModel( + string Name, + string? Primary, + IReadOnlyList ConstraintTypes, + bool DefaultConstructor) { + + /// + /// Structural equality over the constraint types, which the compiler-generated version compares + /// by reference — two identical models built on consecutive runs would never match, and the + /// incremental cache would miss on every keystroke. + /// + public virtual bool Equals(TypeParameterModel? other) => + other is not null && + Name == other.Name && + Primary == other.Primary && + DefaultConstructor == other.DefaultConstructor && + ModelEquality.ListEquals(ConstraintTypes, other.ConstraintTypes); + + public override int GetHashCode() { + unchecked { + var hash = Name.GetHashCode(); + + hash = hash * 31 + (Primary?.GetHashCode() ?? 0); + hash = hash * 31 + DefaultConstructor.GetHashCode(); + hash = hash * 31 + ModelEquality.ListHashCode(ConstraintTypes); + + return hash; + } + } +} /// /// An interceptor named by the attribute, with the interfaces it implements. @@ -147,7 +194,7 @@ public record InterceptedMemberModel( ITypeDefinition? ReturnType, ITypeDefinition ResultType, IReadOnlyList Parameters, - IReadOnlyList TypeParameters, + IReadOnlyList TypeParameters, ReturnShape ReturnShape) { /// @@ -282,6 +329,13 @@ public override int GetHashCode() { /// after the position, so the order is part of the model. /// /// What the wrapper declares, pointing back into the members. +/// +/// The implementation's type parameters, empty for a non-generic service. The wrapper repeats them +/// and their constraints, so Repository<T> where T : class becomes +/// Repository_Intercepted<T> : IRepository<T> where T : class holding a +/// Repository<T>. A constraint that were dropped would leave the wrapper unable to +/// reference what it wraps. +/// public record InterceptorModel( ITypeDefinition ServiceType, ITypeDefinition ImplementationType, @@ -289,7 +343,14 @@ public record InterceptorModel( IReadOnlyList Members, IReadOnlyList Declarations, int Order, - InterceptionRefusal? Refusal = null) { + InterceptionRefusal? Refusal = null, + IReadOnlyList? TypeParameters = null) { + + /// + /// Whether the intercepted service is an open generic, and so registers as an implementation type + /// rather than through a factory. + /// + public bool IsOpenGeneric => TypeParameters is { Count: > 0 }; /// /// Sentinel for a node carrying the attribute that produced no usable model and nothing to say @@ -333,7 +394,8 @@ public bool Equals(InterceptorModel? x, InterceptorModel? y) { Equals(x.Refusal, y.Refusal) && ModelEquality.ListEquals(x.Interceptors, y.Interceptors) && ModelEquality.ListEquals(x.Members, y.Members) && - ModelEquality.ListEquals(x.Declarations, y.Declarations); + ModelEquality.ListEquals(x.Declarations, y.Declarations) && + ModelEquality.ListEquals(x.TypeParameters, y.TypeParameters); } public int GetHashCode(InterceptorModel obj) { @@ -346,6 +408,7 @@ public int GetHashCode(InterceptorModel obj) { hash = hash * 31 + ModelEquality.ListHashCode(obj.Interceptors); hash = hash * 31 + ModelEquality.ListHashCode(obj.Members); hash = hash * 31 + ModelEquality.ListHashCode(obj.Declarations); + hash = hash * 31 + ModelEquality.ListHashCode(obj.TypeParameters); return hash; } diff --git a/src/DependencyModules.SourceGenerator.Impl/Utilities/AttributeTypeMatcher.cs b/src/DependencyModules.SourceGenerator.Impl/Utilities/AttributeTypeMatcher.cs new file mode 100644 index 0000000..14e50d9 --- /dev/null +++ b/src/DependencyModules.SourceGenerator.Impl/Utilities/AttributeTypeMatcher.cs @@ -0,0 +1,97 @@ +using CSharpAuthor; +using Microsoft.CodeAnalysis; +using Microsoft.CodeAnalysis.CSharp.Syntax; + +namespace DependencyModules.SourceGenerator.Impl.Utilities; + +/// +/// Whether an attribute usage is a given attribute type, resolved rather than string-matched. +/// +/// +/// +/// Attribute usages were compared as written — attributeSyntax.Name.ToString() against the +/// type's simple name, and against that name with Attribute appended. Every other legal +/// spelling missed, and missing meant the registration was silently absent: no diagnostic, a green +/// build, and a failure at the first resolve. +/// +/// +/// [SingletonService] and [SingletonServiceAttribute] matched. +/// [DependencyModules.Runtime.Attributes.SingletonService], +/// [global::DependencyModules.Runtime.Attributes.SingletonServiceAttribute] and any +/// using alias did not. +/// +/// +/// This belongs in a transform, never in a predicate: it reads the semantic model, which is what +/// resolves an alias and a qualified name to the same symbol, and what a predicate visiting every +/// node in the compilation must not do. +/// +/// +public static class AttributeTypeMatcher { + + /// + /// Whether resolves to . + /// + /// + /// Falls back to comparing the written name when the symbol cannot be resolved, which happens + /// while a file is mid-edit and the attribute does not yet bind. Answering "no" there would make + /// registrations flicker out of the container between keystrokes. + /// + public static bool Matches( + SemanticModel semanticModel, + AttributeSyntax attributeSyntax, + ITypeDefinition attributeType, + CancellationToken cancellationToken) { + + var symbol = Resolve(semanticModel, attributeSyntax, cancellationToken); + + if (symbol == null) { + return MatchesAsWritten(attributeSyntax, attributeType); + } + + return symbol.Name == attributeType.Name && NamespaceOf(symbol) == attributeType.Namespace; + } + + /// + /// The attribute class an attribute usage names. + /// + /// + /// An attribute usage binds to a constructor, so the type is that constructor's containing type. + /// GetTypeInfo answers for the cases where the constructor could not be chosen — an + /// argument list that does not match any overload still names the attribute unambiguously. + /// + private static INamedTypeSymbol? Resolve( + SemanticModel semanticModel, AttributeSyntax attributeSyntax, CancellationToken cancellationToken) { + + var symbolInfo = semanticModel.GetSymbolInfo(attributeSyntax, cancellationToken); + + if (symbolInfo.Symbol?.ContainingType is { } containingType) { + return containingType; + } + + if (symbolInfo.CandidateSymbols.Length > 0 && + symbolInfo.CandidateSymbols[0].ContainingType is { } candidateType) { + return candidateType; + } + + return semanticModel.GetTypeInfo(attributeSyntax, cancellationToken).Type as INamedTypeSymbol; + } + + /// + /// The old comparison, kept only for the unresolvable case. + /// + private static bool MatchesAsWritten(AttributeSyntax attributeSyntax, ITypeDefinition attributeType) { + var written = attributeSyntax.Name.ToString(); + var lastDot = written.LastIndexOf('.'); + + if (lastDot >= 0) { + written = written.Substring(lastDot + 1); + } + + return written == attributeType.Name || written + "Attribute" == attributeType.Name; + } + + private static string NamespaceOf(INamedTypeSymbol symbol) => + symbol.ContainingNamespace is { IsGlobalNamespace: false } containing + ? containing.ToDisplayString() + : ""; +} diff --git a/src/DependencyModules.SourceGenerator.Impl/Utilities/DecoratorExpansion.cs b/src/DependencyModules.SourceGenerator.Impl/Utilities/DecoratorExpansion.cs index dca92a6..c9f1a75 100644 --- a/src/DependencyModules.SourceGenerator.Impl/Utilities/DecoratorExpansion.cs +++ b/src/DependencyModules.SourceGenerator.Impl/Utilities/DecoratorExpansion.cs @@ -22,13 +22,20 @@ namespace DependencyModules.SourceGenerator.Impl.Utilities; /// public static class DecoratorExpansion { + /// + /// Decorators that name a service the compilation registers as an open generic. Nothing can be + /// emitted for those — see — and the caller reports + /// DM0013 rather than letting them disappear. + /// public static IReadOnlyList Expand( IReadOnlyList decorators, IReadOnlyList registeredServiceTypes, + out IReadOnlyList refusedForOpenGenericRegistration, bool includeNonGeneric = true, Func? canClose = null) { var expanded = new List(decorators.Count); + List? refused = null; foreach (var decorator in decorators) { if (decorator.IsIgnored) { @@ -36,6 +43,17 @@ public static IReadOnlyList Expand( } if (!decorator.IsOpenGeneric) { + // An unbound service type has no legal emission at all: Decorate> is + // CS7003. A generic decorator reaches this state only when nothing closed it, and is + // handled below; a non-generic one never had an expansion step to catch it, so this + // is where it stops. Refused whatever is registered, because the emission is invalid + // on its own terms. + if (decorator.HasUnboundServiceType) { + (refused ??= new List()).Add(decorator); + + continue; + } + // A non-generic decorator names one service type and needs no expansion, so only // the pass that owns the declaration emits it. Anything that cannot be constructed // by generated code is dropped: generated code builds the decorator with a literal @@ -48,6 +66,8 @@ public static IReadOnlyList Expand( continue; } + var closedCount = 0; + foreach (var serviceType in registeredServiceTypes) { if (!ClosesTheSameGeneric(serviceType, decorator.ServiceType)) { continue; @@ -68,12 +88,52 @@ public static IReadOnlyList Expand( } expanded.Add(closed); + closedCount++; + } + + // Nothing closed it. Distinguishing the two reasons is the whole point: a compilation + // that registers the service as an open generic can never be decorated and is worth + // reporting, while a compilation that registers nothing at all is the ordinary + // cross-assembly case — [Decorate] exists to name a service someone else registers, so + // reporting that would fire on the feature's primary use. + if (closedCount == 0 && NamesAnOpenGenericRegistration(decorator, registeredServiceTypes)) { + (refused ??= new List()).Add(decorator); } } + refusedForOpenGenericRegistration = (IReadOnlyList?)refused ?? Array.Empty(); + return expanded; } + /// + /// Whether this compilation registers the decorated service as an open generic. + /// + /// + /// Matched on name, namespace and arity, with every type argument blank — the form + /// services.AddSingleton(typeof(IStore<>), typeof(Store<>)) produces. + /// + private static bool NamesAnOpenGenericRegistration( + DecoratorModel decorator, IReadOnlyList registeredServiceTypes) { + + if (decorator.ServiceType is not GenericTypeDefinition decorated) { + return false; + } + + foreach (var registered in registeredServiceTypes) { + if (registered is GenericTypeDefinition open && + open.TypeArguments.Count == decorated.TypeArguments.Count && + open.Name == decorated.Name && + open.Namespace == decorated.Namespace && + open.TypeArguments.All(argument => string.IsNullOrEmpty(argument.Name))) { + + return true; + } + } + + return false; + } + /// /// Whether a registered service type is a closed construction of the decorated open generic. /// diff --git a/src/DependencyModules.SourceGenerator.Impl/Utilities/InterceptedMemberReader.cs b/src/DependencyModules.SourceGenerator.Impl/Utilities/InterceptedMemberReader.cs index 37238a3..7c96ef3 100644 --- a/src/DependencyModules.SourceGenerator.Impl/Utilities/InterceptedMemberReader.cs +++ b/src/DependencyModules.SourceGenerator.Impl/Utilities/InterceptedMemberReader.cs @@ -158,7 +158,7 @@ private static bool ReadProperty( type, type, indices, - Array.Empty(), + Array.Empty(), ReturnShape.Value)); } @@ -178,7 +178,7 @@ private static bool ReadProperty( null, KnownTypes.DependencyModules.Interception.NoResult, arguments, - Array.Empty(), + Array.Empty(), ReturnShape.Void)); } @@ -234,7 +234,7 @@ private static InterceptedMemberModel EventAccessor( null, KnownTypes.DependencyModules.Interception.NoResult, new InterceptedParameterModel[] { new("value", "value", handlerType, null) }, - Array.Empty(), + Array.Empty(), ReturnShape.Void); /// @@ -337,53 +337,20 @@ private static IEnumerable EnumerateMembers(INamedTypeSymbol serviceTyp /// The member's type parameters and their constraints. The state class repeats both, or the call /// it forwards will not satisfy the constraints the interface declared. /// - private static IReadOnlyList ReadTypeParameters(IMethodSymbol method) { + private static IReadOnlyList ReadTypeParameters(IMethodSymbol method) { if (method.TypeParameters.Length == 0) { - return Array.Empty(); + return Array.Empty(); } - var typeParameters = new List(); + var typeParameters = new List(); foreach (var parameter in method.TypeParameters) { - typeParameters.Add(new InterceptedTypeParameterModel(parameter.Name, RenderConstraints(parameter))); + typeParameters.Add(TypeParameterReader.Read(parameter)); } return typeParameters; } - private static string RenderConstraints(ITypeParameterSymbol parameter) { - var constraints = new List(); - - // Unmanaged implies a value type constraint, so it has to be tested first or the narrower - // constraint would be rendered as the wider one. - if (parameter.HasUnmanagedTypeConstraint) { - constraints.Add("unmanaged"); - } else if (parameter.HasValueTypeConstraint) { - constraints.Add("struct"); - } else if (parameter.HasReferenceTypeConstraint) { - constraints.Add( - parameter.ReferenceTypeConstraintNullableAnnotation == NullableAnnotation.Annotated - ? "class?" - : "class"); - } else if (parameter.HasNotNullConstraint) { - constraints.Add("notnull"); - } - - foreach (var constraintType in parameter.ConstraintTypes) { - var builder = new StringBuilder(); - - constraintType.GetTypeDefinition().WriteTypeName(builder, TypeOutputMode.Global); - - constraints.Add(builder.ToString()); - } - - if (parameter.HasConstructorConstraint) { - constraints.Add("new()"); - } - - return string.Join(", ", constraints); - } - private static string EscapeIdentifier(string name) => Microsoft.CodeAnalysis.CSharp.SyntaxFacts.GetKeywordKind(name) == Microsoft.CodeAnalysis.CSharp.SyntaxKind.None ? name diff --git a/src/DependencyModules.SourceGenerator.Impl/Utilities/InterceptorModelUtility.cs b/src/DependencyModules.SourceGenerator.Impl/Utilities/InterceptorModelUtility.cs index f439694..65b3f23 100644 --- a/src/DependencyModules.SourceGenerator.Impl/Utilities/InterceptorModelUtility.cs +++ b/src/DependencyModules.SourceGenerator.Impl/Utilities/InterceptorModelUtility.cs @@ -42,16 +42,15 @@ public static InterceptorModel GetInterceptorModel( return InterceptorModel.Ignore; } - // A generic implementation registers as an open generic, and decorating one of those is not - // supported: DecoratorHelper rewrites the registration into a factory, which the container - // rejects for an open generic service type. Refusing here turns what would be an - // ArgumentException when the provider is built into a message naming the declaration. - if (implementationSymbol.IsGenericType) { - return InterceptorModel.Refused( - $"'{implementationSymbol.Name}' is generic, so it registers as an open generic, and " + - "decorating an open generic registration is not supported. Register a closed " + - $"construction instead, such as a class deriving from '{implementationSymbol.Name}<...>'"); - } + // A generic implementation registers as an open generic. Decoration cannot touch one — it + // rewrites the registration into a factory, and the container refuses a factory for an open + // generic service type — but interception does not need one: the wrapper is a generated type, + // and an open generic implementation type is exactly what the container does accept. It is + // registered as the service, and takes the implementation by its own type so that resolving + // it does not come back round to the wrapper. + // + // Constraints come along with the parameters. The wrapper is declared over the same ones and + // repeats their constraints, without which it could not reference what it wraps. var interceptorSymbols = new List(); var order = 0; @@ -90,18 +89,19 @@ public static InterceptorModel GetInterceptorModel( } // Nothing here can be placed around anything, so the wrapper would forward every call - // untouched. Not generating one leaves the service registered as it already was. - if (!AnyMemberIsIntercepted(interceptors, members)) { - return InterceptorModel.Ignore; - } - + // untouched, and none is generated. The model is still returned rather than ignored: this is + // the sharpest form of an interceptor that does not run — an interceptor implementing only + // IInterceptor applied to a service whose members are all async never runs at all — and + // returning Ignore here is what kept DM0015 from ever seeing it. The generator drops it after + // reporting. return new InterceptorModel( serviceSymbol.GetTypeDefinition(), ToTypeDefinition(implementationSymbol), interceptors, members, declarations, - order); + order, + TypeParameters: TypeParameterModels(implementationSymbol)); } private static InterceptorModel Refuse(string? reason) => @@ -268,6 +268,24 @@ private static ImmutableArray DeclaredInterfaces(INamedTypeSym return ImmutableArray.Empty; } + /// + /// The implementation's type parameters and their constraints, which the wrapper repeats so its + /// own parameters line up with the ones the service and the implementation are closed over. + /// + private static IReadOnlyList TypeParameterModels(INamedTypeSymbol symbol) { + if (symbol.TypeParameters.Length == 0) { + return Array.Empty(); + } + + var models = new TypeParameterModel[symbol.TypeParameters.Length]; + + for (var i = 0; i < models.Length; i++) { + models[i] = TypeParameterReader.Read(symbol.TypeParameters[i]); + } + + return models; + } + private static ITypeDefinition ToTypeDefinition(INamedTypeSymbol symbol) { var namespaceName = symbol.ContainingNamespace.IsGlobalNamespace ? "" diff --git a/src/DependencyModules.SourceGenerator.Impl/Utilities/ServiceModelUtility.cs b/src/DependencyModules.SourceGenerator.Impl/Utilities/ServiceModelUtility.cs index e0badb7..019b700 100644 --- a/src/DependencyModules.SourceGenerator.Impl/Utilities/ServiceModelUtility.cs +++ b/src/DependencyModules.SourceGenerator.Impl/Utilities/ServiceModelUtility.cs @@ -323,14 +323,17 @@ private static List GetRegistrations(SyntaxTransformCo foreach (var typeDefinition in _attributeTypes) { cancellationToken.ThrowIfCancellationRequested(); - if (attributeSyntax.Name.ToString() == typeDefinition.Name || - attributeSyntax.Name + "Attribute" == typeDefinition.Name) { + // Resolved, not compared as written: a namespace-qualified usage, a global:: prefix + // and a using alias all name the same attribute, and all of them used to be silently + // skipped — leaving the class unregistered with nothing to say so. + if (AttributeTypeMatcher.Matches( + context.SemanticModel, attributeSyntax, typeDefinition, cancellationToken)) { list.Add(GetServiceRegistration(context, attributeSyntax, classDefinition)); } } - if (attributeSyntax.Name.ToString() == _crossWireService.Name || - attributeSyntax.Name + "Attribute" == _crossWireService.Name) { + if (AttributeTypeMatcher.Matches( + context.SemanticModel, attributeSyntax, _crossWireService, cancellationToken)) { list.AddRange(GetCrossWiredService(context, attributeSyntax, classDefinition)); } } diff --git a/src/DependencyModules.SourceGenerator.Impl/Utilities/SyntaxSelector.cs b/src/DependencyModules.SourceGenerator.Impl/Utilities/SyntaxSelector.cs index d370203..a01f5e9 100644 --- a/src/DependencyModules.SourceGenerator.Impl/Utilities/SyntaxSelector.cs +++ b/src/DependencyModules.SourceGenerator.Impl/Utilities/SyntaxSelector.cs @@ -16,12 +16,22 @@ protected BaseSyntaxSelector(params ITypeDefinition[] attributes) { _names = GetAttributeStrings(attributes); } + /// + /// The bare names an attribute may be written as, with and without the Attribute suffix. + /// + /// + /// Qualification is stripped from the usage rather than enumerated here — see + /// . Listing prefixes was the previous approach and it missed the + /// namespace-qualified form without the suffix, so + /// [DependencyModules.Runtime.Attributes.DependencyModule] — valid C# — was silently not a + /// module: no partial written, no diagnostic, and a CS0311 at the consumer's + /// AddModule<T>() naming neither the attribute nor the omission. + /// private List GetAttributeStrings(ITypeDefinition[] attributes) { var returnList = new List(); foreach (var attribute in attributes) { returnList.Add(attribute.Name); - returnList.Add(attribute.Namespace + "." + attribute.Name); if (attribute.Name.EndsWith(_attributeString)) { var simpleName = attribute.Name.Substring(0, attribute.Name.Length - _attributeString.Length); @@ -33,6 +43,31 @@ private List GetAttributeStrings(ITypeDefinition[] attributes) { return returnList; } + /// + /// An attribute usage reduced to the name it ends in, so every way of qualifying it compares + /// equal. + /// + /// + /// Covers Ns.Attr, global::Ns.Attr and alias::Ns.Attr. It does not cover a + /// using alias of the attribute type itself, which resolves only through the semantic + /// model and so cannot be seen from a predicate that must stay syntax-only. + /// + /// This does not widen what matches by namespace: the bare simple name was already accepted + /// regardless of which namespace it came from, so a same-named attribute from elsewhere was + /// always a candidate and is filtered downstream as it always was. + /// + private static string LastSegment(string attributeName) { + var lastDot = attributeName.LastIndexOf('.'); + + if (lastDot >= 0) { + return attributeName.Substring(lastDot + 1); + } + + var lastColon = attributeName.LastIndexOf(':'); + + return lastColon >= 0 ? attributeName.Substring(lastColon + 1) : attributeName; + } + protected abstract bool TestForTypes(SyntaxNode node, CancellationToken token); public bool Where(SyntaxNode node, CancellationToken token) { @@ -51,10 +86,7 @@ public bool Where(SyntaxNode node, CancellationToken token) { } var found = node.DescendantNodes() - .OfType().Any(a => { - var name = a.Name.ToString(); - return _names.Contains(name); - }); + .OfType().Any(a => _names.Contains(LastSegment(a.Name.ToString()))); return found; } @@ -72,9 +104,7 @@ private bool ProcessAttributeList(SyntaxList attributeLists var foundAttribute = false; foreach (var attributeListSyntax in attributeLists) { foreach (var attributeSyntax in attributeListSyntax.Attributes) { - var name = attributeSyntax.Name.ToString(); - - foundAttribute = _names.Contains(name); + foundAttribute = _names.Contains(LastSegment(attributeSyntax.Name.ToString())); if (foundAttribute) { break; diff --git a/src/DependencyModules.SourceGenerator.Impl/Utilities/TypeParameterReader.cs b/src/DependencyModules.SourceGenerator.Impl/Utilities/TypeParameterReader.cs new file mode 100644 index 0000000..8d9d0fa --- /dev/null +++ b/src/DependencyModules.SourceGenerator.Impl/Utilities/TypeParameterReader.cs @@ -0,0 +1,45 @@ +using CSharpAuthor; +using DependencyModules.SourceGenerator.Impl.Models; +using Microsoft.CodeAnalysis; + +namespace DependencyModules.SourceGenerator.Impl.Utilities; + +/// +/// Reads a type parameter's constraints into parts. +/// +/// +/// One reader for both places a wrapper repeats constraints — the class it is declared as, and each +/// generic method it forwards. The rules are subtle enough that two copies would drift: only one +/// primary constraint is legal, unmanaged has to be tested before struct because it +/// implies it, and Roslyn reports a constructor constraint for a struct-constrained parameter +/// even though repeating new() alongside it is CS0451. +/// +public static class TypeParameterReader { + + public static TypeParameterModel Read(ITypeParameterSymbol parameter) { + string? primary = null; + + if (parameter.HasUnmanagedTypeConstraint) { + primary = "unmanaged"; + } else if (parameter.HasValueTypeConstraint) { + primary = "struct"; + } else if (parameter.HasReferenceTypeConstraint) { + primary = parameter.ReferenceTypeConstraintNullableAnnotation == NullableAnnotation.Annotated + ? "class?" + : "class"; + } else if (parameter.HasNotNullConstraint) { + primary = "notnull"; + } + + var constraintTypes = new ITypeDefinition[parameter.ConstraintTypes.Length]; + + for (var i = 0; i < constraintTypes.Length; i++) { + constraintTypes[i] = parameter.ConstraintTypes[i].GetTypeDefinition(); + } + + var defaultConstructor = parameter.HasConstructorConstraint && + primary is not ("struct" or "unmanaged"); + + return new TypeParameterModel(parameter.Name, primary, constraintTypes, defaultConstructor); + } +} diff --git a/src/DependencyModules.SourceGenerator/Conventions/ConventionGenerator.cs b/src/DependencyModules.SourceGenerator/Conventions/ConventionGenerator.cs index 94fb9f8..89177dc 100644 --- a/src/DependencyModules.SourceGenerator/Conventions/ConventionGenerator.cs +++ b/src/DependencyModules.SourceGenerator/Conventions/ConventionGenerator.cs @@ -355,10 +355,13 @@ private static void WriteDecorators( var expanded = DecoratorExpansion.Expand( decorators, registeredServiceTypes, + out var refusedForOpenGenericRegistration, canClose: (decoratorType, closedService) => DecoratorConstraintChecker.CanClose( moduleDecorators.Compilation, decoratorType, closedService)); + ReportOpenGenericDecoration(context, refusedForOpenGenericRegistration, logger); + if (expanded.Count == 0) { return; } @@ -428,6 +431,37 @@ private static IReadOnlyList CollectDecorators( return decorators; } + /// + /// Reports the decorators that were dropped because their service is registered as an open + /// generic. + /// + /// + /// The expansion cannot produce anything for these, and until this existed the two shapes failed + /// differently and both badly: a generic decorator vanished with a green build, and a non-generic + /// one reached emission carrying an unbound service type and produced CS7003 in generated code. + /// + private static void ReportOpenGenericDecoration( + SourceProductionContext context, + IReadOnlyList refused, + FileLogger logger) { + + foreach (var decorator in refused) { + var serviceName = decorator.ServiceType.Name; + var decoratorName = decorator.DecoratorType.Name; + + logger.Error( + $"'{decoratorName}' cannot decorate '{serviceName}' because it is registered as an " + + "open generic."); + + context.ReportDiagnostic( + Diagnostic.Create( + DependencyModuleDiagnostics.OpenGenericCannotBeDecorated, + Location.None, + serviceName, + decoratorName)); + } + } + /// /// Two decorators of one service sharing an order nest in an order nobody declared, so it is /// reported rather than resolved arbitrarily. diff --git a/src/DependencyModules.SourceGenerator/DependencyModules.SourceGenerator.csproj b/src/DependencyModules.SourceGenerator/DependencyModules.SourceGenerator.csproj index 12649c0..3805258 100644 --- a/src/DependencyModules.SourceGenerator/DependencyModules.SourceGenerator.csproj +++ b/src/DependencyModules.SourceGenerator/DependencyModules.SourceGenerator.csproj @@ -39,7 +39,7 @@ all runtime; build; native; contentfiles; analyzers; buildtransitive - + all build @@ -50,9 +50,11 @@ + Impl\%(RecursiveDir)/%(FileName)%(Extension) - PreserveNewest diff --git a/src/DependencyModules.SourceGenerator/InterceptorSourceGenerator.cs b/src/DependencyModules.SourceGenerator/InterceptorSourceGenerator.cs index 5eefc73..d894a98 100644 --- a/src/DependencyModules.SourceGenerator/InterceptorSourceGenerator.cs +++ b/src/DependencyModules.SourceGenerator/InterceptorSourceGenerator.cs @@ -113,9 +113,86 @@ private static IReadOnlyList ReportUnsupported( continue; } + ReportUnservedMembers(context, model, logger); + + // No member has an interceptor that can serve it, so a wrapper would forward every call + // untouched. Dropped after reporting rather than before, which is what makes the case + // visible at all. + if (!model.Members.Any(member => + model.Interceptors.Any(interceptor => interceptor.CanServe(member.Kind)))) { + + continue; + } + usable.Add(model); } return usable; } + + /// + /// Reports interceptors that are quietly absent from some of the members they were applied to. + /// + /// + /// The generator picks per member from the three interceptor interfaces, and an interceptor + /// implementing none of the one a member needs was simply left out of that member's chain. That + /// is an interceptor that does not run, which is a correctness question rather than a style one: + /// an argument-rewriting interceptor stops rewriting, and an authorisation gate stops gating. + /// + /// One diagnostic per interceptor and member shape, so a wide interface produces one line rather + /// than one per member. + /// + private static void ReportUnservedMembers( + SourceProductionContext context, InterceptorModel model, FileLogger logger) { + + foreach (var interceptor in model.Interceptors) { + foreach (var kind in new[] { + InterceptorKind.Sync, InterceptorKind.Async, InterceptorKind.Stream + }) { + + if (interceptor.CanServe(kind)) { + continue; + } + + var unserved = model.Members + .Where(member => member.Kind == kind) + .Select(member => member.Name) + .Distinct() + .OrderBy(name => name, StringComparer.Ordinal) + .ToArray(); + + if (unserved.Length == 0) { + continue; + } + + logger.Error( + $"'{interceptor.Type.Name}' does not implement {InterfaceFor(kind)}, so it is not " + + $"applied to {string.Join(", ", unserved)} on '{model.ServiceType.Name}'."); + + context.ReportDiagnostic( + Diagnostic.Create( + DependencyModuleDiagnostics.InterceptorCannotServeMembers, + Location.None, + interceptor.Type.Name, + InterfaceFor(kind), + DescriptionFor(kind), + model.ServiceType.Name, + string.Join(", ", unserved))); + } + } + } + + private static string InterfaceFor(InterceptorKind kind) => + kind switch { + InterceptorKind.Async => "IAsyncInterceptor", + InterceptorKind.Stream => "IAsyncEnumerableInterceptor", + _ => "IInterceptor" + }; + + private static string DescriptionFor(InterceptorKind kind) => + kind switch { + InterceptorKind.Async => "the members returning a task", + InterceptorKind.Stream => "the members returning an async stream", + _ => "the members returning a value directly" + }; } diff --git a/src/DependencyModules.SourceGenerator/ServiceSourceGenerator.cs b/src/DependencyModules.SourceGenerator/ServiceSourceGenerator.cs index 8d39c5e..b356e56 100644 --- a/src/DependencyModules.SourceGenerator/ServiceSourceGenerator.cs +++ b/src/DependencyModules.SourceGenerator/ServiceSourceGenerator.cs @@ -40,6 +40,8 @@ protected override void GenerateSourceOutput(SourceProductionContext context, var serviceModels = ReportUnconstructableServices(context, inputData.Right, logger); + serviceModels = ReportCrossWiredGenerics(context, serviceModels, logger); + if (serviceModels.Length == 0) { return; } @@ -160,6 +162,58 @@ private static ImmutableArray ReportUnconstructableServices( return builder.ToImmutable(); } + /// + /// Reports cross-wired generic types and removes them from generation. + /// + /// + /// Cross-wiring shares one instance across every service type, which is emitted as a factory per + /// interface. An open generic registration cannot carry a factory, so the emission was invalid on + /// its face: the type parameter leaked into the registration as typeof(ILedger<T>) + /// and the factory read GetRequiredService<Ledger<>>(), giving CS0246 and + /// CS7003 in generated code. + /// + /// The whole model is dropped rather than the cross-wired registrations alone. Keeping the + /// implementation's own registration would honour half an attribute — the instance would no + /// longer be reachable through any of its interfaces, which is the entire reason the attribute + /// was written. + /// + private static ImmutableArray ReportCrossWiredGenerics( + SourceProductionContext context, ImmutableArray serviceModels, FileLogger logger) { + + if (!serviceModels.Any(IsCrossWiredGeneric)) { + return serviceModels; + } + + var builder = ImmutableArray.CreateBuilder(serviceModels.Length); + + foreach (var serviceModel in serviceModels) { + if (!IsCrossWiredGeneric(serviceModel)) { + builder.Add(serviceModel); + + continue; + } + + var typeName = serviceModel.ImplementationType.Name; + + logger.Error($"Skipping '{typeName}' because a generic type cannot be cross-wired."); + + context.ReportDiagnostic( + Diagnostic.Create( + DependencyModuleDiagnostics.CrossWireCannotBeGeneric, + Location.None, + typeName)); + } + + return builder.ToImmutable(); + } + + /// + /// A cross-wired registration on an implementation that is itself generic. + /// + private static bool IsCrossWiredGeneric(ServiceModel serviceModel) => + serviceModel.ImplementationType is GenericTypeDefinition { TypeArguments.Count: > 0 } && + serviceModel.Registrations.Any(registration => registration.CrossWire == true); + /// /// Reports what each conditional registration depends on, and refuses conditions that name /// nothing to test. diff --git a/src/DependencyModules.Testing/Attributes/MockAttribute.cs b/src/DependencyModules.Testing/Attributes/MockAttribute.cs index 8ecf6e3..9f90e12 100644 --- a/src/DependencyModules.Testing/Attributes/MockAttribute.cs +++ b/src/DependencyModules.Testing/Attributes/MockAttribute.cs @@ -55,8 +55,17 @@ public void SetupServiceCollection( } var mockedValue = mockAttribute.ProvideMock(parameter.ParameterType); + var key = ServiceKeyOf(parameter); - serviceCollection.AddSingleton(parameter.ParameterType, _ => mockedValue); + // Registered under the parameter's key when it has one. Registering unkeyed regardless left + // the keyed registration — the one the consumer actually injects — untouched, so the service + // under test kept the real implementation while the test held a double it believed was wired + // in. The arrangement ran, the double recorded nothing, and the assertion failed elsewhere. + if (key == null) { + serviceCollection.AddSingleton(parameter.ParameterType, _ => mockedValue); + } else { + serviceCollection.AddKeyedSingleton(parameter.ParameterType, key, (_, _) => mockedValue); + } } /// @@ -77,6 +86,22 @@ public void SetupServiceCollection( /// public Task GetParameterValueAsync( ITestMethodContext testMethod, IServiceProvider serviceProvider, ParameterInfo parameter) { + var key = ServiceKeyOf(parameter); + + if (key != null && serviceProvider is IKeyedServiceProvider keyedServiceProvider) { + return Task.FromResult(keyedServiceProvider.GetKeyedService(parameter.ParameterType, key)); + } + return Task.FromResult(serviceProvider.GetService(parameter.ParameterType)); } + + /// + /// The key the parameter asks for, or null when it asks for the unkeyed service. + /// + /// + /// The same attribute the container path honours, read here so that a parameter carrying both + /// [Mock] and [FromKeyedServices] means one thing rather than two. + /// + private static object? ServiceKeyOf(ParameterInfo parameter) => + parameter.GetCustomAttribute()?.Key; } diff --git a/tests/DependencyModules.Tests/GeneratorTests/DiagnosticsTests.cs b/tests/DependencyModules.Tests/GeneratorTests/DiagnosticsTests.cs index cd22f73..d4947b8 100644 --- a/tests/DependencyModules.Tests/GeneratorTests/DiagnosticsTests.cs +++ b/tests/DependencyModules.Tests/GeneratorTests/DiagnosticsTests.cs @@ -158,6 +158,339 @@ public partial class TestModule; Assert.DoesNotContain(result.GeneratorDiagnostics, d => d.Id == "DM0002"); } + /// + /// A generic decorator declared on the class, over a service registered as an open generic. The + /// expansion has no closed construction to close over, so the declaration used to disappear with + /// a green build — a decorator in the source that never ran. + /// + [Fact] + public void GenericDecoratorOverOpenGenericRegistration_ReportsDM0013() { + var result = GeneratorTestHarness.Run( + OpenGenericStore( + """ + [Decorator] + public class LoggingStore(IStore inner) : IStore { + public string Read(T key) => inner.Read(key); + } + """)); + + var diagnostic = Assert.Single(result.GeneratorDiagnostics, d => d.Id == "DM0013"); + + Assert.Equal(DiagnosticSeverity.Warning, diagnostic.Severity); + Assert.Contains("IStore", diagnostic.GetMessage()); + Assert.Contains("LoggingStore", diagnostic.GetMessage()); + } + + /// + /// The same service, decorated from the module instead. Both declaration forms reach the same + /// expansion, so both have to report. + /// + [Fact] + public void ModuleDeclaredDecoratorOverOpenGenericRegistration_ReportsDM0013() { + var result = GeneratorTestHarness.Run( + OpenGenericStore( + """ + public class LoggingStore(IStore inner) : IStore { + public string Read(T key) => inner.Read(key); + } + """, + moduleAttributes: "[Decorate(typeof(IStore<>), typeof(LoggingStore<>))]")); + + Assert.Single(result.GeneratorDiagnostics, d => d.Id == "DM0013"); + } + + /// + /// A non-generic decorator named against an unbound service type. This one needed no + /// expansion, so nothing caught it and it reached emission still carrying IStore<> — + /// which is CS7003 in generated code. + /// + [Fact] + public void NonGenericDecoratorOverOpenGenericRegistration_ReportsDM0013() { + var result = GeneratorTestHarness.Run( + OpenGenericStore( + """ + public class StringStoreDecorator(IStore inner) : IStore { + public string Read(string key) => inner.Read(key); + } + """, + moduleAttributes: "[Decorate(typeof(IStore<>), typeof(StringStoreDecorator))]")); + + Assert.Single(result.GeneratorDiagnostics, d => d.Id == "DM0013"); + } + + /// + /// And the generated code compiles, which it did not before: GeneratedAssembly.Create + /// asserts the compilation is clean and emits. + /// + [Fact] + public void NonGenericDecoratorOverOpenGenericRegistration_StillCompiles() { + var generated = GeneratedAssembly.Create( + OpenGenericStore( + """ + public class StringStoreDecorator(IStore inner) : IStore { + public string Read(string key) => inner.Read(key); + } + """, + moduleAttributes: "[Decorate(typeof(IStore<>), typeof(StringStoreDecorator))]")); + + Assert.Contains(generated.Services, d => d.ServiceType == generated.Type("IStore`1")); + } + + /// + /// The case that must keep working: closed registrations, which a generic decorator is expanded + /// across. This is the shape a MediatR-style pipeline is built from. + /// + [Fact] + public void GenericDecoratorOverClosedRegistrations_DoesNotReportDM0013() { + var result = GeneratorTestHarness.Run( + """ + using DependencyModules.Runtime.Attributes; + + namespace TestNamespace; + + public interface IStore { string Read(T key); } + + [SingletonService] public class IntStore : IStore { public string Read(int key) => "int"; } + [SingletonService] public class StringStore : IStore { public string Read(string key) => "string"; } + + [Decorator] + public class LoggingStore(IStore inner) : IStore { + public string Read(T key) => inner.Read(key); + } + + [DependencyModule] + public partial class TestModule; + """); + + Assert.DoesNotContain(result.GeneratorDiagnostics, d => d.Id == "DM0013"); + } + + /// + /// A decorator naming a service this compilation does not register at all stays quiet. Naming a + /// service someone else registers is what [Decorate] is for, so reporting here would fire + /// on the feature's primary use. + /// + [Fact] + public void DecoratorForAServiceThisCompilationDoesNotRegister_DoesNotReportDM0013() { + var result = GeneratorTestHarness.Run( + """ + using DependencyModules.Runtime.Attributes; + + namespace TestNamespace; + + public interface IElsewhere { string Read(T key); } + + public class LoggingElsewhere(IElsewhere inner) : IElsewhere { + public string Read(T key) => inner.Read(key); + } + + [DependencyModule] + [Decorate(typeof(IElsewhere<>), typeof(LoggingElsewhere<>))] + public partial class TestModule; + """); + + Assert.DoesNotContain(result.GeneratorDiagnostics, d => d.Id == "DM0013"); + } + + /// + /// Cross-wiring shares one instance across every service type, which needs a factory — and an + /// open generic registration cannot have one. The emission was invalid on its face: the type + /// parameter leaked into typeof(ILedger<T>) beside + /// GetRequiredService<Ledger<>>(). + /// + [Fact] + public void CrossWiredGenericType_ReportsDM0014() { + var result = GeneratorTestHarness.Run(CrossWiredLedger("public class Ledger : ILedger, IAudit;")); + + var diagnostic = Assert.Single(result.GeneratorDiagnostics, d => d.Id == "DM0014"); + + Assert.Equal(DiagnosticSeverity.Warning, diagnostic.Severity); + Assert.Contains("Ledger", diagnostic.GetMessage()); + } + + [Fact] + public void CrossWiredGenericType_IsNotRegistered() { + var result = GeneratorTestHarness.Run(CrossWiredLedger("public class Ledger : ILedger, IAudit;")); + + Assert.DoesNotContain(result.GeneratedSources.Keys, key => key.Contains("Dependencies")); + } + + /// + /// Cross-wiring a non-generic type is untouched, and still shares one instance across both + /// interfaces. + /// + [Fact] + public void CrossWiredNonGenericType_StillRegisters() { + var generated = GeneratedAssembly.Create(CrossWiredLedger("public class Ledger : ILedger, IAudit;")); + + var provider = generated.BuildProvider(); + + // The point of cross-wiring: both interfaces answer with the one instance. + Assert.Same( + provider.GetService(generated.Type("ILedger`1").MakeGenericType(typeof(int))), + provider.GetService(generated.Type("IAudit`1").MakeGenericType(typeof(int)))); + } + + /// + /// An interceptor implementing only IInterceptor, applied to a service whose members are + /// all async. It never runs, and the build used to be green — the model was ignored before + /// anything could report on it. + /// + [Fact] + public void InterceptorThatServesNoMember_ReportsDM0015() { + var result = GeneratorTestHarness.Run( + Intercepted( + """ + public interface IAsyncOnly { + Task GetAsync(string key); + } + + [SingletonService] + [Intercept(typeof(SyncOnlyInterceptor))] + public class AsyncOnly : IAsyncOnly { + public Task GetAsync(string key) => Task.FromResult(key); + } + """)); + + var diagnostic = Assert.Single(result.GeneratorDiagnostics, d => d.Id == "DM0015"); + + Assert.Equal(DiagnosticSeverity.Warning, diagnostic.Severity); + Assert.Contains("SyncOnlyInterceptor", diagnostic.GetMessage()); + Assert.Contains("IAsyncInterceptor", diagnostic.GetMessage()); + Assert.Contains("GetAsync", diagnostic.GetMessage()); + } + + /// + /// The partial case: the interceptor serves the sync members and is quietly absent from the async + /// one, which is how an argument-rewriting interceptor stops rewriting halfway through a service. + /// + [Fact] + public void InterceptorThatServesSomeMembers_ReportsDM0015ForTheRest() { + var result = GeneratorTestHarness.Run( + Intercepted( + """ + public interface IMixed { + int Count(string key); + Task CountAsync(string key); + } + + [SingletonService] + [Intercept(typeof(SyncOnlyInterceptor))] + public class Mixed : IMixed { + public int Count(string key) => key.Length; + public Task CountAsync(string key) => Task.FromResult(key.Length); + } + """)); + + var diagnostic = Assert.Single(result.GeneratorDiagnostics, d => d.Id == "DM0015"); + + Assert.Contains("CountAsync", diagnostic.GetMessage()); + Assert.DoesNotContain("'Count'", diagnostic.GetMessage()); + } + + /// + /// An interceptor covering every shape the service uses says nothing. + /// + [Fact] + public void InterceptorThatServesEveryMember_DoesNotReportDM0015() { + var result = GeneratorTestHarness.Run( + Intercepted( + """ + public interface ISyncOnly { + int Count(string key); + } + + [SingletonService] + [Intercept(typeof(SyncOnlyInterceptor))] + public class SyncOnly : ISyncOnly { + public int Count(string key) => key.Length; + } + """)); + + Assert.DoesNotContain(result.GeneratorDiagnostics, d => d.Id == "DM0015"); + } + + /// + /// DM0008 drops the whole wrapper, not only the member it names, and the message has to say so — + /// the guide read as though the other members were still intercepted. + /// + [Fact] + public void UnsupportedMember_ReportsThatNoMemberIsIntercepted() { + var result = GeneratorTestHarness.Run( + Intercepted( + """ + public interface IAwkward { + bool TryGet(string key, out string value); + int Fine(string key); + } + + [SingletonService] + [Intercept(typeof(SyncOnlyInterceptor))] + public class Awkward : IAwkward { + public bool TryGet(string key, out string value) { value = key; return true; } + public int Fine(string key) => key.Length; + } + """)); + + var diagnostic = Assert.Single(result.GeneratorDiagnostics, d => d.Id == "DM0008"); + + Assert.Contains("none of its members are intercepted", diagnostic.GetMessage()); + Assert.Contains("TryGet", diagnostic.GetMessage()); + } + + private static string Intercepted(string body) => + $$""" + using System.Threading.Tasks; + using DependencyModules.Runtime.Attributes; + using DependencyModules.Runtime.Interception; + + namespace TestNamespace; + + [SingletonService] + public class SyncOnlyInterceptor : IInterceptor { + public TResult Intercept(InvocationContext context) => context.Proceed(); + } + + {{body}} + + [DependencyModule] + public partial class TestModule; + """; + + private static string OpenGenericStore(string body, string moduleAttributes = "") => + $$""" + using DependencyModules.Runtime.Attributes; + + namespace TestNamespace; + + public interface IStore { string Read(T key); } + + [SingletonService] + public class Store : IStore { public string Read(T key) => "store"; } + + {{body}} + + [DependencyModule] + {{moduleAttributes}} + public partial class TestModule; + """; + + private static string CrossWiredLedger(string implementation) => + $$""" + using DependencyModules.Runtime.Attributes; + + namespace TestNamespace; + + public interface ILedger; + public interface IAudit; + + [CrossWireService] + {{implementation}} + + [DependencyModule] + public partial class TestModule; + """; + private static string Module(string body) => $$""" using DependencyModules.Runtime.Attributes; diff --git a/tests/DependencyModules.Tests/GeneratorTests/GeneratedBehaviourTests.cs b/tests/DependencyModules.Tests/GeneratorTests/GeneratedBehaviourTests.cs index 5dc2a35..0ea87a3 100644 --- a/tests/DependencyModules.Tests/GeneratorTests/GeneratedBehaviourTests.cs +++ b/tests/DependencyModules.Tests/GeneratorTests/GeneratedBehaviourTests.cs @@ -151,6 +151,37 @@ public void ReplaceRegistration_LeavesASingleRegistration() { Assert.Single(generated.Descriptors("IThing")); } + /// + /// Registrations within a module are emitted sorted by implementation type name, and both + /// Replace and Try act on a registration that has to already be there. Named so that the + /// alphabet puts them first, they used to run before their target existed: Replace replaced + /// nothing and added itself, then the registration it meant to displace was added after it and + /// won. Renaming the class fixed it, and nothing said so. + /// + [Fact] + public void ReplaceRegistration_WinsEvenWhenItsTypeNameSortsFirst() { + var generated = GeneratedAssembly.Create(Module( + """ + [SingletonService(Using = RegistrationType.Replace)] public class AaaThing : IThing; + [SingletonService] public class ZzzThing : IThing; + """)); + + Assert.Single(generated.Descriptors("IThing")); + Assert.Equal(generated.Type("AaaThing"), generated.ResolveRequired("IThing").GetType()); + } + + [Fact] + public void TryRegistration_DeclinesEvenWhenItsTypeNameSortsFirst() { + var generated = GeneratedAssembly.Create(Module( + """ + [SingletonService(Using = RegistrationType.Try)] public class AaaThing : IThing; + [SingletonService] public class ZzzThing : IThing; + """)); + + Assert.Single(generated.Descriptors("IThing")); + Assert.Equal(generated.Type("ZzzThing"), generated.ResolveRequired("IThing").GetType()); + } + [Fact] public void ConstructorDependencies_AreInjectedFromTheContainer() { var generated = GeneratedAssembly.Create(Module( diff --git a/tests/DependencyModules.Tests/GeneratorTests/InterceptorGenerationTests.cs b/tests/DependencyModules.Tests/GeneratorTests/InterceptorGenerationTests.cs index 902180d..a2169fc 100644 --- a/tests/DependencyModules.Tests/GeneratorTests/InterceptorGenerationTests.cs +++ b/tests/DependencyModules.Tests/GeneratorTests/InterceptorGenerationTests.cs @@ -579,34 +579,129 @@ public partial class TestModule; } /// - /// A generic implementation registers as an open generic, and decorating one of those rewrites - /// the registration into a factory, which the container rejects for an open generic service - /// type. Refused here so the failure names the declaration rather than surfacing as an - /// ArgumentException when the provider is built. + /// A generic implementation registers as an open generic. Decoration cannot touch one — it + /// rewrites the registration into a factory, and the container refuses a factory for an open + /// generic service type — but interception does not need one: the wrapper is a generated type, + /// and an open generic implementation type is what the container does accept. /// [Fact] - public void GenericImplementation_ReportsDM0008() { + public void GenericImplementation_IsIntercepted() { + var result = GeneratorTestHarness.Run(GenericRepo("public class Repo : IRepo { public void Run() { } }")); + + Assert.DoesNotContain(result.GeneratorDiagnostics, d => d.Id == "DM0008"); + Assert.Contains(result.GeneratedSources.Keys, key => key.Contains("Repo_Intercepted")); + } + + /// + /// The wrapper is generic over the same parameters, so the container can register it as an open + /// generic implementation type and close it per construction. + /// + [Fact] + public void GenericImplementation_EmitsAGenericWrapper() { + var result = GeneratorTestHarness.Run(GenericRepo("public class Repo : IRepo { public void Run() { } }")); + + var wrapper = Assert.Single(result.GeneratedSources, pair => pair.Key.Contains("Repo_Intercepted")).Value; + + Assert.Contains("class Repo_Intercepted", wrapper); + + // It takes the implementation by its own type. Asking for the service would resolve the + // wrapper itself, which is registered as that service, and recurse. + Assert.Contains("Repo inner", wrapper); + } + + /// + /// Registered by swapping the open generic registration rather than through the decorator + /// factory, which an open generic service type cannot carry. Every type is written unbound: no + /// `T` is in scope at the registration. + /// + [Fact] + public void GenericImplementation_RegistersAsAnOpenGenericImplementation() { + var result = GeneratorTestHarness.Run(GenericRepo("public class Repo : IRepo { public void Run() { } }")); + + var registration = Assert.Single(result.GeneratedSources, pair => pair.Key.Contains("Interceptors")).Value; + + Assert.Contains("InterceptOpenGeneric", registration); + Assert.Contains("typeof(global::TestNamespace.IRepo<>)", registration); + Assert.Contains("typeof(global::TestNamespace.Repo_Intercepted<>)", registration); + } + + /// + /// The wrapper repeats the implementation's constraints, without which it could not reference + /// what it wraps. + /// + [Fact] + public void ConstrainedGenericImplementation_RepeatsTheConstraints() { var result = GeneratorTestHarness.Run( - $$""" - {{Preamble}} + GenericRepo( + "public class Repo : IRepo where T : class, IMarker, new() { public void Run() { } }", + supporting: "public interface IMarker;")); - {{Tracing("Tracing", "tracing")}} + Assert.DoesNotContain(result.GeneratorDiagnostics, d => d.Id == "DM0008"); - public interface IRepo { void Run(); } + var wrapper = Assert.Single(result.GeneratedSources, pair => pair.Key.Contains("Repo_Intercepted")).Value; - [SingletonService] - [Intercept(typeof(TracingInterceptor))] - public class Repo : IRepo { public void Run() { } } + Assert.Contains("where T : class, global::TestNamespace.IMarker, new()", wrapper); + } - [DependencyModule] - public partial class TestModule; - """); + /// + /// struct already guarantees a default constructor, and repeating new() alongside it is CS0451. + /// Roslyn reports the constructor constraint for a struct-constrained parameter anyway, so the + /// reader has to drop it rather than pass it through. + /// + [Fact] + public void StructConstrainedGeneric_DoesNotRepeatTheDefaultConstructor() { + var result = GeneratorTestHarness.Run( + GenericRepo("public class Repo : IRepo where T : struct { public void Run() { } }")); - var diagnostic = Assert.Single(result.GeneratorDiagnostics, d => d.Id == "DM0008"); + var wrapper = Assert.Single(result.GeneratedSources, pair => pair.Key.Contains("Repo_Intercepted")).Value; - Assert.Contains("open generic", diagnostic.GetMessage()); + Assert.Contains("where T : struct", wrapper); + Assert.DoesNotContain("new()", wrapper); } + /// + /// And the constrained wrapper is not merely well-formed text: it compiles, loads and runs. + /// + [Fact] + public void ConstrainedGenericImplementation_ResolvesAndIntercepts() { + var generated = GeneratedAssembly.Create( + GenericRepo( + "public class Repo : IRepo where T : class, IMarker, new() { public void Run() { } }", + supporting: """ + public interface IMarker; + + public class Marked : IMarker; + """)); + + var closed = generated.Type("IRepo`1").MakeGenericType(generated.Type("Marked")); + var resolved = generated.BuildProvider().GetService(closed); + + Assert.NotNull(resolved); + Assert.StartsWith("Repo_Intercepted", resolved!.GetType().Name); + } + + /// + /// The attributes land on whatever declares first, so anything + /// the implementation needs alongside it goes in . + /// + private static string GenericRepo(string implementation, string supporting = "") => + $$""" + {{Preamble}} + + {{Tracing("Tracing", "tracing")}} + + public interface IRepo { void Run(); } + + {{supporting}} + + [SingletonService] + [Intercept(typeof(TracingInterceptor))] + {{implementation}} + + [DependencyModule] + public partial class TestModule; + """; + /// /// A closed construction of a generic service, which is the answer to the refusal above. The /// interface is reached through the base rather than declared, and a service registration finds diff --git a/tests/DependencyModules.Tests/Snapshots/ModuleGenerationSnapshotTests.RegistrationTypeVariants.verified.txt b/tests/DependencyModules.Tests/Snapshots/ModuleGenerationSnapshotTests.RegistrationTypeVariants.verified.txt index 108f641..8d135a3 100644 --- a/tests/DependencyModules.Tests/Snapshots/ModuleGenerationSnapshotTests.RegistrationTypeVariants.verified.txt +++ b/tests/DependencyModules.Tests/Snapshots/ModuleGenerationSnapshotTests.RegistrationTypeVariants.verified.txt @@ -13,16 +13,16 @@ namespace TestNamespace private static void ModuleDependencies(global::Microsoft.Extensions.DependencyInjection.IServiceCollection services) { - services.Replace(new global::Microsoft.Extensions.DependencyInjection.ServiceDescriptor( - typeof(global::TestNamespace.IReplace), - typeof(global::TestNamespace.ReplaceThing), - ServiceLifetime.Singleton - )); services.TryAddEnumerable(new global::Microsoft.Extensions.DependencyInjection.ServiceDescriptor( typeof(global::TestNamespace.ITryEnumerable), typeof(global::TestNamespace.TryEnumerableThing), ServiceLifetime.Singleton )); + services.Replace(new global::Microsoft.Extensions.DependencyInjection.ServiceDescriptor( + typeof(global::TestNamespace.IReplace), + typeof(global::TestNamespace.ReplaceThing), + ServiceLifetime.Singleton + )); services.TryAddSingleton( typeof(global::TestNamespace.ITry), typeof(global::TestNamespace.TryThing) diff --git a/tests/DependencyModules.Tests/Snapshots/PublicApiTests.RuntimeApi.verified.txt b/tests/DependencyModules.Tests/Snapshots/PublicApiTests.RuntimeApi.verified.txt index 8bb382a..6f3b991 100644 --- a/tests/DependencyModules.Tests/Snapshots/PublicApiTests.RuntimeApi.verified.txt +++ b/tests/DependencyModules.Tests/Snapshots/PublicApiTests.RuntimeApi.verified.txt @@ -189,6 +189,7 @@ namespace DependencyModules.Runtime.Helpers public static void Decorate(Microsoft.Extensions.DependencyInjection.IServiceCollection services, System.Type serviceType, System.Func decoratorFactory) { } public static void Decorate(Microsoft.Extensions.DependencyInjection.IServiceCollection services, System.Type decoratorIdentity, System.Func decoratorFactory) where TService : class { } + public static void InterceptOpenGeneric(Microsoft.Extensions.DependencyInjection.IServiceCollection services, System.Type serviceType, [System.Diagnostics.CodeAnalysis.DynamicallyAccessedMembers(System.Diagnostics.CodeAnalysis.DynamicallyAccessedMemberTypes.PublicConstructors)] System.Type implementationType, [System.Diagnostics.CodeAnalysis.DynamicallyAccessedMembers(System.Diagnostics.CodeAnalysis.DynamicallyAccessedMemberTypes.PublicConstructors)] System.Type wrapperType) { } } public sealed class DecoratorRegistration { diff --git a/tests/DependencyModules.Tests/Snapshots/PublicApiTests.SourceGeneratorApi.verified.txt b/tests/DependencyModules.Tests/Snapshots/PublicApiTests.SourceGeneratorApi.verified.txt index 0ad69bc..5052420 100644 --- a/tests/DependencyModules.Tests/Snapshots/PublicApiTests.SourceGeneratorApi.verified.txt +++ b/tests/DependencyModules.Tests/Snapshots/PublicApiTests.SourceGeneratorApi.verified.txt @@ -14,7 +14,9 @@ namespace CSharpAuthor { public AttributeDefinition(CSharpAuthor.ITypeDefinition attributeType) { } public System.Collections.Generic.IList? Arguments { get; set; } + public string? Target { get; set; } protected override void WriteComponentOutput(CSharpAuthor.IOutputContext outputContext) { } + public void WriteInline(CSharpAuthor.IOutputContext outputContext) { } } public abstract class BaseBlockDefinition : CSharpAuthor.BaseOutputComponent { @@ -113,6 +115,7 @@ namespace CSharpAuthor public class ClassDefinition : CSharpAuthor.BaseOutputComponent, CSharpAuthor.IConstructContainer, CSharpAuthor.INamedComponent { public ClassDefinition(string name) { } + public System.Collections.Generic.IReadOnlyList Constraints { get; } public System.Collections.Generic.IReadOnlyList Constructors { get; } public int FieldCount { get; } public System.Collections.Generic.IReadOnlyList Fields { get; } @@ -120,11 +123,14 @@ namespace CSharpAuthor public System.Collections.Generic.IReadOnlyList Methods { get; } public string Name { get; } public System.Collections.Generic.IReadOnlyList Properties { get; } + public bool TerminateWithSemicolon { get; set; } public CSharpAuthor.ClassKeyword TypeKeyword { get; set; } public CSharpAuthor.IOutputComponent? WhereStatement { get; set; } public CSharpAuthor.ClassDefinition AddBaseType(CSharpAuthor.ITypeDefinition typeDefinition) { } + public CSharpAuthor.ClassDefinition AddBaseType(CSharpAuthor.ITypeDefinition typeDefinition, params CSharpAuthor.IOutputComponent[] arguments) { } public CSharpAuthor.ClassDefinition AddClass(string name) { } public void AddComponent(CSharpAuthor.IOutputComponent outputComponent) { } + public CSharpAuthor.ConstraintDefinition AddConstraint(string typeParameter) { } public CSharpAuthor.ConstructorDefinition AddConstructor(CSharpAuthor.IOutputComponent? baseComponent = null) { } public CSharpAuthor.EnumDefinition AddEnum(string name) { } public CSharpAuthor.EventDefinition AddEvent(CSharpAuthor.ITypeDefinition handlerType, string name) { } @@ -186,10 +192,25 @@ namespace CSharpAuthor NoAccessibility = 2048, Sealed = 4096, } + public class ConstraintDefinition + { + public ConstraintDefinition(string typeParameter) { } + public bool IsEmpty { get; } + public string TypeParameter { get; } + public CSharpAuthor.ConstraintDefinition Class(bool nullable = false) { } + public CSharpAuthor.ConstraintDefinition Default() { } + public CSharpAuthor.ConstraintDefinition DefaultConstructor() { } + public CSharpAuthor.ConstraintDefinition Implements(CSharpAuthor.ITypeDefinition type) { } + public CSharpAuthor.ConstraintDefinition NotNull() { } + public CSharpAuthor.ConstraintDefinition Struct() { } + public CSharpAuthor.ConstraintDefinition Unmanaged() { } + public void WriteOutput(CSharpAuthor.IOutputContext outputContext) { } + } public class ConstructorDefinition : CSharpAuthor.MethodDefinition { public ConstructorDefinition(string name, CSharpAuthor.IOutputComponent? base = null) { } public CSharpAuthor.IOutputComponent? Base { get; } + public bool IsPrimary { get; set; } protected override void WriteAccessModifier(CSharpAuthor.IOutputContext outputContext) { } protected override void WriteEndOfMethodSignature(CSharpAuthor.IOutputContext outputContext) { } protected override void WriteReturnType(CSharpAuthor.IOutputContext outputContext) { } @@ -214,12 +235,14 @@ namespace CSharpAuthor public CSharpAuthor.EnumDefinition AddFlags() { } public CSharpAuthor.EnumValueDefinition AddValue(string enumValueName) { } public CSharpAuthor.EnumValueDefinition AddValue(string enumValueName, object value) { } + protected override void WriteComment(CSharpAuthor.IOutputContext outputContext) { } protected override void WriteComponentOutput(CSharpAuthor.IOutputContext outputContext) { } } public class EnumValueDefinition : CSharpAuthor.BaseOutputComponent { public EnumValueDefinition(string enumValueName) { } public object? Value { get; set; } + protected override void WriteComment(CSharpAuthor.IOutputContext outputContext) { } protected override void WriteComponentOutput(CSharpAuthor.IOutputContext outputContext) { } } public class EventDefinition : CSharpAuthor.BaseOutputComponent, CSharpAuthor.INamedComponent @@ -445,6 +468,7 @@ namespace CSharpAuthor protected readonly System.Collections.Generic.List ParameterList; protected int VariableCount; public MethodDefinition(string name) { } + public System.Collections.Generic.IReadOnlyList Constraints { get; } public System.Collections.Generic.List GenericParameters { get; } public CSharpAuthor.ITypeDefinition? InterfaceImplementation { get; set; } public string Name { get; } @@ -452,6 +476,7 @@ namespace CSharpAuthor public string? ReturnComment { get; set; } public CSharpAuthor.ITypeDefinition? ReturnType { get; } public CSharpAuthor.IOutputComponent? WhereStatement { get; set; } + public CSharpAuthor.ConstraintDefinition AddConstraint(string typeParameter) { } public void AddGenericParameter(CSharpAuthor.ITypeDefinition typeDefinition) { } public CSharpAuthor.MethodDefinition AddParameter(CSharpAuthor.ParameterDefinition parameterDefinition) { } public CSharpAuthor.ParameterDefinition AddParameter(CSharpAuthor.ITypeDefinition typeDefinition, string name) { } @@ -471,6 +496,7 @@ namespace CSharpAuthor { public NamespaceDefinition(string ns = "") { } public bool FileScopedNamespace { get; set; } + public string Namespace { get; } public CSharpAuthor.ClassDefinition AddClass(string name) { } public void AddComponent(CSharpAuthor.IOutputComponent component) { } public CSharpAuthor.EnumDefinition AddEnum(string name) { } @@ -1059,10 +1085,13 @@ namespace DependencyModules.SourceGenerator.Impl public static readonly Microsoft.CodeAnalysis.DiagnosticDescriptor ConventionCannotBeRead; public static readonly Microsoft.CodeAnalysis.DiagnosticDescriptor ConventionMatchNotConstructable; public static readonly Microsoft.CodeAnalysis.DiagnosticDescriptor ConventionMatchedNothing; + public static readonly Microsoft.CodeAnalysis.DiagnosticDescriptor CrossWireCannotBeGeneric; public static readonly Microsoft.CodeAnalysis.DiagnosticDescriptor EmptyEnvironmentCondition; public static readonly Microsoft.CodeAnalysis.DiagnosticDescriptor ExposedByConvention; public static readonly Microsoft.CodeAnalysis.DiagnosticDescriptor GeneratorFailure; + public static readonly Microsoft.CodeAnalysis.DiagnosticDescriptor InterceptorCannotServeMembers; public static readonly Microsoft.CodeAnalysis.DiagnosticDescriptor ModuleMustBePartial; + public static readonly Microsoft.CodeAnalysis.DiagnosticDescriptor OpenGenericCannotBeDecorated; public static readonly Microsoft.CodeAnalysis.DiagnosticDescriptor RegisteredConditionally; public static readonly Microsoft.CodeAnalysis.DiagnosticDescriptor ServiceCannotBeConstructed; } @@ -1308,7 +1337,7 @@ namespace DependencyModules.SourceGenerator.Impl.Models } public class InterceptedMemberModel : System.IEquatable { - public InterceptedMemberModel(string Name, string Identifier, DependencyModules.SourceGenerator.Impl.Models.AccessorForm Form, CSharpAuthor.ITypeDefinition? ReturnType, CSharpAuthor.ITypeDefinition ResultType, System.Collections.Generic.IReadOnlyList Parameters, System.Collections.Generic.IReadOnlyList TypeParameters, DependencyModules.SourceGenerator.Impl.Models.ReturnShape ReturnShape) { } + public InterceptedMemberModel(string Name, string Identifier, DependencyModules.SourceGenerator.Impl.Models.AccessorForm Form, CSharpAuthor.ITypeDefinition? ReturnType, CSharpAuthor.ITypeDefinition ResultType, System.Collections.Generic.IReadOnlyList Parameters, System.Collections.Generic.IReadOnlyList TypeParameters, DependencyModules.SourceGenerator.Impl.Models.ReturnShape ReturnShape) { } public DependencyModules.SourceGenerator.Impl.Models.AccessorForm Form { get; init; } public string Identifier { get; init; } public DependencyModules.SourceGenerator.Impl.Models.InterceptorKind Kind { get; } @@ -1317,7 +1346,7 @@ namespace DependencyModules.SourceGenerator.Impl.Models public CSharpAuthor.ITypeDefinition ResultType { get; init; } public DependencyModules.SourceGenerator.Impl.Models.ReturnShape ReturnShape { get; init; } public CSharpAuthor.ITypeDefinition? ReturnType { get; init; } - public System.Collections.Generic.IReadOnlyList TypeParameters { get; init; } + public System.Collections.Generic.IReadOnlyList TypeParameters { get; init; } public virtual bool Equals(DependencyModules.SourceGenerator.Impl.Models.InterceptedMemberModel? other) { } public override int GetHashCode() { } } @@ -1330,12 +1359,6 @@ namespace DependencyModules.SourceGenerator.Impl.Models public string Name { get; init; } public CSharpAuthor.ITypeDefinition Type { get; init; } } - public class InterceptedTypeParameterModel : System.IEquatable - { - public InterceptedTypeParameterModel(string Name, string Constraints) { } - public string Constraints { get; init; } - public string Name { get; init; } - } public class InterceptionRefusal : System.IEquatable { public InterceptionRefusal(string Message) { } @@ -1350,15 +1373,17 @@ namespace DependencyModules.SourceGenerator.Impl.Models public class InterceptorModel : System.IEquatable { public static readonly DependencyModules.SourceGenerator.Impl.Models.InterceptorModel Ignore; - public InterceptorModel(CSharpAuthor.ITypeDefinition ServiceType, CSharpAuthor.ITypeDefinition ImplementationType, System.Collections.Generic.IReadOnlyList Interceptors, System.Collections.Generic.IReadOnlyList Members, System.Collections.Generic.IReadOnlyList Declarations, int Order, DependencyModules.SourceGenerator.Impl.Models.InterceptionRefusal? Refusal = null) { } + public InterceptorModel(CSharpAuthor.ITypeDefinition ServiceType, CSharpAuthor.ITypeDefinition ImplementationType, System.Collections.Generic.IReadOnlyList Interceptors, System.Collections.Generic.IReadOnlyList Members, System.Collections.Generic.IReadOnlyList Declarations, int Order, DependencyModules.SourceGenerator.Impl.Models.InterceptionRefusal? Refusal = null, System.Collections.Generic.IReadOnlyList? TypeParameters = null) { } public System.Collections.Generic.IReadOnlyList Declarations { get; init; } public CSharpAuthor.ITypeDefinition ImplementationType { get; init; } public System.Collections.Generic.IReadOnlyList Interceptors { get; init; } public bool IsIgnored { get; } + public bool IsOpenGeneric { get; } public System.Collections.Generic.IReadOnlyList Members { get; init; } public int Order { get; init; } public DependencyModules.SourceGenerator.Impl.Models.InterceptionRefusal? Refusal { get; init; } public CSharpAuthor.ITypeDefinition ServiceType { get; init; } + public System.Collections.Generic.IReadOnlyList? TypeParameters { get; init; } public static DependencyModules.SourceGenerator.Impl.Models.InterceptorModel Refused(string message) { } } public class InterceptorModelComparer : System.Collections.Generic.IEqualityComparer @@ -1518,6 +1543,16 @@ namespace DependencyModules.SourceGenerator.Impl.Models public DependencyModules.SourceGenerator.Impl.Models.RegistrationType? RegistrationType { get; init; } public CSharpAuthor.ITypeDefinition ServiceType { get; init; } } + public class TypeParameterModel : System.IEquatable + { + public TypeParameterModel(string Name, string? Primary, System.Collections.Generic.IReadOnlyList ConstraintTypes, bool DefaultConstructor) { } + public System.Collections.Generic.IReadOnlyList ConstraintTypes { get; init; } + public bool DefaultConstructor { get; init; } + public string Name { get; init; } + public string? Primary { get; init; } + public virtual bool Equals(DependencyModules.SourceGenerator.Impl.Models.TypeParameterModel? other) { } + public override int GetHashCode() { } + } } namespace DependencyModules.SourceGenerator.Impl.Utilities { @@ -1532,6 +1567,10 @@ namespace DependencyModules.SourceGenerator.Impl.Utilities public static System.Collections.Generic.IReadOnlyList GetAttributeModels(DependencyModules.SourceGenerator.Impl.Utilities.SyntaxTransformContext context, Microsoft.CodeAnalysis.SyntaxNode node, System.Threading.CancellationToken cancellationToken, System.Func? filter = null) { } public static System.Collections.Generic.IEnumerable GetAttributes(DependencyModules.SourceGenerator.Impl.Utilities.SyntaxTransformContext context, Microsoft.CodeAnalysis.SyntaxList attributeListSyntax, System.Threading.CancellationToken cancellationToken, System.Func? filter = null) { } } + public static class AttributeTypeMatcher + { + public static bool Matches(Microsoft.CodeAnalysis.SemanticModel semanticModel, Microsoft.CodeAnalysis.CSharp.Syntax.AttributeSyntax attributeSyntax, CSharpAuthor.ITypeDefinition attributeType, System.Threading.CancellationToken cancellationToken) { } + } public abstract class BaseAttributeWriter where T : DependencyModules.SourceGenerator.Impl.Models.IClassModel { @@ -1565,7 +1604,7 @@ namespace DependencyModules.SourceGenerator.Impl.Utilities } public static class DecoratorExpansion { - public static System.Collections.Generic.IReadOnlyList Expand(System.Collections.Generic.IReadOnlyList decorators, System.Collections.Generic.IReadOnlyList registeredServiceTypes, bool includeNonGeneric = true, System.Func? canClose = null) { } + public static System.Collections.Generic.IReadOnlyList Expand(System.Collections.Generic.IReadOnlyList decorators, System.Collections.Generic.IReadOnlyList registeredServiceTypes, out System.Collections.Generic.IReadOnlyList refusedForOpenGenericRegistration, bool includeNonGeneric = true, System.Func? canClose = null) { } } public static class DecoratorModelUtility { @@ -1682,6 +1721,10 @@ namespace DependencyModules.SourceGenerator.Impl.Utilities public static DependencyModules.SourceGenerator.Impl.Utilities.SyntaxTransformContext op_Implicit(Microsoft.CodeAnalysis.GeneratorAttributeSyntaxContext context) { } public static DependencyModules.SourceGenerator.Impl.Utilities.SyntaxTransformContext op_Implicit(Microsoft.CodeAnalysis.GeneratorSyntaxContext context) { } } + public static class TypeParameterReader + { + public static DependencyModules.SourceGenerator.Impl.Models.TypeParameterModel Read(Microsoft.CodeAnalysis.ITypeParameterSymbol parameter) { } + } public static class TypeSyntaxExtensions { public static string GetFullName(this Microsoft.CodeAnalysis.INamespaceSymbol? namespaceSymbol) { } diff --git a/tests/DependencyModules.Tests/TestingTests/TestParameterResolverTests.cs b/tests/DependencyModules.Tests/TestingTests/TestParameterResolverTests.cs index d5b96ab..a3df09e 100644 --- a/tests/DependencyModules.Tests/TestingTests/TestParameterResolverTests.cs +++ b/tests/DependencyModules.Tests/TestingTests/TestParameterResolverTests.cs @@ -133,6 +133,61 @@ public async Task ResolvingBeforeSetupThrows() { Assert.Contains(nameof(TestParameterResolver.SetupServiceCollection), exception.Message); } + /// + /// A [Mock] on a keyed parameter replaces the keyed registration. It used to register the + /// double unkeyed, leaving the keyed registration — the one a consumer injects — untouched, so + /// the service under test kept the real implementation while the test held a double it believed + /// was wired in. + /// + [Fact] + public async Task KeyedMockReplacesTheKeyedRegistration() { + var (resolver, provider) = Build( + nameof(Samples.KeyedMock), + services => services.AddKeyedSingleton("primary")); + + var arguments = await resolver.ResolveArgumentsAsync(provider, []); + + Assert.IsType(Assert.Single(arguments)); + Assert.IsType(provider.GetRequiredKeyedService("primary")); + } + + /// + /// And it does not spill into the unkeyed slot, where nothing asked for it. + /// + [Fact] + public void KeyedMockRegistersNothingUnkeyed() { + var (_, provider) = Build( + nameof(Samples.KeyedMock), + services => services.AddKeyedSingleton("primary")); + + Assert.Null(provider.GetService()); + } + + /// + /// A key the mock did not name is left alone, so mocking one keyed implementation leaves its + /// siblings real. + /// + [Fact] + public void KeyedMockLeavesOtherKeysAlone() { + var (_, provider) = Build( + nameof(Samples.KeyedMock), + services => { + services.AddKeyedSingleton("primary"); + services.AddKeyedSingleton("secondary"); + }); + + Assert.IsType(provider.GetRequiredKeyedService("secondary")); + } + + /// Control: an unkeyed mock still replaces the unkeyed registration. + [Fact] + public async Task UnkeyedMockReplacesTheUnkeyedRegistration() { + var arguments = await Resolve( + nameof(Samples.UnkeyedMock), services => services.AddSingleton()); + + Assert.IsType(Assert.Single(arguments)); + } + [Fact] public void SetupIsOfferedEveryParameter() { var services = new ServiceCollection(); @@ -175,9 +230,14 @@ private class StubContext(MethodInfo method) : ITestMethodContext { /// Signatures only — never invoked. Static so nothing needs constructing; the members are public /// within this private class so one set of binding flags finds them all. /// + [StubMockSupport] private static class Samples { public static void OneService(IThing thing) { } + public static void KeyedMock([Mock] [FromKeyedServices("primary")] IThing thing) { } + + public static void UnkeyedMock([Mock] IThing thing) { } + public static void WantsTheProvider(IServiceProvider provider) { } public static void DataThenService(int number, IThing thing) { } @@ -193,6 +253,15 @@ public static void UnregisteredWithInjectedValue([InjectValues("supplied")] Need public static void TwoRegisteringAttributes([RegistersOther] IThing first, [RegistersOther] IThing second) { } } + /// + /// Stands in for a mocking package, so the real [Mock] can be driven without one. What the + /// double actually is does not matter here; where it gets registered does. + /// + [AttributeUsage(AttributeTargets.Class)] + private class StubMockSupportAttribute : Attribute, IMockSupportAttribute { + public object ProvideMock(Type type) => new Other(); + } + /// /// Stands in for [Mock]: registers a replacement during setup, then lets ordinary container /// resolution hand it back. diff --git a/website/guide/aot.md b/website/guide/aot.md index 882afe3..6a6d801 100644 --- a/website/guide/aot.md +++ b/website/guide/aot.md @@ -55,8 +55,24 @@ a compile-time decision, and belongs to `#if`. See [what conditions cost](/guide/environments#what-conditions-cost). **Open generic registration is the least AOT-friendly part of the container itself**, independent of -this library — the container has to construct a closed type at run time. If you are targeting Native -AOT aggressively, prefer closed registrations. +this library — the container has to construct a closed type at run time, and Native AOT only has code +for the instantiations the compiler could see. + +In practice the line falls between reference and value type arguments. Measured on a published +`osx-arm64` binary, with `[SingletonService]` on `Bin : IBin`: + +``` +GetRequiredService>() works — reference types share one instantiation +GetRequiredService>() InvalidOperationException: Unable to create a generic service + for type 'IBin`1[System.Int32]' because 'System.Int32' is a + ValueType. Native code to support creating generic services + might not be available with native AOT. +``` + +This is the container, not the generator: an [intercepted](/guide/interception) open generic behaves +exactly the same way, because it is registered the same way. If you are targeting Native AOT, register +closed constructions — a [convention](/guide/conventions) over the open generic does that for you, +emitting one registration per implementation. **Runtime assembly discovery is not supported**, because there would be nothing to resolve at build time. See [Scanning a package](/guide/scanning). diff --git a/website/guide/decorators.md b/website/guide/decorators.md index afb54af..2e56973 100644 --- a/website/guide/decorators.md +++ b/website/guide/decorators.md @@ -171,12 +171,11 @@ public class Repository : IRepository { } // registers IRepository<> its public class CachingRepository(IRepository inner) : IRepository { } ``` -``` -InvalidOperationException: 'IRepository`1' is registered as an open generic and cannot be -decorated by 'CachingRepository`1'. … -``` +This is [DM0013](/reference/diagnostics#dm0013) at build time, whichever way the decorator was +declared — on the class, or on the module with `[Decorate]`. -Register closed constructions instead. +Register closed constructions instead. A [convention](/guide/conventions) over the open generic +registers one per implementation, and an open generic decorator is then expanded across them. Note that this is about the **registration**, not the decorator. An open generic decorator over closed registrations — the example further up — works, and is the common case. diff --git a/website/guide/getting-started.md b/website/guide/getting-started.md index fcccc7c..ab9eabd 100644 --- a/website/guide/getting-started.md +++ b/website/guide/getting-started.md @@ -48,6 +48,17 @@ dotnet add package DependencyModules.SourceGenerator Requires .NET 8.0 or later, and ships both `net8.0` and `net10.0` assemblies so a project on either LTS release gets one built against its own framework. +::: tip A console app or class library needs one more +`DependencyModules.Runtime` depends on `Microsoft.Extensions.DependencyInjection.Abstractions`, which +is the right dependency for a library — but `ServiceCollection` and `BuildServiceProvider()` live in +the implementation package. A project using the Web or Worker SDK already has it through its framework +reference. Anything else needs: + +```shell +dotnet add package Microsoft.Extensions.DependencyInjection +``` +::: + Those two are everything the library itself needs — [conventions](/guide/conventions) included. The optional packages are for [testing](/guide/testing), and this guide will tell you when you want them: diff --git a/website/guide/interception.md b/website/guide/interception.md index 49b14d7..1d5fe96 100644 --- a/website/guide/interception.md +++ b/website/guide/interception.md @@ -118,7 +118,7 @@ public async IAsyncEnumerable InterceptStream(StreamInvocationCont |---|---| | `Proceed()` / `ProceedAsync()` | run the rest of the pipeline — more than once to retry, or not at all to skip the implementation | | `Caller.ServiceType`, `Caller.MemberName` | what is being called | -| `Arguments` | by index or by name, and **writable** — a write replaces what the implementation receives | +| `Arguments` | by index, and **writable** — a write replaces what the implementation receives. `NameAt(index)` gives the declared parameter name | Arguments cost nothing until you read one. @@ -135,12 +135,74 @@ dependencies of its own — as `TimingInterceptor` does with its `ILogger`. ## What cannot be intercepted The generator has to emit a real override, so some shapes are impossible. These are reported as -[DM0008](/reference/diagnostics#dm0008) and left unwrapped rather than failing the build: +[DM0008](/reference/diagnostics#dm0008) rather than failing the build: - `ref`, `in` and `out` parameters, and `ref struct` parameters - by-reference returns - `init`-only setters - static members -- generic implementations, which register as an open generic +- a generic *method* whose shape the wrapper cannot forward, by the same rules as above -Write a [decorator](/guide/decorators) for those. +::: warning One such member disables interception for the whole interface +There is no partial wrapper. A single `out` parameter anywhere on the interface means no wrapper is +generated at all, so every other member goes uninterceped too, and `GetRequiredService()` +returns the plain implementation. The diagnostic names the member it found first; fixing it may +uncover another. + +Move the member to an interface that is not intercepted, or write a +[decorator](/guide/decorators) for the service instead. +::: + +## Intercepting a generic service + +A generic implementation registers as an open generic, and a decorator cannot touch one — decoration +rewrites a registration into a factory, and the container refuses a factory for an open generic +service type. Interception does not need a factory: the wrapper is a generated type, and an open +generic implementation type is what the container does accept. + +```csharp +[SingletonService] +[Intercept(typeof(TracingInterceptor))] +public class Repository : IRepository { … } +``` + +The wrapper is generic over the same parameters — `Repository_Intercepted : IRepository` — and +takes `Repository` by its own type rather than the service, which would resolve back to the wrapper +and recurse. The container closes it per construction, so `IRepository` and +`IRepository` each get their own. + +::: warning Native AOT closes this over reference types only +An open generic registration is the container's least AOT-friendly shape, intercepted or not: a +published binary can construct `IRepository` and throws for `IRepository`. That is not +specific to interception — a plain `[SingletonService]` on a generic class behaves identically. See +[Trimming and AOT](/guide/aot#what-it-does-not-cover). +::: + +Constraints come along with the parameters. `Repository where T : class, IEntity, new()` is wrapped +by `Repository_Intercepted : IRepository where T : class, IEntity, new()`, because without them +the wrapper could not reference what it wraps. + +## When an interceptor covers only some members + +Separate from the above, and quieter. Each interceptor is placed only around the members whose shape +it can serve — `IInterceptor` for a direct return, `IAsyncInterceptor` for a task, +`IAsyncEnumerableInterceptor` for a stream — and it is simply absent from the rest: + +```csharp +public class AuditInterceptor : IInterceptor { … } // sync only + +[SingletonService] +[Intercept(typeof(AuditInterceptor))] +public class Orders : IOrders { + public int Count(string customer) { … } // audited + public Task CountAsync(string customer) { … } // not audited +} +``` + +That is [DM0015](/reference/diagnostics#dm0015). It is worth taking seriously rather than silencing: +an interceptor that rewrites arguments stops rewriting them, and one that authorises or audits stops +doing that — on the async members, which are usually the ones doing the work. Implement the missing +interface, or apply the interceptor to a service with no such member. + +One type may implement any combination of the three, which is how a single interceptor covers a mixed +interface. diff --git a/website/guide/modules.md b/website/guide/modules.md index d6a179f..3451320 100644 --- a/website/guide/modules.md +++ b/website/guide/modules.md @@ -124,6 +124,20 @@ public partial class DiagnosticsModule; Convention registrations always name their declaring module as their realm, which is why two modules running conventions over the same interface do not leak into each other. +::: warning Two modules in one assembly, loaded together, register everything twice +"Joins every module in its compilation" is literal. An assembly declaring two modules that neither set +`OnlyRealm` puts the *whole* registration list in both — decorators included — so loading both in one +call runs it twice: + +```csharp +services.AddModules(new AppModule(), new DataModule()); // every service registered twice +``` + +Declaring two modules is fine; loading both is what doubles up. If they are meant to be composed +together, give one a realm, or have one compose the other with its +[generated attribute](#composing-modules) instead of naming both at the call site. +::: + ## Parameters A module can take values from whoever loads it — a connection string, a base URL. Declare them as diff --git a/website/guide/troubleshooting.md b/website/guide/troubleshooting.md index e5da103..5d786bf 100644 --- a/website/guide/troubleshooting.md +++ b/website/guide/troubleshooting.md @@ -45,6 +45,26 @@ the reason**. ``` +::: warning No log appeared? +Every `DependencyModules_*` property reaches the generator through +`build/DependencyModules.SourceGenerator.targets`, which ships **inside the NuGet package**. A project +that references the analyzer as a `ProjectReference` — building this library from source, or vendoring +it — never imports that file, so the property is invisible and silently takes its default. + +Declare them yourself in that project, or in a `Directory.Build.props` above it: + +```xml + + + + + + + + +``` +::: + ## 3. Check for DM diagnostics The generator reports what it can detect at build time, and a good deal of what goes wrong here is diff --git a/website/reference/attributes.md b/website/reference/attributes.md index 127cbcf..7b6f39b 100644 --- a/website/reference/attributes.md +++ b/website/reference/attributes.md @@ -59,6 +59,11 @@ A `[Decorator]` is never a convention candidate — it is not a service. | Property | | |---|---| | `Order` | nesting; lower sits closer to the implementation | +| `Service` | the decorated interface, when it cannot be inferred | +| `Realm` | restrict the decorator to one module, matching `Realm` on the service attributes | + +An unrestricted decorator belongs to every module that is not `OnlyRealm`, exactly as an unrestricted +service registration does — so a decorator with no `Realm` is not picked up by an `OnlyRealm` module. ### `[Intercept(params Type[])]` diff --git a/website/reference/diagnostics.md b/website/reference/diagnostics.md index 5a78bb2..10c056a 100644 --- a/website/reference/diagnostics.md +++ b/website/reference/diagnostics.md @@ -4,15 +4,22 @@ The generator reports what it can work out at build time as `DM####` codes, so a shows up in the IDE rather than as a resolution failure at startup. This page says what each one means and what to do about it. -They behave like any other analyzer diagnostic, so each can be tuned or silenced through -`.editorconfig`: - -```ini -dotnet_diagnostic.DM0010.severity = none +These are reported by a source generator rather than by an analyzer, which decides how they are +tuned. Roslyn applies `.editorconfig` severity mapping to *analyzer* diagnostics, and a generator's +reach the compilation with the severity already fixed — so `dotnet_diagnostic.DM0005.severity = none` +has no effect. Use the compilation-level properties instead, which are applied later and do work: + +```xml + + $(NoWarn);DM0005 + $(WarningsAsErrors);DM0013 + ``` -`DM0010` and `DM0011` are informational and exist to make registration visible at the class. Silence -them if the IDE gets noisy; the rest are worth reading. +`#pragma warning disable DM0005` works too, for silencing one site rather than a project. + +`DM0010` and `DM0011` are informational and exist to make registration visible at the class, which +means they appear in the IDE and never in `dotnet build` at any verbosity. The rest are worth reading. | Code | Severity | Meaning | |---|---|---| @@ -28,6 +35,9 @@ them if the IDE gets noisy; the rest are worth reading. | [DM0010](#dm0010) | Info | A service is registered by convention | | [DM0011](#dm0011) | Info | A service is registered only when a condition holds | | [DM0012](#dm0012) | Warning | An environment condition names nothing to test | +| [DM0013](#dm0013) | Warning | A service registered as an open generic cannot be decorated | +| [DM0014](#dm0014) | Warning | A generic type cannot be cross-wired | +| [DM0015](#dm0015) | Warning | An interceptor does not apply to every member | ## DM0001 {#dm0001} @@ -88,7 +98,10 @@ Their nesting would be ambiguous. See [Decorators](/guide/decorators#ordering). **A service marked for interception cannot be wrapped.** The member uses `ref`, `in`, `out` or a `ref struct` parameter, returns by reference, has an -`init`-only setter, is static, or the implementation is generic. See +`init`-only setter, or is static. + +One such member costs the whole interface: no wrapper is generated, so every other member goes +uninterceped too. The message names the first offender it found. See [Interception](/guide/interception#what-cannot-be-intercepted). ## DM0009 {#dm0009} @@ -123,3 +136,81 @@ Informational, reported at the class. See [Environments](/guide/environments). `[IfEnvironment()]` and `[IfEnvironmentValue("")]` both compile. Written plain they mean the service never registers; written as the `IfNot` form they mean the attribute does nothing at all. + +## DM0013 {#dm0013} + +**A service registered as an open generic cannot be decorated.** + +Decoration replaces a registration with a factory, and the container does not allow one for an open +generic service type — `Open generic service type 'IRepository`1[T]' requires registering an open +generic implementation type`. + +```csharp +[SingletonService] +public class Repository : IRepository { } // registers IRepository<> itself + +[Decorator] +public class CachingRepository(IRepository inner) : IRepository { } // DM0013 +``` + +Reported whichever way the decorator was declared — on the class, or on the module with +`[Decorate]` — and whether or not the decorator is itself generic. + +Register closed constructions instead. A [convention](/guide/conventions) over the open generic +registers one per implementation, and an open generic decorator is expanded across them. See +[Decorators](/guide/decorators#one-limitation). + +## DM0014 {#dm0014} + +**A generic type cannot be cross-wired.** + +`[CrossWireService]` shares one instance across the implementation and every interface it declares, +which is emitted as a factory per interface — and an open generic registration cannot carry one. + +```csharp +[CrossWireService] +public class Ledger : ILedger, IAudit { } // DM0014 +``` + +Registering each interface to the same open generic implementation type would compile, and is a +different contract: the container builds one instance per service type, which is the opposite of what +the attribute promises. + +Use `[SingletonService]`, `[ScopedService]` or `[TransientService]` instead, applying one per +interface if the type needs to answer to more than one. + +## DM0015 {#dm0015} + +**An interceptor does not apply to every member it was applied to.** + +Three interfaces cover the member shapes, and the generator picks per member: + +| Interface | Members | +|---|---| +| `IInterceptor` | returning a value directly, or `void` | +| `IAsyncInterceptor` | returning `Task`, `Task`, `ValueTask`, `ValueTask` | +| `IAsyncEnumerableInterceptor` | returning `IAsyncEnumerable` | + +An interceptor that implements none of the one a member needs is left out of that member's chain, and +those calls run without it: + +```csharp +public class AuditInterceptor : IInterceptor { … } // sync only + +[SingletonService] +[Intercept(typeof(AuditInterceptor))] +public class Orders : IOrders { + public int Count(string customer) { … } // audited + public Task CountAsync(string customer) { … } // DM0015 — not audited +} +``` + +This matters more than it first reads. An interceptor that rewrites arguments stops rewriting them; +one that authorises or audits stops doing that, on exactly the members most likely to be the +interesting ones. In the sharpest case — an `IInterceptor` applied to a service whose members are all +async — it never runs at all. + +Implement the missing interface on the interceptor, or apply it to a service with no such member. + +Reported once per interceptor and member shape, so a wide interface produces one line rather than +one per member. See [Interception](/guide/interception). diff --git a/website/reference/msbuild.md b/website/reference/msbuild.md index fe2bde2..797f886 100644 --- a/website/reference/msbuild.md +++ b/website/reference/msbuild.md @@ -6,11 +6,11 @@ when the packages are installed from NuGet. | Property | Default | | |---|---|---| -| `DependencyModules_GenerateFactories` | `false` | emit a `new` expression instead of `typeof(T)`, so the container does not construct by reflection | +| `DependencyModules_GenerateFactories` | `false` | emit a `new` expression instead of `typeof(T)`, so the container does not construct by reflection — [see the trade-off](#generatefactories-and-container-validation) | | `DependencyModules_RegistrationType` | `Add` | the default registration strategy for the project | | `DependencyModules_AutoGenerateModule` | `true` | generate `ApplicationModule` for a top-level `Program.cs` | | `DependencyModules_RegisterGenerator` | `false` | register discovered `JsonSerializerContext` types | -| `DependencyModules_ExcludeGeneratedCodeFromCoverage` | `true` | apply `[ExcludeFromCodeCoverage]` to generated members | +| `ExcludeGeneratedCodeFromCoverage` | `true` | apply `[ExcludeFromCodeCoverage]` to generated members — note this one carries no `DependencyModules_` prefix | | `DependencyModules_LogOutputDirectory` | *(none)* | write a generator log here — see [Troubleshooting](/guide/troubleshooting) | ```xml @@ -29,3 +29,31 @@ Not a DependencyModules property, but the one you will reach for most: true ``` + +## `GenerateFactories` and container validation {#generatefactories-and-container-validation} + +Worth knowing before turning this on project-wide. + +What it emits is a **factory** per registration: + +```csharp +services.AddSingleton( + typeof(OrderService), + provider => new OrderService(provider.GetRequiredService())); +``` + +`Microsoft.Extensions.DependencyInjection` cannot see inside a factory, so every registration in the +project becomes opaque to its own graph validation. Measured on the same captive dependency — a +singleton taking a scoped service — with only this property differing: + +| | `BuildServiceProvider(ValidateScopes + ValidateOnBuild)` | +|---|---| +| unset | throws — `Cannot consume scoped service 'IUnitOfWork' from singleton 'OrderService'` | +| `true` | builds cleanly | + +A missing registration goes the same way: the `GetRequiredService` call inside a factory is not +checked at build either, so it throws on first resolve instead. + +The property exists for startup cost and for Native AOT, which is exactly the setting a team turns on +late and everywhere. If you rely on `ValidateScopes` and `ValidateOnBuild` in development — and the +standard advice is to — keep this off there and turn it on for the published build.