Add support to fetch logger objects from properties and base classes. (…

…#4828)

- When generating a logging method which doesn't have a logger specified
as a parameter, the generator now supports fetching either a field or
property from the class or any base class. It also supports either
generic or non-generic loggers.

Co-authored-by: Martin Taillefer <[email protected]>

src/Generators/Microsoft.Gen.Logging/Emission/Emitter.Method.cs

@@ -540,8 +540,8 @@ bool GenVariableAssignments(LoggingMethod lm, string lambdaStateName, int numRes
 
         static (string name, bool isNullable) GetLogger(LoggingMethod lm)
         {
-            string logger = lm.LoggerField;
-            bool isNullable = lm.LoggerFieldNullable;
+            string logger = lm.LoggerMember;
+            bool isNullable = lm.LoggerMemberNullable;
 
             foreach (var p in lm.Parameters)
             {

src/Generators/Microsoft.Gen.Logging/Model/LoggingMethod.cs

@@ -24,8 +24,8 @@ internal sealed class LoggingMethod
     public bool IsExtensionMethod;
     public bool IsStatic;
     public string Modifiers = string.Empty;
-    public string LoggerField = "_logger";
-    public bool LoggerFieldNullable;
+    public string LoggerMember = "_logger";
+    public bool LoggerMemberNullable;
     public bool HasXmlDocumentation;
 
     public string GetParameterNameInTemplate(LoggingMethodParameter parameter)

src/Generators/Microsoft.Gen.Logging/Parsing/DiagDescriptors.cs

@@ -108,16 +108,16 @@ internal sealed class DiagDescriptors : DiagDescriptorsBase
         category: Category,
         DiagnosticSeverity.Warning);
 
-    public static DiagnosticDescriptor MissingLoggerField { get; } = Make(
+    public static DiagnosticDescriptor MissingLoggerMember { get; } = Make(
         id: DiagnosticIds.LoggerMessage.LOGGEN015,
-        title: Resources.MissingLoggerFieldTitle,
-        messageFormat: Resources.MissingLoggerFieldMessage,
+        title: Resources.MissingLoggerMemberTitle,
+        messageFormat: Resources.MissingLoggerMemberMessage,
         category: Category);
 
-    public static DiagnosticDescriptor MultipleLoggerFields { get; } = Make(
+    public static DiagnosticDescriptor MultipleLoggerMembers { get; } = Make(
         id: DiagnosticIds.LoggerMessage.LOGGEN016,
-        title: Resources.MultipleLoggerFieldsTitle,
-        messageFormat: Resources.MultipleLoggerFieldsMessage,
+        title: Resources.MultipleLoggerMembersTitle,
+        messageFormat: Resources.MultipleLoggerMembersMessage,
         category: Category);
 
     public static DiagnosticDescriptor CantUseDataClassificationWithLogPropertiesOrTagProvider { get; } = Make(
@@ -176,7 +176,7 @@ internal sealed class DiagDescriptors : DiagDescriptorsBase
         messageFormat: Resources.LoggingMethodParameterRefKindMessage,
         category: Category);
 
-    public static DiagnosticDescriptor LogPropertiesProviderWithRedaction { get; } = Make(
+    public static DiagnosticDescriptor TagProviderWithRedaction { get; } = Make(
         id: DiagnosticIds.LoggerMessage.LOGGEN026,
         title: Resources.TagProviderWithRedactionTitle,
         messageFormat: Resources.TagProviderWithRedactionMessage,

src/Generators/Microsoft.Gen.Logging/Parsing/Parser.cs

@@ -58,9 +58,9 @@ public IReadOnlyList<LoggingType> GetLogTypes(IEnumerable<TypeDeclarationSyntax>
 
                 LoggingType? lt = null;
                 string nspace = string.Empty;
-                string? loggerField = null;
-                bool loggerFieldNullable = false;
-                IFieldSymbol? secondLoggerField = null;
+                string? loggerMember = null;
+                bool loggerMemberNullable = false;
+                ISymbol? secondLoggerMember = null;
 
                 ids.Clear();
                 foreach (var method in typeDec.Members.Where(m => m.IsKind(SyntaxKind.MethodDeclaration)).Cast<MethodDeclarationSyntax>())
@@ -199,25 +199,25 @@ public IReadOnlyList<LoggingType> GetLogTypes(IEnumerable<TypeDeclarationSyntax>
                         {
                             if (!parsingState.FoundLogger)
                             {
-                                if (loggerField == null)
+                                if (loggerMember == null)
                                 {
-                                    (loggerField, secondLoggerField, loggerFieldNullable) = FindField(sm, typeDec, symbols.ILoggerSymbol);
+                                    (loggerMember, secondLoggerMember, loggerMemberNullable) = FindMember(sm, typeDec, symbols.ILoggerSymbol);
                                 }
 
-                                if (secondLoggerField != null)
+                                if (secondLoggerMember != null)
                                 {
-                                    Diag(DiagDescriptors.MultipleLoggerFields, secondLoggerField.GetLocation(), typeDec.Identifier.Text);
+                                    Diag(DiagDescriptors.MultipleLoggerMembers, secondLoggerMember.GetLocation(), typeDec.Identifier.Text);
                                     keepMethod = false;
                                 }
-                                else if (loggerField == null)
+                                else if (loggerMember == null)
                                 {
-                                    Diag(DiagDescriptors.MissingLoggerField, method.Identifier.GetLocation(), typeDec.Identifier.Text);
+                                    Diag(DiagDescriptors.MissingLoggerMember, method.Identifier.GetLocation(), typeDec.Identifier.Text);
                                     keepMethod = false;
                                 }
                                 else
                                 {
-                                    lm.LoggerField = loggerField;
-                                    lm.LoggerFieldNullable = loggerFieldNullable;
+                                    lm.LoggerMember = loggerMember;
+                                    lm.LoggerMemberNullable = loggerMemberNullable;
                                 }
                             }
 
@@ -592,35 +592,67 @@ private void CheckMethodParametersAreUnique(LoggingMethod lm, Dictionary<Logging
         return null;
     }
 
-    private (string? field, IFieldSymbol? secondLoggerField, bool isNullable) FindField(SemanticModel sm, TypeDeclarationSyntax classDec, ITypeSymbol symbol)
+    private (string? member, ISymbol? secondMember, bool isNullable) FindMember(SemanticModel sm, TypeDeclarationSyntax classDec, ITypeSymbol symbol)
     {
-        string? field = null;
+        string? member = null;
         bool isNullable = false;
 
-        foreach (var m in classDec.Members)
+        INamedTypeSymbol? type = sm.GetDeclaredSymbol(classDec);
+        var originType = type!;
+        while (type != null)
         {
-            if (m is FieldDeclarationSyntax fds)
+            foreach (var m in type.GetMembers())
             {
-                foreach (var v in fds.Declaration.Variables)
+                if (m is IPropertySymbol p)
                 {
-                    var fs = sm.GetDeclaredSymbol(v, _cancellationToken) as IFieldSymbol;
-                    if (fs != null && ParserUtilities.IsBaseOrIdentity(fs.Type, symbol, _compilation))
+                    var gm = p.GetMethod;
+                    if (gm != null)
                     {
-                        if (field == null)
+                        if (ParserUtilities.IsBaseOrIdentity(gm.ReturnType, symbol, _compilation))
                         {
-                            field = v.Identifier.Text;
-                            isNullable = fs.Type.NullableAnnotation == NullableAnnotation.Annotated;
+                            if (!originType.CanAccess(gm))
+                            {
+                                continue;
+                            }
+
+                            if (member == null)
+                            {
+                                member = p.Name;
+                                isNullable = gm.ReturnType.NullableAnnotation == NullableAnnotation.Annotated;
+                            }
+                            else
+                            {
+                                return (null, p, isNullable);
+                            }
+                        }
+                    }
+                }
+                else if (m is IFieldSymbol f)
+                {
+                    if (f.AssociatedSymbol == null && ParserUtilities.IsBaseOrIdentity(f.Type, symbol, _compilation))
+                    {
+                        if (!originType.CanAccess(f))
+                        {
+                            continue;
+                        }
+
+                        if (member == null)
+                        {
+                            member = f.Name;
+                            isNullable = f.Type.NullableAnnotation == NullableAnnotation.Annotated;
                         }
                         else
                         {
-                            return (null, fs, isNullable);
+                            return (null, f, isNullable);
                         }
                     }
                 }
             }
+
+            type = type.BaseType;
         }
 
-        return (field, null, isNullable);
+        return (member, null, isNullable);
     }
 
     private void Diag(DiagnosticDescriptor desc, Location? location, params object?[]? messageArgs)

src/Generators/Microsoft.Gen.Logging/Parsing/Resources.resx

@@ -207,17 +207,17 @@
   <data name="ShouldntMentionLoggerInMessageTitle" xml:space="preserve">
     <value>Don't include logger parameters as templates</value>
   </data>
-  <data name="MissingLoggerFieldMessage" xml:space="preserve">
-    <value>Couldn't find a field of type "Microsoft.Extensions.Logging.ILogger" in type "{0}"</value>
+  <data name="MissingLoggerMemberMessage" xml:space="preserve">
+    <value>Couldn't find a field or property of type "Microsoft.Extensions.Logging.ILogger" in type "{0}"</value>
   </data>
-  <data name="MissingLoggerFieldTitle" xml:space="preserve">
-    <value>Couldn't find a field of type "Microsoft.Extensions.Logging.ILogger"</value>
+  <data name="MissingLoggerMemberTitle" xml:space="preserve">
+    <value>Couldn't find a field or property of type "Microsoft.Extensions.Logging.ILogger"</value>
   </data>
-  <data name="MultipleLoggerFieldsMessage" xml:space="preserve">
-    <value>Found multiple fields of type "Microsoft.Extensions.Logging.ILogger" in type "{0}"</value>
+  <data name="MultipleLoggerMembersMessage" xml:space="preserve">
+    <value>Found multiple fields or properties of type "Microsoft.Extensions.Logging.ILogger" in type "{0}"</value>
   </data>
-  <data name="MultipleLoggerFieldsTitle" xml:space="preserve">
-    <value>Multiple fields of type "Microsoft.Extensions.Logging.ILogger" were found</value>
+  <data name="MultipleLoggerMembersTitle" xml:space="preserve">
+    <value>Multiple fields or properties of type "Microsoft.Extensions.Logging.ILogger" were found</value>
   </data>
   <data name="InvalidTypeToLogPropertiesMessage" xml:space="preserve">
     <value>Can't log properties of items of type "{0}"</value>

src/Generators/Shared/SymbolHelpers.cs

@@ -16,4 +16,40 @@ public static string GetFullNamespace(ISymbol symbol)
     {
         return symbol.ContainingNamespace.IsGlobalNamespace ? string.Empty : symbol.ContainingNamespace.ToString();
     }
+
+    /// <summary>
+    /// Can code in a given type access a given member?
+    /// </summary>
+    /// <remarks>
+    /// Note that this implementation assumes that the target member is within the origin type
+    /// or a base class of the origin type.
+    /// </remarks>
+    public static bool CanAccess(this INamedTypeSymbol originType, ISymbol targetMember)
+    {
+        if (SymbolEqualityComparer.Default.Equals(originType, targetMember.ContainingType))
+        {
+            // target member is from the origin type, we're good
+            return true;
+        }
+
+        if (targetMember.DeclaredAccessibility == Accessibility.Private)
+        {
+            // can't access a private member from a different type
+            return false;
+        }
+
+        if (SymbolEqualityComparer.Default.Equals(originType.ContainingAssembly, targetMember.ContainingAssembly))
+        {
+            // target member is in the same assembly as the origin type, so we're good
+            return true;
+        }
+
+        if (targetMember.DeclaredAccessibility is Accessibility.Internal or Accessibility.ProtectedAndInternal)
+        {
+            // can't access internal members of other assemblies (sorry, we don't support IVT right now)
+            return false;
+        }
+
+        return true;
+    }
 }