Skip to content

Commit

Permalink
New rule S2166: Classes named like "Exception" should extend "Excepti…
Browse files Browse the repository at this point in the history
…on" or a subclass (#6727)

* Initial scaffolding

* Add test cases

* Implement C# rule

* Update RSPEC

* Add VB test cases

* Fix indentation

* Add test case for generic classes

* Different handling of static classes / modules

* Add record class test case

* Add intentional finding test cases

* Fix unit tests

* Remove empty line

* Introduce ClassAndModuleDeclarations to ISyntaxKindFacade

* Remove empty line

* Update RSPEC and rule message

* Update test cases

* Add explanation to test cases

* Add compilation error test cases

* Add test case for System.Runtime.InteropServices._Exception
  • Loading branch information
zsolt-kolbay-sonarsource committed Feb 9, 2023
1 parent 543953b commit b9f87fd
Show file tree
Hide file tree
Showing 25 changed files with 548 additions and 5 deletions.
@@ -0,0 +1,15 @@
/*
* <Your-Product-Name>
* Copyright (c) <Year-From>-<Year-To> <Your-Company-Name>
*
* Please configure this header in your SonarCloud/SonarQube quality profile.
* You can also set it in SonarLint.xml additional file for SonarLint or standalone NuGet analyzer.
*/

namespace IntentionalFindings
{
public static class S2166
{
public class CustomException { } // Noncompliant (S2166)
}
}
@@ -0,0 +1,10 @@
' <Your-Product-Name>
' Copyright (c) <Year-From>-<Year-To> <Your-Company-Name>
'
' Please configure this header in your SonarCloud/SonarQube quality profile.
' You can also set it in SonarLint.xml additional file for SonarLint or standalone NuGet analyzer.

Public Class S2166
Public Class CustomException ' Noncompliant (S2166)
End Class
End Class
40 changes: 40 additions & 0 deletions analyzers/rspec/cs/S2166_c#.html
@@ -0,0 +1,40 @@
<p>Clear, communicative naming is important in code. It helps maintainers and API users understand the intentions for and uses of a unit of code.
Using "exception" in the name of a class that does not extend <code>Exception</code> or one of its subclasses is a clear violation of the expectation
that a class' name will indicate what it is and/or does.</p>
<h2>Noncompliant Code Example</h2>
<pre>
public class FruitException // Noncompliant - this has nothing to do with Exception
{
private Fruit expected;
private string unusualCharacteristics;
private bool appropriateForCommercialExploitation;
// ...
}

public class CarException // Noncompliant - does not derive from any Exception-based class
{
public CarException(string message, Exception inner)
{
// ...
}
}
</pre>
<h2>Compliant Solution</h2>
<pre>
public class FruitSport // Compliant - class name does not end with 'Exception'
{
private Fruit expected;
private string unusualCharacteristics;
private bool appropriateForCommercialExploitation;
// ...
}

public class CarException: Exception // Compliant - correctly extends System.Exception
{
public CarException(string message, Exception inner): base(message, inner)
{
// ...
}
}
</pre>

19 changes: 19 additions & 0 deletions analyzers/rspec/cs/S2166_c#.json
@@ -0,0 +1,19 @@
{
"title": "Classes named like \"Exception\" should extend \"Exception\" or a subclass",
"type": "CODE_SMELL",
"status": "ready",
"remediation": {
"func": "Constant\/Issue",
"constantCost": "5min"
},
"tags": [
"convention",
"error-handling",
"pitfall"
],
"defaultSeverity": "Major",
"ruleSpecification": "RSPEC-2166",
"sqKey": "S2166",
"scope": "Main",
"quickfix": "unknown"
}
1 change: 1 addition & 0 deletions analyzers/rspec/cs/Sonar_way_profile.json
Expand Up @@ -62,6 +62,7 @@
"S2114",
"S2115",
"S2123",
"S2166",
"S2178",
"S2183",
"S2184",
Expand Down
35 changes: 35 additions & 0 deletions analyzers/rspec/vbnet/S2166_vb.net.html
@@ -0,0 +1,35 @@
<p>Clear, communicative naming is important in code. It helps maintainers and API users understand the intentions for and uses of a unit of code.
Using "exception" in the name of a class that does not extend <code>Exception</code> or one of its subclasses is a clear violation of the expectation
that a class' name will indicate what it is and/or does.</p>
<h2>Noncompliant Code Example</h2>
<pre>
Public Class FruitException ' Noncompliant - this has nothing to do with Exception
Private expected As Fruit
Private unusualCharacteristics As String
Private appropriateForCommercialExploitation As Boolean
' ...
End Class

Public Class CarException ' Noncompliant - does not derive from any Exception-based class
Public Sub New(message As String, inner As Exception)
' ...
End Sub
End Class
</pre>
<h2>Compliant Solution</h2>
<pre>
Public Class FruitSport ' Compliant - class name does not end with 'Exception'
Private expected As Fruit
Private unusualCharacteristics As String
Private appropriateForCommercialExploitation As Boolean
' ...
End Class

Public Class CarException Inherits Exception ' Compliant - correctly extends System.Exception
Public Sub New(message As String, inner As Exception)
MyBase.New(message, inner)
' ...
End Sub
End Class
</pre>

19 changes: 19 additions & 0 deletions analyzers/rspec/vbnet/S2166_vb.net.json
@@ -0,0 +1,19 @@
{
"title": "Classes named like \"Exception\" should extend \"Exception\" or a subclass",
"type": "CODE_SMELL",
"status": "ready",
"remediation": {
"func": "Constant\/Issue",
"constantCost": "5min"
},
"tags": [
"convention",
"error-handling",
"pitfall"
],
"defaultSeverity": "Major",
"ruleSpecification": "RSPEC-2166",
"sqKey": "S2166",
"scope": "Main",
"quickfix": "unknown"
}
1 change: 1 addition & 0 deletions analyzers/rspec/vbnet/Sonar_way_profile.json
Expand Up @@ -38,6 +38,7 @@
"S1940",
"S2068",
"S2077",
"S2166",
"S2178",
"S2222",
"S2225",
Expand Down
Expand Up @@ -23,12 +23,13 @@ namespace SonarAnalyzer.Helpers.Facade;
internal sealed class CSharpSyntaxKindFacade : ISyntaxKindFacade<SyntaxKind>
{
public SyntaxKind Attribute => SyntaxKind.Attribute;
public SyntaxKind ClassDeclaration => SyntaxKind.ClassDeclaration;
public SyntaxKind[] ClassAndRecordDeclaration => new[]
{
SyntaxKind.ClassDeclaration,
SyntaxKindEx.RecordClassDeclaration,
};
public SyntaxKind ClassDeclaration => SyntaxKind.ClassDeclaration;
public SyntaxKind[] ClassAndModuleDeclarations => new[] { SyntaxKind.ClassDeclaration };
public SyntaxKind[] CommentTrivia => new[]
{
SyntaxKind.SingleLineCommentTrivia,
Expand Down
27 changes: 27 additions & 0 deletions analyzers/src/SonarAnalyzer.CSharp/Rules/ClassNamedException.cs
@@ -0,0 +1,27 @@
/*
* SonarAnalyzer for .NET
* Copyright (C) 2015-2023 SonarSource SA
* mailto: contact AT sonarsource DOT com
*
* This program is free software; you can redistribute it and/or
* modify it under the terms of the GNU Lesser General Public
* License as published by the Free Software Foundation; either
* version 3 of the License, or (at your option) any later version.
*
* This program is distributed in the hope that it will be useful,
* but WITHOUT ANY WARRANTY; without even the implied warranty of
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
* Lesser General Public License for more details.
*
* You should have received a copy of the GNU Lesser General Public License
* along with this program; if not, write to the Free Software Foundation,
* Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA.
*/

namespace SonarAnalyzer.Rules.CSharp;

[DiagnosticAnalyzer(LanguageNames.CSharp)]
public sealed class ClassNamedException : ClassNamedExceptionBase<SyntaxKind>
{
protected override ILanguageFacade<SyntaxKind> Language => CSharpFacade.Instance;
}
Expand Up @@ -24,8 +24,9 @@ public interface ISyntaxKindFacade<out TSyntaxKind>
where TSyntaxKind : struct
{
abstract TSyntaxKind Attribute { get; }
abstract TSyntaxKind[] ClassAndRecordDeclaration { get; }
abstract TSyntaxKind ClassDeclaration { get; }
abstract TSyntaxKind[] ClassAndRecordDeclaration { get; }
abstract TSyntaxKind[] ClassAndModuleDeclarations { get; }
abstract TSyntaxKind[] CommentTrivia { get; }
abstract TSyntaxKind[] ComparisonKinds { get; }
abstract TSyntaxKind ConstructorDeclaration { get; }
Expand Down
1 change: 1 addition & 0 deletions analyzers/src/SonarAnalyzer.Common/Helpers/KnownType.cs
Expand Up @@ -367,6 +367,7 @@ internal sealed partial class KnownType
internal static readonly KnownType System_Runtime_InteropServices_ComVisibleAttribute = new("System.Runtime.InteropServices.ComVisibleAttribute");
internal static readonly KnownType System_Runtime_InteropServices_DefaultParameterValueAttribute = new("System.Runtime.InteropServices.DefaultParameterValueAttribute");
internal static readonly KnownType System_Runtime_InteropServices_DllImportAttribute = new("System.Runtime.InteropServices.DllImportAttribute");
internal static readonly KnownType System_Runtime_InteropServices_Exception = new("System.Runtime.InteropServices._Exception");
internal static readonly KnownType System_Runtime_InteropServices_HandleRef = new("System.Runtime.InteropServices.HandleRef");
internal static readonly KnownType System_Runtime_InteropServices_InterfaceTypeAttribute = new("System.Runtime.InteropServices.InterfaceTypeAttribute");
internal static readonly KnownType System_Runtime_InteropServices_LibraryImportAttribute = new("System.Runtime.InteropServices.LibraryImportAttribute");
Expand Down
@@ -0,0 +1,47 @@
/*
* SonarAnalyzer for .NET
* Copyright (C) 2015-2023 SonarSource SA
* mailto: contact AT sonarsource DOT com
*
* This program is free software; you can redistribute it and/or
* modify it under the terms of the GNU Lesser General Public
* License as published by the Free Software Foundation; either
* version 3 of the License, or (at your option) any later version.
*
* This program is distributed in the hope that it will be useful,
* but WITHOUT ANY WARRANTY; without even the implied warranty of
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
* Lesser General Public License for more details.
*
* You should have received a copy of the GNU Lesser General Public License
* along with this program; if not, write to the Free Software Foundation,
* Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA.
*/

namespace SonarAnalyzer.Rules;

public abstract class ClassNamedExceptionBase<TSyntaxKind> : SonarDiagnosticAnalyzer<TSyntaxKind>
where TSyntaxKind : struct
{
private const string DiagnosticId = "S2166";

protected override string MessageFormat => "Rename this class to remove \"Exception\" or correct its inheritance.";

protected ClassNamedExceptionBase() : base(DiagnosticId) { }

protected override void Initialize(SonarAnalysisContext context) =>
context.RegisterNodeAction(
Language.GeneratedCodeRecognizer,
c =>
{
if (Language.Syntax.NodeIdentifier(c.Node) is { IsMissing: false } classIdentifier
&& classIdentifier.ValueText.EndsWith("Exception", StringComparison.InvariantCultureIgnoreCase)
&& c.SemanticModel.GetDeclaredSymbol(c.Node) is INamedTypeSymbol { } classSymbol
&& !classSymbol.DerivesFrom(KnownType.System_Exception)
&& !classSymbol.Implements(KnownType.System_Runtime_InteropServices_Exception))
{
c.ReportIssue(Diagnostic.Create(Rule, classIdentifier.GetLocation()));
}
},
Language.SyntaxKind.ClassAndModuleDeclarations);
}
Expand Up @@ -23,8 +23,13 @@ namespace SonarAnalyzer.Helpers.Facade;
internal sealed class VisualBasicSyntaxKindFacade : ISyntaxKindFacade<SyntaxKind>
{
public SyntaxKind Attribute => SyntaxKind.Attribute;
public SyntaxKind[] ClassAndRecordDeclaration => new[] { SyntaxKind.ClassBlock };
public SyntaxKind ClassDeclaration => SyntaxKind.ClassBlock;
public SyntaxKind[] ClassAndRecordDeclaration => new[] { SyntaxKind.ClassBlock };
public SyntaxKind[] ClassAndModuleDeclarations => new[]
{
SyntaxKind.ClassBlock,
SyntaxKind.ModuleBlock
};
public SyntaxKind[] CommentTrivia => new[]
{
SyntaxKind.CommentTrivia,
Expand Down
Expand Up @@ -168,6 +168,7 @@ private static bool IsOn(this ExpressionSyntax expression, SyntaxKind onKind)
MemberAccessExpressionSyntax x => x.Name.Identifier,
MethodBlockSyntax x => x.SubOrFunctionStatement?.GetIdentifier(),
MethodStatementSyntax x => x.Identifier,
ModuleBlockSyntax x => x.ModuleStatement.Identifier,
EnumStatementSyntax x => x.Identifier,
EnumMemberDeclarationSyntax x => x.Identifier,
InvocationExpressionSyntax x => x.Expression?.GetIdentifier(),
Expand Down
@@ -0,0 +1,27 @@
/*
* SonarAnalyzer for .NET
* Copyright (C) 2015-2023 SonarSource SA
* mailto: contact AT sonarsource DOT com
*
* This program is free software; you can redistribute it and/or
* modify it under the terms of the GNU Lesser General Public
* License as published by the Free Software Foundation; either
* version 3 of the License, or (at your option) any later version.
*
* This program is distributed in the hope that it will be useful,
* but WITHOUT ANY WARRANTY; without even the implied warranty of
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
* Lesser General Public License for more details.
*
* You should have received a copy of the GNU Lesser General Public License
* along with this program; if not, write to the Free Software Foundation,
* Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA.
*/

namespace SonarAnalyzer.Rules.VisualBasic;

[DiagnosticAnalyzer(LanguageNames.VisualBasic)]
public sealed class ClassNamedException : ClassNamedExceptionBase<SyntaxKind>
{
protected override ILanguageFacade<SyntaxKind> Language => VisualBasicFacade.Instance;
}
Expand Up @@ -2090,7 +2090,7 @@ internal static class RuleTypeMappingCS
// ["S2163"],
// ["S2164"],
// ["S2165"],
// ["S2166"],
["S2166"] = "CODE_SMELL",
// ["S2167"],
// ["S2168"],
// ["S2169"],
Expand Down
Expand Up @@ -2090,7 +2090,7 @@ internal static class RuleTypeMappingVB
// ["S2163"],
// ["S2164"],
// ["S2165"],
// ["S2166"],
["S2166"] = "CODE_SMELL",
// ["S2167"],
// ["S2168"],
// ["S2169"],
Expand Down

0 comments on commit b9f87fd

Please sign in to comment.