Skip to content

Commit

Permalink
Fix S2699 FP: Support overridden methods annotated with AssertionMeth…
Browse files Browse the repository at this point in the history 
…odAttribute (#6652)
  • Loading branch information
@martin-strecker-sonarsource
martin-strecker-sonarsource committed Mar 14, 2023
1 parent 9552ce6 commit befef4b
Show file tree
Hide file tree
Showing 7 changed files with 328 additions and 21 deletions.
2 changes: 1 addition & 1 deletion analyzers/src/SonarAnalyzer.CSharp/Rules/TestMethodShouldContainAssertion.cs
View file
Expand Up @@ -152,6 +152,6 @@ private static bool IsTestIgnored(IMethodSymbol method)
|| methodSymbol.ContainingType.DerivesFromAny(KnownAssertionTypes);

private static bool IsCustomAssertion(ISymbol methodSymbol) =>
methodSymbol.GetAttributes().Any(x => x.AttributeClass.Name == CustomAssertionAttributeName);
methodSymbol.GetAttributesWithInherited().Any(x => x.AttributeClass.Name == CustomAssertionAttributeName);
}
}
8 changes: 8 additions & 0 deletions analyzers/src/SonarAnalyzer.Common/Extensions/AttributeDataExtensions.cs
View file
Expand Up @@ -46,6 +46,14 @@ public static bool TryGetAttributeValue<T>(this AttributeData attribute, string
}
}

public static bool HasAttributeUsageInherited(this AttributeData attribute) =>
attribute.AttributeClass.GetAttributes()
.Where(x => x.AttributeClass.Is(KnownType.System_AttributeUsageAttribute))
.SelectMany(x => x.NamedArguments.Where(x => x.Key == nameof(AttributeUsageAttribute.Inherited)))
.Where(x => x.Value is { Kind: TypedConstantKind.Primitive, Type.SpecialType: SpecialType.System_Boolean })
.Select(x => (bool?)x.Value.Value)
.FirstOrDefault() ?? true; // Default value of Inherited is true

private static bool TryConvertConstant<T>(TypedConstant constant, out T value)
{
value = default;
Expand Down
35 changes: 35 additions & 0 deletions analyzers/src/SonarAnalyzer.Common/Helpers/SymbolHelper.cs
View file
Expand Up @@ -19,6 +19,7 @@
*/

using System.Reflection;
using static System.Net.WebRequestMethods;

namespace SonarAnalyzer.Helpers
{
Expand Down Expand Up @@ -209,6 +210,40 @@ public static bool IsPubliclyAccessible(this ISymbol symbol)
symbol?.GetAttributes().Where(a => a.AttributeClass.IsAny(attributeTypes))
?? Enumerable.Empty<AttributeData>();

/// <summary>
/// Returns attributes for the symbol by also respecting <see cref="AttributeUsageAttribute.Inherited"/>.
/// The returned <see cref="AttributeData"/> is consistent with the results from <see cref="MemberInfo.GetCustomAttributes(bool)"/>.
/// </summary>
public static IEnumerable<AttributeData> GetAttributesWithInherited(this ISymbol symbol)
{
foreach (var attribute in symbol.GetAttributes())
{
yield return attribute;
}

var baseSymbol = BaseSymbol(symbol);
while (baseSymbol is not null)
{
foreach (var attribute in baseSymbol.GetAttributes().Where(x => x.HasAttributeUsageInherited()))
{
yield return attribute;
}

baseSymbol = BaseSymbol(baseSymbol);
}

static ISymbol BaseSymbol(ISymbol symbol) =>
symbol switch
{
INamedTypeSymbol namedType => namedType.BaseType,
IMethodSymbol { OriginalDefinition: { } originalDefinition } method when !method.Equals(originalDefinition) => BaseSymbol(originalDefinition),
IMethodSymbol { OverriddenMethod: { } overridenMethod } => overridenMethod,
// Support for other kinds of symbols needs to be implemented/tested as needed. A full list can be found here:
// https://learn.microsoft.com/dotnet/api/system.attributetargets
_ => null,
};
}

internal static bool AnyAttributeDerivesFrom(this ISymbol symbol, KnownType attributeType) =>
symbol?.GetAttributes().Any(a => a.AttributeClass.DerivesFrom(attributeType)) ?? false;

Expand Down
109 changes: 98 additions & 11 deletions analyzers/tests/SonarAnalyzer.UnitTest/Extensions/AttributeDataExtensionsTest.cs
View file
Expand Up @@ -21,6 +21,7 @@
using Microsoft.CodeAnalysis.CSharp.Syntax;
using Moq;
using Moq.Protected;
using SonarAnalyzer.Common;
using SonarAnalyzer.Extensions;

namespace SonarAnalyzer.UnitTest.Extensions
Expand Down Expand Up @@ -141,6 +142,91 @@ public void TryGetAttributeValue_ObjectConversion(object value, Type expectedTyp
actualValue.Should().Be(value);
}

[DataTestMethod]
[DataRow(true)]
[DataRow(false)]
public void HasAttributeUsageInherited_InheritedSpecified(bool inherited)
{
var code = $$"""
using System;

[AttributeUsage(AttributeTargets.All, Inherited = {{inherited.ToString().ToLower()}})]
public class MyAttribute: Attribute { }

[My]
public class Program { }
""";
CompileAttribute(code).HasAttributeUsageInherited().Should().Be(inherited);
}

[TestMethod]
public void HasAttributeUsageInherited_InheritedUnSpecified()
{
const string code = """
using System;

[AttributeUsage(AttributeTargets.All)]
public class MyAttribute: Attribute { }

[My]
public class Program { }
""";
CompileAttribute(code).HasAttributeUsageInherited().Should().Be(true); // The default for Inherited = true
}

[TestMethod]
public void HasAttributeUsageInherited_NoUsageAttribute()
{
const string code = """
using System;

public class MyAttribute: Attribute { }

[My]
public class Program { }
""";
CompileAttribute(code).HasAttributeUsageInherited().Should().Be(true); // The default for Inherited = true
}

[DataTestMethod]
[DataRow(true, true)]
[DataRow(false, true)] // The "Inherited" flag is not inherited for the AttributeUsage attribute itself. See also the SymbolHelperTest.GetAttributesWithInherited... tests,
// where the reflection behavior of MemberInfo.GetCustomAttributes is also tested.
public void HasAttributeUsageInherited_UsageInherited(bool inherited, bool expected)
{
var code = $$"""
using System;

[AttributeUsage(AttributeTargets.All, Inherited = {{inherited.ToString().ToLower()}})]
public class BaseAttribute: Attribute { }

public class MyAttribute: BaseAttribute { }

[My]
public class Program { }
""";
CompileAttribute(code).HasAttributeUsageInherited().Should().Be(expected);
}

[TestMethod]
public void HasAttributeUsageInherited_DuplicateAttributeUsage()
{
const string code = """
using System;

[AttributeUsage(AttributeTargets.All, Inherited = true)]
[AttributeUsage(AttributeTargets.All, Inherited = false)] // Compiler error
public class MyAttribute: Attribute { }

[My]
public class Program { }
""";
CompileAttribute(code, ignoreErrors: true).HasAttributeUsageInherited().Should().BeTrue();
}

private static AttributeData CompileAttribute(string code, bool ignoreErrors = false) =>
new SnippetCompiler(code, ignoreErrors, AnalyzerLanguage.CSharp).GetTypeSymbol("Program").GetAttributes().Single(x => x.HasName("MyAttribute"));

private static AttributeData AttributeDataWithName(string attributeClassName)
{
var namedType = new Mock<INamedTypeSymbol>();
Expand All @@ -155,20 +241,21 @@ private static AttributeData AttributeDataWithArguments(Dictionary<string, objec
namedArguments ??= new();
constructorArguments ??= new();
var separator = (constructorArguments.Any() && namedArguments.Any()) ? ", " : string.Empty;
var code = $@"
using System;
var code = $$"""
using System;

public class MyAttribute: Attribute
{{
public MyAttribute({constructorArguments.Select(x => $"{TypeName(x.Value)} {x.Key}").JoinStr(", ")})
{{
}}
public class MyAttribute: Attribute
{
public MyAttribute({{constructorArguments.Select(x => $"{TypeName(x.Value)} {x.Key}").JoinStr(", ")}})
{
}

{namedArguments.Select(x => $@"public {TypeName(x.Value)} {x.Key} {{ get; set; }}").JoinStr("\r\n")}
}}
{{namedArguments.Select(x => $@"public {TypeName(x.Value)} {x.Key} {{ get; set; }}").JoinStr("\r\n")}}
}

[My({constructorArguments.Select(x => Quote(x.Value)).JoinStr(", ")}{separator}{namedArguments.Select(x => $"{x.Key}={Quote(x.Value)}").JoinStr(", ")})]
public class Dummy {{ }}";
[My({{constructorArguments.Select(x => Quote(x.Value)).JoinStr(", ")}}{{separator}}{{namedArguments.Select(x => $"{x.Key}={Quote(x.Value)}").JoinStr(", ")}})]
public class Dummy { }
""";
var snippet = new SnippetCompiler(code);
var classDeclaration = snippet.SyntaxTree.GetRoot().DescendantNodes().OfType<ClassDeclarationSyntax>().Last();
var symbol = (INamedTypeSymbol)snippet.SemanticModel.GetDeclaredSymbol(classDeclaration);
Expand Down
169 changes: 169 additions & 0 deletions analyzers/tests/SonarAnalyzer.UnitTest/Helpers/SymbolHelperTest.cs
View file
Expand Up @@ -398,5 +398,174 @@ public void GetClassification_Method(MethodKind methodKind, string expected)

fakeSymbol.Object.GetClassification().Should().Be(expected);
}

[DataTestMethod]
[DataRow("BaseClass<int> ", "VirtualMethod ", "InheritedAttribute", "DerivedInheritedAttribute", "DerivedNotInheritedAttribute", "UnannotatedAttribute", "NotInheritedAttribute")]
[DataRow("DerivedOpenGeneric<int>", "VirtualMethod ", "InheritedAttribute", "DerivedInheritedAttribute", "DerivedNotInheritedAttribute", "UnannotatedAttribute")]
[DataRow("DerivedClosedGeneric ", "VirtualMethod ", "InheritedAttribute", "DerivedInheritedAttribute", "DerivedNotInheritedAttribute", "UnannotatedAttribute")]
[DataRow("DerivedNoOverrides<int>", "VirtualMethod ", "InheritedAttribute", "DerivedInheritedAttribute", "DerivedNotInheritedAttribute", "UnannotatedAttribute", "NotInheritedAttribute")]
[DataRow("DerivedOpenGeneric<int>", "GenericVirtualMethod<int> ", "InheritedAttribute", "DerivedInheritedAttribute", "DerivedNotInheritedAttribute", "UnannotatedAttribute")]
[DataRow("DerivedClosedGeneric ", "GenericVirtualMethod<int> ", "InheritedAttribute", "DerivedInheritedAttribute", "DerivedNotInheritedAttribute", "UnannotatedAttribute")]
[DataRow("DerivedNoOverrides<int>", "GenericVirtualMethod<int> ", "InheritedAttribute", "DerivedInheritedAttribute", "DerivedNotInheritedAttribute", "UnannotatedAttribute", "NotInheritedAttribute")]
[DataRow("DerivedOpenGeneric<int>", "NonVirtualMethod ")]
[DataRow("DerivedClosedGeneric ", "NonVirtualMethod ")]
[DataRow("DerivedNoOverrides<int>", "NonVirtualMethod ", "InheritedAttribute", "DerivedInheritedAttribute", "DerivedNotInheritedAttribute", "UnannotatedAttribute", "NotInheritedAttribute")]
[DataRow("DerivedOpenGeneric<int>", "GenericNonVirtualMethod<int>")]
[DataRow("DerivedClosedGeneric ", "GenericNonVirtualMethod<int>")]
[DataRow("DerivedNoOverrides<int>", "GenericNonVirtualMethod<int>", "InheritedAttribute", "DerivedInheritedAttribute", "DerivedNotInheritedAttribute", "UnannotatedAttribute", "NotInheritedAttribute")]
public void GetAttributesWithInherited_MethodSymbol(string className, string methodName, params string[] expectedAttributes)
{
className = className.TrimEnd();
methodName = methodName.TrimEnd();
var code = $$"""
using System;

[AttributeUsage(AttributeTargets.All, Inherited = true)]
public class InheritedAttribute : Attribute { }

[AttributeUsage(AttributeTargets.All, Inherited = false)]
public class NotInheritedAttribute : Attribute { }

public class DerivedInheritedAttribute: InheritedAttribute { }

public class DerivedNotInheritedAttribute: NotInheritedAttribute { }

public class UnannotatedAttribute : Attribute { }

public class BaseClass<T1>
{
[Inherited]
[DerivedInherited]
[NotInherited]
[DerivedNotInherited]
[Unannotated]
public virtual void VirtualMethod() { }

[Inherited]
[DerivedInherited]
[NotInherited]
[DerivedNotInherited]
[Unannotated]
public void NonVirtualMethod() { }

[Inherited]
[DerivedInherited]
[NotInherited]
[DerivedNotInherited]
[Unannotated]
public void GenericNonVirtualMethod<T2>() { }

[Inherited]
[DerivedInherited]
[NotInherited]
[DerivedNotInherited]
[Unannotated]
public virtual void GenericVirtualMethod<T2>() { }
}

public class DerivedOpenGeneric<T1>: BaseClass<T1>
{
public override void VirtualMethod() { }
public new void NonVirtualMethod() { }
public new void GenericNonVirtualMethod<T2>() { }
public override void GenericVirtualMethod<T2>() { }
}

public class DerivedClosedGeneric: BaseClass<int>
{
public override void VirtualMethod() { }
public new void NonVirtualMethod() { }
public new void GenericNonVirtualMethod<T2>() { }
public override void GenericVirtualMethod<T2>() { }
}

public class DerivedNoOverrides<T>: BaseClass<T> { }

public class Program
{
public static void Main()
{
new {{className}}().{{methodName}}();
}
}
""";
var compiler = new SnippetCompiler(code);
var invocationExpression = compiler.GetNodes<InvocationExpressionSyntax>().Should().ContainSingle().Subject;
var method = compiler.GetSymbol<IMethodSymbol>(invocationExpression);
var actual = method.GetAttributesWithInherited().Select(x => x.AttributeClass.Name).ToList();
actual.Should().BeEquivalentTo(expectedAttributes);

// GetAttributesWithInherited should behave like MemberInfo.GetCustomAttributes from runtime reflection:
var type = compiler.EmitAssembly().GetType(className.Replace("<int>", "`1"), throwOnError: true);
var methodInfo = type.GetMethod(methodName.Replace("<int>", string.Empty));
methodInfo.GetCustomAttributes(inherit: true).Select(x => x.GetType().Name).Should().BeEquivalentTo(expectedAttributes);
}

[DataTestMethod]
[DataRow("BaseClass<int> ", "InheritedAttribute", "DerivedInheritedAttribute", "DerivedNotInheritedAttribute", "UnannotatedAttribute", "NotInheritedAttribute")]
[DataRow("DerivedOpenGeneric<int>", "InheritedAttribute", "DerivedInheritedAttribute", "DerivedNotInheritedAttribute", "UnannotatedAttribute")]
[DataRow("DerivedClosedGeneric ", "InheritedAttribute", "DerivedInheritedAttribute", "DerivedNotInheritedAttribute", "UnannotatedAttribute")]
[DataRow("Implement ")]
public void GetAttributesWithInherited_TypeSymbol(string className, params string[] expectedAttributes)
{
className = className.TrimEnd();
var code = $$"""
using System;

[AttributeUsage(AttributeTargets.All, Inherited = true)]
public class InheritedAttribute : Attribute { }

[AttributeUsage(AttributeTargets.All, Inherited = false)]
public class NotInheritedAttribute : Attribute { }

public class DerivedInheritedAttribute: InheritedAttribute { }

public class DerivedNotInheritedAttribute: NotInheritedAttribute { }

public class UnannotatedAttribute : Attribute { }

[Inherited]
[DerivedInherited]
[NotInherited]
[DerivedNotInherited]
[Unannotated]
public class BaseClass<T1> { }

[Inherited]
[DerivedInherited]
[NotInherited]
[DerivedNotInherited]
[Unannotated]
public interface IInterface { }

public class DerivedOpenGeneric<T1>: BaseClass<T1> { }

public class DerivedClosedGeneric: BaseClass<int> { }

public class Implement: IInterface { }

public class Program
{
public static void Main()
{
new {{className}}();
}
}
""";
var compiler = new SnippetCompiler(code);
var objectCreation = compiler.GetNodes<ObjectCreationExpressionSyntax>().Should().ContainSingle().Subject;
if (compiler.GetSymbol<IMethodSymbol>(objectCreation) is { MethodKind: MethodKind.Constructor, ReceiverType: { } receiver })
{
var actual = receiver.GetAttributesWithInherited().Select(x => x.AttributeClass.Name).ToList();
actual.Should().BeEquivalentTo(expectedAttributes);
}
else
{
Assert.Fail("Constructor could not be found.");
}
// GetAttributesWithInherited should behave like MemberInfo.GetCustomAttributes from runtime reflection:
var type = compiler.EmitAssembly().GetType(className.Replace("<int>", "`1"), throwOnError: true);
type.GetCustomAttributes(inherit: true).Select(x => x.GetType().Name).Should().BeEquivalentTo(expectedAttributes);
}
}
}
2 changes: 1 addition & 1 deletion analyzers/tests/SonarAnalyzer.UnitTest/TestCases/TestMethodShouldContainAssertion.Custom.cs
View file
Expand Up @@ -64,7 +64,7 @@ public class BaseTest
public class DerivedTest : BaseTest
{
[TestMethod]
public void Derived() // Noncompliant FP: The overridden method needs to be annotated because Roslyn does not respect AttributeUsage.Inherited in ISymbol.GetAttributes
public void Derived() // Compliant
{
CustomAssertionMethod();
}
Expand Down

0 comments on commit befef4b

Please sign in to comment.