Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add support for primary constructors in LoggerMessageGenerator #101660

Open
wants to merge 9 commits into
base: main
Choose a base branch
from
Open
2 changes: 1 addition & 1 deletion docs/project/list-of-diagnostics.md
Expand Up @@ -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 |
Expand Down
Expand Up @@ -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}"
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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}
Expand Down
Expand Up @@ -208,5 +208,13 @@ public static class DiagnosticDescriptors
category: "LoggingGenerator",
defaultSeverity: DiagnosticSeverity.Error,
isEnabledByDefault: true);

public static DiagnosticDescriptor PrimaryConstructorParameterLoggerHidden { get; } = DiagnosticDescriptorHelper.Create(
kimsey0 marked this conversation as resolved.
Show resolved Hide resolved
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);
}
}
Expand Up @@ -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)
{
Expand All @@ -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))
kimsey0 marked this conversation as resolved.
Show resolved Hide resolved
{
// 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);
Expand Down
Expand Up @@ -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);
Expand Down
@@ -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>
Expand All @@ -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.
-->
Expand Down Expand Up @@ -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>
Expand Up @@ -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>
Expand Down
Expand Up @@ -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>
Expand Down
Expand Up @@ -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>
Expand Down
Expand Up @@ -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>
Expand Down