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
New rule S2166: Classes named like "Exception" should extend "Exception" or a subclass #6727
Merged
zsolt-kolbay-sonarsource
merged 21 commits into
master
from
Zsolt/S2166-class-named-exception
Feb 9, 2023
Merged
Changes from 20 commits
Commits
Show all changes
21 commits
Select commit
Hold shift + click to select a range
12c49d4
Initial scaffolding
zsolt-kolbay-sonarsource 43d058f
Add test cases
zsolt-kolbay-sonarsource 78dd3fc
Implement C# rule
zsolt-kolbay-sonarsource b2d8e63
Update RSPEC
zsolt-kolbay-sonarsource da7f219
Add VB test cases
zsolt-kolbay-sonarsource af479a1
Fix indentation
zsolt-kolbay-sonarsource a8e4e12
Add test case for generic classes
zsolt-kolbay-sonarsource 5215eca
Different handling of static classes / modules
zsolt-kolbay-sonarsource 6818afe
Add record class test case
zsolt-kolbay-sonarsource fb60d96
Add intentional finding test cases
zsolt-kolbay-sonarsource c49813e
Fix unit tests
zsolt-kolbay-sonarsource df85f92
Remove empty line
zsolt-kolbay-sonarsource 8d95675
Introduce ClassAndModuleDeclarations to ISyntaxKindFacade
zsolt-kolbay-sonarsource 3a755e4
Remove empty line
zsolt-kolbay-sonarsource 540c20b
Update RSPEC and rule message
zsolt-kolbay-sonarsource 8b1e18b
Update test cases
zsolt-kolbay-sonarsource 5715d6d
Add explanation to test cases
zsolt-kolbay-sonarsource fa18201
Add compilation error test cases
zsolt-kolbay-sonarsource 0ad6700
Add test case for System.Runtime.InteropServices._Exception
zsolt-kolbay-sonarsource 044ce8b
Move up ClassDeclaration in CSharpSyntaxKindFacade
zsolt-kolbay-sonarsource 8e0317d
Fix formatting
zsolt-kolbay-sonarsource File filter
Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
There are no files selected for viewing
15 changes: 15 additions & 0 deletions
15
analyzers/its/sources/ManuallyAddedNoncompliantIssues.CS/IntentionalFindings/S2166.cs
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -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) | ||
} | ||
} |
10 changes: 10 additions & 0 deletions
10
analyzers/its/sources/ManuallyAddedNoncompliantIssues.VB/IntentionalFindings/S2166.vb
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -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 |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -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> | ||
|
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -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" | ||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -62,6 +62,7 @@ | |
"S2114", | ||
"S2115", | ||
"S2123", | ||
"S2166", | ||
"S2178", | ||
"S2183", | ||
"S2184", | ||
|
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -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> | ||
|
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -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" | ||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -38,6 +38,7 @@ | |
"S1940", | ||
"S2068", | ||
"S2077", | ||
"S2166", | ||
"S2178", | ||
"S2222", | ||
"S2225", | ||
|
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
27 changes: 27 additions & 0 deletions
27
analyzers/src/SonarAnalyzer.CSharp/Rules/ClassNamedException.cs
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -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; | ||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
47 changes: 47 additions & 0 deletions
47
analyzers/src/SonarAnalyzer.Common/Rules/ClassNamedExceptionBase.cs
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -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( | ||
antonioaversa marked this conversation as resolved.
Show resolved
Hide resolved
|
||
Language.GeneratedCodeRecognizer, | ||
c => | ||
{ | ||
antonioaversa marked this conversation as resolved.
Show resolved
Hide resolved
|
||
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); | ||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -25,6 +25,11 @@ internal sealed class VisualBasicSyntaxKindFacade : ISyntaxKindFacade<SyntaxKind | |
public SyntaxKind Attribute => SyntaxKind.Attribute; | ||
public SyntaxKind[] ClassAndRecordDeclaration => new[] { SyntaxKind.ClassBlock }; | ||
public SyntaxKind ClassDeclaration => SyntaxKind.ClassBlock; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. please move |
||
public SyntaxKind[] ClassAndModuleDeclarations => new[] | ||
{ | ||
SyntaxKind.ClassBlock, | ||
SyntaxKind.ModuleBlock | ||
}; | ||
public SyntaxKind[] CommentTrivia => new[] | ||
{ | ||
SyntaxKind.CommentTrivia, | ||
|
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
27 changes: 27 additions & 0 deletions
27
analyzers/src/SonarAnalyzer.VisualBasic/Rules/ClassNamedException.cs
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -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; | ||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
please move
ClassDeclaration
aboveClassAndRecordDeclaration