Fix false positive on primary constructors.

SonarSource · Apr 3, 2024 · fb87e2c · fb87e2c
1 parent ac87c14
commit fb87e2c
Show file tree

Hide file tree

Showing 3 changed files with 53 additions and 11 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() =>
@@ -64,6 +64,13 @@ public class ClassShouldNotBeEmptyTest
             .WithOptions(ParseOptionsHelper.FromCSharp10)
             .Verify();
 
+    [TestMethod]
+    public void ClassShouldNotBeEmpty_CSharp12() =>
+        builderCS
+            .AddPaths("ClassShouldNotBeEmpty.CSharp12.cs")
+            .WithOptions(ParseOptionsHelper.FromCSharp12)
+            .Verify();
+
     [TestMethod]
     public void ClassShouldNotBeEmpty_Inheritance_CS() =>
         builderCS

diff --git a/analyzers/tests/SonarAnalyzer.Test/TestCases/ClassShouldNotBeEmpty.CSharp12.cs b/analyzers/tests/SonarAnalyzer.Test/TestCases/ClassShouldNotBeEmpty.CSharp12.cs
@@ -0,0 +1,16 @@
+namespace Compliant
+{
+    class ChildClass() : BaseClass(42) { } // Compliant
+
+    class BaseClass(int value) { }
+}
+
+namespace Noncompliant
+{
+    class ChildClass() : BaseClass() { } // Noncompliant
+
+    class BaseClass()
+    {
+        public int Value { get; init; }
+    }
+}