Scope an interceptor to its own implementation, keep module defaults, diagnose a stranded assembly attribute

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;
+}
+
+/// <summary>
+/// A module whose parameters carry C# initialisers, one of each kind that matters.
+/// </summary>
+/// <remarks>
+/// <c>Label</c> 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 <c>Label</c> wrote null over <c>"default-label"</c> and
+/// the failure surfaced as a NullReferenceException inside ConfigureServices.
+///
+/// <c>Size</c> is a value type and is included to pin the documented limit rather than a fix: an
+/// attribute property of type <c>int</c> 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.
+/// </remarks>
+[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));
+    }
+}
+
+/// <summary>Composes the module above without naming either parameter.</summary>
+[DependencyModule]
+[DefaultedParameterModule]
+public partial class DefaultedParameterComposer;
+
+/// <summary>Composes it while naming both, which has always worked.</summary>
+[DependencyModule]
+[DefaultedParameterModule(Label = "named-label", Size = 7)]
+public partial class NamedParameterComposer;

integ-tests/SutProject.Tests/ParameterizedModuleTests/DefaultedParameterModuleTests.cs

@@ -0,0 +1,31 @@
+using DependencyModules.xUnit.Attributes;
+using Xunit;
+
+namespace SutProject.Tests.ParameterizedModuleTests;
+
+public class DefaultedParameterModuleTests {
+
+    /// <summary>
+    /// A composition that names no parameters leaves the module's own initialiser in place.
+    /// </summary>
+    [ModuleTest(typeof(DefaultedParameterComposer))]
+    public void AnUnnamedReferenceParameter_KeepsItsDefault(DefaultedParameterValues values) {
+        Assert.Equal("default-label", values.Label);
+    }
+
+    /// <summary>
+    /// 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.
+    /// </summary>
+    [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);
+    }
+}

src/DependencyModules.Runtime/Helpers/DecoratorHelper.cs

@@ -144,12 +144,53 @@ public static void Decorate<TService>(
         Type decoratorIdentity,
         Func<IServiceProvider, TService, TService> decoratorFactory) where TService : class {
 
+        Decorate(services, decoratorIdentity, decoratorFactory, null);
+    }
+
+    /// <summary>
+    /// Wraps only the registration of <typeparamref name="TService"/> that was built from one
+    /// particular implementation.
+    /// </summary>
+    /// <param name="services">The collection to rewrite.</param>
+    /// <param name="decoratorIdentity">
+    /// The wrapper type, used to refuse a second application of the same wrapper to one registration.
+    /// </param>
+    /// <param name="decoratorFactory">
+    /// Builds the wrapper given the provider and the instance being wrapped.
+    /// </param>
+    /// <param name="implementationType">
+    /// 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.
+    /// </param>
+    /// <remarks>
+    /// <para>
+    /// 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.
+    /// </para>
+    /// <para>
+    /// 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.
+    /// </para>
+    /// </remarks>
+    public static void Decorate<TService>(
+        IServiceCollection services,
+        Type decoratorIdentity,
+        Func<IServiceProvider, TService, TService> decoratorFactory,
+        Type? implementationType) where TService : class {
+
         Decorate(
             services,
             typeof(TService),
             (provider, inner) => decoratorFactory(provider, (TService)inner),
             null,
-            decoratorIdentity);
+            decoratorIdentity,
+            implementationType);
     }
 
     /// <summary>
@@ -201,12 +242,41 @@ private static void RecordApplied(
         Applied.Add(replacement, applied);
     }
 
+    /// <summary>
+    /// The implementation a descriptor was originally built from, across rewrites.
+    /// </summary>
+    /// <remarks>
+    /// 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 <see cref="Applied"/> is.
+    /// </remarks>
+    private static readonly ConditionalWeakTable<ServiceDescriptor, Type> OriginImplementation = new();
+
+    /// <summary>
+    /// 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
+    /// <c>DependencyModules_GenerateFactories</c> emits.
+    /// </summary>
+    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<IServiceProvider, object, object> 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;
         }

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.

src/DependencyModules.SourceGenerator.Impl/DependencyModuleDiagnostics.cs

@@ -77,6 +77,33 @@ public static class DependencyModuleDiagnostics {
         defaultSeverity: DiagnosticSeverity.Error,
         isEnabledByDefault: true);
 
+    /// <summary>
+    /// Raised for an assembly-level module attribute in a file that is not the entry point.
+    /// </summary>
+    /// <remarks>
+    /// Assembly-level module attributes are composed into the generated <c>ApplicationModule</c>,
+    /// 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
+    /// <c>InvalidOperationException</c> 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 <c>ApplicationModule</c> — a class library or a test project has
+    /// no entry point to be in the wrong file relative to.
+    /// </remarks>
+    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);
+
     /// <summary>
     /// Raised for a module carrying settable properties while relying on the generated
     /// <c>Equals</c>/<c>GetHashCode</c>.

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

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));
         }

src/DependencyModules.SourceGenerator/AssemblyModuleAttributeDiagnostics.cs

@@ -32,14 +32,19 @@ internal static class AssemblyModuleAttributeDiagnostics {
     internal sealed class UnitModel : IEquatable<UnitModel> {
 
         public UnitModel(
+            string filePath,
             ImmutableArray<Usage> usages,
             ImmutableArray<string> fileUsings,
             ImmutableArray<string> globalUsings) {
+            FilePath = filePath;
             Usages = usages;
             FileUsings = fileUsings;
             GlobalUsings = globalUsings;
         }
 
+        /// <summary>The file these attributes were written in.</summary>
+        public string FilePath { get; }
+
         public ImmutableArray<Usage> Usages { get; }
 
         /// <summary>Namespaces this file imports, which apply only to this file.</summary>
@@ -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<string>(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)));
+                }
             }
         }
     }