From 64801e50361624245d42359990f4d91289584738 Mon Sep 17 00:00:00 2001 From: JakeYallop <30874283+JakeYallop@users.noreply.github.com> Date: Mon, 31 Jan 2022 22:01:06 +0000 Subject: [PATCH 1/3] Compute unique method names for generated code Calculates and assigns a unique LoggerMethod name as required, and uses this new unique name for generating types in the source generator. This allows using the LoggerMessageAttribute on methods that share the same name but have different method signatures. Fix #61814 --- .../gen/LoggerMessageGenerator.Emitter.cs | 14 ++++---- .../gen/LoggerMessageGenerator.Parser.cs | 18 ++++++++++ .../LoggerMessageGeneratedCodeTests.cs | 34 +++++++++++++++++++ .../MethodOverloadTestExtensions.cs | 21 ++++++++++++ 4 files changed, 80 insertions(+), 7 deletions(-) create mode 100644 src/libraries/Microsoft.Extensions.Logging.Abstractions/tests/Microsoft.Extensions.Logging.Generators.Tests/TestClasses/MethodOverloadTestExtensions.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..eebab2be296144 100644 --- a/src/libraries/Microsoft.Extensions.Logging.Abstractions/gen/LoggerMessageGenerator.Emitter.cs +++ b/src/libraries/Microsoft.Extensions.Logging.Abstractions/gen/LoggerMessageGenerator.Emitter.cs @@ -138,14 +138,14 @@ private void GenStruct(LoggerMethod lm, string nestedIndentation) {nestedIndentation}/// {s_generatedTypeSummary} {nestedIndentation}[{s_generatedCodeAttribute}] {nestedIndentation}[{s_editorBrowsableAttribute}] - {nestedIndentation}private readonly struct __{lm.Name}Struct : global::System.Collections.Generic.IReadOnlyList> + {nestedIndentation}private readonly struct __{lm.UniqueName}Struct : global::System.Collections.Generic.IReadOnlyList> {nestedIndentation}{{"); GenFields(lm, nestedIndentation); if (lm.TemplateParameters.Count > 0) { _builder.Append($@" - {nestedIndentation}public __{lm.Name}Struct("); + {nestedIndentation}public __{lm.UniqueName}Struct("); GenArguments(lm); _builder.Append($@") {nestedIndentation}{{"); @@ -166,7 +166,7 @@ private void GenStruct(LoggerMethod lm, string nestedIndentation) {nestedIndentation}}} "); _builder.Append($@" - {nestedIndentation}public static readonly global::System.Func<__{lm.Name}Struct, global::System.Exception?, string> Format = (state, ex) => state.ToString(); + {nestedIndentation}public static readonly global::System.Func<__{lm.UniqueName}Struct, global::System.Exception?, string> Format = (state, ex) => state.ToString(); {nestedIndentation}public int Count => {lm.TemplateParameters.Count + 1}; @@ -343,7 +343,7 @@ private void GenArguments(LoggerMethod lm) private void GenHolder(LoggerMethod lm) { - string typeName = $"__{lm.Name}Struct"; + string typeName = $"__{lm.UniqueName}Struct"; _builder.Append($"new {typeName}("); foreach (LoggerParameter p in lm.TemplateParameters) @@ -375,7 +375,7 @@ private void GenLogMethod(LoggerMethod lm, string nestedIndentation) GenDefineTypes(lm, brackets: false); - _builder.Append($@"global::System.Exception?> __{lm.Name}Callback = + _builder.Append($@"global::System.Exception?> __{lm.UniqueName}Callback = {nestedIndentation}global::Microsoft.Extensions.Logging.LoggerMessage.Define"); GenDefineTypes(lm, brackets: true); @@ -404,7 +404,7 @@ private void GenLogMethod(LoggerMethod lm, string nestedIndentation) if (UseLoggerMessageDefine(lm)) { _builder.Append($@" - {nestedIndentation}{enabledCheckIndentation}__{lm.Name}Callback({logger}, "); + {nestedIndentation}{enabledCheckIndentation}__{lm.UniqueName}Callback({logger}, "); GenCallbackArguments(lm); @@ -420,7 +420,7 @@ private void GenLogMethod(LoggerMethod lm, string nestedIndentation) GenHolder(lm); _builder.Append($@", {nestedIndentation}{enabledCheckIndentation}{exceptionArg}, - {nestedIndentation}{enabledCheckIndentation}__{lm.Name}Struct.Format);"); + {nestedIndentation}{enabledCheckIndentation}__{lm.UniqueName}Struct.Format);"); } if (!lm.SkipEnabledCheck) 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 ad8a524b0fc8cc..63e85ee16dd89d 100644 --- a/src/libraries/Microsoft.Extensions.Logging.Abstractions/gen/LoggerMessageGenerator.Parser.cs +++ b/src/libraries/Microsoft.Extensions.Logging.Abstractions/gen/LoggerMessageGenerator.Parser.cs @@ -510,6 +510,23 @@ bool IsAllowedKind(SyntaxKind kind) => if (lc != null) { + //once we've collected all methods for the given class, check for overloads + //and provide unique names for logger methods + var methods = new Dictionary(lc.Methods.Count); + foreach (LoggerMethod lm in lc.Methods) + { + if (methods.ContainsKey(lm.Name)) + { + var currentCount = methods[lm.Name]; + lm.UniqueName = $"{lm.Name}{currentCount}"; + methods[lm.Name] = currentCount + 1; + } + else + { + lm.UniqueName = lm.Name; + methods[lm.Name] = 1; //start from 1 + } + } results.Add(lc); } } @@ -693,6 +710,7 @@ internal class LoggerMethod public readonly Dictionary TemplateMap = new(StringComparer.OrdinalIgnoreCase); public readonly List TemplateList = new(); public string Name = string.Empty; + public string UniqueName = string.Empty; public string Message = string.Empty; public int? Level; public int EventId; 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 d55bf4a8028d52..5bac432c7f41a2 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 @@ -498,6 +498,40 @@ public void TemplateTests() } + [Fact] + public void OverloadTests() + { + var logger = new MockLogger(); + + logger.Reset(); + OverloadTestExtensions.M0(logger, 1); + Assert.Null(logger.LastException); + Assert.Equal($"{nameof(OverloadTestExtensions.M0)}1", logger.LastFormattedString); + Assert.Equal(LogLevel.Trace, logger.LastLogLevel); + Assert.Equal("M0", logger.LastEventId.Name); + + logger.Reset(); + OverloadTestExtensions.M0(logger, "string"); + Assert.Null(logger.LastException); + Assert.Equal($"{nameof(OverloadTestExtensions.M0)}string", logger.LastFormattedString); + Assert.Equal(LogLevel.Trace, logger.LastLogLevel); + Assert.Equal("M0", logger.LastEventId.Name); + + logger.Reset(); + OverloadTestExtensions.M1(logger, 1); + Assert.Null(logger.LastException); + Assert.Equal($"{nameof(OverloadTestExtensions.M1)}1", logger.LastFormattedString); + Assert.Equal(LogLevel.Trace, logger.LastLogLevel); + Assert.Equal("M1Custom", logger.LastEventId.Name); + + logger.Reset(); + OverloadTestExtensions.M1(logger, "string"); + Assert.Null(logger.LastException); + Assert.Equal($"{nameof(OverloadTestExtensions.M1)}string", logger.LastFormattedString); + Assert.Equal(LogLevel.Trace, logger.LastLogLevel); + Assert.Equal("M1", logger.LastEventId.Name); + } + private static void AssertLastState(MockLogger logger, params KeyValuePair[] expected) { var rol = (IReadOnlyList>)logger.LastState!; diff --git a/src/libraries/Microsoft.Extensions.Logging.Abstractions/tests/Microsoft.Extensions.Logging.Generators.Tests/TestClasses/MethodOverloadTestExtensions.cs b/src/libraries/Microsoft.Extensions.Logging.Abstractions/tests/Microsoft.Extensions.Logging.Generators.Tests/TestClasses/MethodOverloadTestExtensions.cs new file mode 100644 index 00000000000000..7f22d209fac3ee --- /dev/null +++ b/src/libraries/Microsoft.Extensions.Logging.Abstractions/tests/Microsoft.Extensions.Logging.Generators.Tests/TestClasses/MethodOverloadTestExtensions.cs @@ -0,0 +1,21 @@ +// 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 OverloadTestExtensions + { + [LoggerMessage(EventId = 0, Level = LogLevel.Trace, Message = "M0{p0}")] + public static partial void M0(ILogger logger, int p0); + + [LoggerMessage(EventId = 1, Level = LogLevel.Trace, Message = "M0{p0}")] + public static partial void M0(ILogger logger, string p0); + + + [LoggerMessage(EventId = 2, EventName = "M1Custom", Level = LogLevel.Trace, Message = "M1{p0}")] + public static partial void M1(ILogger logger, int p0); + + [LoggerMessage(EventId = 3, Level = LogLevel.Trace, Message = "M1{p0}")] + public static partial void M1(ILogger logger, string p0); + } +} From 60cc1bec2b87eacf154b896f8f4c03f7cf2c6a8f Mon Sep 17 00:00:00 2001 From: JakeYallop <30874283+JakeYallop@users.noreply.github.com> Date: Tue, 1 Feb 2022 17:48:17 +0000 Subject: [PATCH 2/3] Rename MethodOverloadTestExtensions to OverloadTestExtensions Also resolves other PR feedback, an extra blank line and incorrect use of an implicit type. --- .../gen/LoggerMessageGenerator.Parser.cs | 2 +- ...ethodOverloadTestExtensions.cs => OverloadTestExtensions.cs} | 1 - 2 files changed, 1 insertion(+), 2 deletions(-) rename src/libraries/Microsoft.Extensions.Logging.Abstractions/tests/Microsoft.Extensions.Logging.Generators.Tests/TestClasses/{MethodOverloadTestExtensions.cs => OverloadTestExtensions.cs} (99%) 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 63e85ee16dd89d..0f7aa5539a7d61 100644 --- a/src/libraries/Microsoft.Extensions.Logging.Abstractions/gen/LoggerMessageGenerator.Parser.cs +++ b/src/libraries/Microsoft.Extensions.Logging.Abstractions/gen/LoggerMessageGenerator.Parser.cs @@ -517,7 +517,7 @@ bool IsAllowedKind(SyntaxKind kind) => { if (methods.ContainsKey(lm.Name)) { - var currentCount = methods[lm.Name]; + int currentCount = methods[lm.Name]; lm.UniqueName = $"{lm.Name}{currentCount}"; methods[lm.Name] = currentCount + 1; } diff --git a/src/libraries/Microsoft.Extensions.Logging.Abstractions/tests/Microsoft.Extensions.Logging.Generators.Tests/TestClasses/MethodOverloadTestExtensions.cs b/src/libraries/Microsoft.Extensions.Logging.Abstractions/tests/Microsoft.Extensions.Logging.Generators.Tests/TestClasses/OverloadTestExtensions.cs similarity index 99% rename from src/libraries/Microsoft.Extensions.Logging.Abstractions/tests/Microsoft.Extensions.Logging.Generators.Tests/TestClasses/MethodOverloadTestExtensions.cs rename to src/libraries/Microsoft.Extensions.Logging.Abstractions/tests/Microsoft.Extensions.Logging.Generators.Tests/TestClasses/OverloadTestExtensions.cs index 7f22d209fac3ee..1cbb24f50eca03 100644 --- a/src/libraries/Microsoft.Extensions.Logging.Abstractions/tests/Microsoft.Extensions.Logging.Generators.Tests/TestClasses/MethodOverloadTestExtensions.cs +++ b/src/libraries/Microsoft.Extensions.Logging.Abstractions/tests/Microsoft.Extensions.Logging.Generators.Tests/TestClasses/OverloadTestExtensions.cs @@ -11,7 +11,6 @@ internal static partial class OverloadTestExtensions [LoggerMessage(EventId = 1, Level = LogLevel.Trace, Message = "M0{p0}")] public static partial void M0(ILogger logger, string p0); - [LoggerMessage(EventId = 2, EventName = "M1Custom", Level = LogLevel.Trace, Message = "M1{p0}")] public static partial void M1(ILogger logger, int p0); From a69a3e4f9381e014a18d7d5c24ad1ef71ff9b325 Mon Sep 17 00:00:00 2001 From: JakeYallop <30874283+JakeYallop@users.noreply.github.com> Date: Tue, 1 Feb 2022 18:46:01 +0000 Subject: [PATCH 3/3] Remove un-needed overload tests --- .../LoggerMessageGeneratedCodeTests.cs | 14 -------------- .../TestClasses/OverloadTestExtensions.cs | 6 ------ 2 files changed, 20 deletions(-) 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 5bac432c7f41a2..677dc148f60d25 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 @@ -516,20 +516,6 @@ public void OverloadTests() Assert.Equal($"{nameof(OverloadTestExtensions.M0)}string", logger.LastFormattedString); Assert.Equal(LogLevel.Trace, logger.LastLogLevel); Assert.Equal("M0", logger.LastEventId.Name); - - logger.Reset(); - OverloadTestExtensions.M1(logger, 1); - Assert.Null(logger.LastException); - Assert.Equal($"{nameof(OverloadTestExtensions.M1)}1", logger.LastFormattedString); - Assert.Equal(LogLevel.Trace, logger.LastLogLevel); - Assert.Equal("M1Custom", logger.LastEventId.Name); - - logger.Reset(); - OverloadTestExtensions.M1(logger, "string"); - Assert.Null(logger.LastException); - Assert.Equal($"{nameof(OverloadTestExtensions.M1)}string", logger.LastFormattedString); - Assert.Equal(LogLevel.Trace, logger.LastLogLevel); - Assert.Equal("M1", logger.LastEventId.Name); } private static void AssertLastState(MockLogger logger, params KeyValuePair[] expected) diff --git a/src/libraries/Microsoft.Extensions.Logging.Abstractions/tests/Microsoft.Extensions.Logging.Generators.Tests/TestClasses/OverloadTestExtensions.cs b/src/libraries/Microsoft.Extensions.Logging.Abstractions/tests/Microsoft.Extensions.Logging.Generators.Tests/TestClasses/OverloadTestExtensions.cs index 1cbb24f50eca03..e0c84fcebedd02 100644 --- a/src/libraries/Microsoft.Extensions.Logging.Abstractions/tests/Microsoft.Extensions.Logging.Generators.Tests/TestClasses/OverloadTestExtensions.cs +++ b/src/libraries/Microsoft.Extensions.Logging.Abstractions/tests/Microsoft.Extensions.Logging.Generators.Tests/TestClasses/OverloadTestExtensions.cs @@ -10,11 +10,5 @@ internal static partial class OverloadTestExtensions [LoggerMessage(EventId = 1, Level = LogLevel.Trace, Message = "M0{p0}")] public static partial void M0(ILogger logger, string p0); - - [LoggerMessage(EventId = 2, EventName = "M1Custom", Level = LogLevel.Trace, Message = "M1{p0}")] - public static partial void M1(ILogger logger, int p0); - - [LoggerMessage(EventId = 3, Level = LogLevel.Trace, Message = "M1{p0}")] - public static partial void M1(ILogger logger, string p0); } }