8000 Yet one more attribute bug fix and test. · dotnet/sdk@6b30920 · GitHub
[go: up one dir, main page]
Skip to content

Commit 6b30920

Browse files
carlossanlopcarlossanlop
committed
Yet one more attribute bug fix and test.
1 parent 8edf85f commit 6b30920

File tree

2 files changed

+57
-17
lines changed

2 files changed

+57
-17
lines changed

‎src/Compatibility/ApiDiff/Microsoft.DotNet.ApiDiff/MemoryOutputDiffGenerator.cs

Lines changed: 18 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
// Licensed to the .NET Foundation under one or more agreements.
22
// The .NET Foundation licenses this file to you under the MIT license.
33

4+
using System;
45
using System.Collections.Concurrent;
56
using System.Diagnostics;
67
using System.Diagnostics.CodeAnalysis;
@@ -511,14 +512,16 @@ private Dictionary<string, AttributeSyntax> CollectAttributeNodes(MemberDeclarat
511512
{
512513
foreach (AttributeSyntax attributeNode in attributeListNode.Attributes)
513514
{
514-
if(model.GetSymbolInfo(attributeNode).Symbol is ISymbol attributeSymbol &&
515-
!_attributeSymbolFilter.Include(attributeSymbol.ContainingType))
515+
IMethodSymbol? constructor = model.GetSymbolInfo(attributeNode).Symbol as IMethodSymbol;
516+
if (constructor is null || !_attributeSymbolFilter.Include(constructor.ContainingType))
516517
{
517-
// The attributes decorating an API are actually calls to their constructor method, but the attribute filter needs type docIDs
518+
// The attributes decorating an API are actually calls to their
519+
// constructor method, but the attribute filter needs type docIDs.
518520
continue;
519521
}
520522
var realAttributeNode = attributeNode.WithArgumentList(attributeNode.ArgumentList);
521-
if (!dictionary.TryAdd(GetAttributeDocId(realAttributeNode, model), realAttributeNode))
523+
524+
if (!dictionary.TryAdd(GetAttributeDocId(constructor, attributeNode, model), realAttributeNode))
522525
{
523526
_log.LogWarning(string.Format(Resources.AttributeAlreadyExists, realAttributeNode.ToFullString(), GetDocId(memberNode, model)));
524527
}
@@ -647,27 +650,31 @@ private static IEnumerable<T> GetMembersOfType<T>(SyntaxNode node) where T : Mem
647650
.Where(n => n is T m && IsEnumMemberOrHasPublicOrProtectedModifierOrIsDestructor(m))
648651
.Cast<T>();
649652

650-
private string GetAttributeDocId(AttributeSyntax attribute, SemanticModel model)
653+
private string GetAttributeDocId(IMethodSymbol attributeConstructorSymbol, AttributeSyntax attribute, SemanticModel model)
651654
{
655+
Debug.Assert(_attributeSymbolFilter.Include(attributeConstructorSymbol.ContainingType));
656+
652657
string? attributeDocId = null;
658+
653659
if (attribute.ArgumentList is AttributeArgumentListSyntax list && list.Arguments.Any())
654660
{
655-
// Workaround to ensure that repeated attributes with different arguments are all included
661+
// If the constructor has arguments, it might be added multiple times on top of an API,
662+
// so we need to get the unique full string which includes the actual value of the arguments.
656663
attributeDocId = attribute.ToFullString();
657664
}
658-
else if (model.GetSymbolInfo(attribute).Symbol is IMethodSymbol constructor)
665+
else
659666
{
660667
try
661668
{
662-
attributeDocId = constructor.ContainingType.GetDocumentationCommentId();
669+
attributeDocId = attributeConstructorSymbol.ContainingType.GetDocumentationCommentId();
663670
}
664671
catch (Exception e)
665672
{
666-
_log.LogWarning(e.Message);
673+
_log.LogWarning($"Could not retrieve docId of the attribute constructor of {attribute.ToFullString()}: {e.Message} ");
667674
}
668675
}
669-
// Workaround because some rare cases of attributes in WinForms like System.Windows.Markup.ContentPropertyAttribute
670-
// cannot be found in the current model.
676+
677+
// Temporary workaround because some rare cases of attributes in WinForms cannot be found in the current model.
671678
attributeDocId ??= attribute.ToFullString();
672679

673680
return attributeDocId;

‎test/Microsoft.DotNet.ApiDiff.Tests/Diff.Attribute.Tests.cs

Lines changed: 39 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -913,7 +913,8 @@ public class MyClass
913913
attributesToExclude: ["T:System.Diagnostics.CodeAnalysis.RequiresUnreferencedCodeAttribute"]); // Overrides the default list
914914

915915
[Fact]
916-
public Task AttributesRepeatedWithDifferentArguments() => RunTestAsync(
916+
public Task SuppressAttributesRepeatedWithDifferentArguments() => RunTestAsync(
917+
// Include dummy property
917918
beforeCode: """
918919
namespace MyNamespace
919920
{
@@ -926,24 +927,24 @@ public class MyClass
926927
using System;
927928
namespace MyNamespace
928929
{
929-
[System.Runtime.Versioning.UnsupportedOSPlatformAttribute("tvos")]
930-
[System.Runtime.Versioning.UnsupportedOSPlatformAttribute("linux")]
930+
[System.Runtime.Versioning.UnsupportedOSPlatform("tvos")]
931+
[System.Runtime.Versioning.UnsupportedOSPlatform("linux")]
931932
public class MyClass
932933
{
934+
public int X { get; }
933935
}
934936
}
935937
""",
936938
expectedCode: """
937939
namespace MyNamespace
938940
{
939-
+ [System.Runtime.Versioning.UnsupportedOSPlatformAttribute("tvos")]
940-
+ [System.Runtime.Versioning.UnsupportedOSPlatformAttribute("linux")]
941941
public class MyClass
942942
{
943+
+ public int X { get; }
943944
}
944945
}
945946
""",
946-
attributesToExclude: null); // null forces using the default list
947+
attributesToExclude: ["T:System.Runtime.Versioning.UnsupportedOSPlatformAttribute"]); // Overrides the default list
947948

948949
[Fact]
949950
public Task SuppressTypeAndAttributeUsage() => RunTestAsync(
@@ -1027,6 +1028,38 @@ public class MyClass
10271028
""",
10281029
attributesToExclude: []); // Make sure to show AttributeUsage, which by default is suppressed
10291030

1031+
[Fact]
1032+
public Task AttributesRepeatedWithDifferentArguments() => RunTestAsync(
1033+
beforeCode: """
1034+
namespace MyNamespace
1035+
{
1036+
public class MyClass
1037+
{
1038+
}
1039+
}
1040+
""",
1041+
afterCode: """
1042+
using System;
1043+
namespace MyNamespace
1044+
{
1045+
[System.Runtime.Versioning.UnsupportedOSPlatform("tvos")]
1046+
[System.Runtime.Versioning.UnsupportedOSPlatform("linux")]
1047+
public class MyClass
1048+
{
1049+
}
1050+
}
1051+
""",
1052+
expectedCode: """
1053+
namespace MyNamespace
1054+
{
1055+
+ [System.Runtime.Versioning.UnsupportedOSPlatformAttribute("tvos")]
1056+
+ [System.Runtime.Versioning.UnsupportedOSPlatformAttribute("linux")]
1057+
public class MyClass
1058+
{
1059+
}
1060+
}
1061+
""",
1062+
attributesToExclude: null); // null forces using the default list
10301063

10311064
#endregion
10321065
}

0 commit comments

Comments
 (0)
0