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

SuppressMessageAttribute not working with namespaces #152

Closed
giggio opened this issue Jan 29, 2015 · 13 comments
Closed

SuppressMessageAttribute not working with namespaces #152

giggio opened this issue Jan 29, 2015 · 13 comments
Labels
2 - Ready Area-Analyzers Bug Resolution-By Design The behavior reported in the issue matches the current design Verified
Milestone

Comments

@giggio
Copy link

giggio commented Jan 29, 2015

I am trying to uses the GlobalSuppressions file to suppress a specific rule on a specific namespace on my test project, like so:

[assembly: System.Diagnostics.CodeAnalysis.SuppressMessage("Style", "CC0061", Scope = "namespace", Target = "CodeCracker.Test.Design", Justification = "No need for tests to have async sufix.")]

But it does not work. The diagnostic keeps being generated. The method specific works:

[assembly: System.Diagnostics.CodeAnalysis.SuppressMessage("Style", "CC0061:TaskNameAsyncAnalyzer", Justification = "No need for tests to have async sufix.", Scope = "member", Target = "~M:CodeCracker.Test.Design.CopyEventToVariableBeforeFireTests.IgnoreMemberAccess")]

I guess this is a bug, right?

@mavasani
Copy link
Contributor

@giggio This is as per the design of FxCop source level suppressions, and we have retained that design in Roslyn. Please see the note in section "Global-Level Suppressions" in here: https://msdn.microsoft.com/en-us/library/ms244717.aspx

"When you suppress a warning with namespace scope, it suppresses the warning against the namespace itself. It does not suppress the warning against types within the namespace."

@srivatsn By Design?

@mavasani mavasani added Resolution-By Design The behavior reported in the issue matches the current design and removed Resolution-By Design The behavior reported in the issue matches the current design labels Feb 11, 2015
@mavasani
Copy link
Contributor

@shyamnamboodiripad Can you double check if we are fine with the current design for suppress message attribute applied with namespace target? Seems reasonable to me..

@srivatsn
Copy link
Contributor

Let's stick to the FxCop convention for the suppressmessage attribute otherwise we will end up breaking a bunch of suppressions in existing code.

@shyamnamboodiripad
Copy link
Contributor

I agree it makes sense to stick with the FxCop convention in this case.

@giggio
Copy link
Author

giggio commented Feb 12, 2015

Ok, it makes sense not to break everyone. But how would then I suppress diagnostics against types inside the namespace? Is that at all possible? Or will I have to add each one individually? This would be really annoying.

@mavasani
Copy link
Contributor

You can suppress a diagnostic on the entire type in one of the following ways:

  1. using Scope = "type" and Target = "~T:" in the assembly attribute in GlobalSuppressions file OR
  2. By explicitly applying the attribute on the type node without any scope or target.

If either approach doesn't work, its a bug that must be fixed.

@mavasani
Copy link
Contributor

But yes, you would have to do it on individual type. Alternatively you can suppress it on the entire assembly by having as assembly attribute with no scope and target.

@giggio
Copy link
Author

giggio commented Feb 13, 2015

Ok, this is insufficient. The ability to suppress a diagnostic on a specific namespace, including all its types, is really important. I am seeing that there is no way to do this today? May I suggest that feature? :)

@giggio
Copy link
Author

giggio commented Feb 13, 2015

@srivatsn
Copy link
Contributor

We could just add a new scope called "TypesInNamespace" and specifying a namespace there should suppress in all types in that namespace.

@srivatsn
Copy link
Contributor

Created #486 to track this request.

@mavasani
Copy link
Contributor

Yes, I don't think implementing this is tough for us in Roslyn. However, it would be good to know the original thinking behind the current design. Probably Nate remembers, let me follow up with him.

@giggio
Copy link
Author

giggio commented Feb 13, 2015

I will tell you why I need it, and maybe it helps. You can see on my original post above that I am asking for it on the namespace CodeCracker.Test.Design. This is a test assembly, CodeCracker.Test.dll, and the specific diagnostic we want to suppress is the one that requires the Async suffix for async methods. We don't want to have to add Async to the end of every test method, it looks ugly, and it is unnecessary. And I want the diagnostic reported on all other methods that are not direct test methods, like test helpers, etc, as I want to know if they are async or not.
Right now we suppressed the diagnostic on the whole assembly, but it is not ideal. Ideally we would suppress only the test namespaces and that would be it.
Thanks for following up on this on #486. :)

@shyamnamboodiripad shyamnamboodiripad removed their assignment Mar 6, 2015
dbremner pushed a commit to dbremner/imanagepowershell that referenced this issue Aug 17, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
2 - Ready Area-Analyzers Bug Resolution-By Design The behavior reported in the issue matches the current design Verified
Projects
None yet
Development

No branches or pull requests

5 participants