Fixes some logging source gen bugs:

- Supports usage of "in" modifier - Improves support for generic constraints Fixes dotnet#58550, dotnet#62644
maryamariyan · Feb 1, 2022 · be3a808 · be3a808
1 parent 58db8db
commit be3a808
Show file tree

Hide file tree

Showing 8 changed files with 210 additions and 8 deletions.
diff --git a/...libraries/Microsoft.Extensions.Logging.Abstractions/gen/LoggerMessageGenerator.Emitter.cs b/...libraries/Microsoft.Extensions.Logging.Abstractions/gen/LoggerMessageGenerator.Emitter.cs
@@ -86,7 +86,7 @@ namespace {lc.Namespace}
                 // loop until you find top level nested class
                 while (parent != null)
                 {
-                    parentClasses.Add($"partial {parent.Keyword} {parent.Name} {parent.Constraints}");
+                    parentClasses.Add($"partial {parent.Keyword} {parent.Name} ");
                     parent = parent.ParentClass;
                 }
 
@@ -100,7 +100,7 @@ namespace {lc.Namespace}
                 }
 
                 _builder.Append($@"
-    {nestedIndentation}partial {lc.Keyword} {lc.Name} {lc.Constraints}
+    {nestedIndentation}partial {lc.Keyword} {lc.Name} 
     {nestedIndentation}{{");
 
                 foreach (LoggerMethod lm in lc.Methods)
@@ -319,6 +319,10 @@ private void GenParameters(LoggerMethod lm)
                         _builder.Append(", ");
                     }
 
+                    if (p.Qualifier != null)
+                    {
+                        _builder.Append($"{p.Qualifier} ");
+                    }
                     _builder.Append($"{p.Type} {p.Name}");
                 }
             }

diff --git a/src/libraries/Microsoft.Extensions.Logging.Abstractions/gen/LoggerMessageGenerator.Parser.cs b/src/libraries/Microsoft.Extensions.Logging.Abstractions/gen/LoggerMessageGenerator.Parser.cs
@@ -321,6 +321,11 @@ public IReadOnlyList<LoggerClass> GetLogClasses(IEnumerable<ClassDeclarationSynt
                                                 break;
                                             }
 
+                                            string? qualifier = null;
+                                            if (paramSymbol.RefKind == RefKind.In)
+                                            {
+                                                qualifier = "in";
+                                            }
                                             string typeName = paramTypeSymbol.ToDisplayString(
                                                 SymbolDisplayFormat.FullyQualifiedFormat.WithMiscellaneousOptions(
                                                     SymbolDisplayMiscellaneousOptions.IncludeNullableReferenceTypeModifier));
@@ -329,6 +334,7 @@ public IReadOnlyList<LoggerClass> GetLogClasses(IEnumerable<ClassDeclarationSynt
                                             {
                                                 Name = paramName,
                                                 Type = typeName,
+                                                Qualifier = qualifier,
                                                 IsLogger = !foundLogger && IsBaseOrIdentity(paramTypeSymbol!, loggerSymbol),
                                                 IsException = !foundException && IsBaseOrIdentity(paramTypeSymbol!, exceptionSymbol),
                                                 IsLogLevel = !foundLogLevel && IsBaseOrIdentity(paramTypeSymbol!, logLevelSymbol),
@@ -478,7 +484,6 @@ potentialNamespaceParent is not NamespaceDeclarationSyntax
                                                 Keyword = classDec.Keyword.ValueText,
                                                 Namespace = nspace,
                                                 Name = classDec.Identifier.ToString() + classDec.TypeParameterList,
-                                                Constraints = classDec.ConstraintClauses.ToString(),
                                                 ParentClass = null,
                                             };
 
@@ -497,7 +502,6 @@ bool IsAllowedKind(SyntaxKind kind) =>
                                                     Keyword = parentLoggerClass.Keyword.ValueText,
                                                     Namespace = nspace,
                                                     Name = parentLoggerClass.Identifier.ToString() + parentLoggerClass.TypeParameterList,
-                                                    Constraints = parentLoggerClass.ConstraintClauses.ToString(),
                                                     ParentClass = null,
                                                 };
 
@@ -683,7 +687,6 @@ internal class LoggerClass
             public string Keyword = string.Empty;
             public string Namespace = string.Empty;
             public string Name = string.Empty;
-            public string Constraints = string.Empty;
             public LoggerClass? ParentClass;
         }
 
@@ -714,6 +717,7 @@ internal class LoggerParameter
         {
             public string Name = string.Empty;
             public string Type = string.Empty;
+            public string? Qualifier;
             public bool IsLogger;
             public bool IsException;
             public bool IsLogLevel;

diff --git a/...Microsoft.Extensions.Logging.Generators.Tests/Baselines/TestWithNestedClass.generated.txt b/...Microsoft.Extensions.Logging.Generators.Tests/Baselines/TestWithNestedClass.generated.txt
@@ -9,11 +9,11 @@ namespace Microsoft.Extensions.Logging.Generators.Tests.TestClasses.NestedNamesp
         {
             partial record NestedRecord 
             {
-                partial class NestedClassTestsExtensions<T1> where T1 : Class1
+                partial class NestedClassTestsExtensions<T1> 
                 {
                     partial class NestedMiddleParentClass 
                     {
-                        partial class Nested<T2> where T2 : Class2
+                        partial class Nested<T2> 
                         {
                             [global::System.CodeDom.Compiler.GeneratedCodeAttribute("Microsoft.Extensions.Logging.Generators", "%VERSION%")]
                             private static readonly global::System.Action<global::Microsoft.Extensions.Logging.ILogger, global::System.Exception?> __M9Callback =

diff --git a/...ns/tests/Microsoft.Extensions.Logging.Generators.Tests/LoggerMessageGeneratedCodeTests.cs b/...ns/tests/Microsoft.Extensions.Logging.Generators.Tests/LoggerMessageGeneratedCodeTests.cs
@@ -3,8 +3,12 @@
 
 using System;
 using System.Collections.Generic;
+using Microsoft.Extensions.Logging.Abstractions;
 using Microsoft.Extensions.Logging.Generators.Tests.TestClasses;
+using Microsoft.Extensions.Logging.Generators.Tests.TestClasses.UsesConstraintInAnotherNamespace;
 using Xunit;
+using NamespaceForABC;
+using ConstraintInAnotherNamespace;
 
 namespace Microsoft.Extensions.Logging.Generators.Tests
 {
@@ -430,6 +434,63 @@ public void SkipEnabledCheckTests()
             Assert.Equal(1, logger.CallCount);
             Assert.Equal("LoggerMethodWithTrueSkipEnabledCheck", logger.LastEventId.Name);
         }
+        private struct MyStruct { }
+
+        [Fact]
+        public void ConstraintsTests()
+        {
+            var logger = new MockLogger();
+            var printer = new MessagePrinter<Message>();
+
+            logger.Reset();
+            printer.Print(logger, new Message() { Text = "Hello" });
+            Assert.Equal(LogLevel.Information, logger.LastLogLevel);
+            Assert.Null(logger.LastException);
+            Assert.Equal("The message is Hello.", logger.LastFormattedString);
+            Assert.Equal(1, logger.CallCount);
+
+            logger.Reset();
+            ConstraintsTestExtensions<Object>.M0(logger, 12);
+            Assert.Equal(LogLevel.Debug, logger.LastLogLevel);
+            Assert.Null(logger.LastException);
+            Assert.Equal("M012", logger.LastFormattedString);
+            Assert.Equal(1, logger.CallCount);
+
+            logger.Reset();
+            ConstraintsTestExtensions1<MyStruct>.M0(logger, 12);
+            Assert.Equal(LogLevel.Debug, logger.LastLogLevel);
+            Assert.Null(logger.LastException);
+            Assert.Equal("M012", logger.LastFormattedString);
+            Assert.Equal(1, logger.CallCount);
+
+            logger.Reset();
+            ConstraintsTestExtensions2<MyStruct>.M0(logger, 12);
+            Assert.Equal(LogLevel.Debug, logger.LastLogLevel);
+            Assert.Null(logger.LastException);
+            Assert.Equal("M012", logger.LastFormattedString);
+            Assert.Equal(1, logger.CallCount);
+
+            logger.Reset();
+            ConstraintsTestExtensions3<MyStruct>.M0(logger, 12);
+            Assert.Equal(LogLevel.Debug, logger.LastLogLevel);
+            Assert.Null(logger.LastException);
+            Assert.Equal("M012", logger.LastFormattedString);
+            Assert.Equal(1, logger.CallCount);
+
+            logger.Reset();
+            ConstraintsTestExtensions4<Attribute>.M0(logger, 12);
+            Assert.Equal(LogLevel.Debug, logger.LastLogLevel);
+            Assert.Null(logger.LastException);
+            Assert.Equal("M012", logger.LastFormattedString);
+            Assert.Equal(1, logger.CallCount);
+
+            logger.Reset();
+            ConstraintsTestExtensions5<MyStruct>.M0(logger, 12);
+            Assert.Equal(LogLevel.Debug, logger.LastLogLevel);
+            Assert.Null(logger.LastException);
+            Assert.Equal("M012", logger.LastFormattedString);
+            Assert.Equal(1, logger.CallCount);
+        }
 
         [Fact]
         public void NestedClassTests()

diff --git a/.../tests/Microsoft.Extensions.Logging.Generators.Tests/LoggerMessageGeneratorParserTests.cs b/.../tests/Microsoft.Extensions.Logging.Generators.Tests/LoggerMessageGeneratorParserTests.cs
@@ -616,6 +616,19 @@ partial class C
             Assert.Equal(DiagnosticDescriptors.LoggingMethodIsGeneric.Id, diagnostics[0].Id);
         }
 
+        [Fact]
+        public async Task SupportsRefKindIn()
+        {
+            IReadOnlyList<Diagnostic> diagnostics = await RunGenerator(@"
+                partial class C
+                {
+                    [LoggerMessage(EventId = 0, Level = LogLevel.Debug, Message = ""Parameter {P1}"")]
+                    static partial void M(ILogger logger, in int p1);
+                }");
+
+            Assert.Empty(diagnostics);
+        }
+
         [Fact]
         public async Task Templates()
         {

diff --git a/...sts/Microsoft.Extensions.Logging.Generators.Tests/TestClasses/ConstaintsTestExtensions.cs b/...sts/Microsoft.Extensions.Logging.Generators.Tests/TestClasses/ConstaintsTestExtensions.cs
@@ -0,0 +1,98 @@
+// Licensed to the .NET Foundation under one or more agreements.
+// The .NET Foundation licenses this file to you under the MIT license.
+
+namespace Microsoft.Extensions.Logging.Generators.Tests.TestClasses
+{
+    using ConstraintInAnotherNamespace;
+    namespace UsesConstraintInAnotherNamespace
+    {
+        public partial class MessagePrinter<T>
+            where T : Message
+        {
+            public void Print(ILogger logger, T message)
+            {
+                Log.Message(logger, message.Text);
+            }
+
+            internal static partial class Log
+            {
+                [LoggerMessage(EventId = 1, Level = LogLevel.Information, Message = "The message is {Text}.")]
+                internal static partial void Message(ILogger logger, string? text);
+            }
+        }
+    }
+
+    internal static partial class ConstraintsTestExtensions<T>
+        where T : class
+    {
+        [LoggerMessage(EventId = 0, Level = LogLevel.Debug, Message = "M0{p0}")]
+        public static partial void M0(ILogger logger, int p0);
+
+        public static void Foo(T dummy)
+        {
+        }
+    }
+
+    internal static partial class ConstraintsTestExtensions1<T>
+        where T : struct
+    {
+        [LoggerMessage(EventId = 0, Level = LogLevel.Debug, Message = "M0{p0}")]
+        public static partial void M0(ILogger logger, int p0);
+
+        public static void Foo(T dummy)
+        {
+        }
+    }
+
+    internal static partial class ConstraintsTestExtensions2<T>
+        where T : unmanaged
+    {
+        [LoggerMessage(EventId = 0, Level = LogLevel.Debug, Message = "M0{p0}")]
+        public static partial void M0(ILogger logger, int p0);
+
+        public static void Foo(T dummy)
+        {
+        }
+    }
+
+    internal static partial class ConstraintsTestExtensions3<T>
+        where T : new()
+    {
+        [LoggerMessage(EventId = 0, Level = LogLevel.Debug, Message = "M0{p0}")]
+        public static partial void M0(ILogger logger, int p0);
+
+        public static void Foo(T dummy)
+        {
+        }
+    }
+
+    internal static partial class ConstraintsTestExtensions4<T>
+        where T : System.Attribute
+    {
+        [LoggerMessage(EventId = 0, Level = LogLevel.Debug, Message = "M0{p0}")]
+        public static partial void M0(ILogger logger, int p0);
+
+        public static void Foo(T dummy)
+        {
+        }
+    }
+
+    internal static partial class ConstraintsTestExtensions5<T>
+        where T : notnull
+    {
+        [LoggerMessage(EventId = 0, Level = LogLevel.Debug, Message = "M0{p0}")]
+        public static partial void M0(ILogger logger, int p0);
+
+        public static void Foo(T dummy)
+        {
+        }
+    }
+}
+
+namespace ConstraintInAnotherNamespace
+{
+    public class Message
+    {
+        public string? Text { get; set; }
+    }
+}
diff --git a/...ts/Microsoft.Extensions.Logging.Generators.Tests/TestClasses/InParameterTestExtensions.cs b/...ts/Microsoft.Extensions.Logging.Generators.Tests/TestClasses/InParameterTestExtensions.cs
@@ -0,0 +1,16 @@
+// Licensed to the .NET Foundation under one or more agreements.
+// The .NET Foundation licenses this file to you under the MIT license.
+
+namespace Microsoft.Extensions.Logging.Generators.Tests.TestClasses
+{
+    internal static partial class InParameterTestExtensions
+    {
+        internal struct S
+        {
+            public override string ToString() => "Hello from S";
+        }
+
+        [LoggerMessage(EventId = 0, Level = LogLevel.Information, Message = "M0 {s}")]
+        internal static partial void M0(ILogger logger, in S s);
+    }
+}
diff --git a/...s/Microsoft.Extensions.Logging.Generators.Tests/TestClasses/NestedClassTestsExtensions.cs b/...s/Microsoft.Extensions.Logging.Generators.Tests/TestClasses/NestedClassTestsExtensions.cs
@@ -3,6 +3,8 @@
 
 namespace Microsoft.Extensions.Logging.Generators.Tests.TestClasses
 {
+    using NamespaceForABC;
+
     internal static partial class NestedClassTestsExtensions<T> where T : ABC
     {
         internal static partial class NestedMiddleParentClass
@@ -26,7 +28,6 @@ internal static partial class NestedClass
             }
         }
     }
-    public class ABC {}
 
     public partial struct NestedStruct
     {
@@ -61,3 +62,8 @@ internal static partial class Logger
         }
     }
 }
+
+namespace NamespaceForABC
+{
+    public class ABC {}
+}