Skip to content

Commit

Permalink
New Rule: ExcludeFromCodeCoverage attributes should include a justifi…
Browse files Browse the repository at this point in the history
…cation (#6593)

@Corniel Thank you for the contribution!
  • Loading branch information
Corniel committed Mar 14, 2023
1 parent 145047e commit 938fbbc
Show file tree
Hide file tree
Showing 16 changed files with 675 additions and 3 deletions.
2 changes: 1 addition & 1 deletion README.md
Expand Up @@ -12,7 +12,7 @@ languages in [SonarQube](http://www.sonarqube.org/), [SonarCloud](https://sonarc

## Features

* [380+ C# rules](https://rules.sonarsource.com/csharp) and [170+ VB.​NET rules](https://rules.sonarsource.com/vbnet)
* [390+ C# rules](https://rules.sonarsource.com/csharp) and [170+ VB.​NET rules](https://rules.sonarsource.com/vbnet)
* Metrics (cognitive complexity, duplications, number of lines etc.)
* Import of [test coverage reports](https://community.sonarsource.com/t/9871) from Visual Studio Code Coverage, dotCover, OpenCover, Coverlet, Altcover.
* Import of third party Roslyn Analyzers results
Expand Down
53 changes: 53 additions & 0 deletions analyzers/rspec/cs/S6513_c#.html
@@ -0,0 +1,53 @@
<p>The <a
href="https://learn.microsoft.com/dotnet/api/system.diagnostics.codeanalysis.excludefromcodecoverageattribute">ExcludeFromCodeCoverageAttribute</a> is
used to exclude portions of code from <a href="https://learn.microsoft.com/dotnet/core/testing/unit-testing-code-coverage">code coverage
reporting</a>. It is a bad practice to retain code that is not covered by unit tests. In .Net 5, the <code>Justification</code> property was added to
the <code>ExcludeFromCodeCoverageAttribute</code> as an opportunity to document the rationale for the exclusion. This rule raises an issue when no
such justification is given.</p>
<h2>Noncompliant Code Example</h2>
<pre>
public struct Coordinates
{
public int X { get; }
public int Y { get; }

[ExcludeFromCodeCoverage] // Noncompliant
public override bool Equals(object obj) =&gt; obj is Coordinates coordinates &amp;&amp; X == coordinates.X &amp;&amp; Y == coordinates.Y;

[ExcludeFromCodeCoverage] // Noncompliant
public override int GetHashCode()
{
var hashCode = 1861411795;
hashCode = hashCode * -1521134295 + X.GetHashCode();
hashCode = hashCode * -1521134295 + Y.GetHashCode();
return hashCode;
}
}
</pre>
<h2>Compliant Solution</h2>
<pre>
public struct Coordinates
{
public int X { get; }
public int Y { get; }

[ExcludeFromCodeCoverage(Justification = "Code generated by Visual Studio refactoring")] // Compliant
public override bool Equals(object obj) =&gt; obj is Coordinates coordinates &amp;&amp; X == coordinates.X &amp;&amp; Y == coordinates.Y;

[ExcludeFromCodeCoverage(Justification = "Code generated by Visual Studio refactoring")] // Compliant
public override int GetHashCode()
{
var hashCode = 1861411795;
hashCode = hashCode * -1521134295 + X.GetHashCode();
hashCode = hashCode * -1521134295 + Y.GetHashCode();
return hashCode;
}
}
</pre>
<h2>See</h2>
<ul>
<li> <a href="https://learn.microsoft.com/dotnet/api/system.diagnostics.codeanalysis.excludefromcodecoverageattribute">API browser</a> -
ExcludeFromCodeCoverageAttribute </li>
<li> <a href="https://learn.microsoft.com/dotnet/core/testing/unit-testing-code-coverage">DevOps and testing</a> - Code coverage reporting </li>
</ul>

17 changes: 17 additions & 0 deletions analyzers/rspec/cs/S6513_c#.json
@@ -0,0 +1,17 @@
{
"title": "\"ExcludeFromCodeCoverage\" attributes should include a justification",
"type": "CODE_SMELL",
"status": "ready",
"remediation": {
"func": "Constant\/Issue",
"constantCost": "5min"
},
"tags": [
"bad-practice"
],
"defaultSeverity": "Minor",
"ruleSpecification": "RSPEC-6513",
"sqKey": "S6513",
"scope": "Main",
"quickfix": "unknown"
}
67 changes: 67 additions & 0 deletions analyzers/rspec/vbnet/S6513_vb.net.html
@@ -0,0 +1,67 @@
<p>The <a
href="https://learn.microsoft.com/dotnet/api/system.diagnostics.codeanalysis.excludefromcodecoverageattribute">ExcludeFromCodeCoverageAttribute</a> is
used to exclude portions of code from <a href="https://learn.microsoft.com/dotnet/core/testing/unit-testing-code-coverage">code coverage
reporting</a>. It is a bad practice to retain code that is not covered by unit tests. In .Net 5, the <code>Justification</code> property was added to
the <code>ExcludeFromCodeCoverageAttribute</code> as an opportunity to document the rationale for the exclusion. This rule raises an issue when no
such justification is given.</p>
<h2>Noncompliant Code Example</h2>
<pre>
Public Structure Coordinates

Public ReadOnly Property X As Integer
Public ReadOnly Property Y As Integer

&lt;ExcludeFromCodeCoverage&gt; ' Noncompliant
Public Overrides Function Equals(obj As Object) As Boolean
If Not (TypeOf obj Is Coordinates) Then
Return False
End If

Dim coordinates = DirectCast(obj, Coordinates)
Return X = coordinates.X AndAlso
Y = coordinates.Y
End Function

&lt;ExcludeFromCodeCoverage&gt; ' Noncompliant
Public Overrides Function GetHashCode() As Integer
Dim hashCode As Long = 1861411795
hashCode = (hashCode * -1521134295 + X.GetHashCode()).GetHashCode()
hashCode = (hashCode * -1521134295 + Y.GetHashCode()).GetHashCode()
Return hashCode
End Function
End Structure
</pre>
<h2>Compliant Solution</h2>
<pre>
Public Structure Coordinates

Public ReadOnly Property X As Integer
Public ReadOnly Property Y As Integer

&lt;ExcludeFromCodeCoverage(Justification:="Code generated by Visual Studio refactoring")&gt; ' Compliant
Public Overrides Function Equals(obj As Object) As Boolean
If Not (TypeOf obj Is Coordinates) Then
Return False
End If

Dim coordinates = DirectCast(obj, Coordinates)
Return X = coordinates.X AndAlso
Y = coordinates.Y
End Function

&lt;ExcludeFromCodeCoverage(Justification:="Code generated by Visual Studio refactoring")&gt; ' Compliant
Public Overrides Function GetHashCode() As Integer
Dim hashCode As Long = 1861411795
hashCode = (hashCode * -1521134295 + X.GetHashCode()).GetHashCode()
hashCode = (hashCode * -1521134295 + Y.GetHashCode()).GetHashCode()
Return hashCode
End Function
End Structure
</pre>
<h2>See</h2>
<ul>
<li> <a href="https://learn.microsoft.com/dotnet/api/system.diagnostics.codeanalysis.excludefromcodecoverageattribute">API browser</a> -
ExcludeFromCodeCoverageAttribute </li>
<li> <a href="https://learn.microsoft.com/dotnet/core/testing/unit-testing-code-coverage">DevOps and testing</a> - Code coverage reporting </li>
</ul>

17 changes: 17 additions & 0 deletions analyzers/rspec/vbnet/S6513_vb.net.json
@@ -0,0 +1,17 @@
{
"title": "\"ExcludeFromCodeCoverage\" attributes should include a justification",
"type": "CODE_SMELL",
"status": "ready",
"remediation": {
"func": "Constant\/Issue",
"constantCost": "5min"
},
"tags": [
"bad-practice"
],
"defaultSeverity": "Minor",
"ruleSpecification": "RSPEC-6513",
"sqKey": "S6513",
"scope": "Main",
"quickfix": "unknown"
}
@@ -0,0 +1,33 @@
/*
* 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 ExcludeFromCodeCoverageAttributesNeedJustification : ExcludeFromCodeCoverageAttributesNeedJustificationBase<SyntaxKind>
{
protected override ILanguageFacade<SyntaxKind> Language => CSharpFacade.Instance;

protected override SyntaxNode GetJustificationExpression(SyntaxNode node) =>
node is AttributeSyntax { ArgumentList.Arguments: { Count: 1 } arguments }
&& arguments[0] is { NameEquals.Name.Identifier.ValueText: JustificationPropertyName, Expression: { } expression }
? expression
: null;
}
@@ -0,0 +1,58 @@
/*
* 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 ExcludeFromCodeCoverageAttributesNeedJustificationBase<TSyntaxKind> : SonarDiagnosticAnalyzer<TSyntaxKind>
where TSyntaxKind : struct
{
private const string DiagnosticId = "S6513";

protected const string JustificationPropertyName = "Justification";

protected override string MessageFormat => "Add a justification.";

protected abstract SyntaxNode GetJustificationExpression(SyntaxNode node);

protected ExcludeFromCodeCoverageAttributesNeedJustificationBase() : base(DiagnosticId) { }

protected sealed override void Initialize(SonarAnalysisContext context) =>
context.RegisterNodeAction(
Language.GeneratedCodeRecognizer,
c =>
{
if (NoJustification(c.Node, c.SemanticModel)
&& c.SemanticModel.GetSymbolInfo(c.Node).Symbol is IMethodSymbol attribute
&& attribute.IsInType(KnownType.System_Diagnostics_CodeAnalysis_ExcludeFromCodeCoverageAttribute)
&& HasJustificationProperty(attribute.ContainingType))
{
c.ReportIssue(Diagnostic.Create(Rule, c.Node.GetLocation()));
}
},
Language.SyntaxKind.Attribute);

private bool NoJustification(SyntaxNode node, SemanticModel model) =>
GetJustificationExpression(node) is not { } justification
|| string.IsNullOrWhiteSpace(Language.FindConstantValue(model, justification) as string);

/// <summary>"Justification" was added in .Net 5, while ExcludeFromCodeCoverage in netstandard2.0.</summary>
private static bool HasJustificationProperty(INamedTypeSymbol symbol) =>
symbol.MemberNames.Contains(JustificationPropertyName);
}
@@ -0,0 +1,34 @@
/*
* 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 ExcludeFromCodeCoverageAttributesNeedJustification : ExcludeFromCodeCoverageAttributesNeedJustificationBase<SyntaxKind>
{
protected override ILanguageFacade<SyntaxKind> Language => VisualBasicFacade.Instance;

protected override SyntaxNode GetJustificationExpression(SyntaxNode node) =>
node is AttributeSyntax { ArgumentList.Arguments: { Count: 1 } arguments }
&& arguments[0] is SimpleArgumentSyntax { Expression: { } } argument
&& Language.NameComparer.Equals(argument.GetName(), JustificationPropertyName)
? argument.Expression
: null;
}
Expand Up @@ -6437,7 +6437,7 @@ internal static class RuleTypeMappingCS
// ["S6510"],
// ["S6511"],
// ["S6512"],
// ["S6513"],
["S6513"] = "CODE_SMELL",
// ["S6514"],
// ["S6515"],
// ["S6516"],
Expand Down
Expand Up @@ -6437,7 +6437,7 @@ internal static class RuleTypeMappingVB
// ["S6510"],
// ["S6511"],
// ["S6512"],
// ["S6513"],
["S6513"] = "CODE_SMELL",
// ["S6514"],
// ["S6515"],
// ["S6516"],
Expand Down
@@ -0,0 +1,68 @@
/*
* SonarAnalyzer for .NET
* Copyright (C) 2015-2022 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.
*/

using CS = SonarAnalyzer.Rules.CSharp;
using VB = SonarAnalyzer.Rules.VisualBasic;

namespace SonarAnalyzer.UnitTest.Rules;

[TestClass]
public class ExcludeFromCodeCoverageAttributesNeedJustificationTest
{
private readonly VerifierBuilder builderCS = new VerifierBuilder<CS.ExcludeFromCodeCoverageAttributesNeedJustification>();
private readonly VerifierBuilder builderVB = new VerifierBuilder<VB.ExcludeFromCodeCoverageAttributesNeedJustification>();

#if NET

[TestMethod]
public void ExcludeFromCodeCoverageAttributesNeedJustification_OnAssembly_CS() =>
builderCS.AddSnippet("[assembly:System.Diagnostics.CodeAnalysis.ExcludeFromCodeCoverage] // Noncompliant").Verify();

[TestMethod]
public void ExcludeFromCodeCoverageAttributesNeedJustification_OnAssembly_VB() =>
builderVB.AddSnippet("<Assembly:System.Diagnostics.CodeAnalysis.ExcludeFromCodeCoverage> ' Noncompliant").Verify();

[TestMethod]
public void ExcludeFromCodeCoverageAttributesNeedJustification_CS() =>
builderCS.AddPaths("ExcludeFromCodeCoverageAttributesNeedJustification.cs").Verify();

[TestMethod]
public void ExcludeFromCodeCoverageAttributesNeedJustification_CSharp9() =>
builderCS.AddPaths("ExcludeFromCodeCoverageAttributesNeedJustification.CSharp9.cs").WithTopLevelStatements().Verify();

[TestMethod]
public void ExcludeFromCodeCoverageAttributesNeedJustification_CSharp10() =>
builderCS.AddPaths("ExcludeFromCodeCoverageAttributesNeedJustification.CSharp10.cs").WithOptions(ParseOptionsHelper.FromCSharp10).Verify();

[TestMethod]
public void ExcludeFromCodeCoverageAttributesNeedJustification_VB() =>
builderVB.AddPaths("ExcludeFromCodeCoverageAttributesNeedJustification.vb").Verify();

#endif

#if netframework

[TestMethod]
public void ExcludeFromCodeCoverageAttributesNeedJustification_IgnoredForNet48() =>
builderCS.AddPaths("ExcludeFromCodeCoverageAttributesNeedJustification.net48.cs").Verify();

#endif

}

0 comments on commit 938fbbc

Please sign in to comment.