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 {