[release/6.0] Fix Logging source generator failures with various synt…

…ax (#64779)
dotnet · Feb 9, 2022 · 3334f3b · 3334f3b
1 parent 7a62f17
commit 3334f3b
Show file tree

Hide file tree

Showing 10 changed files with 276 additions and 13 deletions.
diff --git a/...libraries/Microsoft.Extensions.Logging.Abstractions/gen/LoggerMessageGenerator.Emitter.cs b/...libraries/Microsoft.Extensions.Logging.Abstractions/gen/LoggerMessageGenerator.Emitter.cs
@@ -78,7 +78,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;
                 }
 
@@ -92,7 +92,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)
@@ -196,7 +196,7 @@ private void GenFieldAssignments(LoggerMethod lm, string nestedIndentation)
             {
                 foreach (LoggerParameter p in lm.TemplateParameters)
                 {
-                    _builder.AppendLine($"                {nestedIndentation}this._{p.Name} = {p.Name};");
+                    _builder.AppendLine($"                {nestedIndentation}this._{p.Name} = {p.CodeName};");
                 }
             }
 
@@ -255,7 +255,7 @@ private void GenCallbackArguments(LoggerMethod lm)
             {
                 foreach (LoggerParameter p in lm.TemplateParameters)
                 {
-                    _builder.Append($"{p.Name}, ");
+                    _builder.Append($"{p.CodeName}, ");
                 }
             }
 
@@ -309,7 +309,11 @@ private void GenParameters(LoggerMethod lm)
                         _builder.Append(", ");
                     }
 
-                    _builder.Append($"{p.Type} {p.Name}");
+                    if (p.Qualifier != null)
+                    {
+                        _builder.Append($"{p.Qualifier} ");
+                    }
+                    _builder.Append($"{p.Type} {p.CodeName}");
                 }
             }
 
@@ -327,7 +331,7 @@ private void GenArguments(LoggerMethod lm)
                         _builder.Append(", ");
                     }
 
-                    _builder.Append($"{p.Type} {p.Name}");
+                    _builder.Append($"{p.Type} {p.CodeName}");
                 }
             }
 
@@ -343,7 +347,7 @@ private void GenHolder(LoggerMethod lm)
                         _builder.Append(", ");
                     }
 
-                    _builder.Append(p.Name);
+                    _builder.Append(p.CodeName);
                 }
 
                 _builder.Append(')');

diff --git a/src/libraries/Microsoft.Extensions.Logging.Abstractions/gen/LoggerMessageGenerator.Parser.cs b/src/libraries/Microsoft.Extensions.Logging.Abstractions/gen/LoggerMessageGenerator.Parser.cs
@@ -308,6 +308,15 @@ public IReadOnlyList<LoggerClass> GetLogClasses(IEnumerable<ClassDeclarationSynt
                                         foreach (IParameterSymbol paramSymbol in methodSymbol.Parameters)
                                         {
                                             string paramName = paramSymbol.Name;
+                                            bool needsAtSign = false;
+                                            if (paramSymbol.DeclaringSyntaxReferences.Length > 0)
+                                            {
+                                                ParameterSyntax paramSyntax = paramSymbol.DeclaringSyntaxReferences[0].GetSyntax(_cancellationToken) as ParameterSyntax;
+                                                if (paramSyntax != null && !string.IsNullOrEmpty(paramSyntax.Identifier.Text))
+                                                {
+                                                    needsAtSign = paramSyntax.Identifier.Text[0] == '@';
+                                                }
+                                            }
                                             if (string.IsNullOrWhiteSpace(paramName))
                                             {
                                                 // semantic problem, just bail quietly
@@ -323,6 +332,15 @@ public IReadOnlyList<LoggerClass> GetLogClasses(IEnumerable<ClassDeclarationSynt
                                                 break;
                                             }
 
+                                            string? qualifier = null;
+                                            if (paramSymbol.RefKind == RefKind.In)
+                                            {
+                                                qualifier = "in";
+                                            }
+                                            else if (paramSymbol.RefKind == RefKind.Ref)
+                                            {
+                                                qualifier = "ref";
+                                            }
                                             string typeName = paramTypeSymbol.ToDisplayString(
                                                 SymbolDisplayFormat.FullyQualifiedFormat.WithMiscellaneousOptions(
                                                     SymbolDisplayMiscellaneousOptions.IncludeNullableReferenceTypeModifier));
@@ -331,6 +349,8 @@ public IReadOnlyList<LoggerClass> GetLogClasses(IEnumerable<ClassDeclarationSynt
                                             {
                                                 Name = paramName,
                                                 Type = typeName,
+                                                Qualifier = qualifier,
+                                                CodeName = needsAtSign ? "@" + paramName : paramName,
                                                 IsLogger = !foundLogger && IsBaseOrIdentity(paramTypeSymbol!, loggerSymbol),
                                                 IsException = !foundException && IsBaseOrIdentity(paramTypeSymbol!, exceptionSymbol),
                                                 IsLogLevel = !foundLogLevel && IsBaseOrIdentity(paramTypeSymbol!, logLevelSymbol),
@@ -476,7 +496,6 @@ potentialNamespaceParent is not NamespaceDeclarationSyntax
                                                 Keyword = classDec.Keyword.ValueText,
                                                 Namespace = nspace,
                                                 Name = classDec.Identifier.ToString() + classDec.TypeParameterList,
-                                                Constraints = classDec.ConstraintClauses.ToString(),
                                                 ParentClass = null,
                                             };
 
@@ -495,7 +514,6 @@ bool IsAllowedKind(SyntaxKind kind) =>
                                                     Keyword = parentLoggerClass.Keyword.ValueText,
                                                     Namespace = nspace,
                                                     Name = parentLoggerClass.Identifier.ToString() + parentLoggerClass.TypeParameterList,
-                                                    Constraints = parentLoggerClass.ConstraintClauses.ToString(),
                                                     ParentClass = null,
                                                 };
 
@@ -681,7 +699,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;
         }
 
@@ -712,6 +729,8 @@ internal class LoggerParameter
         {
             public string Name = string.Empty;
             public string Type = string.Empty;
+            public string CodeName = string.Empty;
+            public string? Qualifier;
             public bool IsLogger;
             public bool IsException;
             public bool IsLogLevel;

diff --git a/...soft.Extensions.Logging.Abstractions/src/Microsoft.Extensions.Logging.Abstractions.csproj b/...soft.Extensions.Logging.Abstractions/src/Microsoft.Extensions.Logging.Abstractions.csproj
@@ -17,6 +17,8 @@ Microsoft.Extensions.Logging.LogLevel
 Microsoft.Extensions.Logging.Logger&lt;T&gt;
 Microsoft.Extensions.Logging.LoggerMessage
 Microsoft.Extensions.Logging.Abstractions.NullLogger</PackageDescription>
+    <GeneratePackageOnBuild>true</GeneratePackageOnBuild>
+    <ServicingVersion>1</ServicingVersion>
   </PropertyGroup>
 
   <ItemGroup>

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
 {
@@ -410,6 +414,71 @@ 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);
+
+            var printer2 = new MessagePrinterHasConstraintOnLogClassAndLogMethod<Message>();
+            logger.Reset();
+            printer2.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
@@ -606,6 +606,21 @@ partial class C
             Assert.Equal(DiagnosticDescriptors.LoggingMethodIsGeneric.Id, diagnostics[0].Id);
         }
 
+        [Theory]
+        [InlineData("ref")]
+        [InlineData("in")]
+        public async Task SupportsRefKindsInAndRef(string modifier)
+        {
+            IReadOnlyList<Diagnostic> diagnostics = await RunGenerator(@$"
+                partial class C
+                {{
+                    [LoggerMessage(EventId = 0, Level = LogLevel.Debug, Message = ""Parameter {{P1}}"")]
+                    static partial void M(ILogger logger, {modifier} int p1);
+                }}");
+
+            Assert.Empty(diagnostics);
+        }
+
         [Fact]
         public async Task Templates()
         {

diff --git a/...tests/Microsoft.Extensions.Logging.Generators.Tests/TestClasses/AtSymbolTestExtensions.cs b/...tests/Microsoft.Extensions.Logging.Generators.Tests/TestClasses/AtSymbolTestExtensions.cs
@@ -0,0 +1,11 @@
+// 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 AtSymbolTestExtensions
+    {
+        [LoggerMessage(EventId = 0, Level = LogLevel.Information, Message = "M0 {event}")]
+        internal static partial void M0(ILogger logger, string @event);
+    }
+}