diff --git a/analyzers/src/SonarAnalyzer.CSharp/Rules/TestMethodShouldContainAssertion.cs b/analyzers/src/SonarAnalyzer.CSharp/Rules/TestMethodShouldContainAssertion.cs index ed57cc22240..2fadb698e71 100644 --- a/analyzers/src/SonarAnalyzer.CSharp/Rules/TestMethodShouldContainAssertion.cs +++ b/analyzers/src/SonarAnalyzer.CSharp/Rules/TestMethodShouldContainAssertion.cs @@ -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); } } diff --git a/analyzers/src/SonarAnalyzer.Common/Extensions/AttributeDataExtensions.cs b/analyzers/src/SonarAnalyzer.Common/Extensions/AttributeDataExtensions.cs index 1a87da7b96a..d8167fdbb13 100644 --- a/analyzers/src/SonarAnalyzer.Common/Extensions/AttributeDataExtensions.cs +++ b/analyzers/src/SonarAnalyzer.Common/Extensions/AttributeDataExtensions.cs @@ -46,6 +46,14 @@ public static bool TryGetAttributeValue(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(TypedConstant constant, out T value) { value = default; diff --git a/analyzers/src/SonarAnalyzer.Common/Helpers/SymbolHelper.cs b/analyzers/src/SonarAnalyzer.Common/Helpers/SymbolHelper.cs index 8dc39fe6f61..eb30a8ff640 100644 --- a/analyzers/src/SonarAnalyzer.Common/Helpers/SymbolHelper.cs +++ b/analyzers/src/SonarAnalyzer.Common/Helpers/SymbolHelper.cs @@ -19,6 +19,7 @@ */ using System.Reflection; +using static System.Net.WebRequestMethods; namespace SonarAnalyzer.Helpers { @@ -209,6 +210,40 @@ public static bool IsPubliclyAccessible(this ISymbol symbol) symbol?.GetAttributes().Where(a => a.AttributeClass.IsAny(attributeTypes)) ?? Enumerable.Empty(); + /// + /// Returns attributes for the symbol by also respecting . + /// The returned is consistent with the results from . + /// + public static IEnumerable 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; diff --git a/analyzers/tests/SonarAnalyzer.UnitTest/Extensions/AttributeDataExtensionsTest.cs b/analyzers/tests/SonarAnalyzer.UnitTest/Extensions/AttributeDataExtensionsTest.cs index 5c11f571ca1..fd3713b1f6d 100644 --- a/analyzers/tests/SonarAnalyzer.UnitTest/Extensions/AttributeDataExtensionsTest.cs +++ b/analyzers/tests/SonarAnalyzer.UnitTest/Extensions/AttributeDataExtensionsTest.cs @@ -21,6 +21,7 @@ using Microsoft.CodeAnalysis.CSharp.Syntax; using Moq; using Moq.Protected; +using SonarAnalyzer.Common; using SonarAnalyzer.Extensions; namespace SonarAnalyzer.UnitTest.Extensions @@ -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(); @@ -155,20 +241,21 @@ private static AttributeData AttributeDataWithArguments(Dictionary $"{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().Last(); var symbol = (INamedTypeSymbol)snippet.SemanticModel.GetDeclaredSymbol(classDeclaration); diff --git a/analyzers/tests/SonarAnalyzer.UnitTest/Helpers/SymbolHelperTest.cs b/analyzers/tests/SonarAnalyzer.UnitTest/Helpers/SymbolHelperTest.cs index a37c408728e..56b1dc084fa 100644 --- a/analyzers/tests/SonarAnalyzer.UnitTest/Helpers/SymbolHelperTest.cs +++ b/analyzers/tests/SonarAnalyzer.UnitTest/Helpers/SymbolHelperTest.cs @@ -398,5 +398,174 @@ public void GetClassification_Method(MethodKind methodKind, string expected) fakeSymbol.Object.GetClassification().Should().Be(expected); } + + [DataTestMethod] + [DataRow("BaseClass ", "VirtualMethod ", "InheritedAttribute", "DerivedInheritedAttribute", "DerivedNotInheritedAttribute", "UnannotatedAttribute", "NotInheritedAttribute")] + [DataRow("DerivedOpenGeneric", "VirtualMethod ", "InheritedAttribute", "DerivedInheritedAttribute", "DerivedNotInheritedAttribute", "UnannotatedAttribute")] + [DataRow("DerivedClosedGeneric ", "VirtualMethod ", "InheritedAttribute", "DerivedInheritedAttribute", "DerivedNotInheritedAttribute", "UnannotatedAttribute")] + [DataRow("DerivedNoOverrides", "VirtualMethod ", "InheritedAttribute", "DerivedInheritedAttribute", "DerivedNotInheritedAttribute", "UnannotatedAttribute", "NotInheritedAttribute")] + [DataRow("DerivedOpenGeneric", "GenericVirtualMethod ", "InheritedAttribute", "DerivedInheritedAttribute", "DerivedNotInheritedAttribute", "UnannotatedAttribute")] + [DataRow("DerivedClosedGeneric ", "GenericVirtualMethod ", "InheritedAttribute", "DerivedInheritedAttribute", "DerivedNotInheritedAttribute", "UnannotatedAttribute")] + [DataRow("DerivedNoOverrides", "GenericVirtualMethod ", "InheritedAttribute", "DerivedInheritedAttribute", "DerivedNotInheritedAttribute", "UnannotatedAttribute", "NotInheritedAttribute")] + [DataRow("DerivedOpenGeneric", "NonVirtualMethod ")] + [DataRow("DerivedClosedGeneric ", "NonVirtualMethod ")] + [DataRow("DerivedNoOverrides", "NonVirtualMethod ", "InheritedAttribute", "DerivedInheritedAttribute", "DerivedNotInheritedAttribute", "UnannotatedAttribute", "NotInheritedAttribute")] + [DataRow("DerivedOpenGeneric", "GenericNonVirtualMethod")] + [DataRow("DerivedClosedGeneric ", "GenericNonVirtualMethod")] + [DataRow("DerivedNoOverrides", "GenericNonVirtualMethod", "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 + { + [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() { } + + [Inherited] + [DerivedInherited] + [NotInherited] + [DerivedNotInherited] + [Unannotated] + public virtual void GenericVirtualMethod() { } + } + + public class DerivedOpenGeneric: BaseClass + { + public override void VirtualMethod() { } + public new void NonVirtualMethod() { } + public new void GenericNonVirtualMethod() { } + public override void GenericVirtualMethod() { } + } + + public class DerivedClosedGeneric: BaseClass + { + public override void VirtualMethod() { } + public new void NonVirtualMethod() { } + public new void GenericNonVirtualMethod() { } + public override void GenericVirtualMethod() { } + } + + public class DerivedNoOverrides: BaseClass { } + + public class Program + { + public static void Main() + { + new {{className}}().{{methodName}}(); + } + } + """; + var compiler = new SnippetCompiler(code); + var invocationExpression = compiler.GetNodes().Should().ContainSingle().Subject; + var method = compiler.GetSymbol(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("", "`1"), throwOnError: true); + var methodInfo = type.GetMethod(methodName.Replace("", string.Empty)); + methodInfo.GetCustomAttributes(inherit: true).Select(x => x.GetType().Name).Should().BeEquivalentTo(expectedAttributes); + } + + [DataTestMethod] + [DataRow("BaseClass ", "InheritedAttribute", "DerivedInheritedAttribute", "DerivedNotInheritedAttribute", "UnannotatedAttribute", "NotInheritedAttribute")] + [DataRow("DerivedOpenGeneric", "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 { } + + [Inherited] + [DerivedInherited] + [NotInherited] + [DerivedNotInherited] + [Unannotated] + public interface IInterface { } + + public class DerivedOpenGeneric: BaseClass { } + + public class DerivedClosedGeneric: BaseClass { } + + public class Implement: IInterface { } + + public class Program + { + public static void Main() + { + new {{className}}(); + } + } + """; + var compiler = new SnippetCompiler(code); + var objectCreation = compiler.GetNodes().Should().ContainSingle().Subject; + if (compiler.GetSymbol(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("", "`1"), throwOnError: true); + type.GetCustomAttributes(inherit: true).Select(x => x.GetType().Name).Should().BeEquivalentTo(expectedAttributes); + } } } diff --git a/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/TestMethodShouldContainAssertion.Custom.cs b/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/TestMethodShouldContainAssertion.Custom.cs index 982c4c2dc55..0867087d968 100644 --- a/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/TestMethodShouldContainAssertion.Custom.cs +++ b/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/TestMethodShouldContainAssertion.Custom.cs @@ -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(); } diff --git a/analyzers/tests/SonarAnalyzer.UnitTest/TestFramework/SnippetCompiler.cs b/analyzers/tests/SonarAnalyzer.UnitTest/TestFramework/SnippetCompiler.cs index 378e901597c..371f9053711 100644 --- a/analyzers/tests/SonarAnalyzer.UnitTest/TestFramework/SnippetCompiler.cs +++ b/analyzers/tests/SonarAnalyzer.UnitTest/TestFramework/SnippetCompiler.cs @@ -18,6 +18,8 @@ * Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. */ +using System.IO; +using System.Reflection; using Microsoft.CodeAnalysis.CSharp; using Microsoft.CodeAnalysis.VisualBasic; using SonarAnalyzer.AnalysisContext; @@ -32,8 +34,7 @@ namespace SonarAnalyzer.UnitTest.TestFramework /// internal class SnippetCompiler { - private readonly Compilation compilation; - + public Compilation Compilation { get; } public SyntaxTree SyntaxTree { get; } public SemanticModel SemanticModel { get; } @@ -43,25 +44,25 @@ internal class SnippetCompiler public SnippetCompiler(string code, bool ignoreErrors, AnalyzerLanguage language, IEnumerable additionalReferences = null, OutputKind outputKind = OutputKind.DynamicallyLinkedLibrary) { - compilation = SolutionBuilder + Compilation = SolutionBuilder .Create() .AddProject(language, createExtraEmptyFile: false, outputKind) .AddSnippet(code) .AddReferences(additionalReferences ?? Enumerable.Empty()) .GetCompilation(); - if (!ignoreErrors && HasCompilationErrors(compilation)) + if (!ignoreErrors && HasCompilationErrors(Compilation)) { - DumpCompilationErrors(compilation); + DumpCompilationErrors(Compilation); throw new InvalidOperationException("Test setup error: test code snippet did not compile. See output window for details."); } - SyntaxTree = compilation.SyntaxTrees.First(); - SemanticModel = compilation.GetSemanticModel(SyntaxTree); + SyntaxTree = Compilation.SyntaxTrees.First(); + SemanticModel = Compilation.GetSemanticModel(SyntaxTree); } public bool IsCSharp() => - compilation.Language == LanguageNames.CSharp; + Compilation.Language == LanguageNames.CSharp; public IEnumerable GetNodes() where TSyntaxNodeType : SyntaxNode => SyntaxTree.GetRoot().DescendantNodes().OfType(); @@ -146,6 +147,13 @@ public SonarSyntaxNodeReportingContext CreateAnalysisContext(SyntaxNode node) return new(AnalysisScaffolding.CreateSonarAnalysisContext(), nodeContext); } + public Assembly EmitAssembly() + { + using var memoryStream = new MemoryStream(); + Compilation.Emit(memoryStream).Success.Should().BeTrue("The provided snippet should emit assembly."); + return Assembly.Load(memoryStream.ToArray()); + } + private static bool HasCompilationErrors(Compilation compilation) => compilation.GetDiagnostics().Any(IsCompilationError);