From 28098f487452e9b75a644dae4354e09f014b8119 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 25 Aug 2026 21:56:09 -0400 Subject: [PATCH] Scope an interceptor to its own implementation, keep module defaults, diagnose a stranded assembly attribute DM-F02: an interceptor's wrapper is generated from one class and forwards that class's members, but it was applied to every registration of the service type - because interception reuses the decorator rewrite, and wrapping everything behind an interface is correct for a decorator and wrong for this. A sibling implementation carrying no [Intercept] came back wrapped in the first class's wrapper; with both marked, each registration was wrapped once per generated wrapper and every interceptor ran twice per call. Nothing threw. Decorate gains an overload taking the implementation type, and the generator names it. Two details this needed: * The filter is skipped when the descriptor's origin cannot be known - an instance, a hand-written factory, or the factories DependencyModules_GenerateFactories emits. Refusing to wrap there would silently stop intercepting a service that had asked for it, which is a worse failure than the one being fixed. * Decoration rewrites an implementation-type descriptor into a factory, which erases the implementation. An interceptor ordered outside a decorator would then fail to recognise its own registration, so the origin is carried across the replacement the way Applied already is. It is a separate overload rather than an optional parameter: generated code from an earlier version is compiled against the three-argument signature, and widening that one would leave it unable to bind. DM-F04: the generated module attribute assigned every property unconditionally unless it was declared nullable, so `public string Label { get; set; } = "x";` had its initialiser overwritten with null by a composition that never mentioned it. Nullability is an annotation rather than a runtime fact - the attribute property is null until assigned either way - so the guard is now emitted for every property. A value-typed property compares as always-true and is assigned unconditionally, which is unchanged and is the documented limit: 0 is a legitimate value, so null cannot express "unset" for one. DM-F05: DM0019 reports an assembly-level module attribute outside the entry point file. Those attributes are composed into the generated ApplicationModule, which is built from one compilation unit, so written anywhere else they were read by nobody - a clean build and an InvalidOperationException at the first resolve. It stays quiet when nothing generated an ApplicationModule, which is the test-project case the testing guide shows: there the attributes are read at run time, and there is no entry point to be in the wrong file relative to. Nine tests added. Two of the three interception ones fail without the fix; the third guards the decorator-interleaving path the fix could have broken. The value-typed parameter test pins the accepted limit rather than a fix. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01R9DWh8FWTib6hRYURbypWU --- .../DefaultedParameterModule.cs | 46 +++++++ .../DefaultedParameterModuleTests.cs | 31 +++++ .../Helpers/DecoratorHelper.cs | 86 +++++++++++- .../AnalyzerReleases.Unshipped.md | 1 + .../DependencyModuleDiagnostics.cs | 27 ++++ .../InterceptorRegistrationWriter.cs | 8 +- .../ModuleAttributeWriter.cs | 18 ++- .../AssemblyModuleAttributeDiagnostics.cs | 51 +++++-- ...AssemblyModuleAttributeDiagnosticsTests.cs | 65 +++++++++ .../GeneratorTests/InterceptionScopeTests.cs | 128 ++++++++++++++++++ ...ructorParametersAndProperties.verified.txt | 5 +- .../PublicApiTests.RuntimeApi.verified.txt | 2 + ...icApiTests.SourceGeneratorApi.verified.txt | 1 + 13 files changed, 450 insertions(+), 19 deletions(-) create mode 100644 integ-tests/SutProject.Tests/ParameterizedModuleTests/DefaultedParameterModule.cs create mode 100644 integ-tests/SutProject.Tests/ParameterizedModuleTests/DefaultedParameterModuleTests.cs create mode 100644 tests/DependencyModules.Tests/GeneratorTests/InterceptionScopeTests.cs diff --git a/integ-tests/SutProject.Tests/ParameterizedModuleTests/DefaultedParameterModule.cs b/integ-tests/SutProject.Tests/ParameterizedModuleTests/DefaultedParameterModule.cs new file mode 100644 index 0000000..88c9fe6 --- /dev/null +++ b/integ-tests/SutProject.Tests/ParameterizedModuleTests/DefaultedParameterModule.cs @@ -0,0 +1,46 @@ +using DependencyModules.Runtime.Attributes; +using DependencyModules.Runtime.Interfaces; +using Microsoft.Extensions.DependencyInjection; + +namespace SutProject.Tests.ParameterizedModuleTests; + +public class DefaultedParameterValues(string label, int size) { + public string Label { get; } = label; + + public int Size { get; } = size; +} + +/// +/// A module whose parameters carry C# initialisers, one of each kind that matters. +/// +/// +/// Label is a non-nullable reference type, which is the shape that used to lose its default: +/// the generated attribute assigned every property unconditionally unless it was declared nullable, +/// so composing this module without naming Label wrote null over "default-label" and +/// the failure surfaced as a NullReferenceException inside ConfigureServices. +/// +/// Size is a value type and is included to pin the documented limit rather than a fix: an +/// attribute property of type int is 0 until assigned and 0 is a legitimate value, so null +/// cannot express "unset" for one. Its initialiser is not preserved, and that is the accepted +/// behaviour. +/// +[DependencyModule] +public partial class DefaultedParameterModule : IServiceCollectionConfiguration { + public string Label { get; set; } = "default-label"; + + public int Size { get; set; } = 42; + + public void ConfigureServices(IServiceCollection services) { + services.AddSingleton(new DefaultedParameterValues(Label, Size)); + } +} + +/// Composes the module above without naming either parameter. +[DependencyModule] +[DefaultedParameterModule] +public partial class DefaultedParameterComposer; + +/// Composes it while naming both, which has always worked. +[DependencyModule] +[DefaultedParameterModule(Label = "named-label", Size = 7)] +public partial class NamedParameterComposer; diff --git a/integ-tests/SutProject.Tests/ParameterizedModuleTests/DefaultedParameterModuleTests.cs b/integ-tests/SutProject.Tests/ParameterizedModuleTests/DefaultedParameterModuleTests.cs new file mode 100644 index 0000000..362c215 --- /dev/null +++ b/integ-tests/SutProject.Tests/ParameterizedModuleTests/DefaultedParameterModuleTests.cs @@ -0,0 +1,31 @@ +using DependencyModules.xUnit.Attributes; +using Xunit; + +namespace SutProject.Tests.ParameterizedModuleTests; + +public class DefaultedParameterModuleTests { + + /// + /// A composition that names no parameters leaves the module's own initialiser in place. + /// + [ModuleTest(typeof(DefaultedParameterComposer))] + public void AnUnnamedReferenceParameter_KeepsItsDefault(DefaultedParameterValues values) { + Assert.Equal("default-label", values.Label); + } + + /// + /// The documented limit, pinned so it is a decision rather than a surprise: an attribute + /// property of a value type cannot tell "not supplied" from the type's default, so a value-typed + /// module parameter's initialiser does not survive composition by attribute. + /// + [ModuleTest(typeof(DefaultedParameterComposer))] + public void AnUnnamedValueParameter_DoesNotKeepItsDefault(DefaultedParameterValues values) { + Assert.Equal(0, values.Size); + } + + [ModuleTest(typeof(NamedParameterComposer))] + public void NamedParameters_AreCarriedAcross(DefaultedParameterValues values) { + Assert.Equal("named-label", values.Label); + Assert.Equal(7, values.Size); + } +} diff --git a/src/DependencyModules.Runtime/Helpers/DecoratorHelper.cs b/src/DependencyModules.Runtime/Helpers/DecoratorHelper.cs index 7cfb4ed..01976c8 100644 --- a/src/DependencyModules.Runtime/Helpers/DecoratorHelper.cs +++ b/src/DependencyModules.Runtime/Helpers/DecoratorHelper.cs @@ -144,12 +144,53 @@ public static void Decorate( Type decoratorIdentity, Func decoratorFactory) where TService : class { + Decorate(services, decoratorIdentity, decoratorFactory, null); + } + + /// + /// Wraps only the registration of that was built from one + /// particular implementation. + /// + /// The collection to rewrite. + /// + /// The wrapper type, used to refuse a second application of the same wrapper to one registration. + /// + /// + /// Builds the wrapper given the provider and the instance being wrapped. + /// + /// + /// Restricts the rewrite to the registration built from this implementation. Null wraps every + /// registration of the service, which is what a decorator declared against an interface should + /// do — so decoration keeps calling the three-argument overload. + /// + /// + /// + /// An interceptor passes one. Its wrapper is generated from a single class and forwards that + /// class's members, so applying it to a sibling implementation of the same interface wrapped a + /// service that never asked to be intercepted, and reported the sibling's type as the first + /// class's wrapper. With both implementations marked, each registration was wrapped once per + /// generated wrapper and every interceptor ran twice per call — no exception, just doubled + /// metrics, audit rows and retries. + /// + /// + /// A separate overload rather than an optional parameter on the one above: generated code from + /// an earlier version is compiled against the three-argument signature, and widening that one + /// would leave it unable to bind against a newer runtime. + /// + /// + public static void Decorate( + IServiceCollection services, + Type decoratorIdentity, + Func decoratorFactory, + Type? implementationType) where TService : class { + Decorate( services, typeof(TService), (provider, inner) => decoratorFactory(provider, (TService)inner), null, - decoratorIdentity); + decoratorIdentity, + implementationType); } /// @@ -201,12 +242,41 @@ private static void RecordApplied( Applied.Add(replacement, applied); } + /// + /// The implementation a descriptor was originally built from, across rewrites. + /// + /// + /// Decoration replaces an implementation-type descriptor with a factory, which erases the + /// implementation from the descriptor itself. An interceptor ordered after a decorator would + /// then be unable to recognise its own registration, so the origin is carried across the + /// replacement the same way is. + /// + private static readonly ConditionalWeakTable OriginImplementation = new(); + + /// + /// The implementation behind a descriptor, or null when it cannot be known — a registration made + /// from an instance or a hand-written factory, including the factories + /// DependencyModules_GenerateFactories emits. + /// + private static Type? OriginImplementationOf(ServiceDescriptor descriptor) => + OriginImplementation.TryGetValue(descriptor, out var origin) ? origin : ImplementationOf(descriptor); + + private static void RecordOrigin(ServiceDescriptor original, ServiceDescriptor replacement) { + if (OriginImplementationOf(original) is not { } origin) { + return; + } + + OriginImplementation.Remove(replacement); + OriginImplementation.Add(replacement, origin); + } + private static void Decorate( IServiceCollection services, Type serviceType, Func decoratorFactory, Type? decoratorType, - Type? decoratorIdentity) { + Type? decoratorIdentity, + Type? implementationType = null) { var ordinal = 0; @@ -228,6 +298,17 @@ private static void Decorate( continue; } + // An interceptor's wrapper belongs to the one implementation it was generated from. + // Skipped only when the descriptor's origin is known and is a different type: a + // registration made from an instance or a factory cannot be attributed to an + // implementation, and refusing to wrap it there would silently stop intercepting a + // service that had asked for it — a worse failure than the one this filter prevents. + if (implementationType != null && + OriginImplementationOf(descriptor) is { } origin && + origin != implementationType) { + continue; + } + GuardOpenGenericRegistration(descriptor, decoratorType); // Captured before the slot is overwritten. Reading services[i] inside the factory would @@ -248,6 +329,7 @@ private static void Decorate( descriptor.Lifetime); RecordApplied(descriptor, replacement, decoratorIdentity); + RecordOrigin(descriptor, replacement); services[i] = replacement; } diff --git a/src/DependencyModules.SourceGenerator.Impl/AnalyzerReleases.Unshipped.md b/src/DependencyModules.SourceGenerator.Impl/AnalyzerReleases.Unshipped.md index fa9d395..ad3085e 100644 --- a/src/DependencyModules.SourceGenerator.Impl/AnalyzerReleases.Unshipped.md +++ b/src/DependencyModules.SourceGenerator.Impl/AnalyzerReleases.Unshipped.md @@ -7,3 +7,4 @@ Rule ID | Category | Severity | Notes --------|----------|----------|------- DM0017 | DependencyModules | Error | A dependency module is declared inside another type. DM0018 | DependencyModules | Info | A module with settable properties relies on generated equality. +DM0019 | DependencyModules | Error | An assembly-level module attribute is outside the entry point file. diff --git a/src/DependencyModules.SourceGenerator.Impl/DependencyModuleDiagnostics.cs b/src/DependencyModules.SourceGenerator.Impl/DependencyModuleDiagnostics.cs index cb5a394..5d06e80 100644 --- a/src/DependencyModules.SourceGenerator.Impl/DependencyModuleDiagnostics.cs +++ b/src/DependencyModules.SourceGenerator.Impl/DependencyModuleDiagnostics.cs @@ -77,6 +77,33 @@ public static class DependencyModuleDiagnostics { defaultSeverity: DiagnosticSeverity.Error, isEnabledByDefault: true); + /// + /// Raised for an assembly-level module attribute in a file that is not the entry point. + /// + /// + /// Assembly-level module attributes are composed into the generated ApplicationModule, + /// and that module is built from one compilation unit — the entry point. An attribute written + /// anywhere else was read by nobody: a clean build, no diagnostic, and an + /// InvalidOperationException at the first resolve, which is the failure the library + /// exists to prevent. + /// + /// The testing guide shows the same syntax living in its own file, and it works there, because + /// the test integration reads assembly attributes at run time rather than at generation time. + /// That asymmetry is what makes the mistake easy to make, and it is also why this stays quiet + /// when nothing generated an ApplicationModule — a class library or a test project has + /// no entry point to be in the wrong file relative to. + /// + public static readonly DiagnosticDescriptor AssemblyModuleAttributeNotComposed = new( + id: "DM0019", + title: "Assembly-level module attribute is not composed", + messageFormat: + "'{0}' is applied at the assembly level in this file, but the generated ApplicationModule is " + + "built from '{1}', so this composition is ignored and the module's services are not " + + "registered. Move the attribute to '{1}', or load the module explicitly with AddModule.", + category: Category, + defaultSeverity: DiagnosticSeverity.Error, + isEnabledByDefault: true); + /// /// Raised for a module carrying settable properties while relying on the generated /// Equals/GetHashCode. diff --git a/src/DependencyModules.SourceGenerator.Impl/InterceptorRegistrationWriter.cs b/src/DependencyModules.SourceGenerator.Impl/InterceptorRegistrationWriter.cs index 0e86219..37290e8 100644 --- a/src/DependencyModules.SourceGenerator.Impl/InterceptorRegistrationWriter.cs +++ b/src/DependencyModules.SourceGenerator.Impl/InterceptorRegistrationWriter.cs @@ -134,6 +134,11 @@ private static void WriteInterceptor( "provider", "GetRequiredService", new[] { model.Interceptors[i].Type })); } + // The implementation is named so the rewrite lands only on the registration this + // wrapper was generated from. Without it a sibling implementation of the same interface + // — one carrying no [Intercept] at all — came back wrapped in this class's wrapper, and + // two intercepted implementations wrapped each other's registrations so every + // interceptor ran twice per call. method.AddIndentedStatement( SyntaxHelpers.InvokeGeneric( KnownTypes.DependencyModules.Helpers.DecoratorHelper, @@ -144,7 +149,8 @@ private static void WriteInterceptor( new WrapStatement( CodeOutputComponent.Get(" => "), CodeOutputComponent.Get("(provider, inner)"), - New(wrapperType, arguments.ToArray())))); + New(wrapperType, arguments.ToArray())), + TypeOf(model.ImplementationType))); } // A field initializer registers the method, matching how decorator registrations are hooked diff --git a/src/DependencyModules.SourceGenerator.Impl/ModuleAttributeWriter.cs b/src/DependencyModules.SourceGenerator.Impl/ModuleAttributeWriter.cs index f694a46..99d9398 100644 --- a/src/DependencyModules.SourceGenerator.Impl/ModuleAttributeWriter.cs +++ b/src/DependencyModules.SourceGenerator.Impl/ModuleAttributeWriter.cs @@ -23,13 +23,17 @@ protected override void CustomImplementation(IConstructContainer container, Clas continue; } - BaseBlockDefinition block = method; - - if (propertyInfoModel.PropertyType.IsNullable) { - block = - method.If(NotEquals(propertyInfoModel.PropertyName, Null())); - - } + // Guarded whatever the declared nullability. An attribute property is null until + // somebody assigns it, and `?` is an annotation rather than a runtime fact — so gating + // the guard on it meant `public string Label { get; set; } = "default";` had its + // initialiser overwritten with null by a composition that never mentioned Label, while + // the same property written `string?` survived. + // + // A value-typed property compares as always-true here and is assigned unconditionally, + // which is the existing behaviour: `int` is 0 until assigned and 0 is a legitimate + // value, so null cannot express "unset" for one either way. CS0472 says as much, and + // BaseAttributeWriter already wraps the class in a pragma for it. + var block = method.If(NotEquals(propertyInfoModel.PropertyName, Null())); block.Assign(propertyInfoModel.PropertyName).To(newModule.Property(propertyInfoModel.PropertyName)); } diff --git a/src/DependencyModules.SourceGenerator/AssemblyModuleAttributeDiagnostics.cs b/src/DependencyModules.SourceGenerator/AssemblyModuleAttributeDiagnostics.cs index 6b0c742..576f157 100644 --- a/src/DependencyModules.SourceGenerator/AssemblyModuleAttributeDiagnostics.cs +++ b/src/DependencyModules.SourceGenerator/AssemblyModuleAttributeDiagnostics.cs @@ -32,14 +32,19 @@ internal static class AssemblyModuleAttributeDiagnostics { internal sealed class UnitModel : IEquatable { public UnitModel( + string filePath, ImmutableArray usages, ImmutableArray fileUsings, ImmutableArray globalUsings) { + FilePath = filePath; Usages = usages; FileUsings = fileUsings; GlobalUsings = globalUsings; } + /// The file these attributes were written in. + public string FilePath { get; } + public ImmutableArray Usages { get; } /// Namespaces this file imports, which apply only to this file. @@ -50,6 +55,7 @@ public UnitModel( public bool Equals(UnitModel? other) => other != null && + FilePath == other.FilePath && Usages.SequenceEqual(other.Usages) && FileUsings.SequenceEqual(other.FileUsings) && GlobalUsings.SequenceEqual(other.GlobalUsings); @@ -125,7 +131,11 @@ private static UnitModel Read(GeneratorSyntaxContext context, CancellationToken } } - return new UnitModel(usages.ToImmutable(), fileUsings.ToImmutable(), globalUsings.ToImmutable()); + return new UnitModel( + unit.SyntaxTree.FilePath, + usages.ToImmutable(), + fileUsings.ToImmutable(), + globalUsings.ToImmutable()); } internal static void Report( @@ -156,6 +166,19 @@ internal static void Report( return; } + // The file the generated ApplicationModule was built from, which is the only file whose + // assembly-level module attributes are composed into anything. Null when nothing generated + // one — a class library, or a test project, where assembly attributes are read at run time + // by the test integration instead and are perfectly at home in any file. + string? entryPointFile = null; + + foreach (var (module, _) in input.Modules) { + if (module.ModuleFeatures.HasFlag(ModuleEntryPointFeatures.AutoGenerateModule)) { + entryPointFile = module.FileLocation; + break; + } + } + var globalUsings = new HashSet(StringComparer.Ordinal); foreach (var unit in input.Units) { @@ -177,16 +200,28 @@ internal static void Report( continue; } - if (unit.FileUsings.Contains(moduleNamespace) || globalUsings.Contains(moduleNamespace)) { + if (!unit.FileUsings.Contains(moduleNamespace) && !globalUsings.Contains(moduleNamespace)) { + context.ReportDiagnostic( + Diagnostic.Create( + DependencyModuleDiagnostics.ModuleAttributeNamespaceNotImported, + usage.Location, + usage.Name, + moduleNamespace)); + + // It does not compile yet, so which file it belongs in is a later question. continue; } - context.ReportDiagnostic( - Diagnostic.Create( - DependencyModuleDiagnostics.ModuleAttributeNamespaceNotImported, - usage.Location, - usage.Name, - moduleNamespace)); + if (entryPointFile != null && + !string.IsNullOrEmpty(unit.FilePath) && + !string.Equals(unit.FilePath, entryPointFile, StringComparison.Ordinal)) { + context.ReportDiagnostic( + Diagnostic.Create( + DependencyModuleDiagnostics.AssemblyModuleAttributeNotComposed, + usage.Location, + usage.Name, + System.IO.Path.GetFileName(entryPointFile))); + } } } } diff --git a/tests/DependencyModules.Tests/GeneratorTests/AssemblyModuleAttributeDiagnosticsTests.cs b/tests/DependencyModules.Tests/GeneratorTests/AssemblyModuleAttributeDiagnosticsTests.cs index 90a548a..2ba099a 100644 --- a/tests/DependencyModules.Tests/GeneratorTests/AssemblyModuleAttributeDiagnosticsTests.cs +++ b/tests/DependencyModules.Tests/GeneratorTests/AssemblyModuleAttributeDiagnosticsTests.cs @@ -115,10 +115,75 @@ public void AUsingAlias_DoesNotCountAsTheImport() { Assert.Single(result.GeneratorDiagnostics, d => d.Id == "DM0016"); } + /// + /// DM0019. Assembly-level module attributes are composed into the generated + /// ApplicationModule, which is built from one compilation unit. Written anywhere else + /// they were read by nobody: a clean build, and an InvalidOperationException at the first + /// resolve. + /// + [Fact] + public void AnAssemblyAttributeOutsideTheEntryPointFile_IsReported() { + var result = RunWithEntryPoint( + bootstrap: + """ + using MyApp.Composition; + + [assembly: ApplicationModule] + """, + program: "System.Console.WriteLine();"); + + var diagnostic = Assert.Single(result.GeneratorDiagnostics, d => d.Id == "DM0019"); + + Assert.Contains("ApplicationModule", diagnostic.GetMessage()); + Assert.Contains("Program.cs", diagnostic.GetMessage()); + } + + [Fact] + public void AnAssemblyAttributeInTheEntryPointFile_IsSilent() { + var result = RunWithEntryPoint( + bootstrap: "", + program: + """ + using MyApp.Composition; + + [assembly: ApplicationModule] + + System.Console.WriteLine(); + """); + + Assert.DoesNotContain(result.GeneratorDiagnostics, d => d.Id == "DM0019"); + } + + /// + /// The case that must stay quiet. A test project has no entry point of its own, so nothing + /// generates an ApplicationModule and there is no file for the attribute to be in the wrong one + /// relative to — and the test integration reads assembly attributes at run time anyway, which is + /// what the testing guide shows. + /// + [Fact] + public void WithNoGeneratedApplicationModule_IsSilent() { + var result = Run( + """ + using MyApp.Composition; + + [assembly: ApplicationModule] + """); + + Assert.DoesNotContain(result.GeneratorDiagnostics, d => d.Id == "DM0019"); + } + private static GeneratorResult Run(string bootstrap) => GeneratorTestHarness.Run( new Dictionary { ["Module.cs"] = ModuleInNamespace, ["Bootstrap.cs"] = bootstrap }); + + private static GeneratorResult RunWithEntryPoint(string bootstrap, string program) => + GeneratorTestHarness.Run( + new Dictionary { + ["Module.cs"] = ModuleInNamespace, + ["Bootstrap.cs"] = bootstrap, + ["Program.cs"] = program + }); } diff --git a/tests/DependencyModules.Tests/GeneratorTests/InterceptionScopeTests.cs b/tests/DependencyModules.Tests/GeneratorTests/InterceptionScopeTests.cs new file mode 100644 index 0000000..4189126 --- /dev/null +++ b/tests/DependencyModules.Tests/GeneratorTests/InterceptionScopeTests.cs @@ -0,0 +1,128 @@ +using System.Linq; +using DependencyModules.Tests.Infrastructure; +using Microsoft.Extensions.DependencyInjection; +using Xunit; + +namespace DependencyModules.Tests.GeneratorTests; + +/// +/// An interceptor's wrapper is generated from one class and forwards that class's members, so it +/// belongs to that class's registration and no other. +/// +/// It used to be applied to every registration of the service type, because interception +/// reuses the decorator rewrite and a decorator is declared against an interface — where wrapping +/// everything behind it is correct. The symptoms were an implementation carrying no +/// [Intercept] coming back wrapped in another class's wrapper, and, with two implementations +/// marked, every interceptor running twice per call. Neither threw. +/// +public class InterceptionScopeTests { + + private const string Interceptor = + """ + public sealed class CountingInterceptor : IInterceptor { + public static int Calls; + public TResult Intercept(InvocationContext context) { + Calls++; + return context.Proceed(); + } + } + """; + + [Fact] + public void AnUnmarkedSiblingImplementation_IsNotWrapped() { + var generated = GeneratedAssembly.Create( + Source( + """ + [SingletonService] [Intercept(typeof(CountingInterceptor))] + public sealed class Loud : IGreeter { public string Greet() => "loud"; } + + [SingletonService] + public sealed class Quiet : IGreeter { public string Greet() => "quiet"; } + """)); + + var provider = generated.BuildProvider(); + + var resolved = ((System.Collections.IEnumerable)provider + .GetService(typeof(System.Collections.Generic.IEnumerable<>) + .MakeGenericType(generated.Type("IGreeter")))!) + .Cast() + .Select(g => g.GetType().Name) + .OrderBy(n => n) + .ToArray(); + + Assert.Equal(new[] { "Loud_Intercepted", "Quiet" }, resolved); + } + + [Fact] + public void TwoMarkedImplementations_EachGetTheirOwnWrapper() { + var generated = GeneratedAssembly.Create( + Source( + """ + [SingletonService] [Intercept(typeof(CountingInterceptor))] + public sealed class Loud : IGreeter { public string Greet() => "loud"; } + + [SingletonService] [Intercept(typeof(CountingInterceptor))] + public sealed class Quiet : IGreeter { public string Greet() => "quiet"; } + """)); + + var provider = generated.BuildProvider(); + + var resolved = ((System.Collections.IEnumerable)provider + .GetService(typeof(System.Collections.Generic.IEnumerable<>) + .MakeGenericType(generated.Type("IGreeter")))!) + .Cast() + .Select(g => g.GetType().Name) + .OrderBy(n => n) + .ToArray(); + + // Each behind its own wrapper, rather than both behind whichever was emitted last. + Assert.Equal(new[] { "Loud_Intercepted", "Quiet_Intercepted" }, resolved); + } + + /// + /// The registration is rewritten into a factory by the decorator, which erases the + /// implementation type from the descriptor. An interceptor ordered outside it has to recognise + /// its own registration anyway, or narrowing the rewrite would silently stop intercepting a + /// service that had asked for it. + /// + [Fact] + public void AnInterceptorOrderedOutsideADecorator_StillApplies() { + var generated = GeneratedAssembly.Create( + Source( + """ + [SingletonService] [Intercept(typeof(CountingInterceptor), Order = 2000)] + public sealed class Core : IGreeter { public string Greet() => "core"; } + + [Decorator(Order = 1000)] + public sealed class Bracketed(IGreeter inner) : IGreeter { + public string Greet() => "[" + inner.Greet() + "]"; + } + """)); + + var resolved = generated.ResolveRequired("IGreeter"); + + Assert.Equal("Core_Intercepted", resolved.GetType().Name); + + // The decorator is still inside the interception wrapper, so both ran. + var greet = (string)resolved.GetType().GetMethod("Greet")!.Invoke(resolved, null)!; + + Assert.Equal("[core]", greet); + } + + private static string Source(string body) => + $$""" + using DependencyModules.Runtime.Attributes; + using DependencyModules.Runtime.Interception; + + namespace TestNamespace; + + public interface IGreeter { string Greet(); } + + {{Interceptor}} + + {{body}} + + [DependencyModule] + public partial class TestModule; + """; +} diff --git a/tests/DependencyModules.Tests/Snapshots/ModuleGenerationSnapshotTests.ModuleWithConstructorParametersAndProperties.verified.txt b/tests/DependencyModules.Tests/Snapshots/ModuleGenerationSnapshotTests.ModuleWithConstructorParametersAndProperties.verified.txt index 611fd2f..5ee3477 100644 --- a/tests/DependencyModules.Tests/Snapshots/ModuleGenerationSnapshotTests.ModuleWithConstructorParametersAndProperties.verified.txt +++ b/tests/DependencyModules.Tests/Snapshots/ModuleGenerationSnapshotTests.ModuleWithConstructorParametersAndProperties.verified.txt @@ -105,7 +105,10 @@ namespace TestNamespace public global::DependencyModules.Runtime.Interfaces.IDependencyModule GetModule() { var newModule = new global::TestNamespace.TestModule(someFlagField); - newModule.OptionalString = OptionalString; + if (OptionalString != null) + { + newModule.OptionalString = OptionalString; + } return newModule; } } diff --git a/tests/DependencyModules.Tests/Snapshots/PublicApiTests.RuntimeApi.verified.txt b/tests/DependencyModules.Tests/Snapshots/PublicApiTests.RuntimeApi.verified.txt index 33fc788..e1348fd 100644 --- a/tests/DependencyModules.Tests/Snapshots/PublicApiTests.RuntimeApi.verified.txt +++ b/tests/DependencyModules.Tests/Snapshots/PublicApiTests.RuntimeApi.verified.txt @@ -190,6 +190,8 @@ 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 Decorate(Microsoft.Extensions.DependencyInjection.IServiceCollection services, System.Type decoratorIdentity, System.Func decoratorFactory, System.Type? implementationType) + 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 bdf9ec7..0a25ddc 100644 --- a/tests/DependencyModules.Tests/Snapshots/PublicApiTests.SourceGeneratorApi.verified.txt +++ b/tests/DependencyModules.Tests/Snapshots/PublicApiTests.SourceGeneratorApi.verified.txt @@ -1067,6 +1067,7 @@ namespace DependencyModules.SourceGenerator.Impl { public static readonly Microsoft.CodeAnalysis.DiagnosticDescriptor AmbiguousConventionMatch; public static readonly Microsoft.CodeAnalysis.DiagnosticDescriptor AmbiguousDecoratorOrder; + public static readonly Microsoft.CodeAnalysis.DiagnosticDescriptor AssemblyModuleAttributeNotComposed; public static readonly Microsoft.CodeAnalysis.DiagnosticDescriptor CannotIntercept; public static readonly Microsoft.CodeAnalysis.DiagnosticDescriptor ConventionCannotBeRead; public static readonly Microsoft.CodeAnalysis.DiagnosticDescriptor ConventionMatchNotConstructable;