From 97fc50089afb67daf695c472287c0b3348db911f Mon Sep 17 00:00:00 2001 From: Maryam Ariyan Date: Mon, 14 Jun 2021 13:54:51 -0700 Subject: [PATCH 1/6] Support log methods in nested classes --- .../gen/LoggerMessageGenerator.Emitter.cs | 153 +++++++++++------- .../gen/LoggerMessageGenerator.Parser.cs | 42 +++-- .../TestWithNestedClass.generated.txt | 27 ++++ .../LoggerMessageGeneratedCodeTests.cs | 13 ++ .../LoggerMessageGeneratorEmitterTests.cs | 26 +++ .../LoggerMessageGeneratorParserTests.cs | 5 +- .../TestClasses/NestedClassTestsExtensions.cs | 18 +++ 7 files changed, 207 insertions(+), 77 deletions(-) create mode 100644 src/libraries/Microsoft.Extensions.Logging.Abstractions/tests/Microsoft.Extensions.Logging.Generators.Tests/Baselines/TestWithNestedClass.generated.txt create mode 100644 src/libraries/Microsoft.Extensions.Logging.Abstractions/tests/Microsoft.Extensions.Logging.Generators.Tests/TestClasses/NestedClassTestsExtensions.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 748bfd375bbe34..e1daec3e13cea6 100644 --- a/src/libraries/Microsoft.Extensions.Logging.Abstractions/gen/LoggerMessageGenerator.Emitter.cs +++ b/src/libraries/Microsoft.Extensions.Logging.Abstractions/gen/LoggerMessageGenerator.Emitter.cs @@ -65,6 +65,8 @@ private static bool UseLoggerMessageDefine(LoggerMethod lm) private void GenType(LoggerClass lc) { + int indentationSize = 0; + string nestedIndentation = ""; if (!string.IsNullOrWhiteSpace(lc.Namespace)) { _builder.Append($@" @@ -72,22 +74,51 @@ namespace {lc.Namespace} {{"); } + LoggerClass parent = lc.ParentClass; + Stack parentClasses = new Stack(); + // loop until you find top level nested class + while (parent != null) + { + parentClasses.Push($@"partial class {parent?.Name + " " + parent?.Constraints}"); + parent = parent.ParentClass; + } + + // write down top level nested class first + foreach (var parentClass in parentClasses) + { + _builder.Append($@" + {nestedIndentation}{parentClass} + {nestedIndentation}{{"); + indentationSize += 4; + nestedIndentation = new String(' ', indentationSize); + } + _builder.Append($@" - partial class {lc.Name} {lc.Constraints} - {{"); + {nestedIndentation}partial class {lc.Name} {lc.Constraints} + {nestedIndentation}{{"); foreach (LoggerMethod lm in lc.Methods) { if (!UseLoggerMessageDefine(lm)) { - GenStruct(lm); + GenStruct(lm, nestedIndentation); } - GenLogMethod(lm); + GenLogMethod(lm, nestedIndentation); } _builder.Append($@" - }}"); + {nestedIndentation}}}"); + + parent = lc.ParentClass; + while (parent != null) + { + indentationSize -= 4; + nestedIndentation = new String(' ', indentationSize); + _builder.Append($@" + {nestedIndentation}}}"); + parent = parent.ParentClass; + } if (!string.IsNullOrWhiteSpace(lc.Namespace)) { @@ -96,83 +127,83 @@ partial class {lc.Name} {lc.Constraints} } } - private void GenStruct(LoggerMethod lm) + private void GenStruct(LoggerMethod lm, string nestedIndentation) { _builder.AppendLine($@" - [{s_generatedCodeAttribute}] - private readonly struct __{lm.Name}Struct : global::System.Collections.Generic.IReadOnlyList> - {{"); - GenFields(lm); + {nestedIndentation}[{s_generatedCodeAttribute}] + {nestedIndentation}private readonly struct __{lm.Name}Struct : global::System.Collections.Generic.IReadOnlyList> + {nestedIndentation}{{"); + GenFields(lm, nestedIndentation); if (lm.TemplateParameters.Count > 0) { _builder.Append($@" - public __{lm.Name}Struct("); + {nestedIndentation}public __{lm.Name}Struct("); GenArguments(lm); _builder.Append($@") - {{"); + {nestedIndentation}{{"); _builder.AppendLine(); - GenFieldAssignments(lm); + GenFieldAssignments(lm, nestedIndentation); _builder.Append($@" - }} + {nestedIndentation}}} "); } _builder.Append($@" - public override string ToString() - {{ + {nestedIndentation}public override string ToString() + {nestedIndentation}{{ "); - GenVariableAssignments(lm); + GenVariableAssignments(lm, nestedIndentation); _builder.Append($@" - return $""{lm.Message}""; - }} + {nestedIndentation}return $""{lm.Message}""; + {nestedIndentation}}} "); _builder.Append($@" - public static string Format(__{lm.Name}Struct state, global::System.Exception? ex) => state.ToString(); + {nestedIndentation}public static string Format(__{lm.Name}Struct state, global::System.Exception? ex) => state.ToString(); - public int Count => {lm.TemplateParameters.Count + 1}; + {nestedIndentation}public int Count => {lm.TemplateParameters.Count + 1}; - public global::System.Collections.Generic.KeyValuePair this[int index] - {{ - get => index switch - {{ + {nestedIndentation}public global::System.Collections.Generic.KeyValuePair this[int index] + {nestedIndentation}{{ + {nestedIndentation}get => index switch + {nestedIndentation}{{ "); - GenCases(lm); + GenCases(lm, nestedIndentation); _builder.Append($@" - _ => throw new global::System.IndexOutOfRangeException(nameof(index)), // return the same exception LoggerMessage.Define returns in this case - }}; + {nestedIndentation}_ => throw new global::System.IndexOutOfRangeException(nameof(index)), // return the same exception LoggerMessage.Define returns in this case + {nestedIndentation}}}; }} - public global::System.Collections.Generic.IEnumerator> GetEnumerator() - {{ - for (int i = 0; i < {lm.TemplateParameters.Count + 1}; i++) - {{ - yield return this[i]; - }} - }} + {nestedIndentation}public global::System.Collections.Generic.IEnumerator> GetEnumerator() + {nestedIndentation}{{ + {nestedIndentation}for (int i = 0; i < {lm.TemplateParameters.Count + 1}; i++) + {nestedIndentation}{{ + {nestedIndentation}yield return this[i]; + {nestedIndentation}}} + {nestedIndentation}}} - global::System.Collections.IEnumerator global::System.Collections.IEnumerable.GetEnumerator() => GetEnumerator(); - }} + {nestedIndentation}global::System.Collections.IEnumerator global::System.Collections.IEnumerable.GetEnumerator() => GetEnumerator(); + {nestedIndentation}}} "); } - private void GenFields(LoggerMethod lm) + private void GenFields(LoggerMethod lm, string nestedIndentation) { foreach (LoggerParameter p in lm.TemplateParameters) { - _builder.AppendLine($" private readonly {p.Type} _{p.Name};"); + _builder.AppendLine($" {nestedIndentation}private readonly {p.Type} _{p.Name};"); } } - private void GenFieldAssignments(LoggerMethod lm) + private void GenFieldAssignments(LoggerMethod lm, string nestedIndentation) { foreach (LoggerParameter p in lm.TemplateParameters) { - _builder.AppendLine($" this._{p.Name} = {p.Name};"); + _builder.AppendLine($" {nestedIndentation}this._{p.Name} = {p.Name};"); } } - private void GenVariableAssignments(LoggerMethod lm) + private void GenVariableAssignments(LoggerMethod lm, string nestedIndentation) { foreach (KeyValuePair t in lm.TemplateMap) { @@ -192,20 +223,20 @@ private void GenVariableAssignments(LoggerMethod lm) { if (lm.TemplateParameters[index].IsEnumerable) { - _builder.AppendLine($" var {t.Key} = " + _builder.AppendLine($" {nestedIndentation}var {t.Key} = " + $"global::__LoggerMessageGenerator.Enumerate((global::System.Collections.IEnumerable ?)this._{lm.TemplateParameters[index].Name});"); _needEnumerationHelper = true; } else { - _builder.AppendLine($" var {t.Key} = this._{lm.TemplateParameters[index].Name};"); + _builder.AppendLine($" {nestedIndentation}var {t.Key} = this._{lm.TemplateParameters[index].Name};"); } } } } - private void GenCases(LoggerMethod lm) + private void GenCases(LoggerMethod lm, string nestedIndentation) { int index = 0; foreach (LoggerParameter p in lm.TemplateParameters) @@ -217,10 +248,10 @@ private void GenCases(LoggerMethod lm) name = lm.TemplateMap[name]; } - _builder.AppendLine($" {index++} => new global::System.Collections.Generic.KeyValuePair(\"{name}\", this._{p.Name}),"); + _builder.AppendLine($" {nestedIndentation}{index++} => new global::System.Collections.Generic.KeyValuePair(\"{name}\", this._{p.Name}),"); } - _builder.AppendLine($" {index++} => new global::System.Collections.Generic.KeyValuePair(\"{{OriginalFormat}}\", \"{ConvertEndOfLineAndQuotationCharactersToEscapeForm(lm.Message)}\"),"); + _builder.AppendLine($" {nestedIndentation}{index++} => new global::System.Collections.Generic.KeyValuePair(\"{{OriginalFormat}}\", \"{ConvertEndOfLineAndQuotationCharactersToEscapeForm(lm.Message)}\"),"); } private void GenCallbackArguments(LoggerMethod lm) @@ -321,7 +352,7 @@ private void GenHolder(LoggerMethod lm) _builder.Append(')'); } - private void GenLogMethod(LoggerMethod lm) + private void GenLogMethod(LoggerMethod lm, string nestedIndentation) { string level = GetLogLevel(lm); string extension = lm.IsExtensionMethod ? "this " : string.Empty; @@ -332,13 +363,13 @@ private void GenLogMethod(LoggerMethod lm) if (UseLoggerMessageDefine(lm)) { _builder.Append($@" - [{s_generatedCodeAttribute}] - private static readonly global::System.Action __{lm.Name}Callback = - global::Microsoft.Extensions.Logging.LoggerMessage.Define"); + _builder.Append($@"global::System.Exception?> __{lm.Name}Callback = + {nestedIndentation}global::Microsoft.Extensions.Logging.LoggerMessage.Define"); GenDefineTypes(lm, brackets: true); @@ -347,20 +378,20 @@ private void GenLogMethod(LoggerMethod lm) } _builder.Append($@" - [{s_generatedCodeAttribute}] - {lm.Modifiers} void {lm.Name}({extension}"); + {nestedIndentation}[{s_generatedCodeAttribute}] + {nestedIndentation}{lm.Modifiers} void {lm.Name}({extension}"); GenParameters(lm); _builder.Append($@") - {{ - if ({logger}.IsEnabled({level})) - {{"); + {nestedIndentation}{{ + {nestedIndentation}if ({logger}.IsEnabled({level})) + {nestedIndentation}{{"); if (UseLoggerMessageDefine(lm)) { _builder.Append($@" - __{lm.Name}Callback({logger}, "); + {nestedIndentation}__{lm.Name}Callback({logger}, "); GenCallbackArguments(lm); @@ -369,7 +400,7 @@ private void GenLogMethod(LoggerMethod lm) else { _builder.Append($@" - {logger}.Log( + {nestedIndentation}{logger}.Log( {level}, new global::Microsoft.Extensions.Logging.EventId({lm.EventId}, {eventName}), "); @@ -380,8 +411,8 @@ private void GenLogMethod(LoggerMethod lm) } _builder.Append($@" - }} - }}"); + {nestedIndentation}}} + {nestedIndentation}}}"); static string GetException(LoggerMethod lm) { 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 03c5ad2cf57624..c7df7a50b2e51a 100644 --- a/src/libraries/Microsoft.Extensions.Logging.Abstractions/gen/LoggerMessageGenerator.Parser.cs +++ b/src/libraries/Microsoft.Extensions.Logging.Abstractions/gen/LoggerMessageGenerator.Parser.cs @@ -331,28 +331,25 @@ public IReadOnlyList GetLogClasses(IEnumerable GetLogClasses(IEnumerable 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 new file mode 100644 index 00000000000000..b93c4028c1f7c4 --- /dev/null +++ b/src/libraries/Microsoft.Extensions.Logging.Abstractions/tests/Microsoft.Extensions.Logging.Generators.Tests/Baselines/TestWithNestedClass.generated.txt @@ -0,0 +1,27 @@ +// +#nullable enable + +namespace Microsoft.Extensions.Logging.Generators.Tests.TestClasses.NestedNamespace +{ + partial class NestedClassTestsExtensions where T1 : Class1 + { + partial class NestedMiddleParentClass + { + partial class Nested where T2 : Class2 + { + [global::System.CodeDom.Compiler.GeneratedCodeAttribute("Microsoft.Extensions.Logging.Generators", "6.0.0.0")] + private static readonly global::System.Action __M9Callback = + global::Microsoft.Extensions.Logging.LoggerMessage.Define(global::Microsoft.Extensions.Logging.LogLevel.Debug, new global::Microsoft.Extensions.Logging.EventId(9, nameof(M9)), "M9", true); + + [global::System.CodeDom.Compiler.GeneratedCodeAttribute("Microsoft.Extensions.Logging.Generators", "6.0.0.0")] + public static partial void M9(global::Microsoft.Extensions.Logging.ILogger logger) + { + if (logger.IsEnabled(global::Microsoft.Extensions.Logging.LogLevel.Debug)) + { + __M9Callback(logger, null); + } + } + } + } + } +} \ No newline at end of file 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 7e0661d77a6993..8ff72bd7247ebf 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 @@ -345,6 +345,19 @@ public void EventNameTests() Assert.Equal("CustomEventName", logger.LastEventId.Name); } + [Fact] + public void NestedClassTests() + { + var logger = new MockLogger(); + + logger.Reset(); + NestedClassTestsExtensions.NestedMiddleParentClass.NestedClass.M9(logger); + Assert.Null(logger.LastException); + Assert.Equal("M9", logger.LastFormattedString); + Assert.Equal(LogLevel.Debug, logger.LastLogLevel); + Assert.Equal(1, logger.CallCount); + } + [Fact] public void TemplateTests() { diff --git a/src/libraries/Microsoft.Extensions.Logging.Abstractions/tests/Microsoft.Extensions.Logging.Generators.Tests/LoggerMessageGeneratorEmitterTests.cs b/src/libraries/Microsoft.Extensions.Logging.Abstractions/tests/Microsoft.Extensions.Logging.Generators.Tests/LoggerMessageGeneratorEmitterTests.cs index d727420f52a291..fe22173a3f6029 100644 --- a/src/libraries/Microsoft.Extensions.Logging.Abstractions/tests/Microsoft.Extensions.Logging.Generators.Tests/LoggerMessageGeneratorEmitterTests.cs +++ b/src/libraries/Microsoft.Extensions.Logging.Abstractions/tests/Microsoft.Extensions.Logging.Generators.Tests/LoggerMessageGeneratorEmitterTests.cs @@ -80,6 +80,32 @@ internal static partial class TestWithDynamicLogLevel await VerifyAgainstBaselineUsingFile("TestWithDynamicLogLevel.generated.txt", testSourceCode); } + [Fact] + public async Task TestBaseline_TestWithNestedClass_Success() + { + string testSourceCode = @" +namespace Microsoft.Extensions.Logging.Generators.Tests.TestClasses +{ + namespace NestedNamespace + { + internal static partial class NestedClassTestsExtensions where T1 : Class1 + { + internal static partial class NestedMiddleParentClass + { + internal static partial class Nested where T2 : Class2 + { + [LoggerMessage(EventId = 9, Level = LogLevel.Debug, Message = ""M9"")] + public static partial void M9(ILogger logger); + } + } + } + internal class Class1 { } + internal class Class2 { } + } +}"; + await VerifyAgainstBaselineUsingFile("TestWithNestedClass.generated.txt", testSourceCode); + } + private async Task VerifyAgainstBaselineUsingFile(string filename, string testSourceCode) { string[] expectedLines = await File.ReadAllLinesAsync(Path.Combine("Baselines", filename)).ConfigureAwait(false); 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 e3e62f72b10cf7..a6ea15141ed72d 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 @@ -249,7 +249,7 @@ partial class C } [Fact] - public async Task NestedType() + public async Task NestedTypeOK() { IReadOnlyList diagnostics = await RunGenerator(@" partial class C @@ -262,8 +262,7 @@ public partial class Nested } "); - Assert.Single(diagnostics); - Assert.Equal(DiagnosticDescriptors.LoggingMethodInNestedType.Id, diagnostics[0].Id); + Assert.Empty(diagnostics); } [Fact] 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 new file mode 100644 index 00000000000000..c207dbe534ea61 --- /dev/null +++ b/src/libraries/Microsoft.Extensions.Logging.Abstractions/tests/Microsoft.Extensions.Logging.Generators.Tests/TestClasses/NestedClassTestsExtensions.cs @@ -0,0 +1,18 @@ +// 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 NestedClassTestsExtensions where T : ABC + { + internal static partial class NestedMiddleParentClass + { + internal static partial class NestedClass + { + [LoggerMessage(EventId = 9, Level = LogLevel.Debug, Message = "M9")] + public static partial void M9(ILogger logger); + } + } + } + public class ABC {} +} \ No newline at end of file From 359fa8140b544054d9ecc6137d321ffcc368a0a8 Mon Sep 17 00:00:00 2001 From: Maryam Ariyan Date: Mon, 14 Jun 2021 13:55:18 -0700 Subject: [PATCH 2/6] Remove message for nested error --- .../gen/DiagnosticDescriptors.cs | 8 -------- .../gen/Resources/Strings.resx | 3 --- .../gen/Resources/xlf/Strings.cs.xlf | 5 ----- .../gen/Resources/xlf/Strings.de.xlf | 5 ----- .../gen/Resources/xlf/Strings.es.xlf | 5 ----- .../gen/Resources/xlf/Strings.fr.xlf | 5 ----- .../gen/Resources/xlf/Strings.it.xlf | 5 ----- .../gen/Resources/xlf/Strings.ja.xlf | 5 ----- .../gen/Resources/xlf/Strings.ko.xlf | 5 ----- .../gen/Resources/xlf/Strings.pl.xlf | 5 ----- .../gen/Resources/xlf/Strings.pt-BR.xlf | 5 ----- .../gen/Resources/xlf/Strings.ru.xlf | 5 ----- .../gen/Resources/xlf/Strings.tr.xlf | 5 ----- .../gen/Resources/xlf/Strings.zh-Hans.xlf | 5 ----- .../gen/Resources/xlf/Strings.zh-Hant.xlf | 5 ----- 15 files changed, 76 deletions(-) diff --git a/src/libraries/Microsoft.Extensions.Logging.Abstractions/gen/DiagnosticDescriptors.cs b/src/libraries/Microsoft.Extensions.Logging.Abstractions/gen/DiagnosticDescriptors.cs index 2e7e0ac340180d..2e3f31a508702f 100644 --- a/src/libraries/Microsoft.Extensions.Logging.Abstractions/gen/DiagnosticDescriptors.cs +++ b/src/libraries/Microsoft.Extensions.Logging.Abstractions/gen/DiagnosticDescriptors.cs @@ -32,14 +32,6 @@ public static class DiagnosticDescriptors DiagnosticSeverity.Error, isEnabledByDefault: true); - public static DiagnosticDescriptor LoggingMethodInNestedType { get; } = new DiagnosticDescriptor( - id: "SYSLIB1004", - title: new LocalizableResourceString(nameof(SR.LoggingMethodInNestedTypeMessage), SR.ResourceManager, typeof(FxResources.Microsoft.Extensions.Logging.Generators.SR)), - messageFormat: new LocalizableResourceString(nameof(SR.LoggingMethodInNestedTypeMessage), SR.ResourceManager, typeof(FxResources.Microsoft.Extensions.Logging.Generators.SR)), - category: "LoggingGenerator", - DiagnosticSeverity.Error, - isEnabledByDefault: true); - public static DiagnosticDescriptor MissingRequiredType { get; } = new DiagnosticDescriptor( id: "SYSLIB1005", title: new LocalizableResourceString(nameof(SR.MissingRequiredTypeTitle), SR.ResourceManager, typeof(FxResources.Microsoft.Extensions.Logging.Generators.SR)), diff --git a/src/libraries/Microsoft.Extensions.Logging.Abstractions/gen/Resources/Strings.resx b/src/libraries/Microsoft.Extensions.Logging.Abstractions/gen/Resources/Strings.resx index 13a23bcd2118b8..dc3d9f85d91ff3 100644 --- a/src/libraries/Microsoft.Extensions.Logging.Abstractions/gen/Resources/Strings.resx +++ b/src/libraries/Microsoft.Extensions.Logging.Abstractions/gen/Resources/Strings.resx @@ -123,9 +123,6 @@ Logging method parameter names cannot start with _ - - Logging class cannot be in nested types - Could not find a required type definition diff --git a/src/libraries/Microsoft.Extensions.Logging.Abstractions/gen/Resources/xlf/Strings.cs.xlf b/src/libraries/Microsoft.Extensions.Logging.Abstractions/gen/Resources/xlf/Strings.cs.xlf index 8a6e5be5b381c6..0058abe6a27ead 100644 --- a/src/libraries/Microsoft.Extensions.Logging.Abstractions/gen/Resources/xlf/Strings.cs.xlf +++ b/src/libraries/Microsoft.Extensions.Logging.Abstractions/gen/Resources/xlf/Strings.cs.xlf @@ -37,11 +37,6 @@ Logging methods cannot have a body - - Logging class cannot be in nested types - Logging class cannot be in nested types - - Logging methods cannot be generic Logging methods cannot be generic diff --git a/src/libraries/Microsoft.Extensions.Logging.Abstractions/gen/Resources/xlf/Strings.de.xlf b/src/libraries/Microsoft.Extensions.Logging.Abstractions/gen/Resources/xlf/Strings.de.xlf index 835dfbbc7fbf1a..71c5282dbb774c 100644 --- a/src/libraries/Microsoft.Extensions.Logging.Abstractions/gen/Resources/xlf/Strings.de.xlf +++ b/src/libraries/Microsoft.Extensions.Logging.Abstractions/gen/Resources/xlf/Strings.de.xlf @@ -37,11 +37,6 @@ Logging methods cannot have a body - - Logging class cannot be in nested types - Logging class cannot be in nested types - - Logging methods cannot be generic Logging methods cannot be generic diff --git a/src/libraries/Microsoft.Extensions.Logging.Abstractions/gen/Resources/xlf/Strings.es.xlf b/src/libraries/Microsoft.Extensions.Logging.Abstractions/gen/Resources/xlf/Strings.es.xlf index 0db7b4e17b3667..473c36cb21e7f0 100644 --- a/src/libraries/Microsoft.Extensions.Logging.Abstractions/gen/Resources/xlf/Strings.es.xlf +++ b/src/libraries/Microsoft.Extensions.Logging.Abstractions/gen/Resources/xlf/Strings.es.xlf @@ -37,11 +37,6 @@ Logging methods cannot have a body - - Logging class cannot be in nested types - Logging class cannot be in nested types - - Logging methods cannot be generic Logging methods cannot be generic diff --git a/src/libraries/Microsoft.Extensions.Logging.Abstractions/gen/Resources/xlf/Strings.fr.xlf b/src/libraries/Microsoft.Extensions.Logging.Abstractions/gen/Resources/xlf/Strings.fr.xlf index 757ff655a6b142..2e6014fdf9f9b4 100644 --- a/src/libraries/Microsoft.Extensions.Logging.Abstractions/gen/Resources/xlf/Strings.fr.xlf +++ b/src/libraries/Microsoft.Extensions.Logging.Abstractions/gen/Resources/xlf/Strings.fr.xlf @@ -37,11 +37,6 @@ Logging methods cannot have a body - - Logging class cannot be in nested types - Logging class cannot be in nested types - - Logging methods cannot be generic Logging methods cannot be generic diff --git a/src/libraries/Microsoft.Extensions.Logging.Abstractions/gen/Resources/xlf/Strings.it.xlf b/src/libraries/Microsoft.Extensions.Logging.Abstractions/gen/Resources/xlf/Strings.it.xlf index 8717aa30c3c287..23c38cbfaba33e 100644 --- a/src/libraries/Microsoft.Extensions.Logging.Abstractions/gen/Resources/xlf/Strings.it.xlf +++ b/src/libraries/Microsoft.Extensions.Logging.Abstractions/gen/Resources/xlf/Strings.it.xlf @@ -37,11 +37,6 @@ Logging methods cannot have a body - - Logging class cannot be in nested types - Logging class cannot be in nested types - - Logging methods cannot be generic Logging methods cannot be generic diff --git a/src/libraries/Microsoft.Extensions.Logging.Abstractions/gen/Resources/xlf/Strings.ja.xlf b/src/libraries/Microsoft.Extensions.Logging.Abstractions/gen/Resources/xlf/Strings.ja.xlf index 9ed2e744c66a4c..2a614f89b8e3bd 100644 --- a/src/libraries/Microsoft.Extensions.Logging.Abstractions/gen/Resources/xlf/Strings.ja.xlf +++ b/src/libraries/Microsoft.Extensions.Logging.Abstractions/gen/Resources/xlf/Strings.ja.xlf @@ -37,11 +37,6 @@ Logging methods cannot have a body - - Logging class cannot be in nested types - Logging class cannot be in nested types - - Logging methods cannot be generic Logging methods cannot be generic diff --git a/src/libraries/Microsoft.Extensions.Logging.Abstractions/gen/Resources/xlf/Strings.ko.xlf b/src/libraries/Microsoft.Extensions.Logging.Abstractions/gen/Resources/xlf/Strings.ko.xlf index 5b7bbb9010784c..6fd91f0b134f96 100644 --- a/src/libraries/Microsoft.Extensions.Logging.Abstractions/gen/Resources/xlf/Strings.ko.xlf +++ b/src/libraries/Microsoft.Extensions.Logging.Abstractions/gen/Resources/xlf/Strings.ko.xlf @@ -37,11 +37,6 @@ Logging methods cannot have a body - - Logging class cannot be in nested types - Logging class cannot be in nested types - - Logging methods cannot be generic Logging methods cannot be generic diff --git a/src/libraries/Microsoft.Extensions.Logging.Abstractions/gen/Resources/xlf/Strings.pl.xlf b/src/libraries/Microsoft.Extensions.Logging.Abstractions/gen/Resources/xlf/Strings.pl.xlf index eed366ed41893d..8227e907f69f2e 100644 --- a/src/libraries/Microsoft.Extensions.Logging.Abstractions/gen/Resources/xlf/Strings.pl.xlf +++ b/src/libraries/Microsoft.Extensions.Logging.Abstractions/gen/Resources/xlf/Strings.pl.xlf @@ -37,11 +37,6 @@ Logging methods cannot have a body - - Logging class cannot be in nested types - Logging class cannot be in nested types - - Logging methods cannot be generic Logging methods cannot be generic diff --git a/src/libraries/Microsoft.Extensions.Logging.Abstractions/gen/Resources/xlf/Strings.pt-BR.xlf b/src/libraries/Microsoft.Extensions.Logging.Abstractions/gen/Resources/xlf/Strings.pt-BR.xlf index c33ad19bb8cd91..f838d277c033ee 100644 --- a/src/libraries/Microsoft.Extensions.Logging.Abstractions/gen/Resources/xlf/Strings.pt-BR.xlf +++ b/src/libraries/Microsoft.Extensions.Logging.Abstractions/gen/Resources/xlf/Strings.pt-BR.xlf @@ -37,11 +37,6 @@ Logging methods cannot have a body - - Logging class cannot be in nested types - Logging class cannot be in nested types - - Logging methods cannot be generic Logging methods cannot be generic diff --git a/src/libraries/Microsoft.Extensions.Logging.Abstractions/gen/Resources/xlf/Strings.ru.xlf b/src/libraries/Microsoft.Extensions.Logging.Abstractions/gen/Resources/xlf/Strings.ru.xlf index 37548b1750a5cc..e919044fc3a918 100644 --- a/src/libraries/Microsoft.Extensions.Logging.Abstractions/gen/Resources/xlf/Strings.ru.xlf +++ b/src/libraries/Microsoft.Extensions.Logging.Abstractions/gen/Resources/xlf/Strings.ru.xlf @@ -37,11 +37,6 @@ Logging methods cannot have a body - - Logging class cannot be in nested types - Logging class cannot be in nested types - - Logging methods cannot be generic Logging methods cannot be generic diff --git a/src/libraries/Microsoft.Extensions.Logging.Abstractions/gen/Resources/xlf/Strings.tr.xlf b/src/libraries/Microsoft.Extensions.Logging.Abstractions/gen/Resources/xlf/Strings.tr.xlf index dccdbd55e57aa9..2dab9b27a8ee77 100644 --- a/src/libraries/Microsoft.Extensions.Logging.Abstractions/gen/Resources/xlf/Strings.tr.xlf +++ b/src/libraries/Microsoft.Extensions.Logging.Abstractions/gen/Resources/xlf/Strings.tr.xlf @@ -37,11 +37,6 @@ Logging methods cannot have a body - - Logging class cannot be in nested types - Logging class cannot be in nested types - - Logging methods cannot be generic Logging methods cannot be generic diff --git a/src/libraries/Microsoft.Extensions.Logging.Abstractions/gen/Resources/xlf/Strings.zh-Hans.xlf b/src/libraries/Microsoft.Extensions.Logging.Abstractions/gen/Resources/xlf/Strings.zh-Hans.xlf index 98b2d4b7185e9b..cf22a7fa2d22fd 100644 --- a/src/libraries/Microsoft.Extensions.Logging.Abstractions/gen/Resources/xlf/Strings.zh-Hans.xlf +++ b/src/libraries/Microsoft.Extensions.Logging.Abstractions/gen/Resources/xlf/Strings.zh-Hans.xlf @@ -37,11 +37,6 @@ Logging methods cannot have a body - - Logging class cannot be in nested types - Logging class cannot be in nested types - - Logging methods cannot be generic Logging methods cannot be generic diff --git a/src/libraries/Microsoft.Extensions.Logging.Abstractions/gen/Resources/xlf/Strings.zh-Hant.xlf b/src/libraries/Microsoft.Extensions.Logging.Abstractions/gen/Resources/xlf/Strings.zh-Hant.xlf index 0a54ae2ca73273..5b656e8813a121 100644 --- a/src/libraries/Microsoft.Extensions.Logging.Abstractions/gen/Resources/xlf/Strings.zh-Hant.xlf +++ b/src/libraries/Microsoft.Extensions.Logging.Abstractions/gen/Resources/xlf/Strings.zh-Hant.xlf @@ -37,11 +37,6 @@ Logging methods cannot have a body - - Logging class cannot be in nested types - Logging class cannot be in nested types - - Logging methods cannot be generic Logging methods cannot be generic From cf9affdd44ab0cd189e79fe10a8fff816d8cba97 Mon Sep 17 00:00:00 2001 From: Maryam Ariyan Date: Mon, 14 Jun 2021 22:24:47 -0700 Subject: [PATCH 3/6] Apply some PR feedback --- .../gen/LoggerMessageGenerator.Emitter.cs | 8 ++++---- .../gen/LoggerMessageGenerator.Parser.cs | 7 +++---- 2 files changed, 7 insertions(+), 8 deletions(-) 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 e1daec3e13cea6..1a79c775663785 100644 --- a/src/libraries/Microsoft.Extensions.Logging.Abstractions/gen/LoggerMessageGenerator.Emitter.cs +++ b/src/libraries/Microsoft.Extensions.Logging.Abstractions/gen/LoggerMessageGenerator.Emitter.cs @@ -75,19 +75,19 @@ namespace {lc.Namespace} } LoggerClass parent = lc.ParentClass; - Stack parentClasses = new Stack(); + var parentClasses = new List(); // loop until you find top level nested class while (parent != null) { - parentClasses.Push($@"partial class {parent?.Name + " " + parent?.Constraints}"); + parentClasses.Add($"partial class {parent?.Name + " " + parent?.Constraints}"); parent = parent.ParentClass; } // write down top level nested class first - foreach (var parentClass in parentClasses) + for (int i = parentClasses.Count - 1; i >= 0; i--) { _builder.Append($@" - {nestedIndentation}{parentClass} + {nestedIndentation}{parentClasses[i]} {nestedIndentation}{{"); indentationSize += 4; nestedIndentation = new String(' ', indentationSize); 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 c7df7a50b2e51a..105d545a29a03f 100644 --- a/src/libraries/Microsoft.Extensions.Logging.Abstractions/gen/LoggerMessageGenerator.Parser.cs +++ b/src/libraries/Microsoft.Extensions.Logging.Abstractions/gen/LoggerMessageGenerator.Parser.cs @@ -331,13 +331,12 @@ public IReadOnlyList GetLogClasses(IEnumerable From 1314e813dc0814b9fd0a1691644fbd629d9d57ab Mon Sep 17 00:00:00 2001 From: Maryam Ariyan Date: Tue, 15 Jun 2021 11:54:59 -0700 Subject: [PATCH 4/6] - Remove variable indentationSize - Add test for non static nested case - Add support for log method directly in structs - Support log methods in nested records/structs --- .../gen/LoggerMessageGenerator.Emitter.cs | 11 ++--- .../gen/LoggerMessageGenerator.Parser.cs | 29 ++++++++++---- .../gen/LoggerMessageGenerator.cs | 13 ++++-- .../LoggerMessageGeneratedCodeTests.cs | 9 ++++- .../TestClasses/NestedClassTestsExtensions.cs | 40 +++++++++++++++++++ 5 files changed, 83 insertions(+), 19 deletions(-) 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 1a79c775663785..aa3c0c521be3e0 100644 --- a/src/libraries/Microsoft.Extensions.Logging.Abstractions/gen/LoggerMessageGenerator.Emitter.cs +++ b/src/libraries/Microsoft.Extensions.Logging.Abstractions/gen/LoggerMessageGenerator.Emitter.cs @@ -65,7 +65,6 @@ private static bool UseLoggerMessageDefine(LoggerMethod lm) private void GenType(LoggerClass lc) { - int indentationSize = 0; string nestedIndentation = ""; if (!string.IsNullOrWhiteSpace(lc.Namespace)) { @@ -79,7 +78,7 @@ namespace {lc.Namespace} // loop until you find top level nested class while (parent != null) { - parentClasses.Add($"partial class {parent?.Name + " " + parent?.Constraints}"); + parentClasses.Add($"partial {parent?.Keyword} {parent?.Name} {parent?.Constraints}"); parent = parent.ParentClass; } @@ -89,12 +88,11 @@ namespace {lc.Namespace} _builder.Append($@" {nestedIndentation}{parentClasses[i]} {nestedIndentation}{{"); - indentationSize += 4; - nestedIndentation = new String(' ', indentationSize); + nestedIndentation += " "; } _builder.Append($@" - {nestedIndentation}partial class {lc.Name} {lc.Constraints} + {nestedIndentation}partial {lc.Keyword} {lc.Name} {lc.Constraints} {nestedIndentation}{{"); foreach (LoggerMethod lm in lc.Methods) @@ -113,8 +111,7 @@ namespace {lc.Namespace} parent = lc.ParentClass; while (parent != null) { - indentationSize -= 4; - nestedIndentation = new String(' ', indentationSize); + nestedIndentation = new String(' ', nestedIndentation.Length - 4); _builder.Append($@" {nestedIndentation}}}"); parent = parent.ParentClass; 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 105d545a29a03f..a1aea3ee05fd4a 100644 --- a/src/libraries/Microsoft.Extensions.Logging.Abstractions/gen/LoggerMessageGenerator.Parser.cs +++ b/src/libraries/Microsoft.Extensions.Logging.Abstractions/gen/LoggerMessageGenerator.Parser.cs @@ -28,9 +28,9 @@ public Parser(Compilation compilation, Action reportDiagnostic, Canc } /// - /// Gets the set of logging classes containing methods to output. + /// Gets the set of logging classes or structs containing methods to output. /// - public IReadOnlyList GetLogClasses(IEnumerable classes) + public IReadOnlyList GetLogClasses(IEnumerable classesOrStructs) { const string LoggerMessageAttribute = "Microsoft.Extensions.Logging.LoggerMessageAttribute"; @@ -69,11 +69,18 @@ public IReadOnlyList GetLogClasses(IEnumerable(); // we enumerate by syntax tree, to minimize the need to instantiate semantic models (since they're expensive) - foreach (var group in classes.GroupBy(x => x.SyntaxTree)) + foreach (var group in classesOrStructs.GroupBy(x => x.SyntaxTree)) { SemanticModel? sm = null; - foreach (ClassDeclarationSyntax classDec in group) + foreach (TypeDeclarationSyntax classDec in group) { + SyntaxKind kind = classDec.Kind(); + if (kind != SyntaxKind.ClassDeclaration && kind != SyntaxKind.StructDeclaration) + { + // only supporting log methods directly under structs or classes + continue; + } + // stop if we're asked to _cancellationToken.ThrowIfCancellationRequested(); @@ -357,6 +364,7 @@ public IReadOnlyList GetLogClasses(IEnumerable GetLogClasses(IEnumerable + kind == SyntaxKind.ClassDeclaration || + kind == SyntaxKind.StructDeclaration || + kind == SyntaxKind.RecordDeclaration; - while (parentLoggerClass != null) + while (parentLoggerClass != null && IsAllowedKind(parentLoggerClass.Kind())) { currentLoggerClass.ParentClass = new LoggerClass { + Keyword = parentLoggerClass.Keyword.ValueText, Namespace = nspace, Name = parentLoggerClass.Identifier.ToString() + parentLoggerClass.TypeParameterList, Constraints = parentLoggerClass.ConstraintClauses.ToString(), @@ -377,7 +391,7 @@ public IReadOnlyList GetLogClasses(IEnumerable Methods = new (); + public string Keyword = string.Empty; public string Namespace = string.Empty; public string Name = string.Empty; public string Constraints = string.Empty; diff --git a/src/libraries/Microsoft.Extensions.Logging.Abstractions/gen/LoggerMessageGenerator.cs b/src/libraries/Microsoft.Extensions.Logging.Abstractions/gen/LoggerMessageGenerator.cs index 73ca6a738aa860..b0910963791476 100644 --- a/src/libraries/Microsoft.Extensions.Logging.Abstractions/gen/LoggerMessageGenerator.cs +++ b/src/libraries/Microsoft.Extensions.Logging.Abstractions/gen/LoggerMessageGenerator.cs @@ -26,14 +26,14 @@ public void Initialize(GeneratorInitializationContext context) [ExcludeFromCodeCoverage] public void Execute(GeneratorExecutionContext context) { - if (context.SyntaxReceiver is not SyntaxReceiver receiver || receiver.ClassDeclarations.Count == 0) + if (context.SyntaxReceiver is not SyntaxReceiver receiver || receiver.ClassOrStructDeclarations.Count == 0) { // nothing to do yet return; } var p = new Parser(context.Compilation, context.ReportDiagnostic, context.CancellationToken); - IReadOnlyList logClasses = p.GetLogClasses(receiver.ClassDeclarations); + IReadOnlyList logClasses = p.GetLogClasses(receiver.ClassOrStructDeclarations); if (logClasses.Count > 0) { var e = new Emitter(); @@ -51,13 +51,18 @@ internal static ISyntaxReceiver Create() return new SyntaxReceiver(); } - public List ClassDeclarations { get; } = new (); + public List ClassOrStructDeclarations { get; } = new (); public void OnVisitSyntaxNode(SyntaxNode syntaxNode) { if (syntaxNode is ClassDeclarationSyntax classSyntax) { - ClassDeclarations.Add(classSyntax); + ClassOrStructDeclarations.Add(classSyntax); + } + + if (syntaxNode is StructDeclarationSyntax structSyntax) + { + ClassOrStructDeclarations.Add(structSyntax); } } } 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 8ff72bd7247ebf..e30d30b558ccf3 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 @@ -351,7 +351,14 @@ public void NestedClassTests() var logger = new MockLogger(); logger.Reset(); - NestedClassTestsExtensions.NestedMiddleParentClass.NestedClass.M9(logger); + NestedClassTestsExtensions.NestedMiddleParentClass.NestedClass.M8(logger); + Assert.Null(logger.LastException); + Assert.Equal("M8", logger.LastFormattedString); + Assert.Equal(LogLevel.Debug, logger.LastLogLevel); + Assert.Equal(1, logger.CallCount); + + logger.Reset(); + NonStaticNestedClassTestsExtensions.NonStaticNestedMiddleParentClass.NestedClass.M9(logger); Assert.Null(logger.LastException); Assert.Equal("M9", logger.LastFormattedString); Assert.Equal(LogLevel.Debug, logger.LastLogLevel); 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 c207dbe534ea61..fcdb4ddd118b76 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,9 +3,31 @@ namespace Microsoft.Extensions.Logging.Generators.Tests.TestClasses { + internal partial struct MyStruct + { + private ILogger _logger; + + public MyStruct(ILogger logger) { _logger = logger; } + + [LoggerMessage(EventId = 7, Level = LogLevel.Debug, Message = "M7")] + public partial void M7(); + } + internal static partial class NestedClassTestsExtensions where T : ABC { internal static partial class NestedMiddleParentClass + { + internal static partial class NestedClass + { + [LoggerMessage(EventId = 8, Level = LogLevel.Debug, Message = "M8")] + public static partial void M8(ILogger logger); + } + } + } + + internal partial class NonStaticNestedClassTestsExtensions where T : ABC + { + internal partial class NonStaticNestedMiddleParentClass { internal static partial class NestedClass { @@ -15,4 +37,22 @@ internal static partial class NestedClass } } public class ABC {} + + public partial struct NestedStruct + { + private static partial class Logger + { + [LoggerMessage(EventId = 10, Level = LogLevel.Debug, Message = "M10")] + public static partial void M10(ILogger logger); + } + } + + public partial record NestedRecord(string Name, string Address) + { + private static partial class Logger + { + [LoggerMessage(EventId = 11, Level = LogLevel.Debug, Message = "M11")] + public static partial void M11(ILogger logger); + } + } } \ No newline at end of file From 2b22cd7aacf17fc18c6ac8afd7d6b43eff12b2c5 Mon Sep 17 00:00:00 2001 From: Maryam Ariyan Date: Tue, 15 Jun 2021 12:33:34 -0700 Subject: [PATCH 5/6] - Cleanup tests - Undo support for loggers directrly under struct --- .../gen/LoggerMessageGenerator.Parser.cs | 13 ++------ .../gen/LoggerMessageGenerator.cs | 13 +++----- .../TestWithNestedClass.generated.txt | 31 ++++++++++++------- .../LoggerMessageGeneratedCodeTests.cs | 21 +++++++++++++ .../LoggerMessageGeneratorEmitterTests.cs | 21 +++++++++---- .../TestClasses/NestedClassTestsExtensions.cs | 31 +++++++++++-------- 6 files changed, 81 insertions(+), 49 deletions(-) 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 a1aea3ee05fd4a..36b5d3716b84a3 100644 --- a/src/libraries/Microsoft.Extensions.Logging.Abstractions/gen/LoggerMessageGenerator.Parser.cs +++ b/src/libraries/Microsoft.Extensions.Logging.Abstractions/gen/LoggerMessageGenerator.Parser.cs @@ -30,7 +30,7 @@ public Parser(Compilation compilation, Action reportDiagnostic, Canc /// /// Gets the set of logging classes or structs containing methods to output. /// - public IReadOnlyList GetLogClasses(IEnumerable classesOrStructs) + public IReadOnlyList GetLogClasses(IEnumerable classes) { const string LoggerMessageAttribute = "Microsoft.Extensions.Logging.LoggerMessageAttribute"; @@ -69,18 +69,11 @@ public IReadOnlyList GetLogClasses(IEnumerable(); // we enumerate by syntax tree, to minimize the need to instantiate semantic models (since they're expensive) - foreach (var group in classesOrStructs.GroupBy(x => x.SyntaxTree)) + foreach (var group in classes.GroupBy(x => x.SyntaxTree)) { SemanticModel? sm = null; - foreach (TypeDeclarationSyntax classDec in group) + foreach (ClassDeclarationSyntax classDec in group) { - SyntaxKind kind = classDec.Kind(); - if (kind != SyntaxKind.ClassDeclaration && kind != SyntaxKind.StructDeclaration) - { - // only supporting log methods directly under structs or classes - continue; - } - // stop if we're asked to _cancellationToken.ThrowIfCancellationRequested(); diff --git a/src/libraries/Microsoft.Extensions.Logging.Abstractions/gen/LoggerMessageGenerator.cs b/src/libraries/Microsoft.Extensions.Logging.Abstractions/gen/LoggerMessageGenerator.cs index b0910963791476..73ca6a738aa860 100644 --- a/src/libraries/Microsoft.Extensions.Logging.Abstractions/gen/LoggerMessageGenerator.cs +++ b/src/libraries/Microsoft.Extensions.Logging.Abstractions/gen/LoggerMessageGenerator.cs @@ -26,14 +26,14 @@ public void Initialize(GeneratorInitializationContext context) [ExcludeFromCodeCoverage] public void Execute(GeneratorExecutionContext context) { - if (context.SyntaxReceiver is not SyntaxReceiver receiver || receiver.ClassOrStructDeclarations.Count == 0) + if (context.SyntaxReceiver is not SyntaxReceiver receiver || receiver.ClassDeclarations.Count == 0) { // nothing to do yet return; } var p = new Parser(context.Compilation, context.ReportDiagnostic, context.CancellationToken); - IReadOnlyList logClasses = p.GetLogClasses(receiver.ClassOrStructDeclarations); + IReadOnlyList logClasses = p.GetLogClasses(receiver.ClassDeclarations); if (logClasses.Count > 0) { var e = new Emitter(); @@ -51,18 +51,13 @@ internal static ISyntaxReceiver Create() return new SyntaxReceiver(); } - public List ClassOrStructDeclarations { get; } = new (); + public List ClassDeclarations { get; } = new (); public void OnVisitSyntaxNode(SyntaxNode syntaxNode) { if (syntaxNode is ClassDeclarationSyntax classSyntax) { - ClassOrStructDeclarations.Add(classSyntax); - } - - if (syntaxNode is StructDeclarationSyntax structSyntax) - { - ClassOrStructDeclarations.Add(structSyntax); + ClassDeclarations.Add(classSyntax); } } } 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 b93c4028c1f7c4..024e0464c715f8 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 @@ -3,22 +3,31 @@ namespace Microsoft.Extensions.Logging.Generators.Tests.TestClasses.NestedNamespace { - partial class NestedClassTestsExtensions where T1 : Class1 + partial class MultiLevelNestedClass { - partial class NestedMiddleParentClass + partial struct NestedStruct { - partial class Nested where T2 : Class2 + partial record NestedRecord { - [global::System.CodeDom.Compiler.GeneratedCodeAttribute("Microsoft.Extensions.Logging.Generators", "6.0.0.0")] - private static readonly global::System.Action __M9Callback = - global::Microsoft.Extensions.Logging.LoggerMessage.Define(global::Microsoft.Extensions.Logging.LogLevel.Debug, new global::Microsoft.Extensions.Logging.EventId(9, nameof(M9)), "M9", true); - - [global::System.CodeDom.Compiler.GeneratedCodeAttribute("Microsoft.Extensions.Logging.Generators", "6.0.0.0")] - public static partial void M9(global::Microsoft.Extensions.Logging.ILogger logger) + partial class NestedClassTestsExtensions where T1 : Class1 { - if (logger.IsEnabled(global::Microsoft.Extensions.Logging.LogLevel.Debug)) + partial class NestedMiddleParentClass { - __M9Callback(logger, null); + partial class Nested where T2 : Class2 + { + [global::System.CodeDom.Compiler.GeneratedCodeAttribute("Microsoft.Extensions.Logging.Generators", "6.0.0.0")] + private static readonly global::System.Action __M9Callback = + global::Microsoft.Extensions.Logging.LoggerMessage.Define(global::Microsoft.Extensions.Logging.LogLevel.Debug, new global::Microsoft.Extensions.Logging.EventId(9, nameof(M9)), "M9", true); + + [global::System.CodeDom.Compiler.GeneratedCodeAttribute("Microsoft.Extensions.Logging.Generators", "6.0.0.0")] + public static partial void M9(global::Microsoft.Extensions.Logging.ILogger logger) + { + if (logger.IsEnabled(global::Microsoft.Extensions.Logging.LogLevel.Debug)) + { + __M9Callback(logger, null); + } + } + } } } } 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 e30d30b558ccf3..c9180b70f23202 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 @@ -363,6 +363,27 @@ public void NestedClassTests() Assert.Equal("M9", logger.LastFormattedString); Assert.Equal(LogLevel.Debug, logger.LastLogLevel); Assert.Equal(1, logger.CallCount); + + logger.Reset(); + NestedStruct.Logger.M10(logger); + Assert.Null(logger.LastException); + Assert.Equal("M10", logger.LastFormattedString); + Assert.Equal(LogLevel.Debug, logger.LastLogLevel); + Assert.Equal(1, logger.CallCount); + + logger.Reset(); + NestedRecord.Logger.M11(logger); + Assert.Null(logger.LastException); + Assert.Equal("M11", logger.LastFormattedString); + Assert.Equal(LogLevel.Debug, logger.LastLogLevel); + Assert.Equal(1, logger.CallCount); + + logger.Reset(); + MultiLevelNestedClass.NestedStruct.NestedRecord.Logger.M12(logger); + Assert.Null(logger.LastException); + Assert.Equal("M12", logger.LastFormattedString); + Assert.Equal(LogLevel.Debug, logger.LastLogLevel); + Assert.Equal(1, logger.CallCount); } [Fact] diff --git a/src/libraries/Microsoft.Extensions.Logging.Abstractions/tests/Microsoft.Extensions.Logging.Generators.Tests/LoggerMessageGeneratorEmitterTests.cs b/src/libraries/Microsoft.Extensions.Logging.Abstractions/tests/Microsoft.Extensions.Logging.Generators.Tests/LoggerMessageGeneratorEmitterTests.cs index fe22173a3f6029..433d601c3d940a 100644 --- a/src/libraries/Microsoft.Extensions.Logging.Abstractions/tests/Microsoft.Extensions.Logging.Generators.Tests/LoggerMessageGeneratorEmitterTests.cs +++ b/src/libraries/Microsoft.Extensions.Logging.Abstractions/tests/Microsoft.Extensions.Logging.Generators.Tests/LoggerMessageGeneratorEmitterTests.cs @@ -88,14 +88,23 @@ namespace Microsoft.Extensions.Logging.Generators.Tests.TestClasses { namespace NestedNamespace { - internal static partial class NestedClassTestsExtensions where T1 : Class1 + public static partial class MultiLevelNestedClass { - internal static partial class NestedMiddleParentClass + public partial struct NestedStruct { - internal static partial class Nested where T2 : Class2 - { - [LoggerMessage(EventId = 9, Level = LogLevel.Debug, Message = ""M9"")] - public static partial void M9(ILogger logger); + internal partial record NestedRecord(string Name, string Address) + { + internal static partial class NestedClassTestsExtensions where T1 : Class1 + { + internal static partial class NestedMiddleParentClass + { + internal static partial class Nested where T2 : Class2 + { + [LoggerMessage(EventId = 9, Level = LogLevel.Debug, Message = ""M9"")] + public static partial void M9(ILogger logger); + } + } + } } } } 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 fcdb4ddd118b76..4b5b6cdbf90a26 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,16 +3,6 @@ namespace Microsoft.Extensions.Logging.Generators.Tests.TestClasses { - internal partial struct MyStruct - { - private ILogger _logger; - - public MyStruct(ILogger logger) { _logger = logger; } - - [LoggerMessage(EventId = 7, Level = LogLevel.Debug, Message = "M7")] - public partial void M7(); - } - internal static partial class NestedClassTestsExtensions where T : ABC { internal static partial class NestedMiddleParentClass @@ -40,7 +30,7 @@ public class ABC {} public partial struct NestedStruct { - private static partial class Logger + internal static partial class Logger { [LoggerMessage(EventId = 10, Level = LogLevel.Debug, Message = "M10")] public static partial void M10(ILogger logger); @@ -49,10 +39,25 @@ private static partial class Logger public partial record NestedRecord(string Name, string Address) { - private static partial class Logger + internal static partial class Logger { [LoggerMessage(EventId = 11, Level = LogLevel.Debug, Message = "M11")] public static partial void M11(ILogger logger); } } -} \ No newline at end of file + + public static partial class MultiLevelNestedClass + { + public partial struct NestedStruct + { + internal partial record NestedRecord(string Name, string Address) + { + internal static partial class Logger + { + [LoggerMessage(EventId = 12, Level = LogLevel.Debug, Message = "M12")] + public static partial void M12(ILogger logger); + } + } + } + } +} From 61e76f54ed9a4421e80a36908a96d2e45ad7dafd Mon Sep 17 00:00:00 2001 From: Maryam Ariyan Date: Tue, 15 Jun 2021 15:01:30 -0700 Subject: [PATCH 6/6] Address PR feedback --- .../gen/LoggerMessageGenerator.Emitter.cs | 2 +- .../gen/LoggerMessageGenerator.Parser.cs | 3 +-- 2 files changed, 2 insertions(+), 3 deletions(-) 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 aa3c0c521be3e0..2b0bc4eca00d5d 100644 --- a/src/libraries/Microsoft.Extensions.Logging.Abstractions/gen/LoggerMessageGenerator.Emitter.cs +++ b/src/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.Constraints}"); parent = parent.ParentClass; } 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 36b5d3716b84a3..45db67d5f601e5 100644 --- a/src/libraries/Microsoft.Extensions.Logging.Abstractions/gen/LoggerMessageGenerator.Parser.cs +++ b/src/libraries/Microsoft.Extensions.Logging.Abstractions/gen/LoggerMessageGenerator.Parser.cs @@ -336,9 +336,8 @@ public IReadOnlyList GetLogClasses(IEnumerable