From cad625723f02d3a04987fc88c1cdf8e026601a87 Mon Sep 17 00:00:00 2001 From: Ian Johnson Date: Thu, 24 Sep 2026 07:44:22 -0400 Subject: [PATCH] Read a module's parameters and equality from its own members, and skip modules DM0003 or DM0017 rejects A property with a private set accessor became a module parameter, and so did every settable property of a class nested in the module. Both gave generated code that did not compile. Only the module's own properties count now, and the set accessor must be one the attribute can use. The Equals check looked at one declaration and took any method named Equals. It now reads the symbol, so Equals and GetHashCode in another partial declaration count. A module that declares only Equals for its own type gets a generated Equals(object) that calls it, and GetHashCode is generated only when the module has none. RegistrationTargets now leaves out a module that is not partial or is nested, so no writer emits a partial class for it. Co-Authored-By: Claude Opus 5.5 (1M context) --- .../BaseSourceGenerator.cs | 72 +++++- .../DependencyModuleWriter.cs | 16 +- .../Models/ModuleEntryPointModel.cs | 11 + .../Models/PropertyInfoModel.cs | 7 +- .../ModuleEntryPointDiagnostics.cs | 3 +- .../Utilities/AttributeModelHelper.cs | 53 ++-- .../Utilities/EntryModelUtil.cs | 10 +- .../GeneratorTests/ModuleDeclarationTests.cs | 241 ++++++++++++++++++ ...icApiTests.SourceGeneratorApi.verified.txt | 2 + 9 files changed, 372 insertions(+), 43 deletions(-) create mode 100644 tests/DependencyModules.Tests/GeneratorTests/ModuleDeclarationTests.cs diff --git a/src/DependencyModules.SourceGenerator.Impl/BaseSourceGenerator.cs b/src/DependencyModules.SourceGenerator.Impl/BaseSourceGenerator.cs index c8507b0..9e8c5e2 100644 --- a/src/DependencyModules.SourceGenerator.Impl/BaseSourceGenerator.cs +++ b/src/DependencyModules.SourceGenerator.Impl/BaseSourceGenerator.cs @@ -327,7 +327,6 @@ typeDefinition is GenericTypeDefinition } var dependencyFlags = GetDependencyFlags(context); - var implementsEqualsFlag = GetEqualsFlag(context); var modelInfo = AttributeModelHelper.GetAttributeClassInfo(context, cancellation); if (dependencyFlags.OnlyRealm) @@ -339,9 +338,9 @@ typeDefinition is GenericTypeDefinition { features |= ModuleEntryPointFeatures.IsRecord; } - else if (!implementsEqualsFlag) + else { - features |= ModuleEntryPointFeatures.ShouldImplementEquals; + features |= GetEqualityFeatures(context, typeDeclarationSyntax, cancellation); } if (!typeDeclarationSyntax.Modifiers.Any(m => m.IsKind(SyntaxKind.PartialKeyword))) @@ -439,12 +438,69 @@ syntax is GlobalStatementSyntax ); } - private bool GetEqualsFlag(GeneratorSyntaxContext context) + /// + /// Which equality members the generator writes for a module. + /// + /// + /// Read from the symbol, so a member in any partial declaration counts. Only + /// Equals(object) is what the load operation calls, so a module that declares only + /// Equals for its own type still gets a generated Equals(object), which calls it. + /// + private static ModuleEntryPointFeatures GetEqualityFeatures( + GeneratorSyntaxContext context, + TypeDeclarationSyntax typeDeclarationSyntax, + CancellationToken cancellation + ) { - return context - .Node.DescendantNodes() - .OfType() - .Any(m => m.Identifier.ToString().Equals("Equals")); + if ( + context.SemanticModel.GetDeclaredSymbol(typeDeclarationSyntax, cancellation) + is not { } module + ) + { + return ModuleEntryPointFeatures.ShouldImplementEquals + | ModuleEntryPointFeatures.ShouldImplementGetHashCode; + } + + var features = ModuleEntryPointFeatures.None; + + var declaresEqualsObject = false; + + foreach (var member in module.GetMembers("Equals")) + { + if (member is not IMethodSymbol { IsStatic: false, Parameters.Length: 1 } equals) + { + continue; + } + + var parameterType = equals.Parameters[0].Type; + + if (parameterType.SpecialType == SpecialType.System_Object) + { + declaresEqualsObject = true; + } + else if (SymbolEqualityComparer.Default.Equals(parameterType, module)) + { + features |= ModuleEntryPointFeatures.DeclaresTypedEquals; + } + } + + if (declaresEqualsObject) + { + return features; + } + + features |= ModuleEntryPointFeatures.ShouldImplementEquals; + + var declaresGetHashCode = module + .GetMembers("GetHashCode") + .Any(member => member is IMethodSymbol { IsStatic: false, Parameters.Length: 0 }); + + if (!declaresGetHashCode) + { + features |= ModuleEntryPointFeatures.ShouldImplementGetHashCode; + } + + return features; } private record DependencyFlags( diff --git a/src/DependencyModules.SourceGenerator.Impl/DependencyModuleWriter.cs b/src/DependencyModules.SourceGenerator.Impl/DependencyModuleWriter.cs index a8325f1..440bb9d 100644 --- a/src/DependencyModules.SourceGenerator.Impl/DependencyModuleWriter.cs +++ b/src/DependencyModules.SourceGenerator.Impl/DependencyModuleWriter.cs @@ -285,14 +285,14 @@ private void GenerateModuleClass(ModuleEntryPointModel model, CSharpFileDefiniti FeatureMethod(classDefinition, model); - if ( - (model.ModuleFeatures & ModuleEntryPointFeatures.ShouldImplementEquals) - == ModuleEntryPointFeatures.ShouldImplementEquals - ) + if (model.ModuleFeatures.HasFlag(ModuleEntryPointFeatures.ShouldImplementEquals)) { EqualMethod(classDefinition, model); - HashMethod(classDefinition, model); + if (model.ModuleFeatures.HasFlag(ModuleEntryPointFeatures.ShouldImplementGetHashCode)) + { + HashMethod(classDefinition, model); + } } } @@ -388,7 +388,11 @@ private void EqualMethod(ClassDefinition classDefinition, ModuleEntryPointModel equalMethod.AddParameter(TypeDefinition.Get(typeof(object)).MakeNullable(), "obj"); - equalMethod.Return($"obj is {model.EntryPointType.Name}"); + equalMethod.Return( + model.ModuleFeatures.HasFlag(ModuleEntryPointFeatures.DeclaresTypedEquals) + ? $"obj is {model.EntryPointType.Name} other && Equals(other)" + : $"obj is {model.EntryPointType.Name}" + ); } /// diff --git a/src/DependencyModules.SourceGenerator.Impl/Models/ModuleEntryPointModel.cs b/src/DependencyModules.SourceGenerator.Impl/Models/ModuleEntryPointModel.cs index 1ee7cb1..d97b632 100644 --- a/src/DependencyModules.SourceGenerator.Impl/Models/ModuleEntryPointModel.cs +++ b/src/DependencyModules.SourceGenerator.Impl/Models/ModuleEntryPointModel.cs @@ -25,6 +25,17 @@ public enum ModuleEntryPointFeatures /// namespace level, the nested declaration never became a module, and the build stayed green. /// NestedInType = 32, + + /// + /// The module declares no GetHashCode(). Read only with . + /// + ShouldImplementGetHashCode = 64, + + /// + /// The module declares Equals for its own type, which the generated + /// Equals(object) calls. + /// + DeclaresTypedEquals = 128, } public record ModuleEntryPointModel( diff --git a/src/DependencyModules.SourceGenerator.Impl/Models/PropertyInfoModel.cs b/src/DependencyModules.SourceGenerator.Impl/Models/PropertyInfoModel.cs index 2ff94b2..022b3cd 100644 --- a/src/DependencyModules.SourceGenerator.Impl/Models/PropertyInfoModel.cs +++ b/src/DependencyModules.SourceGenerator.Impl/Models/PropertyInfoModel.cs @@ -8,9 +8,10 @@ namespace DependencyModules.SourceGenerator.Impl.Models; /// No set accessor. Nothing to configure, so not a parameter. /// Belongs to the type rather than the instance, so not a parameter. /// -/// Reachable from another type in the same assembly, which is where the generated attribute sits. -/// public, internal and protected internal are; private, -/// protected, private protected and an unmodified declaration are not. +/// The property and its set accessor are reachable from another type in the same assembly, which +/// is where the generated attribute sits. public, internal and +/// protected internal are; private, protected, private protected and an +/// unmodified declaration are not. /// public record PropertyInfoModel( ITypeDefinition PropertyType, diff --git a/src/DependencyModules.SourceGenerator.Impl/ModuleEntryPointDiagnostics.cs b/src/DependencyModules.SourceGenerator.Impl/ModuleEntryPointDiagnostics.cs index b24495d..51ca3ec 100644 --- a/src/DependencyModules.SourceGenerator.Impl/ModuleEntryPointDiagnostics.cs +++ b/src/DependencyModules.SourceGenerator.Impl/ModuleEntryPointDiagnostics.cs @@ -57,7 +57,8 @@ public static bool IsNestedInType(ModuleEntryPointModel model) => /// public static bool ReliesOnGeneratedEquality(ModuleEntryPointModel model) => model.PropertyInfoModels.Any(p => p.IsModuleParameter) - && model.ModuleFeatures.HasFlag(ModuleEntryPointFeatures.ShouldImplementEquals); + && model.ModuleFeatures.HasFlag(ModuleEntryPointFeatures.ShouldImplementEquals) + && !model.ModuleFeatures.HasFlag(ModuleEntryPointFeatures.DeclaresTypedEquals); public static void Report( SourceProductionContext context, diff --git a/src/DependencyModules.SourceGenerator.Impl/Utilities/AttributeModelHelper.cs b/src/DependencyModules.SourceGenerator.Impl/Utilities/AttributeModelHelper.cs index 5da88ef..e73a5cb 100644 --- a/src/DependencyModules.SourceGenerator.Impl/Utilities/AttributeModelHelper.cs +++ b/src/DependencyModules.SourceGenerator.Impl/Utilities/AttributeModelHelper.cs @@ -43,32 +43,38 @@ CancellationToken cancellationToken { var propertyList = new List(); - foreach (var syntax in context.Node.DescendantNodes()) + // The module's own members only. A property of a nested type is not a module parameter. + var members = context.Node is TypeDeclarationSyntax typeDeclaration + ? typeDeclaration.Members + : default; + + foreach (var propertyDeclarationSyntax in members.OfType()) { cancellationToken.ThrowIfCancellationRequested(); - if (syntax is PropertyDeclarationSyntax propertyDeclarationSyntax) - { - var setter = propertyDeclarationSyntax.AccessorList?.Accessors.FirstOrDefault(x => - x.IsKind(SyntaxKind.SetAccessorDeclaration) - ); + var setter = propertyDeclarationSyntax.AccessorList?.Accessors.FirstOrDefault(x => + x.IsKind(SyntaxKind.SetAccessorDeclaration) + ); - var propertyType = propertyDeclarationSyntax.Type.GetTypeDefinition(context); + var propertyType = propertyDeclarationSyntax.Type.GetTypeDefinition(context); - if (propertyType != null) - { - propertyList.Add( - new PropertyInfoModel( - propertyType, - propertyDeclarationSyntax.Identifier.ToString(), - setter == null, - propertyDeclarationSyntax.Modifiers.Any(m => - m.IsKind(SyntaxKind.StaticKeyword) - ), - IsVisibleToAttribute(propertyDeclarationSyntax.Modifiers) - ) - ); - } + if (propertyType != null) + { + propertyList.Add( + new PropertyInfoModel( + propertyType, + propertyDeclarationSyntax.Identifier.ToString(), + setter == null, + propertyDeclarationSyntax.Modifiers.Any(m => + m.IsKind(SyntaxKind.StaticKeyword) + ), + IsVisibleToAttribute(propertyDeclarationSyntax.Modifiers) + && ( + setter is not { Modifiers.Count: > 0 } + || IsVisibleToAttribute(setter.Modifiers) + ) + ) + ); } } @@ -80,8 +86,9 @@ CancellationToken cancellationToken } /// - /// Whether a property with these modifiers can be read and written from another type in the - /// same assembly — which is what the generated attribute is. + /// Whether a property or a set accessor with these modifiers can be used from another type in + /// the same assembly — which is what the generated attribute is. A set accessor with no + /// modifier has the access of its property. /// /// /// Read from syntax rather than from the symbol, because everything else in this transform is diff --git a/src/DependencyModules.SourceGenerator.Impl/Utilities/EntryModelUtil.cs b/src/DependencyModules.SourceGenerator.Impl/Utilities/EntryModelUtil.cs index c7e7046..76b3dc5 100644 --- a/src/DependencyModules.SourceGenerator.Impl/Utilities/EntryModelUtil.cs +++ b/src/DependencyModules.SourceGenerator.Impl/Utilities/EntryModelUtil.cs @@ -82,7 +82,9 @@ IEnumerable allEntryPoints /// /// Every writer that emits registrations, decorations or interceptions into a module filters /// through this, so an auto-generated module that defers to a declared one is skipped by all of - /// them rather than by whichever ones remembered to. + /// them rather than by whichever ones remembered to. So is a module that DM0003 or DM0017 + /// rejects. A partial class written for a module that is not partial is CS0260, and one written + /// for a nested module is a second type at namespace level. /// public static IList RegistrationTargets( IList entryPoints @@ -92,7 +94,11 @@ IList entryPoints for (var i = 0; i < entryPoints.Count; i++) { - if (DelegateTargetFor(entryPoints[i], entryPoints) == null) + if ( + DelegateTargetFor(entryPoints[i], entryPoints) == null + && !ModuleEntryPointDiagnostics.IsNotPartial(entryPoints[i]) + && !ModuleEntryPointDiagnostics.IsNestedInType(entryPoints[i]) + ) { filtered?.Add(entryPoints[i]); continue; diff --git a/tests/DependencyModules.Tests/GeneratorTests/ModuleDeclarationTests.cs b/tests/DependencyModules.Tests/GeneratorTests/ModuleDeclarationTests.cs new file mode 100644 index 0000000..897a913 --- /dev/null +++ b/tests/DependencyModules.Tests/GeneratorTests/ModuleDeclarationTests.cs @@ -0,0 +1,241 @@ +using DependencyModules.Runtime; +using DependencyModules.Runtime.Interfaces; +using DependencyModules.Tests.Infrastructure; +using Microsoft.Extensions.DependencyInjection; +using Xunit; + +namespace DependencyModules.Tests.GeneratorTests; + +/// +/// What the generator reads from a module declaration: its parameters, its equality members, +/// and whether it can complete the module at all. +/// +public class ModuleDeclarationTests +{ + private const string Preamble = """ + using DependencyModules.Runtime.Attributes; + using DependencyModules.Runtime.Interfaces; + using Microsoft.Extensions.DependencyInjection; + + namespace TestNamespace; + + """; + + private static GeneratorResult Run(string source) => + GeneratorTestHarness.Run(Preamble + source); + + [Fact] + public void APropertyWithAPrivateSetter_IsNotAModuleParameter() + { + var result = Run( + """ + [DependencyModule] + public partial class TestModule + { + public string Label { get; private set; } = "shop"; + + public string? Host { get; internal set; } + } + """ + ); + + result.AssertNoErrors(); + + var module = result.SourceContaining("Module.g.cs"); + + Assert.DoesNotContain("newModule.Label", module); + Assert.Contains("newModule.Host", module); + } + + [Fact] + public void APropertyOfANestedClass_IsNotAModuleParameter() + { + var result = Run( + """ + [DependencyModule] + public partial class TestModule + { + public class Options + { + public int Size { get; set; } + } + } + """ + ); + + result.AssertNoErrors(); + + Assert.DoesNotContain("newModule.Size", result.SourceContaining("Module.g.cs")); + Assert.DoesNotContain(result.GeneratorDiagnostics, d => d.Id == "DM0018"); + } + + [Fact] + public void EqualsInAnotherPartialDeclaration_IsNotGeneratedAgain() + { + var result = GeneratorTestHarness.Run( + new Dictionary + { + ["TestModule.cs"] = + Preamble + + """ + [DependencyModule] + public partial class TestModule + { + public string? Host { get; set; } + } + """, + ["TestModule.Equality.cs"] = """ + namespace TestNamespace; + + public partial class TestModule + { + public override bool Equals(object? obj) => + obj is TestModule other && other.Host == Host; + + public override int GetHashCode() => Host?.GetHashCode() ?? 0; + } + """, + } + ); + + result.AssertNoErrors(); + + Assert.DoesNotContain(result.GeneratorDiagnostics, d => d.Id == "DM0018"); + } + + [Fact] + public void AGetHashCodeOfItsOwn_IsNotGeneratedAgain() + { + var result = Run( + """ + [DependencyModule] + public partial class TestModule + { + public override int GetHashCode() => 7; + } + """ + ); + + result.AssertNoErrors(); + + var module = result.SourceContaining("Module.g.cs"); + + Assert.Contains("override bool Equals", module); + Assert.DoesNotContain("override int GetHashCode", module); + } + + [Theory] + [InlineData("other is not null", "a", "a", 1)] + [InlineData("other is not null", "a", "b", 1)] + [InlineData("other is not null && other.Host == Host", "a", "a", 1)] + [InlineData("other is not null && other.Host == Host", "a", "b", 2)] + public void AModuleWithOnlyTypedEquals_IsLoadedByThatEquals( + string equality, + string firstHost, + string secondHost, + int expectedLoads + ) + { + var source = + Preamble + + $$""" + public record AuditMarker; + + [DependencyModule] + public partial class TestModule : IServiceCollectionConfiguration, System.IEquatable + { + public string? Host { get; set; } + + public bool Equals(TestModule? other) => {{equality}}; + + public void ConfigureServices(IServiceCollection services) => + services.AddSingleton(new AuditMarker()); + } + """; + + Assert.DoesNotContain( + GeneratorTestHarness.Run(source).GeneratorDiagnostics, + d => d.Id == "DM0018" + ); + + var assembly = GeneratedAssembly.Create(source); + + var moduleType = assembly.Type("TestModule"); + + IDependencyModule Module(string host) + { + var module = (IDependencyModule)Activator.CreateInstance(moduleType)!; + moduleType.GetProperty("Host")!.SetValue(module, host); + return module; + } + + var services = new ServiceCollection().AddModules(Module(firstHost), Module(secondHost)); + + Assert.Equal( + expectedLoads, + services.Count(d => d.ServiceType == assembly.Type("AuditMarker")) + ); + } + + [Fact] + public void AModuleThatIsNotPartial_GetsOnlyDM0003() + { + var result = Run( + """ + public interface IClock { } + + [SingletonService] + public class Clock : IClock { } + + [DependencyModule] + public class ShopModule { } + """ + ); + + Assert.Equal("DM0003", Assert.Single(result.Errors).Id); + Assert.DoesNotContain(result.GeneratedSources.Keys, key => key.Contains("ShopModule")); + } + + [Fact] + public void ANestedModule_GetsOnlyDM0017() + { + var result = Run( + """ + using DependencyModules.Runtime.Interception; + + public interface IClock + { + string Now(); + } + + public class PassInterceptor : IInterceptor + { + public TResult Intercept(InvocationContext context) => + context.Proceed(); + } + + [SingletonService] + [Intercept(typeof(PassInterceptor))] + public class Clock : IClock + { + public string Now() => "now"; + } + + [Decorator] + public class LoggingClock(IClock inner) : IClock + { + public string Now() => inner.Now(); + } + + public static class Modules + { + [DependencyModule] + public partial class ShopModule; + } + """ + ); + + Assert.Equal("DM0017", Assert.Single(result.Errors).Id); + Assert.DoesNotContain(result.GeneratedSources.Keys, key => key.Contains("ShopModule")); + } +} diff --git a/tests/DependencyModules.Tests/Snapshots/PublicApiTests.SourceGeneratorApi.verified.txt b/tests/DependencyModules.Tests/Snapshots/PublicApiTests.SourceGeneratorApi.verified.txt index 992817a..6304441 100644 --- a/tests/DependencyModules.Tests/Snapshots/PublicApiTests.SourceGeneratorApi.verified.txt +++ b/tests/DependencyModules.Tests/Snapshots/PublicApiTests.SourceGeneratorApi.verified.txt @@ -1676,6 +1676,8 @@ namespace DependencyModules.SourceGenerator.Impl.Models IsRecord = 8, NotPartial = 16, NestedInType = 32, + ShouldImplementGetHashCode = 64, + DeclaresTypedEquals = 128, } public class ModuleEntryPointModel : DependencyModules.SourceGenerator.Impl.Models.IClassModel, System.IEquatable {