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: Allow to have empty class with non-empty ctor for base class #6960

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.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
52 changes: 52 additions & 0 deletions analyzers/its/expected/Net5/Net5--net5.0-S2094.json
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,58 @@
},
{
"id": "S2094",
"message": "Remove this empty record, write its code or make it an "interface".",
"location": {
"uri": "sources\Net5\Net5\Records.cs",
"region": {
"startLine": 109,
"startColumn": 19,
"endLine": 109,
"endColumn": 20
}
}
},
{
"id": "S2094",
"message": "Remove this empty record, write its code or make it an "interface".",
"location": {
"uri": "sources\Net5\Net5\Records.cs",
"region": {
"startLine": 130,
"startColumn": 19,
"endLine": 130,
"endColumn": 20
}
}
},
{
"id": "S2094",
"message": "Remove this empty record, write its code or make it an "interface".",
"location": {
"uri": "sources\Net5\Net5\S2219.cs",
"region": {
"startLine": 5,
"startColumn": 16,
"endLine": 5,
"endColumn": 21
}
}
},
{
"id": "S2094",
"message": "Remove this empty record, write its code or make it an "interface".",
"location": {
"uri": "sources\Net5\Net5\S2219.cs",
"region": {
"startLine": 6,
"startColumn": 23,
"endLine": 6,
"endColumn": 28
}
}
},
{
"id": "S2094",
"message": "Remove this empty class, write its code or make it an "interface".",
"location": {
"uri": "sources\Net5\Net5\S2330.cs",
Expand Down
16 changes: 14 additions & 2 deletions analyzers/src/SonarAnalyzer.CSharp/Rules/ClassShouldNotBeEmpty.cs
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,19 @@ protected override bool HasConditionalCompilationDirectives(SyntaxNode node) =>
SyntaxKind.ElseDirectiveTrivia,
SyntaxKind.EndIfDirectiveTrivia));

private bool IsParameterlessRecord(SyntaxNode node) =>
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 };

private static RecordDeclarationSyntaxWrapper? RecordDeclarationSyntax(SyntaxNode node) =>
RecordDeclarationSyntaxWrapper.IsInstance(node)
&& (RecordDeclarationSyntaxWrapper)node is { ParameterList.Parameters.Count: 0 };
? (RecordDeclarationSyntaxWrapper)node
: null;

private static PrimaryConstructorBaseTypeSyntaxWrapper? BaseTypeSyntax(RecordDeclarationSyntaxWrapper node) =>
node.BaseList?.Types.FirstOrDefault() is { } type
&& PrimaryConstructorBaseTypeSyntaxWrapper.IsInstance(type)
? (PrimaryConstructorBaseTypeSyntaxWrapper)type
: null;
}
Original file line number Diff line number Diff line change
@@ -1,13 +1,22 @@

record class EmptyRecordClass1(); // Noncompliant {{Remove this empty record, write its code or make it an "interface".}}
// ^^^^^^^^^^^^^^^^^
record class EmptyRecordClass2() { }; // Noncompliant
namespace Compliant
{
record class NotEmptyRecordClass1(int RecordMember);
record class NotEmptyRecordClass2()
{
int RecordMember => 42;
};
}
namespace NonCompliant
{

record struct EmptyRecordStruct1(); // Compliant - this rule only deals with classes
record struct EmptyRecordStruct2() { };
record class EmptyRecordClass(); // Noncompliant {{Remove this empty record, write its code or make it an "interface".}}
// ^^^^^^^^^^^^^^^^
record class EmptyRecordClassWithEmptyBody() { }; // Noncompliant

record class NotEmptyRecordClass1(int RecordMember);
record class NotEmptyRecordClass2()
record class EmptyChildWithoutBrackets : EmptyRecordClass; // Noncompliant
}
namespace Ignored
{
int RecordMember => 42;
};
record struct EmptyRecordStruct(); // Compliant - this rule only deals with classes
record struct EmptyRecordStructWithEmptyBody() { }; // Compliant - this rule only deals with classes
}
Original file line number Diff line number Diff line change
@@ -1,44 +1,63 @@
using System;

record EmptyRecord1(); // Noncompliant {{Remove this empty record, write its code or make it an "interface".}}
// ^^^^^^^^^^^^
record EmptyRecord2() { }; // Noncompliant
namespace Compliant
{
record RecordWithParameters(int Parameter);

record RecordWithParameters(int RecordMember);
record RecordWithProperty
{
int SomeProperty => 42;
}

record RecordWithProperty
{
int SomeProperty => 42;
}
record RecordWithField
{
int SomeField = 42;
}
record RecordWithMethod
{
void Method() { }
}
record RecordWithMethodOverride
{
public override string ToString() => "";
}
record RecordWithIndexer
{
int this[int index] => 42;
record RecordWithField
{
int SomeField = 42;
}
record RecordWithMethod
{
void Method() { }
}
record RecordWithMethodOverride
{
public override string ToString() => "";
}
record RecordWithIndexer
{
int this[int index] => 42;
}
record RecordWithDelegate
{
delegate void MethodDelegate();
}
record RecordWithEvent
{
event EventHandler CustomEvent;
}

record NotEmptyGenericRecord<T>(T RecordMember);

record NotEmptyGenericRecordWithContraint<T>(T RecordMember) where T : class;

record EmptyRecordProvidingParameterToBase() : RecordWithParameters(42);
}
record RecordWithDelegate

namespace Noncompliant
{
delegate void MethodDelegate();
record EmptyRecord(); // Noncompliant {{Remove this empty record, write its code or make it an "interface".}}
// ^^^^^^^^^^^
record EmptyRecordWithEmptyBody() { }; // Noncompliant

record EmptyChildWithoutBrackets : EmptyRecord; // Noncompliant

record EmptyChildRecord() : EmptyRecord(); // Noncompliant

record EmptyGenericRecord<T>(); // Noncompliant
// ^^^^^^^^^^^^^^^^^^
record EmptyGenericRecordWithContraint<T>() where T : class; // Noncompliant
}
record RecordWithEvent

namespace Ignore
{
event EventHandler CustomEvent;
partial record EmptyPartialRecord(); // Compliant - partial classes are ignored, so partial record classes are ignored as well
}

partial record EmptyPartialRecord(); // Compliant - partial classes are ignored, so partial record classes are ignored as well

record EmptyGenericRecord<T>(); // Noncompliant
// ^^^^^^^^^^^^^^^^^^
record EmptyGenericRecordWithContraint<T>() where T : class; // Noncompliant
record NotEmptyGenericRecord<T>(T RecordMember);
record NotEmptyGenericRecordWithContraint<T>(T RecordMember) where T : class;
Original file line number Diff line number Diff line change
Expand Up @@ -2,26 +2,36 @@
using Microsoft.AspNetCore.Mvc.RazorPages;
using System;

namespace Compliant
{
class DefaultImplementation : AbstractBaseWithoutAbstractMethods { } // Compliant - the class will use the default implementation of DefaultMethod
class CustomException : Exception { } // Compliant - empty exception classes are allowed, the name of the class already provides information
class CustomAttribute : Attribute { } // Compliant - empty attribute classes are allowed, the name of the class already provides information
class EmptyPageModel : PageModel { } // Compliant - an empty PageModel can be fully functional, the C# code can be in the cshtml file
class CustomActionResult : ActionResult { } // Compliant - an empty action result can still provide information by its name
}

namespace NonCompliant
{
class SubClass : BaseClass { } // Noncompliant - not derived from any special base class
}

namespace Ignore
{
class NoImplementation : AbstractBaseWithAbstractMethods { } // Error - abstract methods should be implemented
}

class BaseClass
{
int Prop => 42;
}
class SubClass: BaseClass { } // Noncompliant - not derived from any special base class

abstract class AbstractBaseWithAbstractMethods
{
public abstract void AbstractMethod();
}

abstract class AbstractBaseWithoutAbstractMethods
{
public virtual void DefaultMethod() { }
}
class NoImplementation: AbstractBaseWithAbstractMethods { } // Error - abstract methods should be implemented
class DefaultImplementation: AbstractBaseWithoutAbstractMethods { } // Compliant - the class will use the default implementation of DefaultMethod

class CustomException: Exception { } // Compliant - empty exception classes are allowed, the name of the class already provides information
class CustomAttribute: Attribute { } // Compliant - empty attribute classes are allowed, the name of the class already provides information

class EmptyPageModel: PageModel { } // Compliant - an empty PageModel can be fully functional, the C# code can be in the cshtml file
class CustomActionResult: ActionResult { } // Compliant - an empty action result can still provide information by its name

Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,46 @@
Imports Microsoft.AspNetCore.Mvc.RazorPages
Imports System

Namespace Compliant

Class DefaultImplementation ' Compliant - the class will use the default implementation of DefaultMethod
Inherits AbstractBaseWithoutAbstractMethods
End Class

Public Class CustomException ' Compliant - empty exception classes are allowed, the name of the class already provides information
Inherits Exception
End Class

Public Class CustomAttribute ' Compliant - empty attribute classes are allowed, the name of the class already provides information
Inherits Exception
End Class

Public Class EmptyPageModel ' Compliant - an empty PageModel can be fully functional, the VB code can be in the vbhtml file
Inherits PageModel
End Class

Public Class CustomActionResult ' Compliant - an empty action result can still provide information by its name
Inherits ActionResult
End Class

End Namespace

Namespace NonComplaint

Public Class SubClass ' Noncompliant - not derived from any special base class
Inherits BaseClass
End Class

End Namespace

Namespace Ignore

Class NoImplementation ' Error - abstract methods should be implemented
Inherits AbstractBaseWithAbstractMethods
End Class

End Namespace

Public Class BaseClass
Private ReadOnly Property Prop As Integer
Get
Expand All @@ -10,10 +50,6 @@ Public Class BaseClass
End Property
End Class

Public Class SubClass ' Noncompliant - not derived from any special base class
Inherits BaseClass
End Class

MustInherit Class AbstractBaseWithAbstractMethods
Public MustOverride Sub AbstractMethod()
End Class
Expand All @@ -22,27 +58,3 @@ MustInherit Class AbstractBaseWithoutAbstractMethods
Public Overridable Sub DefaultMethod()
End Sub
End Class

Class NoImplementation ' Error - abstract methods should be implemented
Inherits AbstractBaseWithAbstractMethods
End Class

Class DefaultImplementation ' Compliant - the class will use the default implementation of DefaultMethod
Inherits AbstractBaseWithoutAbstractMethods
End Class

Public Class CustomException ' Compliant - empty exception classes are allowed, the name of the class already provides information
Inherits Exception
End Class

Public Class CustomAttribute ' Compliant - empty attribute classes are allowed, the name of the class already provides information
Inherits Exception
End Class

Public Class EmptyPageModel ' Compliant - an empty PageModel can be fully functional, the VB code can be in the vbhtml file
Inherits PageModel
End Class

Public Class CustomActionResult ' Compliant - an empty action result can still provide information by its name
Inherits ActionResult
End Class
Loading