Add support for primary constructors in LoggerMessageGenerator (dotne…

…t#101660)

* Add support for primary constructors in LoggerMessageGenerator

* Get the primary constructor parameters types from the constructor symbol instead of from the semantic model

* Prioritize fields over primary constructor parameters and ignore shadowed parameters when finding a logger

* Make checking for primary constructors non-conditional on Roslyn version and simplify project setup

* Reintroduce Roslyn 4.8 test project

* Add info-level diagnostic for logger primary constructor parameters that are shadowed by field

* Update list of diagnostics with new logging message generator diagnostic

* Only add non-logger field names to set of shadowed names

* Add comment explaining the use of the set of shadowed names with an example

docs/project/list-of-diagnostics.md

@@ -145,7 +145,7 @@ The diagnostic id values reserved for .NET Libraries analyzer warnings are `SYSL
 |  __`SYSLIB1024`__ | Argument is using the unsupported out parameter modifier |
 |  __`SYSLIB1025`__ | Multiple logging methods cannot use the same event name within a class |
 |  __`SYSLIB1026`__ | C# language version not supported by the logging source generator. |
-|  __`SYSLIB1027`__ | _`SYSLIB1001`-`SYSLIB1029` reserved for logging._ |
+|  __`SYSLIB1027`__ | Primary constructor parameter of type Microsoft.Extensions.Logging.ILogger is hidden by a field |
 |  __`SYSLIB1028`__ | _`SYSLIB1001`-`SYSLIB1029` reserved for logging._ |
 |  __`SYSLIB1029`__ | _`SYSLIB1001`-`SYSLIB1029` reserved for logging._ |
 |  __`SYSLIB1030`__ | JsonSourceGenerator did not generate serialization metadata for type |

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/DiagnosticDescriptors.cs

@@ -208,5 +208,13 @@ public static class DiagnosticDescriptors
             category: "LoggingGenerator",
             defaultSeverity: DiagnosticSeverity.Error,
             isEnabledByDefault: true);
+
+        public static DiagnosticDescriptor PrimaryConstructorParameterLoggerHidden { get; } = DiagnosticDescriptorHelper.Create(
+            id: "SYSLIB1027",
+            title: new LocalizableResourceString(nameof(SR.PrimaryConstructorParameterLoggerHiddenTitle), SR.ResourceManager, typeof(FxResources.Microsoft.Extensions.Logging.Generators.SR)),
+            messageFormat: new LocalizableResourceString(nameof(SR.PrimaryConstructorParameterLoggerHiddenMessage), SR.ResourceManager, typeof(FxResources.Microsoft.Extensions.Logging.Generators.SR)),
+            category: "LoggingGenerator",
+            DiagnosticSeverity.Info,
+            isEnabledByDefault: true);
     }
 }

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

@@ -630,11 +630,23 @@ 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 })
+                // We keep track of the names of all non-logger fields, since they prevent referring to logger
+                // primary constructor parameters with the same name. Example:
+                // partial class C(ILogger logger)
+                // {
+                //     private readonly object logger = logger;
+                //
+                //     [LoggerMessage(EventId = 0, Level = LogLevel.Debug, Message = ""M1"")]
+                //     public partial void M1(); // The ILogger primary constructor parameter cannot be used here.
+                // }
+                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 +663,52 @@ private static string GenerateClassName(TypeDeclarationSyntax typeDeclaration)
                                 return (null, true);
                             }
                         }
+                        else
+                        {
+                            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.
+                                Diag(DiagnosticDescriptors.PrimaryConstructorParameterLoggerHidden, parameter.Locations[0], classDec.Identifier.Text);
+
+                                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/gen/Resources/Strings.resx

@@ -1,17 +1,17 @@
 <?xml version="1.0" encoding="utf-8"?>
 <root>
-  <!--
-    Microsoft ResX Schema
-
+  <!-- 
+    Microsoft ResX Schema 
+    
     Version 2.0
-
-    The primary goals of this format is to allow a simple XML format
-    that is mostly human readable. The generation and parsing of the
-    various data types are done through the TypeConverter classes
+    
+    The primary goals of this format is to allow a simple XML format 
+    that is mostly human readable. The generation and parsing of the 
+    various data types are done through the TypeConverter classes 
     associated with the data types.
-
+    
     Example:
-
+    
     ... ado.net/XML headers & schema ...
     <resheader name="resmimetype">text/microsoft-resx</resheader>
     <resheader name="version">2.0</resheader>
@@ -26,36 +26,36 @@
         <value>[base64 mime encoded string representing a byte array form of the .NET Framework object]</value>
         <comment>This is a comment</comment>
     </data>
-
-    There are any number of "resheader" rows that contain simple
+                
+    There are any number of "resheader" rows that contain simple 
     name/value pairs.
-
-    Each data row contains a name, and value. The row also contains a
-    type or mimetype. Type corresponds to a .NET class that support
-    text/value conversion through the TypeConverter architecture.
-    Classes that don't support this are serialized and stored with the
+    
+    Each data row contains a name, and value. The row also contains a 
+    type or mimetype. Type corresponds to a .NET class that support 
+    text/value conversion through the TypeConverter architecture. 
+    Classes that don't support this are serialized and stored with the 
     mimetype set.
-
-    The mimetype is used for serialized objects, and tells the
-    ResXResourceReader how to depersist the object. This is currently not
+    
+    The mimetype is used for serialized objects, and tells the 
+    ResXResourceReader how to depersist the object. This is currently not 
     extensible. For a given mimetype the value must be set accordingly:
-
-    Note - application/x-microsoft.net.object.binary.base64 is the format
-    that the ResXResourceWriter will generate, however the reader can
+    
+    Note - application/x-microsoft.net.object.binary.base64 is the format 
+    that the ResXResourceWriter will generate, however the reader can 
     read any of the formats listed below.
-
+    
     mimetype: application/x-microsoft.net.object.binary.base64
-    value   : The object must be serialized with
+    value   : The object must be serialized with 
             : System.Runtime.Serialization.Formatters.Binary.BinaryFormatter
             : and then encoded with base64 encoding.
-
+    
     mimetype: application/x-microsoft.net.object.soap.base64
-    value   : The object must be serialized with
+    value   : The object must be serialized with 
             : System.Runtime.Serialization.Formatters.Soap.SoapFormatter
             : and then encoded with base64 encoding.
 
     mimetype: application/x-microsoft.net.object.bytearray.base64
-    value   : The object must be serialized into a byte array
+    value   : The object must be serialized into a byte array 
             : using a System.ComponentModel.TypeConverter
             : and then encoded with base64 encoding.
     -->
@@ -237,4 +237,12 @@
   <data name="LoggingUnsupportedLanguageVersionMessageFormat" xml:space="preserve">
     <value>The Logging source generator is not available in C# {0}. Please use language version {1} or greater.</value>
   </data>
+  <data name="PrimaryConstructorParameterLoggerHiddenMessage" xml:space="preserve">
+    <value>Class '{0}' has a primary constructor parameter of type Microsoft.Extensions.Logging.ILogger that is hidden by a field in the class or a base class, preventing its use</value>
+    <comment>{Locked="Microsoft.Extensions.Logging.ILogger"}</comment>
+  </data>
+  <data name="PrimaryConstructorParameterLoggerHiddenTitle" xml:space="preserve">
+    <value>Primary constructor parameter of type Microsoft.Extensions.Logging.ILogger is hidden by a field</value>
+    <comment>{Locked="Microsoft.Extensions.Logging.ILogger"}</comment>
+  </data>
 </root>

src/libraries/Microsoft.Extensions.Logging.Abstractions/gen/Resources/xlf/Strings.cs.xlf

@@ -132,6 +132,16 @@
         <target state="translated">Našlo se několik polí typu Microsoft.Extensions.Logging.ILogger</target>
         <note>{Locked="Microsoft.Extensions.Logging.ILogger"}</note>
       </trans-unit>
+      <trans-unit id="PrimaryConstructorParameterLoggerHiddenMessage">
+        <source>Class '{0}' has a primary constructor parameter of type Microsoft.Extensions.Logging.ILogger that is hidden by a field in the class or a base class, preventing its use</source>
+        <target state="new">Class '{0}' has a primary constructor parameter of type Microsoft.Extensions.Logging.ILogger that is hidden by a field in the class or a base class, preventing its use</target>
+        <note>{Locked="Microsoft.Extensions.Logging.ILogger"}</note>
+      </trans-unit>
+      <trans-unit id="PrimaryConstructorParameterLoggerHiddenTitle">
+        <source>Primary constructor parameter of type Microsoft.Extensions.Logging.ILogger is hidden by a field</source>
+        <target state="new">Primary constructor parameter of type Microsoft.Extensions.Logging.ILogger is hidden by a field</target>
+        <note>{Locked="Microsoft.Extensions.Logging.ILogger"}</note>
+      </trans-unit>
       <trans-unit id="RedundantQualifierInMessageMessage">
         <source>Remove redundant qualifier (Info:, Warning:, Error:, etc) from the logging message since it is implicit in the specified log level.</source>
         <target state="translated">Odeberte redundantní kvalifikátor (Informace:, Upozornění:, Chyba: atd.) ze zprávy o protokolování, protože je na zadané úrovni protokolu implicitní.</target>

src/libraries/Microsoft.Extensions.Logging.Abstractions/gen/Resources/xlf/Strings.de.xlf

@@ -132,6 +132,16 @@
         <target state="translated">Mehrere Felder vom Typ "Microsoft.Extensions.Logging.ILogger" gefunden</target>
         <note>{Locked="Microsoft.Extensions.Logging.ILogger"}</note>
       </trans-unit>
+      <trans-unit id="PrimaryConstructorParameterLoggerHiddenMessage">
+        <source>Class '{0}' has a primary constructor parameter of type Microsoft.Extensions.Logging.ILogger that is hidden by a field in the class or a base class, preventing its use</source>
+        <target state="new">Class '{0}' has a primary constructor parameter of type Microsoft.Extensions.Logging.ILogger that is hidden by a field in the class or a base class, preventing its use</target>
+        <note>{Locked="Microsoft.Extensions.Logging.ILogger"}</note>
+      </trans-unit>
+      <trans-unit id="PrimaryConstructorParameterLoggerHiddenTitle">
+        <source>Primary constructor parameter of type Microsoft.Extensions.Logging.ILogger is hidden by a field</source>
+        <target state="new">Primary constructor parameter of type Microsoft.Extensions.Logging.ILogger is hidden by a field</target>
+        <note>{Locked="Microsoft.Extensions.Logging.ILogger"}</note>
+      </trans-unit>
       <trans-unit id="RedundantQualifierInMessageMessage">
         <source>Remove redundant qualifier (Info:, Warning:, Error:, etc) from the logging message since it is implicit in the specified log level.</source>
         <target state="translated">Entfernen Sie den redundanten Qualifizierer (z. B. "Info:", "Warnung:" oder "Fehler:") aus der Protokollierungsmeldung, weil er auf der angegebenen Protokollebene implizit enthalten ist.</target>

src/libraries/Microsoft.Extensions.Logging.Abstractions/gen/Resources/xlf/Strings.es.xlf

@@ -132,6 +132,16 @@
         <target state="translated">Se encontraron varios campos de tipo Microsoft.Extensions.Logging.ILogger</target>
         <note>{Locked="Microsoft.Extensions.Logging.ILogger"}</note>
       </trans-unit>
+      <trans-unit id="PrimaryConstructorParameterLoggerHiddenMessage">
+        <source>Class '{0}' has a primary constructor parameter of type Microsoft.Extensions.Logging.ILogger that is hidden by a field in the class or a base class, preventing its use</source>
+        <target state="new">Class '{0}' has a primary constructor parameter of type Microsoft.Extensions.Logging.ILogger that is hidden by a field in the class or a base class, preventing its use</target>
+        <note>{Locked="Microsoft.Extensions.Logging.ILogger"}</note>
+      </trans-unit>
+      <trans-unit id="PrimaryConstructorParameterLoggerHiddenTitle">
+        <source>Primary constructor parameter of type Microsoft.Extensions.Logging.ILogger is hidden by a field</source>
+        <target state="new">Primary constructor parameter of type Microsoft.Extensions.Logging.ILogger is hidden by a field</target>
+        <note>{Locked="Microsoft.Extensions.Logging.ILogger"}</note>
+      </trans-unit>
       <trans-unit id="RedundantQualifierInMessageMessage">
         <source>Remove redundant qualifier (Info:, Warning:, Error:, etc) from the logging message since it is implicit in the specified log level.</source>
         <target state="translated">Quitar calificadores redundantes (Información:, Advertencia:, Error:, etc.) del mensaje de registro, ya que está implícito en el nivel de registro especificado.</target>

src/libraries/Microsoft.Extensions.Logging.Abstractions/gen/Resources/xlf/Strings.fr.xlf

@@ -132,6 +132,16 @@
         <target state="translated">Plusieurs champs de type Microsoft.Extensions.Logging.ILogger ont été trouvés</target>
         <note>{Locked="Microsoft.Extensions.Logging.ILogger"}</note>
       </trans-unit>
+      <trans-unit id="PrimaryConstructorParameterLoggerHiddenMessage">
+        <source>Class '{0}' has a primary constructor parameter of type Microsoft.Extensions.Logging.ILogger that is hidden by a field in the class or a base class, preventing its use</source>
+        <target state="new">Class '{0}' has a primary constructor parameter of type Microsoft.Extensions.Logging.ILogger that is hidden by a field in the class or a base class, preventing its use</target>
+        <note>{Locked="Microsoft.Extensions.Logging.ILogger"}</note>
+      </trans-unit>
+      <trans-unit id="PrimaryConstructorParameterLoggerHiddenTitle">
+        <source>Primary constructor parameter of type Microsoft.Extensions.Logging.ILogger is hidden by a field</source>
+        <target state="new">Primary constructor parameter of type Microsoft.Extensions.Logging.ILogger is hidden by a field</target>
+        <note>{Locked="Microsoft.Extensions.Logging.ILogger"}</note>
+      </trans-unit>
       <trans-unit id="RedundantQualifierInMessageMessage">
         <source>Remove redundant qualifier (Info:, Warning:, Error:, etc) from the logging message since it is implicit in the specified log level.</source>
         <target state="translated">Supprimez le qualificateur redondant (Info:, Warning:, Error:, etc.) du message de journalisation, car il est implicite dans le niveau de journalisation spécifié.</target>