S3431: Optimize rule (#9627)

SonarSource · Aug 9, 2024 · abd975c · abd975c
1 parent ca7bacd
commit abd975c
Show file tree

Hide file tree

Showing 5 changed files with 97 additions and 8 deletions.
diff --git a/analyzers/src/SonarAnalyzer.CSharp/Rules/ExpectedExceptionAttributeShouldNotBeUsed.cs b/analyzers/src/SonarAnalyzer.CSharp/Rules/ExpectedExceptionAttributeShouldNotBeUsed.cs
@@ -27,6 +27,9 @@ public sealed class ExpectedExceptionAttributeShouldNotBeUsed : ExpectedExceptio
     {
         protected override ILanguageFacade<SyntaxKind> Language => CSharpFacade.Instance;
 
+        protected override SyntaxNode FindExpectedExceptionAttribute(SyntaxNode node) =>
+            ((MethodDeclarationSyntax)node).AttributeLists.SelectMany(x => x.Attributes).FirstOrDefault(x => x.GetName() is "ExpectedException" or "ExpectedExceptionAttribute");
+
         protected override bool HasMultiLineBody(SyntaxNode node)
         {
             var declaration = (MethodDeclarationSyntax)node;

diff --git a/analyzers/src/SonarAnalyzer.Common/Rules/ExpectedExceptionAttributeShouldNotBeUsedBase.cs b/analyzers/src/SonarAnalyzer.Common/Rules/ExpectedExceptionAttributeShouldNotBeUsedBase.cs
@@ -25,6 +25,7 @@ public abstract class ExpectedExceptionAttributeShouldNotBeUsedBase<TSyntaxKind>
     {
         internal const string DiagnosticId = "S3431";
 
+        protected abstract SyntaxNode FindExpectedExceptionAttribute(SyntaxNode node);
         protected abstract bool HasMultiLineBody(SyntaxNode node);
         protected abstract bool AssertInCatchFinallyBlock(SyntaxNode node);
 
@@ -33,16 +34,27 @@ public abstract class ExpectedExceptionAttributeShouldNotBeUsedBase<TSyntaxKind>
         protected ExpectedExceptionAttributeShouldNotBeUsedBase() : base(DiagnosticId) { }
 
         protected override void Initialize(SonarAnalysisContext context) =>
-            context.RegisterNodeAction(Language.GeneratedCodeRecognizer, c =>
+            context.RegisterCompilationStartAction(c =>
             {
-                if (HasMultiLineBody(c.Node)
-                    && !AssertInCatchFinallyBlock(c.Node)
-                    && c.SemanticModel.GetDeclaredSymbol(c.Node) is { } methodSymbol
-                    && methodSymbol.GetAttributes(UnitTestHelper.KnownExpectedExceptionAttributes).FirstOrDefault() is { } attribute)
+                if (!ContainExpectedExceptionType(c.Compilation))
                 {
-                    c.ReportIssue(Rule, attribute.ApplicationSyntaxReference.GetSyntax());
+                    return;
                 }
-            },
-            Language.SyntaxKind.MethodDeclarations);
+
+                c.RegisterNodeAction(Language.GeneratedCodeRecognizer, cc =>
+                    {
+                        if (FindExpectedExceptionAttribute(cc.Node) is {} attribute
+                            && HasMultiLineBody(cc.Node)
+                            && !AssertInCatchFinallyBlock(cc.Node))
+                        {
+                            cc.ReportIssue(Rule, attribute);
+                        }
+                    },
+                    Language.SyntaxKind.MethodDeclarations);
+            });
+
+        private static bool ContainExpectedExceptionType(Compilation compilation) =>
+            compilation.GetTypeByMetadataName(KnownType.Microsoft_VisualStudio_TestTools_UnitTesting_ExpectedExceptionAttribute) is not null
+            || compilation.GetTypeByMetadataName(KnownType.NUnit_Framework_ExpectedExceptionAttribute) is not null;
     }
 }
diff --git a/analyzers/src/SonarAnalyzer.VisualBasic/Rules/ExpectedExceptionAttributeShouldNotBeUsed.cs b/analyzers/src/SonarAnalyzer.VisualBasic/Rules/ExpectedExceptionAttributeShouldNotBeUsed.cs
@@ -25,6 +25,9 @@ public sealed class ExpectedExceptionAttributeShouldNotBeUsed : ExpectedExceptio
 {
     protected override ILanguageFacade<SyntaxKind> Language => VisualBasicFacade.Instance;
 
+    protected override SyntaxNode FindExpectedExceptionAttribute(SyntaxNode node) =>
+        ((MethodStatementSyntax)node).AttributeLists.SelectMany(x => x.Attributes).FirstOrDefault(x => x.GetName() is "ExpectedException" or "ExpectedExceptionAttribute");
+
     protected override bool HasMultiLineBody(SyntaxNode node) =>
         node.Parent is MethodBlockSyntax { Statements.Count: > 1 };
 

diff --git a/...rs/tests/SonarAnalyzer.Test/TestCases/ExpectedExceptionAttributeShouldNotBeUsed.MsTest.cs b/...rs/tests/SonarAnalyzer.Test/TestCases/ExpectedExceptionAttributeShouldNotBeUsed.MsTest.cs
@@ -13,6 +13,30 @@ public void TestFoo1()
             x.ToString();
         }
 
+        [TestMethod]
+        [ExpectedExceptionAttribute(typeof(ArgumentNullException))]  // Noncompliant
+        public void WithAttributeSuffix()
+        {
+            var x = true;
+            x.ToString();
+        }
+
+        [TestMethod]
+        [Microsoft.VisualStudio.TestTools.UnitTesting.ExpectedException(typeof(ArgumentNullException))]  // Noncompliant
+        public void FullyQualifiedAttribute()
+        {
+            var x = true;
+            x.ToString();
+        }
+
+        [TestMethod]
+        [Unrelated.ExpectedException(typeof(ArgumentNullException))]  // Noncompliant - FP
+        public void UnrelatedAttribute()
+        {
+            var x = true;
+            x.ToString();
+        }
+
         [TestMethod]
         [ExpectedException(typeof(ArgumentNullException))] // Compliant - one line
         public void TestFoo3()
@@ -206,3 +230,17 @@ public void AssertInCatchWithFinally()
         }
     }
 }
+
+namespace Unrelated
+{
+    [AttributeUsage(AttributeTargets.Method)]
+    public class ExpectedExceptionAttribute : Attribute
+    {
+        public Type ExceptionType { get; private set; }
+
+        public ExpectedExceptionAttribute(Type exceptionType)
+        {
+            ExceptionType = exceptionType;
+        }
+    }
+}
diff --git a/...rs/tests/SonarAnalyzer.Test/TestCases/ExpectedExceptionAttributeShouldNotBeUsed.MsTest.vb b/...rs/tests/SonarAnalyzer.Test/TestCases/ExpectedExceptionAttributeShouldNotBeUsed.MsTest.vb
@@ -11,6 +11,27 @@ Public Class ExceptionTests
         instance.ToString()
     End Sub
 
+    <TestMethod>
+    <ExpectedExceptionAttribute(GetType(ArgumentNullException))>  ' Noncompliant
+    Public Sub WithAttributeSuffix()
+        Dim x As Boolean = True
+        x.ToString()
+    End Sub
+
+    <TestMethod>
+    <Microsoft.VisualStudio.TestTools.UnitTesting.ExpectedException(GetType(ArgumentNullException))>  ' Noncompliant
+    Public Sub FullyQualifiedAttribute()
+        Dim x As Boolean = True
+        x.ToString()
+    End Sub
+
+    <TestMethod>
+    <Unrelated.ExpectedException(GetType(ArgumentNullException))>  ' Noncompliant - FPgit
+    Public Sub UnrelatedAttribute()
+        Dim x As Boolean = True
+        x.ToString()
+    End Sub
+
     ' Noncompliant@+2
     <TestMethod>
     <ExpectedException(GetType(DivideByZeroException))>
@@ -142,3 +163,15 @@ Class Repro_8300
     End Sub
 End Class
 
+Namespace Unrelated
+    <AttributeUsage(AttributeTargets.Method)>
+    Public Class ExpectedExceptionAttribute
+        Inherits Attribute
+
+        Public ReadOnly Property ExceptionType As Type
+
+        Public Sub New(exceptionType As Type)
+            Me.ExceptionType = exceptionType
+        End Sub
+    End Class
+End Namespace