Skip to content

Commit b54886b

Browse files
github-actions[bot]martincostellotarekgh
authored
[release/8.0-staging] Fix compilation error with generic type attributes (#97717)
* Fix compilation error with generic type attributes Strip generic type parameter attributes from partial classes emitted by the logging source generated to avoid CS0579 errors from duplicate attributes. Fixes #97498. * Update comment Update comment as suggested by code review. * Verify type parameters retained Verify that attributes on generic type parameters are not lost. * Add package authoring --------- Co-authored-by: martincostello <[email protected]> Co-authored-by: Tarek Mahmoud Sayed <[email protected]>
1 parent 1901707 commit b54886b

File tree

6 files changed

+153
-3
lines changed

6 files changed

+153
-3
lines changed

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

Lines changed: 25 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -536,7 +536,7 @@ potentialNamespaceParent is not NamespaceDeclarationSyntax
536536
{
537537
Keyword = classDec.Keyword.ValueText,
538538
Namespace = nspace,
539-
Name = classDec.Identifier.ToString() + classDec.TypeParameterList,
539+
Name = GenerateClassName(classDec),
540540
ParentClass = null,
541541
};
542542

@@ -554,7 +554,7 @@ static bool IsAllowedKind(SyntaxKind kind) =>
554554
{
555555
Keyword = parentLoggerClass.Keyword.ValueText,
556556
Namespace = nspace,
557-
Name = parentLoggerClass.Identifier.ToString() + parentLoggerClass.TypeParameterList,
557+
Name = GenerateClassName(parentLoggerClass),
558558
ParentClass = null,
559559
};
560560

@@ -601,6 +601,29 @@ static bool IsAllowedKind(SyntaxKind kind) =>
601601
return results;
602602
}
603603

604+
private static string GenerateClassName(TypeDeclarationSyntax typeDeclaration)
605+
{
606+
if (typeDeclaration.TypeParameterList != null &&
607+
typeDeclaration.TypeParameterList.Parameters.Count != 0)
608+
{
609+
// The source generator produces a partial class that the compiler merges with the original
610+
// class definition in the user code. If the user applies attributes to the generic types
611+
// of the class, it is necessary to remove these attribute annotations from the generated
612+
// code. Failure to do so may result in a compilation error (CS0579: Duplicate attribute).
613+
for (int i = 0; i < typeDeclaration.TypeParameterList.Parameters.Count; i++)
614+
{
615+
TypeParameterSyntax parameter = typeDeclaration.TypeParameterList.Parameters[i];
616+
617+
if (parameter.AttributeLists.Count > 0)
618+
{
619+
typeDeclaration = typeDeclaration.ReplaceNode(parameter, parameter.WithAttributeLists([]));
620+
}
621+
}
622+
}
623+
624+
return typeDeclaration.Identifier.ToString() + typeDeclaration.TypeParameterList;
625+
}
626+
604627
private (string? loggerField, bool multipleLoggerFields) FindLoggerField(SemanticModel sm, TypeDeclarationSyntax classDec, ITypeSymbol loggerSymbol)
605628
{
606629
string? loggerField = null;

src/libraries/Microsoft.Extensions.Logging.Abstractions/src/Microsoft.Extensions.Logging.Abstractions.csproj

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,8 @@
55
<EnableDefaultItems>true</EnableDefaultItems>
66
<AllowUnsafeBlocks>true</AllowUnsafeBlocks>
77
<IsPackable>true</IsPackable>
8+
<GeneratePackageOnBuild>true</GeneratePackageOnBuild>
9+
<ServicingVersion>1</ServicingVersion>
810
<PackageDescription>Logging abstractions for Microsoft.Extensions.Logging.
911

1012
Commonly Used Types:
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,24 @@
1+
// <auto-generated/>
2+
#nullable enable
3+
4+
namespace Microsoft.Extensions.Logging.Generators.Tests.TestClasses
5+
{
6+
partial class GenericTypeWithAttribute<A, B, C>
7+
{
8+
partial class Log<D>
9+
{
10+
[global::System.CodeDom.Compiler.GeneratedCodeAttribute("Microsoft.Extensions.Logging.Generators", "%VERSION%")]
11+
private static readonly global::System.Action<global::Microsoft.Extensions.Logging.ILogger, A, B, C, global::System.Exception?> __M0Callback =
12+
global::Microsoft.Extensions.Logging.LoggerMessage.Define<A, B, C>(global::Microsoft.Extensions.Logging.LogLevel.Debug, new global::Microsoft.Extensions.Logging.EventId(42, nameof(M0)), "a = {a}; b = {b}; c = {c}", new global::Microsoft.Extensions.Logging.LogDefineOptions() { SkipEnabledCheck = true });
13+
14+
[global::System.CodeDom.Compiler.GeneratedCodeAttribute("Microsoft.Extensions.Logging.Generators", "%VERSION%")]
15+
public static partial void M0(global::Microsoft.Extensions.Logging.ILogger logger, A a, B b, C c)
16+
{
17+
if (logger.IsEnabled(global::Microsoft.Extensions.Logging.LogLevel.Debug))
18+
{
19+
__M0Callback(logger, a, b, c, null);
20+
}
21+
}
22+
}
23+
}
24+
}

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

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

44
using System;
55
using System.IO;
6+
using System.Reflection;
67
using System.Threading.Tasks;
7-
using Microsoft.CodeAnalysis.Text;
88
using SourceGenerators.Tests;
99
using Xunit;
1010

@@ -163,6 +163,40 @@ internal static partial class TestWithDefaultValues
163163
}
164164
#endif
165165

166+
[Fact]
167+
public async Task TestBaseline_TestWithNestedClassWithGenericTypesWithAttributes_Success()
168+
{
169+
string testSourceCode = @"
170+
namespace Microsoft.Extensions.Logging.Generators.Tests.TestClasses
171+
{
172+
public partial class GenericTypeWithAttribute<[Foo] A, [Bar] B, C>
173+
{
174+
public void M0<D>(A a, B b, C c, ILogger logger) => Log<D>.M0(logger, a, b, c);
175+
private static partial class Log<[Foo] D>
176+
{
177+
[LoggerMessage(EventId = 42, Level = LogLevel.Debug, Message = ""a = {a}; b = {b}; c = {c}"")]
178+
public static partial void M0(ILogger logger, A a, B b, C c);
179+
}
180+
}
181+
182+
[AttributeUsage(AttributeTargets.GenericParameter)]
183+
public sealed class FooAttribute : Attribute { }
184+
[AttributeUsage(AttributeTargets.GenericParameter)]
185+
public sealed class BarAttribute : Attribute { }
186+
}";
187+
await VerifyAgainstBaselineUsingFile("TestWithNestedClassWithGenericTypesWithAttributes.generated.txt", testSourceCode);
188+
}
189+
190+
[Fact]
191+
public void GenericTypeParameterAttributesAreRetained()
192+
{
193+
var type = typeof(TestClasses.NestedClassWithGenericTypesWithAttributesTestsExtensions<,,>).GetTypeInfo();
194+
195+
Assert.Equal(3, type.GenericTypeParameters.Length);
196+
Assert.NotNull(type.GenericTypeParameters[0].GetCustomAttribute<TestClasses.FooAttribute>());
197+
Assert.NotNull(type.GenericTypeParameters[1].GetCustomAttribute<TestClasses.BarAttribute>());
198+
}
199+
166200
private async Task VerifyAgainstBaselineUsingFile(string filename, string testSourceCode)
167201
{
168202
string baseline = LineEndingsHelper.Normalize(await File.ReadAllTextAsync(Path.Combine("Baselines", filename)).ConfigureAwait(false));

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

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -359,6 +359,44 @@ public partial class Nested
359359
Assert.Empty(diagnostics);
360360
}
361361

362+
[Fact]
363+
public async Task NestedTypeWithGenericParameterOK()
364+
{
365+
IReadOnlyList<Diagnostic> diagnostics = await RunGenerator(@"
366+
partial class C<T>
367+
{
368+
public partial class Nested<U>
369+
{
370+
[LoggerMessage(EventId = 0, Level = LogLevel.Debug, Message = ""M1"")]
371+
static partial void M1(ILogger logger);
372+
}
373+
}
374+
");
375+
376+
Assert.Empty(diagnostics);
377+
}
378+
379+
[Fact]
380+
public async Task NestedTypeWithGenericParameterWithAttributeOK()
381+
{
382+
IReadOnlyList<Diagnostic> diagnostics = await RunGenerator(@"
383+
using System;
384+
using System.Diagnostics.CodeAnalysis;
385+
partial class C<[DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.PublicProperties)] T>
386+
{
387+
public partial class Nested<[MarkerAttribute] U>
388+
{
389+
[LoggerMessage(EventId = 0, Level = LogLevel.Debug, Message = ""M1"")]
390+
static partial void M1(ILogger logger);
391+
}
392+
}
393+
[AttributeUsage(AttributeTargets.GenericParameter)]
394+
class MarkerAttribute : Attribute { }
395+
");
396+
397+
Assert.Empty(diagnostics);
398+
}
399+
362400
#if ROSLYN4_0_OR_GREATER
363401
[Fact]
364402
public async Task FileScopedNamespaceOK()

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

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

44
namespace Microsoft.Extensions.Logging.Generators.Tests.TestClasses
55
{
6+
using System;
67
using NamespaceForABC;
78

9+
public partial class NestedClassWithNoTypeConstraintTestsExtensions<T>
10+
{
11+
public void M7(ILogger logger) => Log.M7(logger);
12+
13+
private static partial class Log
14+
{
15+
[LoggerMessage(EventId = 7, Level = LogLevel.Debug, Message = "M7")]
16+
public static partial void M7(ILogger logger);
17+
}
18+
}
19+
820
internal static partial class NestedClassTestsExtensions<T> where T : ABC
921
{
1022
internal static partial class NestedMiddleParentClass
@@ -61,6 +73,23 @@ internal static partial class Logger
6173
}
6274
}
6375
}
76+
77+
public partial class NestedClassWithGenericTypesWithAttributesTestsExtensions<[Foo] A, [Bar] B, C>
78+
{
79+
public void M13<D>(A a, B b, C c, ILogger logger) => Log<D>.M13(logger, a, b, c);
80+
81+
private static partial class Log<[Foo] D>
82+
{
83+
[LoggerMessage(EventId = 13, Level = LogLevel.Debug, Message = "M13: A = {a}; B = {b}; C = {c}")]
84+
public static partial void M13(ILogger logger, A a, B b, C c);
85+
}
86+
}
87+
88+
[AttributeUsage(AttributeTargets.GenericParameter)]
89+
public sealed class FooAttribute : Attribute { }
90+
91+
[AttributeUsage(AttributeTargets.GenericParameter)]
92+
public sealed class BarAttribute : Attribute { }
6493
}
6594

6695
namespace NamespaceForABC

0 commit comments

Comments
 (0)