From be3a808bb40a229b9c1f07ea7866e97a3dae82f8 Mon Sep 17 00:00:00 2001 From: Maryam Ariyan Date: Mon, 31 Jan 2022 18:54:28 -0800 Subject: [PATCH 1/3] 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 69f87fdcd48f6c..5b07aecd7eb774 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 2f624f6cec974e..bd3deb53c11eb0 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 b7b93bf27bd02d..07cd2a3aff4211 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 eda529fade4ef1..3ca395f04ca6b8 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 4697e5603cd0f5..0b3c7979895e36 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 00000000000000..557d69ad9a4f71 --- /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 00000000000000..4bd050c9ca6f2c --- /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 4b5b6cdbf90a26..ba6b87f6d5308a 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 From bcae28c846d8eee88612917ae7e08fc87a9faf2b Mon Sep 17 00:00:00 2001 From: Maryam Ariyan Date: Mon, 31 Jan 2022 19:24:09 -0800 Subject: [PATCH 2/3] Apply PR feedback --- .../gen/LoggerMessageGenerator.Parser.cs | 4 ++++ .../LoggerMessageGeneratorParserTests.cs | 16 +++++++++------- ...tExtensions.cs => ParameterTestExtensions.cs} | 9 ++++++--- 3 files changed, 19 insertions(+), 10 deletions(-) rename src/libraries/Microsoft.Extensions.Logging.Abstractions/tests/Microsoft.Extensions.Logging.Generators.Tests/TestClasses/{InParameterTestExtensions.cs => ParameterTestExtensions.cs} (53%) 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 bd3deb53c11eb0..070415f3144d17 100644 --- a/src/libraries/Microsoft.Extensions.Logging.Abstractions/gen/LoggerMessageGenerator.Parser.cs +++ b/src/libraries/Microsoft.Extensions.Logging.Abstractions/gen/LoggerMessageGenerator.Parser.cs @@ -326,6 +326,10 @@ public IReadOnlyList GetLogClasses(IEnumerable diagnostics = await RunGenerator(@" + 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); - }"); + {{ + [LoggerMessage(EventId = 0, Level = LogLevel.Debug, Message = ""Parameter {{P1}}"")] + static partial void M(ILogger logger, {modifier} int p1); + }}"); Assert.Empty(diagnostics); } 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/ParameterTestExtensions.cs similarity index 53% rename from src/libraries/Microsoft.Extensions.Logging.Abstractions/tests/Microsoft.Extensions.Logging.Generators.Tests/TestClasses/InParameterTestExtensions.cs rename to src/libraries/Microsoft.Extensions.Logging.Abstractions/tests/Microsoft.Extensions.Logging.Generators.Tests/TestClasses/ParameterTestExtensions.cs index 4bd050c9ca6f2c..f51fc2e62f6a96 100644 --- 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/ParameterTestExtensions.cs @@ -3,14 +3,17 @@ namespace Microsoft.Extensions.Logging.Generators.Tests.TestClasses { - internal static partial class InParameterTestExtensions + internal static partial class ParameterTestExtensions { 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); + [LoggerMessage(EventId = 0, Level = LogLevel.Information, Message = "UseInParameter {s}")] + internal static partial void UseInParameter(ILogger logger, in S s); + + [LoggerMessage(EventId = 1, Level = LogLevel.Information, Message = "UseRefParameter {s}")] + internal static partial void UseRefParameter(ILogger logger, ref S s); } } From 2cf72e17b110dc78f31cc739b8ad1477911175c9 Mon Sep 17 00:00:00 2001 From: Maryam Ariyan Date: Tue, 1 Feb 2022 12:52:43 -0800 Subject: [PATCH 3/3] Add another test --- .../LoggerMessageGeneratedCodeTests.cs | 10 +++++++++- .../TestClasses/ConstaintsTestExtensions.cs | 20 +++++++++++++++++++ 2 files changed, 29 insertions(+), 1 deletion(-) 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 3ca395f04ca6b8..fb9c13aa1cade4 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 @@ -440,8 +440,8 @@ private struct MyStruct { } public void ConstraintsTests() { var logger = new MockLogger(); - var printer = new MessagePrinter(); + var printer = new MessagePrinter(); logger.Reset(); printer.Print(logger, new Message() { Text = "Hello" }); Assert.Equal(LogLevel.Information, logger.LastLogLevel); @@ -449,6 +449,14 @@ public void ConstraintsTests() Assert.Equal("The message is Hello.", logger.LastFormattedString); Assert.Equal(1, logger.CallCount); + var printer2 = new MessagePrinterHasConstraintOnLogClassAndLogMethod(); + 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.M0(logger, 12); Assert.Equal(LogLevel.Debug, logger.LastLogLevel); 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 index 557d69ad9a4f71..7e6f87271b535f 100644 --- 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 @@ -20,6 +20,21 @@ internal static partial class Log internal static partial void Message(ILogger logger, string? text); } } + + public partial class MessagePrinterHasConstraintOnLogClassAndLogMethod + where T : Message + { + public void Print(ILogger logger, T message) + { + Log.Message(logger, message); + } + + internal static partial class Log where U : Message + { + [LoggerMessage(EventId = 1, Level = LogLevel.Information, Message = "The message is {Text}.")] + internal static partial void Message(ILogger logger, U text); + } + } } internal static partial class ConstraintsTestExtensions @@ -94,5 +109,10 @@ namespace ConstraintInAnotherNamespace public class Message { public string? Text { get; set; } + + public override string ToString() + { + return $"`{Text}`"; + } } }