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

CA1801: Exempt parameters documented "The parameter is not used" #2805

Closed
peteroupc opened this issue Sep 2, 2019 · 6 comments
Closed

CA1801: Exempt parameters documented "The parameter is not used" #2805

peteroupc opened this issue Sep 2, 2019 · 6 comments

Comments

@peteroupc
Copy link

peteroupc commented Sep 2, 2019

Microsoft.CodeAnalysis.FxCopAnalyzers v2.9.4.

Exempt a parameter from CA1801, "Review unused parameters", if the parameter is documented with the text "The parameter is not used." or its equivalent in other languages. This is the exact text used when exempting documentation elements from duplicate checks by StyleCop, and this text already signals that the parameter in question is intentionally unused.

Example:

/// <summary>This is a test.</summary>
/// <param name='param'>The parameter is not used.</param>
public static void Test(int param) {
}
@mavasani
Copy link
Contributor

mavasani commented Sep 2, 2019

There is no such convention that a comment with a specific text should indicate a suppression - this gets especially confusing when comments are localized. A better approach is to port the logic used in Roslyn's implementation of this analyzer to allow using special parameter names to indicate discard parameters: http://source.roslyn.io/#Microsoft.CodeAnalysis.Features/RemoveUnusedParametersAndValues/AbstractRemoveUnusedParametersAndValuesDiagnosticAnalyzer.cs,333

Let us use this issue to track porting this logic.

@peteroupc
Copy link
Author

In case you're not familiar, here is the StyleCop Analyzers rule making an exception for "The parameter is not used": SA1625: Element documentation must not be copied and pasted; implementation.

@mavasani
Copy link
Contributor

mavasani commented Sep 2, 2019

@peteroupc Every third party analyzer package has the freedom to define its own heuristics for bailing out or not reporting diagnostics. It is not feasible to keep all analyzer packages in sync in terms of the heuristics used.

@peteroupc
Copy link
Author

peteroupc commented Sep 2, 2019

Note that StyleCop Analyzers is far from being a "third-party analyzer package" in relation to this one — at least it seems to me. I agree that this would not be so relevant if the relation between FxCop Analyzers and StyleCop Analyzers weren't as close as it is.

@mavasani
Copy link
Contributor

mavasani commented Sep 2, 2019

@peteroupc I am not sure how you came to that conclusion. For example, does StyleCop analyzer respect Roslyn's heuristic for marking unused parameter: http://source.roslyn.io/#Microsoft.CodeAnalysis.Features/RemoveUnusedParametersAndValues/AbstractRemoveUnusedParametersAndValuesDiagnosticAnalyzer.cs,333. Majority of analyzer packages available out there have lot of overlapping rules, I am sure StyleCop analyzers has overlap with Roslyn's inbuilt IDE code style rules and some of the analyzers in this repo - it is not possible to make these implementations uniform. I would suggest that anyone using multiple analyzer packages should not expect uniformity between identical rules in packages. Instead, when you come across overlapping rules, you should choose a particular implementation that suits you better and disable the other one through ruleset file or nowarn entry.

@mavasani
Copy link
Contributor

mavasani commented Sep 3, 2019

Seems like #2638 already ensures that parameters named as _, _1, _2, etc. are already treated as special discard parameter names.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants