From e2f74db49c34439bdc1e0c46883b4d6ff7d35b4c Mon Sep 17 00:00:00 2001 From: Corniel Nobel Date: Mon, 20 Mar 2023 16:38:24 +0100 Subject: [PATCH 1/6] Apply fix. --- .../Rules/ClassShouldNotBeEmpty.cs | 14 ++- .../ClassShouldNotBeEmpty.CSharp9.cs | 85 +++++++++++-------- 2 files changed, 63 insertions(+), 36 deletions(-) diff --git a/analyzers/src/SonarAnalyzer.CSharp/Rules/ClassShouldNotBeEmpty.cs b/analyzers/src/SonarAnalyzer.CSharp/Rules/ClassShouldNotBeEmpty.cs index 556e80feaa1..b0a75c42ffc 100644 --- a/analyzers/src/SonarAnalyzer.CSharp/Rules/ClassShouldNotBeEmpty.cs +++ b/analyzers/src/SonarAnalyzer.CSharp/Rules/ClassShouldNotBeEmpty.cs @@ -54,7 +54,17 @@ protected override bool HasConditionalCompilationDirectives(SyntaxNode node) => SyntaxKind.ElseDirectiveTrivia, SyntaxKind.EndIfDirectiveTrivia)); - private bool IsParameterlessRecord(SyntaxNode node) => + private static bool IsParameterlessRecord(SyntaxNode node) => + RecordDeclarationSyntax(node) is { ParameterList.Parameters.Count: 0 } declaration + && !ProvidesBaseWithParameters(declaration); + + private static bool ProvidesBaseWithParameters(RecordDeclarationSyntaxWrapper node) => + node.BaseList?.Types.FirstOrDefault() is { } type + && PrimaryConstructorBaseTypeSyntaxWrapper.IsInstance(type) + && (PrimaryConstructorBaseTypeSyntaxWrapper)type is { ArgumentList.Arguments.Count: >= 1 }; + + private static RecordDeclarationSyntaxWrapper? RecordDeclarationSyntax(SyntaxNode node) => RecordDeclarationSyntaxWrapper.IsInstance(node) - && (RecordDeclarationSyntaxWrapper)node is { ParameterList.Parameters.Count: 0 }; + ? (RecordDeclarationSyntaxWrapper)node + : null; } diff --git a/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/ClassShouldNotBeEmpty.CSharp9.cs b/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/ClassShouldNotBeEmpty.CSharp9.cs index 7383f176f20..6fc80b42159 100644 --- a/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/ClassShouldNotBeEmpty.CSharp9.cs +++ b/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/ClassShouldNotBeEmpty.CSharp9.cs @@ -1,44 +1,61 @@ 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 RecordMember); + + record NotEmptyGenericRecordWithContraint(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 EmptyChildRecord() : EmptyRecord(); // Noncompliant + + record EmptyGenericRecord(); // Noncompliant + // ^^^^^^^^^^^^^^^^^^ + record EmptyGenericRecordWithContraint() 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(); // Noncompliant -// ^^^^^^^^^^^^^^^^^^ -record EmptyGenericRecordWithContraint() where T : class; // Noncompliant -record NotEmptyGenericRecord(T RecordMember); -record NotEmptyGenericRecordWithContraint(T RecordMember) where T : class; From f42f0537f821c2303870d0faa07538beafe36659 Mon Sep 17 00:00:00 2001 From: Corniel Nobel Date: Thu, 30 Mar 2023 12:57:40 +0200 Subject: [PATCH 2/6] Simplify. --- .../Rules/ClassShouldNotBeEmpty.cs | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/analyzers/src/SonarAnalyzer.CSharp/Rules/ClassShouldNotBeEmpty.cs b/analyzers/src/SonarAnalyzer.CSharp/Rules/ClassShouldNotBeEmpty.cs index b0a75c42ffc..502d01b27f2 100644 --- a/analyzers/src/SonarAnalyzer.CSharp/Rules/ClassShouldNotBeEmpty.cs +++ b/analyzers/src/SonarAnalyzer.CSharp/Rules/ClassShouldNotBeEmpty.cs @@ -56,15 +56,16 @@ protected override bool HasConditionalCompilationDirectives(SyntaxNode node) => private static bool IsParameterlessRecord(SyntaxNode node) => RecordDeclarationSyntax(node) is { ParameterList.Parameters.Count: 0 } declaration - && !ProvidesBaseWithParameters(declaration); - - private static bool ProvidesBaseWithParameters(RecordDeclarationSyntaxWrapper node) => - node.BaseList?.Types.FirstOrDefault() is { } type - && PrimaryConstructorBaseTypeSyntaxWrapper.IsInstance(type) - && (PrimaryConstructorBaseTypeSyntaxWrapper)type is { ArgumentList.Arguments.Count: >= 1 }; + && BaseTypeSyntax(declaration) is not { ArgumentList.Arguments.Count: >= 1 }; 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; } From 4501141df78ddcc36aa4d03153786c36faaa6f8f Mon Sep 17 00:00:00 2001 From: Corniel Nobel Date: Thu, 30 Mar 2023 13:19:35 +0200 Subject: [PATCH 3/6] Regroup tests. --- .../ClassShouldNotBeEmpty.CSharp10.cs | 28 ++- .../ClassShouldNotBeEmpty.Inheritance.cs | 30 ++- .../ClassShouldNotBeEmpty.Inheritance.vb | 68 +++--- .../TestCases/ClassShouldNotBeEmpty.cs | 191 ++++++++------- .../TestCases/ClassShouldNotBeEmpty.vb | 229 +++++++++--------- 5 files changed, 297 insertions(+), 249 deletions(-) diff --git a/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/ClassShouldNotBeEmpty.CSharp10.cs b/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/ClassShouldNotBeEmpty.CSharp10.cs index 02fd205d0ae..f97d3622525 100644 --- a/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/ClassShouldNotBeEmpty.CSharp10.cs +++ b/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/ClassShouldNotBeEmpty.CSharp10.cs @@ -1,13 +1,19 @@ - -record class EmptyRecordClass1(); // Noncompliant {{Remove this empty record, write its code or make it an "interface".}} -// ^^^^^^^^^^^^^^^^^ -record class EmptyRecordClass2() { }; // Noncompliant - -record struct EmptyRecordStruct1(); // Compliant - this rule only deals with classes -record struct EmptyRecordStruct2() { }; +namespace Compliant +{ + record class NotEmptyRecordClass1(int RecordMember); + record class NotEmptyRecordClass2() + { + int RecordMember => 42; + }; +} +namespace NonCompliant +{ -record class NotEmptyRecordClass1(int RecordMember); -record class NotEmptyRecordClass2() + record class EmptyRecordClass(); // Noncompliant {{Remove this empty record, write its code or make it an "interface".}} + // ^^^^^^^^^^^^^^^^ + record class EmptyRecordClassWithEmptyBody() { }; // Noncompliant +} +namespace Ignore { - int RecordMember => 42; -}; + record struct EmptyRecordStruct(); // Compliant - this rule only deals with classes +} diff --git a/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/ClassShouldNotBeEmpty.Inheritance.cs b/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/ClassShouldNotBeEmpty.Inheritance.cs index 3896e880e1a..338fabd4def 100644 --- a/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/ClassShouldNotBeEmpty.Inheritance.cs +++ b/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/ClassShouldNotBeEmpty.Inheritance.cs @@ -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 - diff --git a/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/ClassShouldNotBeEmpty.Inheritance.vb b/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/ClassShouldNotBeEmpty.Inheritance.vb index c28782f9c07..35bc4db97f2 100644 --- a/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/ClassShouldNotBeEmpty.Inheritance.vb +++ b/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/ClassShouldNotBeEmpty.Inheritance.vb @@ -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 @@ -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 @@ -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 diff --git a/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/ClassShouldNotBeEmpty.cs b/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/ClassShouldNotBeEmpty.cs index dfadc29fada..4e5f009b1b0 100644 --- a/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/ClassShouldNotBeEmpty.cs +++ b/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/ClassShouldNotBeEmpty.cs @@ -2,119 +2,128 @@ using System.Collections.Generic; using System.Runtime.InteropServices; -class Empty { } // Noncompliant {{Remove this empty class, write its code or make it an "interface".}} -// ^^^^^ - -public class PublicEmpty { } // Noncompliant -internal class InternalEmpty { } // Noncompliant - -class EmptyWithComments // Noncompliant +namespace Compliant { - // Some comment -} + class ClassWithProperty + { + int SomeProperty => 42; + } + class ClassWithField + { + int SomeField = 42; + } + class ClassWithMethod + { + void Method() { } + } + class ClassWithIndexer + { + int this[int index] => 42; + } + class ClassWithDelegate + { + delegate void MethodDelegate(); + } + class ClassWithEvent + { + event EventHandler CustomEvent; + } -class ClassWithProperty -{ - int SomeProperty => 42; -} -class ClassWithField -{ - int SomeField = 42; -} -class ClassWithMethod -{ - void Method() { } -} -class ClassWithIndexer -{ - int this[int index] => 42; -} -class ClassWithDelegate -{ - delegate void MethodDelegate(); -} -class ClassWithEvent -{ - event EventHandler CustomEvent; -} + class GenericNotEmpty + { + void Method(T arg) { } + } -class OuterClass -{ - class InnerEmpty1 { } // Noncompliant - private class InnerEmpty2 { } // Noncompliant - protected class InnerEmpty3 { } // Noncompliant - internal class InnerEmpty4 { } // Noncompliant - protected internal class InnerEmpty5 { } // Noncompliant - public class InnerEmpty6 { } // Noncompliant - - public class InnerEmptyWithComments // Noncompliant + class GenericNotEmptyWithConstraints + where T : class { - // Some comment + void Method(T arg) { } } - class InnerNonEmpty + partial class PartialNotEmpty { - public int SomeProperty => 42; + int Prop => 42; } -} -class GenericEmpty { } // Noncompliant -// ^^^^^^^^^^^^ -class GenericEmptyWithConstraints // Noncompliant - where T : class -{ -} + partial class PartialEmpty { } // Compliant - Source Generators and some frameworks use empty partial classes as placeholders -class GenericNotEmpty -{ - void Method(T arg) { } -} -class GenericNotEmptyWithConstraints - where T : class -{ - void Method(T arg) { } -} + [ComVisible(true)] + class ClassWithAttribute { } // Compliant - types with attributes are ignored -class IntegerList: List { } // Compliant - creates a more specific type without adding new members to it (similar to using the typedef keyword in C/C++) -class StringLookup: Dictionary { } -interface IIntegerSet: ISet { } + [ComVisible(true), Obsolete] + class ClassWithMultipleAttributes { } -[ComVisible(true)] -class ClassWithAttribute { } // Compliant - types with attributes are ignored + class OuterClass + { + class InnerEmpty1 { } // Noncompliant + private class InnerEmpty2 { } // Noncompliant + protected class InnerEmpty3 { } // Noncompliant + internal class InnerEmpty4 { } // Noncompliant + protected internal class InnerEmpty5 { } // Noncompliant + public class InnerEmpty6 { } // Noncompliant + + public class InnerEmptyWithComments // Noncompliant + { + // Some comment + } + + class InnerNonEmpty + { + public int SomeProperty => 42; + } + } -[ComVisible(true), Obsolete] -class ClassWithMultipleAttributes { } + class IntegerList : List { } // Compliant - creates a more specific type without adding new members to it (similar to using the typedef keyword in C/C++) + class StringLookup : Dictionary { } + interface IIntegerSet : ISet { } -[] // Error -class AttributeError { } + class Conditional // Compliant - it's not empty when the given symbol is defined + { +#if NOTDEFINED + public override string ToString() + { + return "Debug Text"; + } +#endif + } +} -static class StaticEmpty { } // Noncompliant +namespace NonCompliant +{ + class Empty { } // Noncompliant {{Remove this empty class, write its code or make it an "interface".}} + // ^^^^^ -abstract class AbstractEmpty { } // Noncompliant + public class PublicEmpty { } // Noncompliant -partial class PartialEmpty { } // Compliant - Source Generators and some frameworks use empty partial classes as placeholders + internal class InternalEmpty { } // Noncompliant -partial class PartialNotEmpty -{ - int Prop => 42; -} + class EmptyWithComments // Noncompliant + { + // Some comment + } -interface IMarker { } // Compliant - this rule only deals with classes -class ImplementsMarker: IMarker { } // Compliant - implements a marker interface + static class StaticEmpty { } // Noncompliant -struct EmptyStruct { } // Compliant - this rule only deals with classes + abstract class AbstractEmpty { } // Noncompliant -enum EmptyEnum { } // Compliant - this rule only deals with classes + class GenericEmpty { } // Noncompliant + // ^^^^^^^^^^^^ + class GenericEmptyWithConstraints // Noncompliant + where T : class { } +} -class Conditional // Compliant - it's not empty when the given symbol is defined +namespace Ignore { -#if NOTDEFINED - public override string ToString() - { - return "Debug Text"; - } -#endif -} + class { } // Error + + [] // Error + class AttributeError { } -class { } // Error + interface IMarker { } // Compliant - this rule only deals with classes + + class ImplementsMarker: IMarker { } // Compliant - implements a marker interface + struct EmptyStruct { } // Compliant - this rule only deals with classes + + enum EmptyEnum { } // Compliant - this rule only deals with classes +} diff --git a/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/ClassShouldNotBeEmpty.vb b/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/ClassShouldNotBeEmpty.vb index d8b1e8c013e..2605683a682 100644 --- a/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/ClassShouldNotBeEmpty.vb +++ b/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/ClassShouldNotBeEmpty.vb @@ -2,158 +2,169 @@ Imports System.Collections.Generic Imports System.Runtime.InteropServices -Class Empty ' Noncompliant {{Remove this empty class, write its code or make it an "interface".}} - ' ^^^^^ -End Class - - -Public Class PublicEmpty ' Noncompliant -End Class +Namespace Compliant + Public Class PublicEmpty ' Noncompliant + End Class -Friend Class InternalEmpty ' Noncompliant -End Class + Friend Class InternalEmpty ' Noncompliant + End Class -Class EmptyWithComments ' Noncompliant - ' Some comment -End Class + Class EmptyWithComments ' Noncompliant + ' Some comment + End Class -Class ClassWithProperty - Private ReadOnly Property SomeProperty As Integer - Get - Return 42 - End Get - End Property -End Class + Class ClassWithProperty + Private ReadOnly Property SomeProperty As Integer + Get + Return 42 + End Get + End Property + End Class -Class ClassWithField - Private SomeField As Integer = 42 -End Class + Class ClassWithField + Private SomeField As Integer = 42 + End Class -Class ClassWithMethod - Private Sub Method() - End Sub -End Class + Class ClassWithMethod + Private Sub Method() + End Sub + End Class -Class ClassWithIndexer - Private ReadOnly Property Item(index As Integer) As Integer - Get - Return 42 - End Get - End Property -End Class + Class ClassWithIndexer + Private ReadOnly Property Item(index As Integer) As Integer + Get + Return 42 + End Get + End Property + End Class -Class ClassWithDelegate - Delegate Sub MethodDelegate() -End Class + Class ClassWithDelegate + Delegate Sub MethodDelegate() + End Class -Class ClassWithEvent - Private Event CustomEvent As EventHandler -End Class + Class ClassWithEvent + Private Event CustomEvent As EventHandler + End Class -Class OuterClass - Class InnerEmpty1 ' Noncompliant + Class GenericNotEmpty(Of T) + Private Sub Method(arg As T) + End Sub End Class - Private Class InnerEmpty2 ' Noncompliant + Class GenericNotEmptyWithConstraints(Of T As Class) + Private Sub Method(arg As T) + End Sub End Class - Protected Class InnerEmpty3 ' Noncompliant + Class IntegerList ' Compliant - creates a more specific type without adding new members to it (similar to using the typedef keyword in C/C++) + Inherits List(Of Integer) End Class - Friend Class InnerEmpty4 ' Noncompliant + Class StringLookup(Of T) + Inherits Dictionary(Of String, T) End Class - Protected Friend Class InnerEmpty5 ' Noncompliant + Interface IIntegerSet(Of T) + Inherits ISet(Of Integer) + End Interface + + + Class ClassWithAttribute ' Compliant - types with attributes are ignored End Class - Public Class InnerEmpty6 ' Noncompliant + + Class ClassWithMultipleAttributes End Class - Public Class InnerEmptyWithComments ' Noncompliant - ' Some comment + Partial Class PartialEmpty ' Compliant - Source Generators and some frameworks use empty partial classes as placeholders End Class - Class InnerNonEmpty - Public ReadOnly Property SomeProperty As Integer + Partial Class PartialNotEmpty + Public ReadOnly Property Prop As Integer Get Return 42 End Get End Property End Class -End Class -Class GenericEmpty(Of T) ' Noncompliant - ' ^^^^^^^^^^^^ -End Class + Class Conditional ' Compliant - it's not empty when the given symbol is defined +#If NOTDEFINED Then + Public Overrides Function ToString() As String + Return "Debug Text" + End Function +#End If + End Class + +End Namespace -Class GenericEmptyWithConstraints(Of T As Class) ' Noncompliant -End Class +Namespace NonCompliant -Class GenericNotEmpty(Of T) - Private Sub Method(arg As T) - End Sub -End Class + Class Empty ' Noncompliant {{Remove this empty class, write its code or make it an "interface".}} + ' ^^^^^ + End Class -Class GenericNotEmptyWithConstraints(Of T As Class) - Private Sub Method(arg As T) - End Sub -End Class + Class OuterClass -Class IntegerList ' Compliant - creates a more specific type without adding new members to it (similar to using the typedef keyword in C/C++) - Inherits List(Of Integer) -End Class + Class InnerEmpty1 ' Noncompliant + End Class -Class StringLookup(Of T) - Inherits Dictionary(Of String, T) -End Class + Private Class InnerEmpty2 ' Noncompliant + End Class -Interface IIntegerSet(Of T) - Inherits ISet(Of Integer) -End Interface + Protected Class InnerEmpty3 ' Noncompliant + End Class - -Class ClassWithAttribute ' Compliant - types with attributes are ignored -End Class + Friend Class InnerEmpty4 ' Noncompliant + End Class - -Class ClassWithMultipleAttributes -End Class + Protected Friend Class InnerEmpty5 ' Noncompliant + End Class -<> ' Error -Class AttributeError ' Noncompliant -End Class + Public Class InnerEmpty6 ' Noncompliant + End Class -MustInherit Class AbstractEmpty ' Noncompliant -End Class + Public Class InnerEmptyWithComments ' Noncompliant + ' Some comment + End Class -Partial Class PartialEmpty ' Compliant - Source Generators and some frameworks use empty partial classes as placeholders -End Class + Class InnerNonEmpty + Public ReadOnly Property SomeProperty As Integer + Get + Return 42 + End Get + End Property + End Class + End Class -Partial Class PartialNotEmpty - Public ReadOnly Property Prop As Integer - Get - Return 42 - End Get - End Property -End Class + Class GenericEmpty(Of T) ' Noncompliant + ' ^^^^^^^^^^^^ + End Class -Interface IMarker ' Compliant - this rule only deals with classes -End Interface + Class GenericEmptyWithConstraints(Of T As Class) ' Noncompliant + End Class -Class ImplementsMarker ' Compliant - implements a marker interface - Implements IMarker -End Class + MustInherit Class AbstractEmpty ' Noncompliant + End Class -Structure EmptyStruct ' Compliant - this rule only deals with classes -End Structure +End Namespace -Class Conditional ' Compliant - it's not empty when the given symbol is defined -#If NOTDEFINED Then - Public Overrides Function ToString() As String - Return "Debug Text" - End Function -#End If -End Class +Namespace Ignore + + Class ' Error + End Class + + <> ' Error + Class AttributeError ' Noncompliant + End Class + + Interface IMarker ' Compliant - this rule only deals with classes + End Interface + + Class ImplementsMarker ' Compliant - implements a marker interface + Implements IMarker + End Class + + Structure EmptyStruct ' Compliant - this rule only deals with classes + End Structure -Class ' Error -End Class +End Namespace From 1edd57649eb0ca1e450b3dc507f534b291062c21 Mon Sep 17 00:00:00 2001 From: Corniel Nobel Date: Thu, 6 Apr 2023 17:59:52 +0200 Subject: [PATCH 4/6] Nitpicks. --- .../SonarAnalyzer.CSharp/Rules/ClassShouldNotBeEmpty.cs | 8 ++++---- .../TestCases/ClassShouldNotBeEmpty.CSharp10.cs | 9 +++++---- 2 files changed, 9 insertions(+), 8 deletions(-) diff --git a/analyzers/src/SonarAnalyzer.CSharp/Rules/ClassShouldNotBeEmpty.cs b/analyzers/src/SonarAnalyzer.CSharp/Rules/ClassShouldNotBeEmpty.cs index 502d01b27f2..ebde92b5a56 100644 --- a/analyzers/src/SonarAnalyzer.CSharp/Rules/ClassShouldNotBeEmpty.cs +++ b/analyzers/src/SonarAnalyzer.CSharp/Rules/ClassShouldNotBeEmpty.cs @@ -60,12 +60,12 @@ private static bool IsParameterlessRecord(SyntaxNode node) => private static RecordDeclarationSyntaxWrapper? RecordDeclarationSyntax(SyntaxNode node) => RecordDeclarationSyntaxWrapper.IsInstance(node) - ? (RecordDeclarationSyntaxWrapper)node - : null; + ? (RecordDeclarationSyntaxWrapper)node + : null; private static PrimaryConstructorBaseTypeSyntaxWrapper? BaseTypeSyntax(RecordDeclarationSyntaxWrapper node) => node.BaseList?.Types.FirstOrDefault() is { } type && PrimaryConstructorBaseTypeSyntaxWrapper.IsInstance(type) - ? (PrimaryConstructorBaseTypeSyntaxWrapper)type - : null; + ? (PrimaryConstructorBaseTypeSyntaxWrapper)type + : null; } diff --git a/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/ClassShouldNotBeEmpty.CSharp10.cs b/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/ClassShouldNotBeEmpty.CSharp10.cs index f97d3622525..da29fa53f32 100644 --- a/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/ClassShouldNotBeEmpty.CSharp10.cs +++ b/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/ClassShouldNotBeEmpty.CSharp10.cs @@ -9,11 +9,12 @@ record class NotEmptyRecordClass2() namespace NonCompliant { - record class EmptyRecordClass(); // Noncompliant {{Remove this empty record, write its code or make it an "interface".}} + record class EmptyRecordClass(); // Noncompliant {{Remove this empty record, write its code or make it an "interface".}} // ^^^^^^^^^^^^^^^^ - record class EmptyRecordClassWithEmptyBody() { }; // Noncompliant + record class EmptyRecordClassWithEmptyBody() { }; // Noncompliant } -namespace Ignore +namespace Ignored { - record struct EmptyRecordStruct(); // Compliant - this rule only deals with classes + record struct EmptyRecordStruct(); // Compliant - this rule only deals with classes + record struct EmptyRecordStructWithEmptyBody() { }; // Compliant - this rule only deals with classes } From 36ae73ea4baeb916cdeffe134bcac3605d836634 Mon Sep 17 00:00:00 2001 From: Corniel Nobel Date: Tue, 11 Apr 2023 18:02:39 +0200 Subject: [PATCH 5/6] Support record (class) without ParameterList. --- .../SonarAnalyzer.CSharp/Rules/ClassShouldNotBeEmpty.cs | 3 ++- .../TestCases/ClassShouldNotBeEmpty.CSharp10.cs | 6 ++++-- .../TestCases/ClassShouldNotBeEmpty.CSharp9.cs | 8 +++++--- 3 files changed, 11 insertions(+), 6 deletions(-) diff --git a/analyzers/src/SonarAnalyzer.CSharp/Rules/ClassShouldNotBeEmpty.cs b/analyzers/src/SonarAnalyzer.CSharp/Rules/ClassShouldNotBeEmpty.cs index ebde92b5a56..90354db762e 100644 --- a/analyzers/src/SonarAnalyzer.CSharp/Rules/ClassShouldNotBeEmpty.cs +++ b/analyzers/src/SonarAnalyzer.CSharp/Rules/ClassShouldNotBeEmpty.cs @@ -55,7 +55,8 @@ protected override bool HasConditionalCompilationDirectives(SyntaxNode node) => SyntaxKind.EndIfDirectiveTrivia)); private static bool IsParameterlessRecord(SyntaxNode node) => - RecordDeclarationSyntax(node) is { ParameterList.Parameters.Count: 0 } declaration + 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) => diff --git a/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/ClassShouldNotBeEmpty.CSharp10.cs b/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/ClassShouldNotBeEmpty.CSharp10.cs index da29fa53f32..6bc52c276b1 100644 --- a/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/ClassShouldNotBeEmpty.CSharp10.cs +++ b/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/ClassShouldNotBeEmpty.CSharp10.cs @@ -9,9 +9,11 @@ record class NotEmptyRecordClass2() namespace NonCompliant { - record class EmptyRecordClass(); // Noncompliant {{Remove this empty record, write its code or make it an "interface".}} + record class EmptyRecordClass(); // Noncompliant {{Remove this empty record, write its code or make it an "interface".}} // ^^^^^^^^^^^^^^^^ - record class EmptyRecordClassWithEmptyBody() { }; // Noncompliant + record class EmptyRecordClassWithEmptyBody() { }; // Noncompliant + + record class EmptyChildWithoutBrackets : EmptyRecordClass; // Noncompliant } namespace Ignored { diff --git a/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/ClassShouldNotBeEmpty.CSharp9.cs b/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/ClassShouldNotBeEmpty.CSharp9.cs index 6fc80b42159..6fa76cadc8c 100644 --- a/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/ClassShouldNotBeEmpty.CSharp9.cs +++ b/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/ClassShouldNotBeEmpty.CSharp9.cs @@ -43,11 +43,13 @@ record EmptyRecordProvidingParameterToBase() : RecordWithParameters(42); namespace Noncompliant { - record EmptyRecord(); // Noncompliant {{Remove this empty record, write its code or make it an "interface".}} + record EmptyRecord(); // Noncompliant {{Remove this empty record, write its code or make it an "interface".}} // ^^^^^^^^^^^ - record EmptyRecordWithEmptyBody() { }; // Noncompliant + record EmptyRecordWithEmptyBody() { }; // Noncompliant - record EmptyChildRecord() : EmptyRecord(); // Noncompliant + record EmptyChildWithoutBrackets : EmptyRecord; // Noncompliant + + record EmptyChildRecord() : EmptyRecord(); // Noncompliant record EmptyGenericRecord(); // Noncompliant // ^^^^^^^^^^^^^^^^^^ From 11a533d952f9806c5f0c6e5e814f5c9c70d04157 Mon Sep 17 00:00:00 2001 From: Zsolt Kolbay Date: Wed, 19 Apr 2023 16:28:41 +0200 Subject: [PATCH 6/6] Update ITs --- .../its/expected/Net5/Net5--net5.0-S2094.json | 52 +++++++++++++++++++ 1 file changed, 52 insertions(+) diff --git a/analyzers/its/expected/Net5/Net5--net5.0-S2094.json b/analyzers/its/expected/Net5/Net5--net5.0-S2094.json index 6410000afd1..836c5818006 100644 --- a/analyzers/its/expected/Net5/Net5--net5.0-S2094.json +++ b/analyzers/its/expected/Net5/Net5--net5.0-S2094.json @@ -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",