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

New Rule: ExcludeFromCodeCoverage attributes should include a justification #6593

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
22 commits
Select commit Hold shift + click to select a range
a31056a
Crreate analyzers.
Dec 24, 2022
052c78b
Support null string.empty and whitsepace, ignore older versions of .NET.
Mar 9, 2023
fa7c65d
390+
Mar 9, 2023
830ccb5
Update analyzers/src/SonarAnalyzer.Common/Rules/ExcludeFromCodeCovera…
Corniel Mar 9, 2023
9dfe3e1
Update analyzers/tests/SonarAnalyzer.UnitTest/Rules/ExcludeFromCodeCo…
Corniel Mar 9, 2023
5f215a6
Update analyzers/tests/SonarAnalyzer.UnitTest/TestCases/ExcludeFromCo…
Corniel Mar 9, 2023
530a0ae
Get Justification content.
Mar 9, 2023
22535ed
Oops.
Mar 9, 2023
77c9a98
Update analyzers/src/SonarAnalyzer.Common/Rules/ExcludeFromCodeCovera…
Corniel Mar 9, 2023
5b7e905
Add test case to indicate type alias support.
Mar 9, 2023
3fe1bd4
Merge branch 'corniel/justify-exclude-from-code-coverage' of https://…
Mar 9, 2023
b537f13
Added comment.
Mar 9, 2023
93469be
Update analyzers/src/SonarAnalyzer.Common/Rules/ExcludeFromCodeCovera…
Corniel Mar 9, 2023
f285304
Move comment,
Mar 9, 2023
5098ff3
Merge branch 'corniel/justify-exclude-from-code-coverage' of https://…
Mar 9, 2023
5a5e61a
Indentation fix.
Mar 10, 2023
c46bd1c
Update analyzers/src/SonarAnalyzer.Common/Rules/ExcludeFromCodeCovera…
Corniel Mar 13, 2023
7d7bff5
Update analyzers/src/SonarAnalyzer.Common/Rules/ExcludeFromCodeCovera…
Corniel Mar 13, 2023
4e43f03
Run rspec script.
martin-strecker-sonarsource Mar 13, 2023
916c251
Whitespace
martin-strecker-sonarsource Mar 13, 2023
2342fd4
Update RSpec
martin-strecker-sonarsource Mar 14, 2023
bf74d6a
Fix file header
martin-strecker-sonarsource Mar 14, 2023
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
2 changes: 1 addition & 1 deletion README.md
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
@@ -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
Original file line number Diff line number Diff line change
@@ -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
Original file line number Diff line number Diff line change
@@ -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
Original file line number Diff line number Diff line change
@@ -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"
}
Original file line number Diff line number Diff line change
@@ -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;
}
Original file line number Diff line number Diff line change
@@ -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))
martin-strecker-sonarsource marked this conversation as resolved.
Show resolved Hide resolved
{
c.ReportIssue(Diagnostic.Create(Rule, c.Node.GetLocation()));
}
},
Language.SyntaxKind.Attribute);

private bool NoJustification(SyntaxNode node, SemanticModel model) =>
martin-strecker-sonarsource marked this conversation as resolved.
Show resolved Hide resolved
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) =>
Corniel marked this conversation as resolved.
Show resolved Hide resolved
symbol.MemberNames.Contains(JustificationPropertyName);
}
Original file line number Diff line number Diff line change
@@ -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;
}
Original file line number Diff line number Diff line change
Expand Up @@ -6437,7 +6437,7 @@ internal static class RuleTypeMappingCS
// ["S6510"],
// ["S6511"],
// ["S6512"],
// ["S6513"],
["S6513"] = "CODE_SMELL",
// ["S6514"],
// ["S6515"],
// ["S6516"],
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6437,7 +6437,7 @@ internal static class RuleTypeMappingVB
// ["S6510"],
// ["S6511"],
// ["S6512"],
// ["S6513"],
["S6513"] = "CODE_SMELL",
// ["S6514"],
// ["S6515"],
// ["S6516"],
Expand Down
Original file line number Diff line number Diff line change
@@ -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

}
Loading