Add support for primary constructors in LoggerMessageGenerator

src/libraries/Microsoft.Extensions.Logging.Abstractions/Microsoft.Extensions.Logging.Abstractions.sln

@@ -23,6 +23,8 @@ Project("{9A19103F-16F7-4668-BE54-9A1E7A4F7556}") = "Microsoft.Extensions.Loggin
 EndProject
 Project("{9A19103F-16F7-4668-BE54-9A1E7A4F7556}") = "Microsoft.Extensions.Logging.Generators.Roslyn4.0.Tests", "tests\Microsoft.Extensions.Logging.Generators.Tests\Microsoft.Extensions.Logging.Generators.Roslyn4.0.Tests.csproj", "{1CB869A7-2EEC-4A53-9C33-DF9E0C75825B}"
 EndProject
+Project("{9A19103F-16F7-4668-BE54-9A1E7A4F7556}") = "Microsoft.Extensions.Logging.Generators.Roslyn4.8.Tests", "tests\Microsoft.Extensions.Logging.Generators.Tests\Microsoft.Extensions.Logging.Generators.Roslyn4.8.Tests.csproj", "{D6167506-0671-46A3-94E5-7A98032DCEC6}"
+EndProject
 Project("{9A19103F-16F7-4668-BE54-9A1E7A4F7556}") = "LibraryImportGenerator", "..\System.Runtime.InteropServices\gen\LibraryImportGenerator\LibraryImportGenerator.csproj", "{852D4E16-58C3-47C2-A6BC-A5B12B37209F}"
 EndProject
 Project("{9A19103F-16F7-4668-BE54-9A1E7A4F7556}") = "Microsoft.Interop.SourceGeneration", "..\System.Runtime.InteropServices\gen\Microsoft.Interop.SourceGeneration\Microsoft.Interop.SourceGeneration.csproj", "{6645D0C4-83D1-4426-B9CD-67096CB7A60F}"
@@ -135,6 +137,10 @@ Global
 		{F3186815-B9A5-455F-B0DF-E39D4235C24F}.Debug|Any CPU.Build.0 = Debug|Any CPU
 		{F3186815-B9A5-455F-B0DF-E39D4235C24F}.Release|Any CPU.ActiveCfg = Release|Any CPU
 		{F3186815-B9A5-455F-B0DF-E39D4235C24F}.Release|Any CPU.Build.0 = Release|Any CPU
+		{D6167506-0671-46A3-94E5-7A98032DCEC6}.Debug|Any CPU.ActiveCfg = Debug|Any CPU
+		{D6167506-0671-46A3-94E5-7A98032DCEC6}.Debug|Any CPU.Build.0 = Debug|Any CPU
+		{D6167506-0671-46A3-94E5-7A98032DCEC6}.Release|Any CPU.ActiveCfg = Release|Any CPU
+		{D6167506-0671-46A3-94E5-7A98032DCEC6}.Release|Any CPU.Build.0 = Release|Any CPU
 	EndGlobalSection
 	GlobalSection(SolutionProperties) = preSolution
 		HideSolutionNode = FALSE
@@ -162,6 +168,7 @@ Global
 		{8215F79E-510B-4CA1-B775-50C47BB58360} = {58760833-B4F5-429D-9ABD-15FDF83E25CD}
 		{F3186815-B9A5-455F-B0DF-E39D4235C24F} = {14DFA192-3C7E-4F10-B5FD-3953BC82A6B1}
 		{14DFA192-3C7E-4F10-B5FD-3953BC82A6B1} = {58760833-B4F5-429D-9ABD-15FDF83E25CD}
+		{D6167506-0671-46A3-94E5-7A98032DCEC6} = {4DE63935-DCA9-4D63-9C1F-AAE79C89CA8B}
 	EndGlobalSection
 	GlobalSection(ExtensibilityGlobals) = postSolution
 		SolutionGuid = {450DA749-CBDC-4BDC-950F-8A491CF59D49}

src/libraries/Microsoft.Extensions.Logging.Abstractions/gen/LoggerMessageGenerator.Parser.cs

@@ -630,11 +630,14 @@ private static string GenerateClassName(TypeDeclarationSyntax typeDeclaration)
 
                 INamedTypeSymbol? classType = sm.GetDeclaredSymbol(classDec, _cancellationToken);
 
+                INamedTypeSymbol? currentClassType = classType;
                 bool onMostDerivedType = true;
 
-                while (classType is { SpecialType: not SpecialType.System_Object })
+                HashSet<string> shadowedNames = new(StringComparer.Ordinal);
+
+                while (currentClassType is { SpecialType: not SpecialType.System_Object })
                 {
-                    foreach (IFieldSymbol fs in classType.GetMembers().OfType<IFieldSymbol>())
+                    foreach (IFieldSymbol fs in currentClassType.GetMembers().OfType<IFieldSymbol>())
                     {
                         if (!onMostDerivedType && fs.DeclaredAccessibility == Accessibility.Private)
                         {
@@ -651,10 +654,48 @@ private static string GenerateClassName(TypeDeclarationSyntax typeDeclaration)
                                 return (null, true);
                             }
                         }
+
+                        shadowedNames.Add(fs.Name);
                     }
 
                     onMostDerivedType = false;
-                    classType = classType.BaseType;
+                    currentClassType = currentClassType.BaseType;
+                }
+
+                // We prioritize fields over primary constructor parameters and avoid warnings if both exist.
+                if (loggerField is not null)
+                {
+                    return (loggerField, false);
+                }
+
+                IEnumerable<IMethodSymbol> primaryConstructors = classType.InstanceConstructors
+                    .Where(ic => ic.DeclaringSyntaxReferences
+                        .Any(ds => ds.GetSyntax() is ClassDeclarationSyntax));
+
+                foreach (IMethodSymbol primaryConstructor in primaryConstructors)
+                {
+                    foreach (IParameterSymbol parameter in primaryConstructor.Parameters)
+                    {
+                        if (IsBaseOrIdentity(parameter.Type, loggerSymbol))
+                        {
+                            if (shadowedNames.Contains(parameter.Name))
+                            {
+                                // Accessible fields always shadow primary constructor parameters,
+                                // so we can't use the primary constructor parameter,
+                                // even if the field is not a valid logger.
+                                continue;
+                            }
+
+                            if (loggerField == null)
+                            {
+                                loggerField = parameter.Name;
+                            }
+                            else
+                            {
+                                return (null, true);
+                            }
+                        }
+                    }
                 }
 
                 return (loggerField, false);

src/libraries/Microsoft.Extensions.Logging.Abstractions/gen/LoggerMessageGenerator.Roslyn4.0.cs

@@ -45,7 +45,7 @@ private static void Execute(Compilation compilation, ImmutableArray<ClassDeclara
                 return;
             }
 
-            IEnumerable<ClassDeclarationSyntax> distinctClasses = classes.Distinct();
+            ImmutableHashSet<ClassDeclarationSyntax> distinctClasses = classes.ToImmutableHashSet();
 
             var p = new Parser(compilation, context.ReportDiagnostic, context.CancellationToken);
             IReadOnlyList<LoggerClass> logClasses = p.GetLogClasses(distinctClasses);

src/libraries/Microsoft.Extensions.Logging.Abstractions/tests/Microsoft.Extensions.Logging.Generators.Tests/Baselines/TestWithLoggerFromPrimaryConstructor.generated.txt

@@ -0,0 +1,21 @@
+// <auto-generated/>
+#nullable enable
+
+namespace Microsoft.Extensions.Logging.Generators.Tests.TestClasses
+{
+    partial class TestWithLoggerFromPrimaryConstructor
+    {
+        [global::System.CodeDom.Compiler.GeneratedCodeAttribute("Microsoft.Extensions.Logging.Generators", "%VERSION%")]
+        private static readonly global::System.Action<global::Microsoft.Extensions.Logging.ILogger, global::System.Exception?> __M0Callback =
+            global::Microsoft.Extensions.Logging.LoggerMessage.Define(global::Microsoft.Extensions.Logging.LogLevel.Debug, new global::Microsoft.Extensions.Logging.EventId(0, nameof(M0)), "M0", new global::Microsoft.Extensions.Logging.LogDefineOptions() { SkipEnabledCheck = true }); 
+
+        [global::System.CodeDom.Compiler.GeneratedCodeAttribute("Microsoft.Extensions.Logging.Generators", "%VERSION%")]
+        public partial void M0()
+        {
+            if (logger.IsEnabled(global::Microsoft.Extensions.Logging.LogLevel.Debug))
+            {
+                __M0Callback(logger, null);
+            }
+        }
+    }
+}

src/libraries/Microsoft.Extensions.Logging.Abstractions/tests/Microsoft.Extensions.Logging.Generators.Tests/Baselines/TestWithLoggerInFieldAndFromPrimaryConstructor.generated.txt

@@ -0,0 +1,21 @@
+// <auto-generated/>
+#nullable enable
+
+namespace Microsoft.Extensions.Logging.Generators.Tests.TestClasses
+{
+    partial class TestWithLoggerFromPrimaryConstructor
+    {
+        [global::System.CodeDom.Compiler.GeneratedCodeAttribute("Microsoft.Extensions.Logging.Generators", "%VERSION%")]
+        private static readonly global::System.Action<global::Microsoft.Extensions.Logging.ILogger, global::System.Exception?> __M0Callback =
+            global::Microsoft.Extensions.Logging.LoggerMessage.Define(global::Microsoft.Extensions.Logging.LogLevel.Debug, new global::Microsoft.Extensions.Logging.EventId(0, nameof(M0)), "M0", new global::Microsoft.Extensions.Logging.LogDefineOptions() { SkipEnabledCheck = true }); 
+
+        [global::System.CodeDom.Compiler.GeneratedCodeAttribute("Microsoft.Extensions.Logging.Generators", "%VERSION%")]
+        public partial void M0()
+        {
+            if (_logger.IsEnabled(global::Microsoft.Extensions.Logging.LogLevel.Debug))
+            {
+                __M0Callback(_logger, null);
+            }
+        }
+    }
+}

src/libraries/Microsoft.Extensions.Logging.Abstractions/tests/Microsoft.Extensions.Logging.Generators.Tests/LoggerMessageGeneratedCodeTests.cs

@@ -26,7 +26,7 @@ public void FindsLoggerFieldInBaseClass()
         }
 
         [Fact]
-        public void FindsLoggerFieldInAnotherParialClass()
+        public void FindsLoggerFieldInAnotherPartialClass()
         {
             var logger = new MockLogger();
 
@@ -36,6 +36,41 @@ public void FindsLoggerFieldInAnotherParialClass()
             Assert.Equal("Test.", logger.LastFormattedString);
         }
 
+#if ROSLYN4_8_OR_GREATER
+        [Fact]
+        public void FindsLoggerInPrimaryConstructorParameter()
+        {
+            var logger = new MockLogger();
+
+            logger.Reset();
+
+            new ClassWithPrimaryConstructor(logger).Test();
+            Assert.Equal("Test.", logger.LastFormattedString);
+        }
+
+        [Fact]
+        public void FindsLoggerInPrimaryConstructorParameterInDifferentPartialDeclaration()
+        {
+            var logger = new MockLogger();
+
+            logger.Reset();
+
+            new ClassWithPrimaryConstructorInDifferentPartialDeclaration(logger).Test();
+            Assert.Equal("Test.", logger.LastFormattedString);
+        }
+
+        [Fact]
+        public void FindsLoggerInFieldInitializedFromPrimaryConstructorParameter()
+        {
+            var logger = new MockLogger();
+
+            logger.Reset();
+
+            new ClassWithPrimaryConstructor(logger).Test();
+            Assert.Equal("Test.", logger.LastFormattedString);
+        }
+#endif
+
         [Fact]
         public void BasicTests()
         {

src/libraries/Microsoft.Extensions.Logging.Abstractions/tests/Microsoft.Extensions.Logging.Generators.Tests/LoggerMessageGeneratorEmitterTests.cs

@@ -187,6 +187,40 @@ public sealed class BarAttribute : Attribute { }
             await VerifyAgainstBaselineUsingFile("TestWithNestedClassWithGenericTypesWithAttributes.generated.txt", testSourceCode);
         }
 
+#if ROSLYN4_8_OR_GREATER
+        [Fact]
+        public async Task TestBaseline_TestWithLoggerFromPrimaryConstructor_Success()
+        {
+            string testSourceCode = @"
+namespace Microsoft.Extensions.Logging.Generators.Tests.TestClasses
+{
+    internal partial class TestWithLoggerFromPrimaryConstructor(ILogger logger)
+    {
+        [LoggerMessage(EventId = 0, Level = LogLevel.Debug, Message = ""M0"")]
+        public partial void M0();
+    }
+}";
+            await VerifyAgainstBaselineUsingFile("TestWithLoggerFromPrimaryConstructor.generated.txt", testSourceCode);
+        }
+
+        [Fact]
+        public async Task TestBaseline_TestWithLoggerInFieldAndFromPrimaryConstructor_UsesField()
+        {
+            string testSourceCode = @"
+namespace Microsoft.Extensions.Logging.Generators.Tests.TestClasses
+{
+    internal partial class TestWithLoggerFromPrimaryConstructor(ILogger logger)
+    {
+        private readonly ILogger _logger = logger;
+
+        [LoggerMessage(EventId = 0, Level = LogLevel.Debug, Message = ""M0"")]
+        public partial void M0();
+    }
+}";
+            await VerifyAgainstBaselineUsingFile("TestWithLoggerInFieldAndFromPrimaryConstructor.generated.txt", testSourceCode);
+        }
+#endif
+
         [Fact]
         public void GenericTypeParameterAttributesAreRetained()
         {

src/libraries/Microsoft.Extensions.Logging.Abstractions/tests/Microsoft.Extensions.Logging.Generators.Tests/LoggerMessageGeneratorParserTests.cs

@@ -417,6 +417,130 @@ internal partial class Logger
         }
 #endif
 
+        [Fact]
+        public async Task FieldOnOtherPartialDeclarationOK()
+        {
+            IReadOnlyList<Diagnostic> diagnostics = await RunGenerator(@"
+                partial class C
+                {
+                    private ILogger _logger;
+
+                    public C(ILogger logger)
+                    {
+                        _logger = logger;
+                    }
+                }
+
+                partial class C
+                {
+                    [LoggerMessage(EventId = 0, Level = LogLevel.Debug, Message = ""M1"")]
+                    public partial void M1();
+                }
+            ");
+
+            Assert.Empty(diagnostics);
+        }
+
+#if ROSLYN4_8_OR_GREATER
+        [Fact]
+        public async Task PrimaryConstructorOK()
+        {
+            IReadOnlyList<Diagnostic> diagnostics = await RunGenerator(@"
+                partial class C(ILogger logger)
+                {
+                    [LoggerMessage(EventId = 0, Level = LogLevel.Debug, Message = ""M1"")]
+                    public partial void M1();
+                }
+            ");
+
+            Assert.Empty(diagnostics);
+        }
+
+        [Fact]
+        public async Task PrimaryConstructorOnOtherPartialDeclarationOK()
+        {
+            IReadOnlyList<Diagnostic> diagnostics = await RunGenerator(@"
+                partial class C(ILogger logger);
+
+                partial class C
+                {
+                    [LoggerMessage(EventId = 0, Level = LogLevel.Debug, Message = ""M1"")]
+                    public partial void M1();
+                }
+            ");
+
+            Assert.Empty(diagnostics);
+        }
+
+        [Fact]
+        public async Task PrimaryConstructorWithDifferentNameLoggerFieldOK()
+        {
+            IReadOnlyList<Diagnostic> diagnostics = await RunGenerator(@"
+                partial class C(ILogger logger)
+                {
+                    private readonly ILogger _logger = logger;
+
+                    [LoggerMessage(EventId = 0, Level = LogLevel.Debug, Message = ""M1"")]
+                    public partial void M1();
+                }
+            ");
+
+            Assert.Empty(diagnostics);
+        }
+
+        [Fact]
+        public async Task PrimaryConstructorWithSameNameLoggerFieldOK()
+        {
+            IReadOnlyList<Diagnostic> diagnostics = await RunGenerator(@"
+                partial class C(ILogger logger)
+                {
+                    private readonly ILogger logger = logger;
+
+                    [LoggerMessage(EventId = 0, Level = LogLevel.Debug, Message = ""M1"")]
+                    public partial void M1();
+                }
+            ");
+
+            Assert.Empty(diagnostics);
+        }
+
+        [Fact]
+        public async Task PrimaryConstructorLoggerShadowedByField()
+        {
+            IReadOnlyList<Diagnostic> diagnostics = await RunGenerator(@"
+                partial class C(ILogger logger)
+                {
+                    private readonly object logger = logger;
+
+                    [LoggerMessage(EventId = 0, Level = LogLevel.Debug, Message = ""M1"")]
+                    public partial void M1();
+                }
+            ");
+
+            Assert.Single(diagnostics);
+            Assert.Equal(DiagnosticDescriptors.MissingLoggerField.Id, diagnostics[0].Id);
+        }
+
+        [Fact]
+        public async Task PrimaryConstructorLoggerShadowedByBaseClass()
+        {
+            IReadOnlyList<Diagnostic> diagnostics = await RunGenerator(@"
+                class Base(object logger) {
+                    protected readonly object logger = logger;
+                }
+
+                partial class Derived(ILogger logger) : Base(logger)
+                {
+                    [LoggerMessage(EventId = 0, Level = LogLevel.Debug, Message = ""M1"")]
+                    public partial void M1();
+                }
+            ");
+
+            Assert.Single(diagnostics);
+            Assert.Equal(DiagnosticDescriptors.MissingLoggerField.Id, diagnostics[0].Id);
+        }
+#endif
+
         [Theory]
         [InlineData("false")]
         [InlineData("true")]

src/libraries/Microsoft.Extensions.Logging.Abstractions/tests/Microsoft.Extensions.Logging.Generators.Tests/Microsoft.Extensions.Logging.Generators.Roslyn4.8.Tests.csproj

@@ -0,0 +1,22 @@
+<Project Sdk="Microsoft.NET.Sdk">
+
+  <PropertyGroup>
+    <RoslynApiVersion>$(MicrosoftCodeAnalysisVersion_4_8)</RoslynApiVersion>
+    <DefineConstants>$(DefineConstants);ROSLYN4_0_OR_GREATER;ROSLYN4_8_OR_GREATER</DefineConstants>
+    <IsHighAotMemoryUsageTest>true</IsHighAotMemoryUsageTest>
+    <EmccLinkOptimizationFlag Condition="'$(ContinuousIntegrationBuild)' == 'true'">-O1</EmccLinkOptimizationFlag>
+    <!-- this Roslyn version brings in NS1.x dependencies -->
+    <FlagNetStandard1XDependencies>false</FlagNetStandard1XDependencies>
+  </PropertyGroup>
+
+  <ItemGroup>
+    <HighAotMemoryUsageAssembly Include="Microsoft.CodeAnalysis.CSharp.dll" />
+  </ItemGroup>
+
+  <Import Project="Microsoft.Extensions.Logging.Generators.targets" />
+
+  <ItemGroup>
+    <ProjectReference Include="..\..\gen\Microsoft.Extensions.Logging.Generators.Roslyn4.4.csproj" OutputItemType="Analyzer" ReferenceOutputAssembly="true" />
+  </ItemGroup>
+
+</Project>