Skip to content

Commit

Permalink
Fix S2094 FP: Primary constructors for classes should be handled equa…
Browse files Browse the repository at this point in the history
…l to records (#9019)
  • Loading branch information
Corniel committed Apr 9, 2024
1 parent 47f3e97 commit e8fb8ab
Show file tree
Hide file tree
Showing 3 changed files with 38 additions and 19 deletions.
29 changes: 20 additions & 9 deletions analyzers/src/SonarAnalyzer.CSharp/Rules/ClassShouldNotBeEmpty.cs
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,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
Expand All @@ -42,7 +42,7 @@ public sealed class ClassShouldNotBeEmpty : ClassShouldNotBeEmptyBase<SyntaxKind
|| declaration.BaseList.Types.Any(x => x.Type is GenericNameSyntax); // or a generic class/interface

protected override bool HasAnyAttribute(SyntaxNode node) =>
node is TypeDeclarationSyntax { AttributeLists.Count: > 0 };
node is TypeDeclarationSyntax { AttributeLists.Count: > 0 };

protected override string DeclarationTypeKeyword(SyntaxNode node) =>
((TypeDeclarationSyntax)node).Keyword.ValueText;
Expand All @@ -54,19 +54,30 @@ 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) =>
node is ClassDeclarationSyntax declaration
&& LacksParameters(declaration.ParameterList(), declaration.BaseList);

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: > 0 }
&& BaseTypeSyntax(baseList) is not { ArgumentList.Arguments.Count: > 0 };

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;
}
Original file line number Diff line number Diff line change
Expand Up @@ -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() =>
Expand Down
Original file line number Diff line number Diff line change
@@ -1,10 +1,18 @@
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 ChildClassWithParameters(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; }
}
}

0 comments on commit e8fb8ab

Please sign in to comment.