From be3a808bb40a229b9c1f07ea7866e97a3dae82f8 Mon Sep 17 00:00:00 2001 From: Maryam Ariyan Date: Mon, 31 Jan 2022 18:54:28 -0800 Subject: [PATCH] Fixes some logging source gen bugs: - Supports usage of "in" modifier - Improves support for generic constraints Fixes #58550, #62644 --- .../gen/LoggerMessageGenerator.Emitter.cs | 8 +- .../gen/LoggerMessageGenerator.Parser.cs | 10 +- .../TestWithNestedClass.generated.txt | 4 +- .../LoggerMessageGeneratedCodeTests.cs | 61 ++++++++++++ .../LoggerMessageGeneratorParserTests.cs | 13 +++ .../TestClasses/ConstaintsTestExtensions.cs | 98 +++++++++++++++++++ .../TestClasses/InParameterTestExtensions.cs | 16 +++ .../TestClasses/NestedClassTestsExtensions.cs | 8 +- 8 files changed, 210 insertions(+), 8 deletions(-) create mode 100644 src/libraries/Microsoft.Extensions.Logging.Abstractions/tests/Microsoft.Extensions.Logging.Generators.Tests/TestClasses/ConstaintsTestExtensions.cs create mode 100644 src/libraries/Microsoft.Extensions.Logging.Abstractions/tests/Microsoft.Extensions.Logging.Generators.Tests/TestClasses/InParameterTestExtensions.cs diff --git a/src/libraries/Microsoft.Extensions.Logging.Abstractions/gen/LoggerMessageGenerator.Emitter.cs b/src/libraries/Microsoft.Extensions.Logging.Abstractions/gen/LoggerMessageGenerator.Emitter.cs index 69f87fdcd48f6..5b07aecd7eb77 100644 --- a/src/libraries/Microsoft.Extensions.Logging.Abstractions/gen/LoggerMessageGenerator.Emitter.cs +++ b/src/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 index 2f624f6cec974..bd3deb53c11eb 100644 --- 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 GetLogClasses(IEnumerable GetLogClasses(IEnumerable 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/src/libraries/Microsoft.Extensions.Logging.Abstractions/tests/Microsoft.Extensions.Logging.Generators.Tests/Baselines/TestWithNestedClass.generated.txt b/src/libraries/Microsoft.Extensions.Logging.Abstractions/tests/Microsoft.Extensions.Logging.Generators.Tests/Baselines/TestWithNestedClass.generated.txt index b7b93bf27bd02..07cd2a3aff421 100644 --- a/src/libraries/Microsoft.Extensions.Logging.Abstractions/tests/Microsoft.Extensions.Logging.Generators.Tests/Baselines/TestWithNestedClass.generated.txt +++ b/src/libraries/Microsoft.Extensions.Logging.Abstractions/tests/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 where T1 : Class1 + partial class NestedClassTestsExtensions { partial class NestedMiddleParentClass { - partial class Nested where T2 : Class2 + partial class Nested { [global::System.CodeDom.Compiler.GeneratedCodeAttribute("Microsoft.Extensions.Logging.Generators", "%VERSION%")] private static readonly global::System.Action __M9Callback = diff --git a/src/libraries/Microsoft.Extensions.Logging.Abstractions/tests/Microsoft.Extensions.Logging.Generators.Tests/LoggerMessageGeneratedCodeTests.cs b/src/libraries/Microsoft.Extensions.Logging.Abstractions/tests/Microsoft.Extensions.Logging.Generators.Tests/LoggerMessageGeneratedCodeTests.cs index eda529fade4ef..3ca395f04ca6b 100644 --- a/src/libraries/Microsoft.Extensions.Logging.Abstractions/tests/Microsoft.Extensions.Logging.Generators.Tests/LoggerMessageGeneratedCodeTests.cs +++ b/src/libraries/Microsoft.Extensions.Logging.Abstractions/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(); + + 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.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.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.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.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.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.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/src/libraries/Microsoft.Extensions.Logging.Abstractions/tests/Microsoft.Extensions.Logging.Generators.Tests/LoggerMessageGeneratorParserTests.cs b/src/libraries/Microsoft.Extensions.Logging.Abstractions/tests/Microsoft.Extensions.Logging.Generators.Tests/LoggerMessageGeneratorParserTests.cs index 4697e5603cd0f..0b3c7979895e3 100644 --- a/src/libraries/Microsoft.Extensions.Logging.Abstractions/tests/Microsoft.Extensions.Logging.Generators.Tests/LoggerMessageGeneratorParserTests.cs +++ b/src/libraries/Microsoft.Extensions.Logging.Abstractions/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 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/src/libraries/Microsoft.Extensions.Logging.Abstractions/tests/Microsoft.Extensions.Logging.Generators.Tests/TestClasses/ConstaintsTestExtensions.cs b/src/libraries/Microsoft.Extensions.Logging.Abstractions/tests/Microsoft.Extensions.Logging.Generators.Tests/TestClasses/ConstaintsTestExtensions.cs new file mode 100644 index 0000000000000..557d69ad9a4f7 --- /dev/null +++ b/src/libraries/Microsoft.Extensions.Logging.Abstractions/tests/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 + 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 + 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 + 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 + 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 + 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 + 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 + 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/src/libraries/Microsoft.Extensions.Logging.Abstractions/tests/Microsoft.Extensions.Logging.Generators.Tests/TestClasses/InParameterTestExtensions.cs b/src/libraries/Microsoft.Extensions.Logging.Abstractions/tests/Microsoft.Extensions.Logging.Generators.Tests/TestClasses/InParameterTestExtensions.cs new file mode 100644 index 0000000000000..4bd050c9ca6f2 --- /dev/null +++ b/src/libraries/Microsoft.Extensions.Logging.Abstractions/tests/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/src/libraries/Microsoft.Extensions.Logging.Abstractions/tests/Microsoft.Extensions.Logging.Generators.Tests/TestClasses/NestedClassTestsExtensions.cs b/src/libraries/Microsoft.Extensions.Logging.Abstractions/tests/Microsoft.Extensions.Logging.Generators.Tests/TestClasses/NestedClassTestsExtensions.cs index 4b5b6cdbf90a2..ba6b87f6d5308 100644 --- a/src/libraries/Microsoft.Extensions.Logging.Abstractions/tests/Microsoft.Extensions.Logging.Generators.Tests/TestClasses/NestedClassTestsExtensions.cs +++ b/src/libraries/Microsoft.Extensions.Logging.Abstractions/tests/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 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 {} +} \ No newline at end of file