Fix S2368 FP/FN: Don't raise for extension methods and raise for cons…

…tructors (#8152)
SonarSource · Oct 18, 2023 · 5bf048e · 5bf048e
1 parent e8ef0ad
commit 5bf048e
Show file tree

Hide file tree

Showing 8 changed files with 122 additions and 97 deletions.
diff --git a/analyzers/src/SonarAnalyzer.CSharp/Extensions/ISymbolExtensions.cs b/analyzers/src/SonarAnalyzer.CSharp/Extensions/ISymbolExtensions.cs
@@ -98,6 +98,7 @@ public static ImmutableArray<SyntaxToken> DeclaringReferenceIdentifiers(this ISy
             {
                 AttributeArgumentSyntax x => x.NameColon?.Name.Identifier,
                 BaseTypeDeclarationSyntax x => x.Identifier,
+                ConstructorDeclarationSyntax x => x.Identifier,
                 DelegateDeclarationSyntax x => x.Identifier,
                 EnumMemberDeclarationSyntax x => x.Identifier,
                 EventDeclarationSyntax x => x.Identifier,

diff --git a/analyzers/src/SonarAnalyzer.CSharp/Rules/PublicMethodWithMultidimensionalArray.cs b/analyzers/src/SonarAnalyzer.CSharp/Rules/PublicMethodWithMultidimensionalArray.cs
@@ -18,21 +18,16 @@
  * Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301, USA.
  */
 
-namespace SonarAnalyzer.Rules.CSharp
-{
-    [DiagnosticAnalyzer(LanguageNames.CSharp)]
-    public sealed class PublicMethodWithMultidimensionalArray : PublicMethodWithMultidimensionalArrayBase<SyntaxKind, MethodDeclarationSyntax>
-    {
-
-        private static readonly DiagnosticDescriptor rule =
-            DescriptorFactory.Create(DiagnosticId, MessageFormat);
-        public override ImmutableArray<DiagnosticDescriptor> SupportedDiagnostics { get; } = ImmutableArray.Create(rule);
+namespace SonarAnalyzer.Rules.CSharp;
 
-        private static readonly ImmutableArray<SyntaxKind> kindsOfInterest = ImmutableArray.Create(SyntaxKind.MethodDeclaration);
-        public override ImmutableArray<SyntaxKind> SyntaxKindsOfInterest => kindsOfInterest;
+[DiagnosticAnalyzer(LanguageNames.CSharp)]
+public sealed class PublicMethodWithMultidimensionalArray : PublicMethodWithMultidimensionalArrayBase<SyntaxKind>
+{
+    protected override ILanguageFacade<SyntaxKind> Language => CSharpFacade.Instance;
 
-        protected override SyntaxToken GetIdentifier(MethodDeclarationSyntax method) => method.Identifier;
+    protected override Location GetIssueLocation(SyntaxNode node) =>
+        Language.Syntax.NodeIdentifier(node)?.GetLocation();
 
-        protected override GeneratedCodeRecognizer GeneratedCodeRecognizer => CSharpGeneratedCodeRecognizer.Instance;
-    }
+    protected override string GetType(SyntaxNode node) =>
+        node is MethodDeclarationSyntax ? "method" : "constructor";
 }
diff --git a/analyzers/src/SonarAnalyzer.Common/Rules/PublicMethodWithMultidimensionalArrayBase.cs b/analyzers/src/SonarAnalyzer.Common/Rules/PublicMethodWithMultidimensionalArrayBase.cs
@@ -18,55 +18,63 @@
  * Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301, USA.
  */
 
-namespace SonarAnalyzer.Rules
+namespace SonarAnalyzer.Rules;
+
+public abstract class PublicMethodWithMultidimensionalArrayBase<TSyntaxKind> : SonarDiagnosticAnalyzer<TSyntaxKind>
+    where TSyntaxKind : struct
 {
-    public abstract class PublicMethodWithMultidimensionalArrayBase : SonarDiagnosticAnalyzer
+    private const string DiagnosticId = "S2368";
+    protected override string MessageFormat => "Make this {0} private or simplify its parameters to not use multidimensional/jagged arrays.";
+
+    protected abstract Location GetIssueLocation(SyntaxNode node);
+    protected abstract string GetType(SyntaxNode node);
+
+    protected PublicMethodWithMultidimensionalArrayBase() : base(DiagnosticId) { }
+
+    protected sealed override void Initialize(SonarAnalysisContext context)
     {
-        protected const string DiagnosticId = "S2368";
-        protected const string MessageFormat = "Make this method private or simplify its parameters to not use multidimensional/jagged arrays.";
+        context.RegisterNodeAction(Language.GeneratedCodeRecognizer, AnalyzeDeclaration, Language.SyntaxKind.MethodDeclarations);
+        context.RegisterNodeAction(Language.GeneratedCodeRecognizer, AnalyzeDeclaration, Language.SyntaxKind.ConstructorDeclaration);
+    }
 
-        protected abstract GeneratedCodeRecognizer GeneratedCodeRecognizer { get; }
+    private void AnalyzeDeclaration(SonarSyntaxNodeReportingContext c)
+    {
+        if (c.SemanticModel.GetDeclaredSymbol(c.Node) is IMethodSymbol methodSymbol
+            && methodSymbol.GetInterfaceMember() == null
+            && methodSymbol.GetOverriddenMember() == null
+            && methodSymbol.IsPubliclyAccessible()
+            && MethodHasMultidimensionalArrayParameters(methodSymbol))
+        {
+            c.ReportIssue(Diagnostic.Create(SupportedDiagnostics[0], GetIssueLocation(c.Node), GetType(c.Node)));
+        }
     }
 
-    public abstract class PublicMethodWithMultidimensionalArrayBase<TLanguageKindEnum, TMethodSyntax> : PublicMethodWithMultidimensionalArrayBase
-        where TLanguageKindEnum : struct
-        where TMethodSyntax : SyntaxNode
+    private static bool MethodHasMultidimensionalArrayParameters(IMethodSymbol methodSymbol)
     {
-        protected sealed override void Initialize(SonarAnalysisContext context)
+        if (methodSymbol.IsExtensionMethod)
         {
-            context.RegisterNodeAction(
-                GeneratedCodeRecognizer,
-                c =>
+            for (var i = 1; i < methodSymbol.Parameters.Length; i++)
+            {
+                if (IsMultidimensionalArrayParameter(methodSymbol.Parameters[i]))
                 {
-                    var method = (TMethodSyntax)c.Node;
-
-                    if (!(c.SemanticModel.GetDeclaredSymbol(method) is IMethodSymbol methodSymbol) ||
-                        methodSymbol.GetInterfaceMember() != null ||
-                        methodSymbol.GetOverriddenMember() != null ||
-                        !methodSymbol.IsPubliclyAccessible() ||
-                        !MethodHasMultidimensionalArrayParameters(methodSymbol))
-                    {
-                        return;
-                    }
-
-                    var identifier = GetIdentifier(method);
-                    c.ReportIssue(Diagnostic.Create(SupportedDiagnostics[0], identifier.GetLocation()));
-                },
-                SyntaxKindsOfInterest.ToArray());
+                    return true;
+                }
+            }
+            return false;
         }
+        else
+        {
+            return methodSymbol.Parameters.Any(param => IsMultidimensionalArrayParameter(param));
+        }
+    }
 
-        private static bool MethodHasMultidimensionalArrayParameters(IMethodSymbol methodSymbol) =>
-            methodSymbol.Parameters.Any(param => param.Type is IArrayTypeSymbol arrayType
-                                                 && (arrayType.Rank > 1
-                                                     || IsJaggedArrayParam(param, arrayType)));
-
-        private static bool IsJaggedArrayParam(IParameterSymbol param, IArrayTypeSymbol arrayType) =>
-            param.IsParams
-                ? arrayType.ElementType is IArrayTypeSymbol subType && subType.ElementType is IArrayTypeSymbol
-                : arrayType.ElementType is IArrayTypeSymbol;
-
-        protected abstract SyntaxToken GetIdentifier(TMethodSyntax method);
+    private static bool IsMultidimensionalArrayParameter(IParameterSymbol param) =>
+        param.Type is IArrayTypeSymbol arrayType
+        && (arrayType.Rank > 1
+            || IsJaggedArrayParam(param, arrayType));
 
-        public abstract ImmutableArray<TLanguageKindEnum> SyntaxKindsOfInterest { get; }
-    }
+    private static bool IsJaggedArrayParam(IParameterSymbol param, IArrayTypeSymbol arrayType) =>
+        param.IsParams
+            ? arrayType.ElementType is IArrayTypeSymbol subType && subType.ElementType is IArrayTypeSymbol
+            : arrayType.ElementType is IArrayTypeSymbol;
 }
diff --git a/analyzers/src/SonarAnalyzer.VisualBasic/Rules/PublicMethodWithMultidimensionalArray.cs b/analyzers/src/SonarAnalyzer.VisualBasic/Rules/PublicMethodWithMultidimensionalArray.cs
@@ -18,21 +18,18 @@
  * Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301, USA.
  */
 
-namespace SonarAnalyzer.Rules.VisualBasic
-{
-    [DiagnosticAnalyzer(LanguageNames.VisualBasic)]
-    public sealed class PublicMethodWithMultidimensionalArray : PublicMethodWithMultidimensionalArrayBase<SyntaxKind, MethodStatementSyntax>
-    {
-        private static readonly DiagnosticDescriptor rule =
-            DescriptorFactory.Create(DiagnosticId, MessageFormat);
-
-        public override ImmutableArray<DiagnosticDescriptor> SupportedDiagnostics { get; } = ImmutableArray.Create(rule);
+namespace SonarAnalyzer.Rules.VisualBasic;
 
-        private static readonly ImmutableArray<SyntaxKind> kindsOfInterest = ImmutableArray.Create(SyntaxKind.SubStatement, SyntaxKind.FunctionStatement);
-        public override ImmutableArray<SyntaxKind> SyntaxKindsOfInterest => kindsOfInterest;
+[DiagnosticAnalyzer(LanguageNames.VisualBasic)]
+public sealed class PublicMethodWithMultidimensionalArray : PublicMethodWithMultidimensionalArrayBase<SyntaxKind>
+{
+    protected override ILanguageFacade<SyntaxKind> Language => VisualBasicFacade.Instance;
 
-        protected override SyntaxToken GetIdentifier(MethodStatementSyntax method) => method.Identifier;
+    protected override Location GetIssueLocation(SyntaxNode node) =>
+        node is ConstructorBlockSyntax x
+            ? x.SubNewStatement.NewKeyword.GetLocation()
+            : Language.Syntax.NodeIdentifier(node)?.GetLocation();
 
-        protected override GeneratedCodeRecognizer GeneratedCodeRecognizer => VisualBasicGeneratedCodeRecognizer.Instance;
-    }
+    protected override string GetType(SyntaxNode node) =>
+        node is MethodStatementSyntax ? "method" : "constructor";
 }
diff --git a/analyzers/tests/SonarAnalyzer.UnitTest/Rules/PublicMethodWithMultidimensionalArrayTest.cs b/analyzers/tests/SonarAnalyzer.UnitTest/Rules/PublicMethodWithMultidimensionalArrayTest.cs
@@ -21,38 +21,37 @@
 using CS = SonarAnalyzer.Rules.CSharp;
 using VB = SonarAnalyzer.Rules.VisualBasic;
 
-namespace SonarAnalyzer.UnitTest.Rules
+namespace SonarAnalyzer.UnitTest.Rules;
+
+[TestClass]
+public class PublicMethodWithMultidimensionalArrayTest
 {
-    [TestClass]
-    public class PublicMethodWithMultidimensionalArrayTest
-    {
-        private readonly VerifierBuilder builderCS = new VerifierBuilder<CS.PublicMethodWithMultidimensionalArray>();
-        private readonly VerifierBuilder builderVB = new VerifierBuilder<VB.PublicMethodWithMultidimensionalArray>();
+    private readonly VerifierBuilder builderCS = new VerifierBuilder<CS.PublicMethodWithMultidimensionalArray>();
+    private readonly VerifierBuilder builderVB = new VerifierBuilder<VB.PublicMethodWithMultidimensionalArray>();
 
-        [TestMethod]
-        public void PublicMethodWithMultidimensionalArray_CS() =>
-            builderCS.AddPaths("PublicMethodWithMultidimensionalArray.cs")
-                .Verify();
+    [TestMethod]
+    public void PublicMethodWithMultidimensionalArray_CS() =>
+        builderCS.AddPaths("PublicMethodWithMultidimensionalArray.cs")
+            .Verify();
 
 #if NET
 
-        [TestMethod]
-        public void PublicMethodWithMultidimensionalArray_CSharp11() =>
-            builderCS.AddPaths("PublicMethodWithMultidimensionalArray.CSharp11.cs")
-                .WithOptions(ParseOptionsHelper.FromCSharp11)
-                .Verify();
+    [TestMethod]
+    public void PublicMethodWithMultidimensionalArray_CSharp11() =>
+        builderCS.AddPaths("PublicMethodWithMultidimensionalArray.CSharp11.cs")
+            .WithOptions(ParseOptionsHelper.FromCSharp11)
+            .Verify();
 
-        [TestMethod]
-        public void PublicMethodWithMultidimensionalArray_CSharp12() =>
-            builderCS.AddPaths("PublicMethodWithMultidimensionalArray.CSharp12.cs")
-                .WithOptions(ParseOptionsHelper.FromCSharp12)
-                .Verify();
+    [TestMethod]
+    public void PublicMethodWithMultidimensionalArray_CSharp12() =>
+        builderCS.AddPaths("PublicMethodWithMultidimensionalArray.CSharp12.cs")
+            .WithOptions(ParseOptionsHelper.FromCSharp12)
+            .Verify();
 
 #endif
 
-        [TestMethod]
-        public void PublicMethodWithMultidimensionalArray_VB() =>
-            builderVB.AddPaths("PublicMethodWithMultidimensionalArray.vb")
-                .Verify();
-    }
+    [TestMethod]
+    public void PublicMethodWithMultidimensionalArray_VB() =>
+        builderVB.AddPaths("PublicMethodWithMultidimensionalArray.vb")
+            .Verify();
 }
diff --git a/.../tests/SonarAnalyzer.UnitTest/TestCases/PublicMethodWithMultidimensionalArray.CSharp12.cs b/.../tests/SonarAnalyzer.UnitTest/TestCases/PublicMethodWithMultidimensionalArray.CSharp12.cs
@@ -14,7 +14,7 @@ public class C5(int i);              // Compliant, not a multi-dimensional array
 
     public class Aliases(IntMatrix a)                    // FN
     {
-        public Aliases(IntMatrix a, int i) : this(a) { } // FN
+        public Aliases(IntMatrix a, int i) : this(a) { } // Noncompliant
 
         public void AMethod1(IntMatrix a) { }            // Noncompliant
         public void AMethod2(params IntMatrix a) { }     // Compliant

diff --git a/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/PublicMethodWithMultidimensionalArray.cs b/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/PublicMethodWithMultidimensionalArray.cs
@@ -68,10 +68,16 @@ namespace Repro_8083
 {
     public class Constructors
     {
-        public Constructors(int[][] a) { }          // FN, the ctor is publicly exposed
-        public Constructors(int[,] a) { }           // FN
+        public Constructors(int[][] a) { }          // Noncompliant {{Make this constructor private or simplify its parameters to not use multidimensional/jagged arrays.}}
+        public Constructors(int[,] a) { }           // Noncompliant
         public Constructors(params int[] a) { }     // Compliant, params of int
-        public Constructors(params int[][][] a) { } // FN, params of int[][]
+        public Constructors(params int[][][] a) { } // Noncompliant
         public Constructors(int i) { }              // Compliant, not a multi-dimensional array
     }
 }
+
+public static class ExtensionMethod
+{
+    public static void Method1<T>(this T[,] dataMatrix, int a) { }
+    public static void Method2<T>(this T data, T[,] dataMatrix) { } // Noncompliant
+}
diff --git a/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/PublicMethodWithMultidimensionalArray.vb b/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/PublicMethodWithMultidimensionalArray.vb
@@ -65,4 +65,23 @@ Namespace Tests.Diagnostics
         End Sub
 
     End Class
+
+    Public Class Constructors
+        Public Sub New(A As Integer()()) ' Noncompliant {{Make this constructor private or simplify its parameters to not use multidimensional/jagged arrays.}}
+'                  ^^^
+        End Sub
+
+        Public Sub New(A As Integer(,)) ' Noncompliant
+        End Sub
+
+        Public Sub New(ParamArray A As Integer())
+        End Sub
+
+        Public Sub New(ParamArray A As Integer()()()) ' Noncompliant
+        End Sub
+
+        Public Sub New(I As Integer)
+        End Sub
+    End Class
+
 End Namespace