Skip to content

Commit f531df6

Browse files
maryamariyanmaryamariyan
authored
Fixes two logging source gen bugs - when using "in" or "ref" modifier / when dealing with constraints (#64593)
* Fixes some logging source gen bugs: - Supports usage of "in" modifier - Improves support for generic constraints Fixes #58550, #62644 * Apply PR feedback * Add another test
1 parent 2e35741 commit f531df6

8 files changed

Lines changed: 247 additions & 8 deletions

File tree

‎src/libraries/Microsoft.Extensions.Logging.Abstractions/gen/LoggerMessageGenerator.Emitter.cs‎

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -86,7 +86,7 @@ namespace {lc.Namespace}
8686
// loop until you find top level nested class
8787
while (parent != null)
8888
{
89-
parentClasses.Add($"partial {parent.Keyword} {parent.Name} {parent.Constraints}");
89+
parentClasses.Add($"partial {parent.Keyword} {parent.Name} ");
9090
parent = parent.ParentClass;
9191
}
9292

@@ -100,7 +100,7 @@ namespace {lc.Namespace}
100100
}
101101

102102
_builder.Append($@"
103-
{nestedIndentation}partial {lc.Keyword} {lc.Name} {lc.Constraints}
103+
{nestedIndentation}partial {lc.Keyword} {lc.Name}
104104
{nestedIndentation}{{");
105105

106106
foreach (LoggerMethod lm in lc.Methods)
@@ -319,6 +319,10 @@ private void GenParameters(LoggerMethod lm)
319319
_builder.Append(", ");
320320
}
321321

322+
if (p.Qualifier != null)
323+
{
324+
_builder.Append($"{p.Qualifier} ");
325+
}
322326
_builder.Append($"{p.Type} {p.Name}");
323327
}
324328
}

‎src/libraries/Microsoft.Extensions.Logging.Abstractions/gen/LoggerMessageGenerator.Parser.cs‎

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -321,6 +321,15 @@ public IReadOnlyList<LoggerClass> GetLogClasses(IEnumerable<ClassDeclarationSynt
321321
break;
322322
}
323323

324+
string? qualifier = null;
325+
if (paramSymbol.RefKind == RefKind.In)
326+
{
327+
qualifier = "in";
328+
}
329+
else if (paramSymbol.RefKind == RefKind.Ref)
330+
{
331+
qualifier = "ref";
332+
}
324333
string typeName = paramTypeSymbol.ToDisplayString(
325334
SymbolDisplayFormat.FullyQualifiedFormat.WithMiscellaneousOptions(
326335
SymbolDisplayMiscellaneousOptions.IncludeNullableReferenceTypeModifier));
@@ -329,6 +338,7 @@ public IReadOnlyList<LoggerClass> GetLogClasses(IEnumerable<ClassDeclarationSynt
329338
{
330339
Name = paramName,
331340
Type = typeName,
341+
Qualifier = qualifier,
332342
IsLogger = !foundLogger && IsBaseOrIdentity(paramTypeSymbol!, loggerSymbol),
333343
IsException = !foundException && IsBaseOrIdentity(paramTypeSymbol!, exceptionSymbol),
334344
IsLogLevel = !foundLogLevel && IsBaseOrIdentity(paramTypeSymbol!, logLevelSymbol),
@@ -478,7 +488,6 @@ potentialNamespaceParent is not NamespaceDeclarationSyntax
478488
Keyword = classDec.Keyword.ValueText,
479489
Namespace = nspace,
480490
Name = classDec.Identifier.ToString() + classDec.TypeParameterList,
481-
Constraints = classDec.ConstraintClauses.ToString(),
482491
ParentClass = null,
483492
};
484493

@@ -497,7 +506,6 @@ bool IsAllowedKind(SyntaxKind kind) =>
497506
Keyword = parentLoggerClass.Keyword.ValueText,
498507
Namespace = nspace,
499508
Name = parentLoggerClass.Identifier.ToString() + parentLoggerClass.TypeParameterList,
500-
Constraints = parentLoggerClass.ConstraintClauses.ToString(),
501509
ParentClass = null,
502510
};
503511

@@ -700,7 +708,6 @@ internal class LoggerClass
700708
public string Keyword = string.Empty;
701709
public string Namespace = string.Empty;
702710
public string Name = string.Empty;
703-
public string Constraints = string.Empty;
704711
public LoggerClass? ParentClass;
705712
}
706713

@@ -732,6 +739,7 @@ internal class LoggerParameter
732739
{
733740
public string Name = string.Empty;
734741
public string Type = string.Empty;
742+
public string? Qualifier;
735743
public bool IsLogger;
736744
public bool IsException;
737745
public bool IsLogLevel;

‎src/libraries/Microsoft.Extensions.Logging.Abstractions/tests/Microsoft.Extensions.Logging.Generators.Tests/Baselines/TestWithNestedClass.generated.txt‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9,11 +9,11 @@ namespace Microsoft.Extensions.Logging.Generators.Tests.TestClasses.NestedNamesp
99
{
1010
partial record NestedRecord
1111
{
12-
partial class NestedClassTestsExtensions<T1> where T1 : Class1
12+
partial class NestedClassTestsExtensions<T1>
1313
{
1414
partial class NestedMiddleParentClass
1515
{
16-
partial class Nested<T2> where T2 : Class2
16+
partial class Nested<T2>
1717
{
1818
[global::System.CodeDom.Compiler.GeneratedCodeAttribute("Microsoft.Extensions.Logging.Generators", "%VERSION%")]
1919
private static readonly global::System.Action<global::Microsoft.Extensions.Logging.ILogger, global::System.Exception?> __M9Callback =

‎src/libraries/Microsoft.Extensions.Logging.Abstractions/tests/Microsoft.Extensions.Logging.Generators.Tests/LoggerMessageGeneratedCodeTests.cs‎

Lines changed: 69 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,8 +3,12 @@
33

44
using System;
55
using System.Collections.Generic;
6+
using Microsoft.Extensions.Logging.Abstractions;
67
using Microsoft.Extensions.Logging.Generators.Tests.TestClasses;
8+
using Microsoft.Extensions.Logging.Generators.Tests.TestClasses.UsesConstraintInAnotherNamespace;
79
using Xunit;
10+
using NamespaceForABC;
11+
using ConstraintInAnotherNamespace;
812

913
namespace Microsoft.Extensions.Logging.Generators.Tests
1014
{
@@ -430,6 +434,71 @@ public void SkipEnabledCheckTests()
430434
Assert.Equal(1, logger.CallCount);
431435
Assert.Equal("LoggerMethodWithTrueSkipEnabledCheck", logger.LastEventId.Name);
432436
}
437+
private struct MyStruct { }
438+
439+
[Fact]
440+
public void ConstraintsTests()
441+
{
442+
var logger = new MockLogger();
443+
444+
var printer = new MessagePrinter<Message>();
445+
logger.Reset();
446+
printer.Print(logger, new Message() { Text = "Hello" });
447+
Assert.Equal(LogLevel.Information, logger.LastLogLevel);
448+
Assert.Null(logger.LastException);
449+
Assert.Equal("The message is Hello.", logger.LastFormattedString);
450+
Assert.Equal(1, logger.CallCount);
451+
452+
var printer2 = new MessagePrinterHasConstraintOnLogClassAndLogMethod<Message>();
453+
logger.Reset();
454+
printer2.Print(logger, new Message() { Text = "Hello" });
455+
Assert.Equal(LogLevel.Information, logger.LastLogLevel);
456+
Assert.Null(logger.LastException);
457+
Assert.Equal("The message is `Hello`.", logger.LastFormattedString);
458+
Assert.Equal(1, logger.CallCount);
459+
460+
logger.Reset();
461+
ConstraintsTestExtensions<Object>.M0(logger, 12);
462+
Assert.Equal(LogLevel.Debug, logger.LastLogLevel);
463+
Assert.Null(logger.LastException);
464+
Assert.Equal("M012", logger.LastFormattedString);
465+
Assert.Equal(1, logger.CallCount);
466+
467+
logger.Reset();
468+
ConstraintsTestExtensions1<MyStruct>.M0(logger, 12);
469+
Assert.Equal(LogLevel.Debug, logger.LastLogLevel);
470+
Assert.Null(logger.LastException);
471+
Assert.Equal("M012", logger.LastFormattedString);
472+
Assert.Equal(1, logger.CallCount);
473+
474+
logger.Reset();
475+
ConstraintsTestExtensions2<MyStruct>.M0(logger, 12);
476+
Assert.Equal(LogLevel.Debug, logger.LastLogLevel);
477+
Assert.Null(logger.LastException);
478+
Assert.Equal("M012", logger.LastFormattedString);
479+
Assert.Equal(1, logger.CallCount);
480+
481+
logger.Reset();
482+
ConstraintsTestExtensions3<MyStruct>.M0(logger, 12);
483+
Assert.Equal(LogLevel.Debug, logger.LastLogLevel);
484+
Assert.Null(logger.LastException);
485+
Assert.Equal("M012", logger.LastFormattedString);
486+
Assert.Equal(1, logger.CallCount);
487+
488+
logger.Reset();
489+
ConstraintsTestExtensions4<Attribute>.M0(logger, 12);
490+
Assert.Equal(LogLevel.Debug, logger.LastLogLevel);
491+
Assert.Null(logger.LastException);
492+
Assert.Equal("M012", logger.LastFormattedString);
493+
Assert.Equal(1, logger.CallCount);
494+
495+
logger.Reset();
496+
ConstraintsTestExtensions5<MyStruct>.M0(logger, 12);
497+
Assert.Equal(LogLevel.Debug, logger.LastLogLevel);
498+
Assert.Null(logger.LastException);
499+
Assert.Equal("M012", logger.LastFormattedString);
500+
Assert.Equal(1, logger.CallCount);
501+
}
433502

434503
[Fact]
435504
public void NestedClassTests()

‎src/libraries/Microsoft.Extensions.Logging.Abstractions/tests/Microsoft.Extensions.Logging.Generators.Tests/LoggerMessageGeneratorParserTests.cs‎

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -616,6 +616,21 @@ partial class C
616616
Assert.Equal(DiagnosticDescriptors.LoggingMethodIsGeneric.Id, diagnostics[0].Id);
617617
}
618618

619+
[Theory]
620+
[InlineData("ref")]
621+
[InlineData("in")]
622+
public async Task SupportsRefKindsInAndRef(string modifier)
623+
{
624+
IReadOnlyList<Diagnostic> diagnostics = await RunGenerator(@$"
625+
partial class C
626+
{{
627+
[LoggerMessage(EventId = 0, Level = LogLevel.Debug, Message = ""Parameter {{P1}}"")]
628+
static partial void M(ILogger logger, {modifier} int p1);
629+
}}");
630+
631+
Assert.Empty(diagnostics);
632+
}
633+
619634
[Fact]
620635
public async Task Templates()
621636
{

‎src/libraries/Microsoft.Extensions.Logging.Abstractions/tests/Microsoft.Extensions.Logging.Generators.Tests/TestClasses/ConstaintsTestExtensions.cs‎

Lines changed: 118 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,118 @@
1+
// Licensed to the .NET Foundation under one or more agreements.
2+
// The .NET Foundation licenses this file to you under the MIT license.
3+
4+
namespace Microsoft.Extensions.Logging.Generators.Tests.TestClasses
5+
{
6+
using ConstraintInAnotherNamespace;
7+
namespace UsesConstraintInAnotherNamespace
8+
{
9+
public partial class MessagePrinter<T>
10+
where T : Message
11+
{
12+
public void Print(ILogger logger, T message)
13+
{
14+
Log.Message(logger, message.Text);
15+
}
16+
17+
internal static partial class Log
18+
{
19+
[LoggerMessage(EventId = 1, Level = LogLevel.Information, Message = "The message is {Text}.")]
20+
internal static partial void Message(ILogger logger, string? text);
21+
}
22+
}
23+
24+
public partial class MessagePrinterHasConstraintOnLogClassAndLogMethod<T>
25+
where T : Message
26+
{
27+
public void Print(ILogger logger, T message)
28+
{
29+
Log<Message>.Message(logger, message);
30+
}
31+
32+
internal static partial class Log<U> where U : Message
33+
{
34+
[LoggerMessage(EventId = 1, Level = LogLevel.Information, Message = "The message is {Text}.")]
35+
internal static partial void Message(ILogger logger, U text);
36+
}
37+
}
38+
}
39+
40+
internal static partial class ConstraintsTestExtensions<T>
41+
where T : class
42+
{
43+
[LoggerMessage(EventId = 0, Level = LogLevel.Debug, Message = "M0{p0}")]
44+
public static partial void M0(ILogger logger, int p0);
45+
46+
public static void Foo(T dummy)
47+
{
48+
}
49+
}
50+
51+
internal static partial class ConstraintsTestExtensions1<T>
52+
where T : struct
53+
{
54+
[LoggerMessage(EventId = 0, Level = LogLevel.Debug, Message = "M0{p0}")]
55+
public static partial void M0(ILogger logger, int p0);
56+
57+
public static void Foo(T dummy)
58+
{
59+
}
60+
}
61+
62+
internal static partial class ConstraintsTestExtensions2<T>
63+
where T : unmanaged
64+
{
65+
[LoggerMessage(EventId = 0, Level = LogLevel.Debug, Message = "M0{p0}")]
66+
public static partial void M0(ILogger logger, int p0);
67+
68+
public static void Foo(T dummy)
69+
{
70+
}
71+
}
72+
73+
internal static partial class ConstraintsTestExtensions3<T>
74+
where T : new()
75+
{
76+
[LoggerMessage(EventId = 0, Level = LogLevel.Debug, Message = "M0{p0}")]
77+
public static partial void M0(ILogger logger, int p0);
78+
79+
public static void Foo(T dummy)
80+
{
81+
}
82+
}
83+
84+
internal static partial class ConstraintsTestExtensions4<T>
85+
where T : System.Attribute
86+
{
87+
[LoggerMessage(EventId = 0, Level = LogLevel.Debug, Message = "M0{p0}")]
88+
public static partial void M0(ILogger logger, int p0);
89+
90+
public static void Foo(T dummy)
91+
{
92+
}
93+
}
94+
95+
internal static partial class ConstraintsTestExtensions5<T>
96+
where T : notnull
97+
{
98+
[LoggerMessage(EventId = 0, Level = LogLevel.Debug, Message = "M0{p0}")]
99+
public static partial void M0(ILogger logger, int p0);
100+
101+
public static void Foo(T dummy)
102+
{
103+
}
104+
}
105+
}
106+
107+
namespace ConstraintInAnotherNamespace
108+
{
109+
public class Message
110+
{
111+
public string? Text { get; set; }
112+
113+
public override string ToString()
114+
{
115+
return $"`{Text}`";
116+
}
117+
}
118+
}

‎src/libraries/Microsoft.Extensions.Logging.Abstractions/tests/Microsoft.Extensions.Logging.Generators.Tests/TestClasses/NestedClassTestsExtensions.cs‎

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,8 @@
33

44
namespace Microsoft.Extensions.Logging.Generators.Tests.TestClasses
55
{
6+
using NamespaceForABC;
7+
68
internal static partial class NestedClassTestsExtensions<T> where T : ABC
79
{
810
internal static partial class NestedMiddleParentClass
@@ -26,7 +28,6 @@ internal static partial class NestedClass
2628
}
2729
}
2830
}
29-
public class ABC {}
3031

3132
public partial struct NestedStruct
3233
{
@@ -61,3 +62,8 @@ internal static partial class Logger
6162
}
6263
}
6364
}
65+
66+
namespace NamespaceForABC
67+
{
68+
public class ABC {}
69+
}

‎src/libraries/Microsoft.Extensions.Logging.Abstractions/tests/Microsoft.Extensions.Logging.Generators.Tests/TestClasses/ParameterTestExtensions.cs‎

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
1+
// Licensed to the .NET Foundation under one or more agreements.
2+
// The .NET Foundation licenses this file to you under the MIT license.
3+
4+
namespace Microsoft.Extensions.Logging.Generators.Tests.TestClasses
5+
{
6+
internal static partial class ParameterTestExtensions
7+
{
8+
internal struct S
9+
{
10+
public override string ToString() => "Hello from S";
11+
}
12+
13+
[LoggerMessage(EventId = 0, Level = LogLevel.Information, Message = "UseInParameter {s}")]
14+
internal static partial void UseInParameter(ILogger logger, in S s);
15+
16+
[LoggerMessage(EventId = 1, Level = LogLevel.Information, Message = "UseRefParameter {s}")]
17+
internal static partial void UseRefParameter(ILogger logger, ref S s);
18+
}
19+
}

0 commit comments

Comments
 (0)