Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix S2094 FP: Primary constructors for classes should be handled equal to records #9019

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
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; }
}
}