Create rule S6513: ExcludeFromCodeCoverage attributes should include a justification #1624
Conversation
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
martin-strecker-sonarsource
changed the title
rules/S6513/vbnet/rule.adoc
Outdated
| Y = coordinates.Y | ||
| End Function | ||
|
|
||
| <ExcludeFromCodeCoverage(Justification:="Code generated by the Visual Studio refactoring.")> |
There was a problem hiding this comment.
I would drop "the" (4 times).
rules/S6513/description.adoc
Outdated
| @@ -0,0 +1 @@ | |||
| The https://learn.microsoft.com/dotnet/api/system.diagnostics.codeanalysis.excludefromcodecoverageattribute[ExcludeFromCodeCoverageAttribute] is used to exclude portions of code from https://learn.microsoft.com/dotnet/core/testing/unit-testing-code-coverage[code coverage reporting]. Code uncovered by unit tests is bad practice and needs a justification. | |||
There was a problem hiding this comment.
"not covered" instead of "uncovered"
"is a bad practice"
for Compliant/Noncompliant code
rules/S6513/description.adoc
Outdated
| @@ -0,0 +1 @@ | |||
| The https://learn.microsoft.com/dotnet/api/system.diagnostics.codeanalysis.excludefromcodecoverageattribute[ExcludeFromCodeCoverageAttribute] is used to exclude portions of code from https://learn.microsoft.com/dotnet/core/testing/unit-testing-code-coverage[code coverage reporting]. Code not covered by unit tests is a bad practice and needs a justification. | |||
There was a problem hiding this comment.
I would say that the bad practice is the use of the attribute, to avoid covering code, rather than the code itself. The code is an object and can't be seen as a practice.
There was a problem hiding this comment.
This change wasn't applied.
What I meant with my comment above was that Code that [...] is a bad practice. should actually be something like Leaving code that [...] is a bad practice. since an object such as code can't be seen as a practice, which is an action (on the object).
Having code that [...] is a bad practice doesn't count as an action, imo.
In other words, the code is not responsible for the mistakes of its author :-)
|
Happy that you addressed .NET 5. @antonioaversa I agree that leaving out the not relevant code makes the snippets easier to process. |
There was a problem hiding this comment.
LGTM! I agree with the change of severity.
Concerning the scope: I would argue that providing a justification when you exclude code from coverage would be a good practice in test code too (e.g. "Used for mocking."). At the same time I would expect a lot of positive cases when using the rule in test projects, and I understand the conservative approach.
|
SonarQube Quality Gate for 'rspec-tools'
|
|
SonarQube Quality Gate for 'rspec-frontend'
|
You can preview this rule here (updated a few minutes after each push).
Implemented by SonarSource/sonar-dotnet#6593