Fix false positive on primary constructors.

SonarSource · Apr 5, 2024 · b1ce1e2 · b1ce1e2
1 parent 4d47966
commit b1ce1e2
Show file tree

Hide file tree

Showing 3 changed files with 48 additions and 17 deletions.
diff --git a/analyzers/src/SonarAnalyzer.CSharp/Rules/ClassShouldNotBeEmpty.cs b/analyzers/src/SonarAnalyzer.CSharp/Rules/ClassShouldNotBeEmpty.cs
@@ -18,6 +18,8 @@
  * Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301, USA.
  */
 
+using System.Xml.Linq;
+
 namespace SonarAnalyzer.Rules.CSharp;
 
 [DiagnosticAnalyzer(LanguageNames.CSharp)]
@@ -28,7 +30,7 @@ public sealed class ClassShouldNotBeEmpty : ClassShouldNotBeEmptyBase<SyntaxKind
     protected override bool IsEmptyAndNotPartial(SyntaxNode node) =>
         node is TypeDeclarationSyntax { Members.Count: 0 } typeDeclaration
         && !typeDeclaration.Modifiers.Any(x => x.IsKind(SyntaxKind.PartialKeyword))
-        && (node is ClassDeclarationSyntax || IsParameterlessRecord(node));
+        && LacksParameterizedPrimaryConstructor(node);
 
     protected override BaseTypeDeclarationSyntax GetIfHasDeclaredBaseClassOrInterface(SyntaxNode node) =>
         node is TypeDeclarationSyntax { BaseList: not null } declaration
@@ -54,19 +56,36 @@ public sealed class ClassShouldNotBeEmpty : ClassShouldNotBeEmptyBase<SyntaxKind
             SyntaxKind.ElseDirectiveTrivia,
             SyntaxKind.EndIfDirectiveTrivia));
 
+    private static bool LacksParameterizedPrimaryConstructor(SyntaxNode node) =>
+        IsParameterlessClass(node)
+        || IsParameterlessRecord(node);
+
+    private static bool IsParameterlessClass(SyntaxNode node) =>
+        ClassDeclarationSyntax(node) is { } declaration
+            ? LacksParameters(declaration.ParameterList, declaration.BaseList)
+            : node is ClassDeclarationSyntax;
+
     private static bool IsParameterlessRecord(SyntaxNode node) =>
         RecordDeclarationSyntax(node) is { } declaration
-        && (declaration.ParameterList is null || declaration.ParameterList.Parameters.Count == 0)
-        && BaseTypeSyntax(declaration) is not { ArgumentList.Arguments.Count: >= 1 };
+        && LacksParameters(declaration.ParameterList, declaration.BaseList);
+
+    private static bool LacksParameters(ParameterListSyntax parameterList, BaseListSyntax baseList) =>
+        parameterList?.Parameters is not { Count: >= 1 }
+        && BaseTypeSyntax(baseList) is not { ArgumentList.Arguments.Count: >= 1 };
+
+    private static ClassDeclarationSyntaxWrapper? ClassDeclarationSyntax(SyntaxNode node) =>
+       ClassDeclarationSyntaxWrapper.IsInstance(node)
+           ? (ClassDeclarationSyntaxWrapper)node
+           : null;
 
     private static RecordDeclarationSyntaxWrapper? RecordDeclarationSyntax(SyntaxNode node) =>
         RecordDeclarationSyntaxWrapper.IsInstance(node)
             ? (RecordDeclarationSyntaxWrapper)node
             : null;
 
-    private static PrimaryConstructorBaseTypeSyntaxWrapper? BaseTypeSyntax(RecordDeclarationSyntaxWrapper node) =>
-        node.BaseList?.Types.FirstOrDefault() is { } type
-        && PrimaryConstructorBaseTypeSyntaxWrapper.IsInstance(type)
-            ? (PrimaryConstructorBaseTypeSyntaxWrapper)type
-            : null;
+    private static PrimaryConstructorBaseTypeSyntaxWrapper? BaseTypeSyntax(BaseListSyntax list) =>
+       list?.Types.FirstOrDefault() is { } type
+       && PrimaryConstructorBaseTypeSyntaxWrapper.IsInstance(type)
+           ? (PrimaryConstructorBaseTypeSyntaxWrapper)type
+           : null;
 }
diff --git a/analyzers/tests/SonarAnalyzer.Test/Rules/ClassShouldNotBeEmptyTest.cs b/analyzers/tests/SonarAnalyzer.Test/Rules/ClassShouldNotBeEmptyTest.cs
@@ -43,12 +43,12 @@ public class ClassShouldNotBeEmptyTest
 
 #if NET
 
-    private static readonly MetadataReference[] AdditionalReferences = new[]
-    {
+    private static readonly MetadataReference[] AdditionalReferences =
+    [
         AspNetCoreMetadataReference.MicrosoftAspNetCoreMvcAbstractions,
         AspNetCoreMetadataReference.MicrosoftAspNetCoreMvcCore,
         AspNetCoreMetadataReference.MicrosoftAspNetCoreRazorPages,
-    };
+    ];
 
     [TestMethod]
     public void ClassShouldNotBeEmpty_CSharp9() =>

diff --git a/analyzers/tests/SonarAnalyzer.Test/TestCases/ClassShouldNotBeEmpty.CSharp12.cs b/analyzers/tests/SonarAnalyzer.Test/TestCases/ClassShouldNotBeEmpty.CSharp12.cs
@@ -1,10 +1,22 @@
-using System.Collections.Generic;
 
-public class TestCaseData(List<int> list, int expectedCount)
+
+}
+
+namespace Compliant
 {
-    public List<int> List { get; }
-    public int ExpectedCount { get; }
+    class ChildClass() : BaseClass(42) { } // Compliant
+
+    class ChildClassWithParamters(int value) : BaseClass(value) { } // Compliant
+
+    class BaseClass(int value) { }
 }
 
-// See https://github.com/SonarSource/sonar-dotnet/issues/9011
-public class CountTestCaseData(List<int> list, int expectedCount) : TestCaseData(list, expectedCount); // Noncompliant - FP
+namespace Noncompliant
+{
+    class ChildClass() : BaseClass() { } // Noncompliant
+
+    class BaseClass()
+    {
+        public int Value { get; init; }
+    }
+}