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

[CompilerGenerated] not honored #931

Open
KristianWedberg opened this issue Jun 9, 2017 · 7 comments
Open

[CompilerGenerated] not honored #931

KristianWedberg opened this issue Jun 9, 2017 · 7 comments

Comments

@KristianWedberg
Copy link

Bug or Enhancement

With v1.0.3 the following generates warnings CC0091 and CC0021:

    [CompilerGenerated] 
    public class TestCompilerGenerated
    {
        public string MyMethod()
        {
            return "MyMethod";
        }
    }

Despite

  • The class is tagged with [CompilerGenerated]
  • "Project Properties > Code Analysis > Suppress results from generated code (managed only)" is enabled

Expected

Anything (classes etc.) tagged with [CompilerGenerated] should be excluded from code analysis if
"Project Properties > Code Analysis > Suppress results from generated code (managed only)" is enabled. This is described here:

https://docs.microsoft.com/en-gb/visualstudio/code-quality/how-to-suppress-code-analysis-warnings-for-generated-code

@giggio
Copy link
Member

giggio commented Jun 13, 2017

There are 2 code analysis tools on VS right now, the Roslyn based, and the legacy one. The one you are referring to on "Project Properties > Code Analysis > Suppress results from generated code (managed only)" applies to the legacy one, which runs when you go the Analyze menu, and select to run a code analysis. We cannot control what happens there.

But we do have our own way to ignore generated code. We haven't been looking at the CompilerGenerated attribute, but we surely can. This is a welcome enhancement, thanks for suggesting it.

@giggio giggio added this to Ready in CodeCracker Jun 13, 2017
@MaStr11
Copy link
Contributor

MaStr11 commented Jun 26, 2017

Related discussions in roslyn:

@giggio You have been involved in some of the discussions there. What is your understanding of the current state in roslyn? Should we wait for the roslyn team to figure this out or should we improve on our own logic?

The PR mentioned introduces the required API to ignore generated code: AnalysisContext.ConfigureGeneratedCodeAnalysis

    /// <summary>
    /// Flags to configure mode of generated code analysis.
    /// </summary>
    [Flags]
    public enum GeneratedCodeAnalysisFlags
    {
        /// <summary>
        /// Disable analyzer action callbacks and diagnostic reporting for generated code.
        /// Analyzer driver will not make callbacks into the analyzer for entities (source files, symbols, etc.) that it classifies as generated code.
        /// Additionally, any diagnostic reported by the analyzer with location in generated code will not be reported.
        /// </summary>
        None = 0x00,

        /// <summary>
        /// Enable analyzer action callbacks for generated code.
        /// Analyzer driver will make callbacks into the analyzer for all entities (source files, symbols, etc.) in the compilation, including generated code.
        /// </summary>
        Analyze = 0x01,

        /// <summary>
        /// Enable reporting diagnostics on generated code.
        /// Analyzer driver will not suppress any analyzer diagnostic based on whether or not it's location is in generated code.
        /// </summary>
        ReportDiagnostics = 0x02,
    }

@giggio
Copy link
Member

giggio commented Jun 28, 2017

The master branch was just updated to Roslyn 1.3.2 (from 1.0.0), and the new API is available from 1.2. We could verify if this is working and available, but I have not yet done so. And I am not sure if CompilerGenerated will be caught. We have to check.
Ideally we would remove our implementation, that is what I would like to do.

@giggio
Copy link
Member

giggio commented Jun 28, 2017

I just checked and adding:

context.ConfigureGeneratedCodeAnalysis(GeneratedCodeAnalysisFlags.None);

To the initialize method works to supress the analyzer on generated code. Only on the master branch, which is updated.

Now we need to check how far does the Roslyn support for generated code goes. Maybe we are doing something extra that they are not. But, where we can, we should switch to theirs and remove ours.

@sharwell
Copy link
Contributor

sharwell commented Jun 28, 2017

@KristianWedberg @giggio Currently Roslyn does consider code marked with this attribute as generated code. However, you should not rely on this behavior in the long run because it has a substantial negative performance impact on the build and I'm working to make ignoring this attribute standard practice (the default wouldn't change, but I would encourage users to disable the setting at their earliest convenience). See dotnet/roslyn#20498.

@giggio You are correct that you must opt-in to having reports suppressed in generated code. The alternative would have been a breaking change for shipped, stable analyzers that implemented and documented their own exclusion policies.

@giggio
Copy link
Member

giggio commented Jun 28, 2017

@sharwell If I understand correctly, you are suggesting that we do not use Roslyn's feature to disable the analysis on generated code. Is my understanding correct?
If so, we would continue to use our own implementation to ignore generated code. I don't think this is efficient in the long run. Every analyzer author will have to implement theirs on their own. Some will be good, some will not. Wouldn't it be better if we improve Roslyn's, if it is not good today?

@sharwell
Copy link
Contributor

sharwell commented Jun 28, 2017

@giggio We switched StyleCop Analyzers over to Roslyn's generated code analysis for the 1.1.0-beta001 release many months ago and haven't had problems. My recommendation was actually for end users to try and avoid depending on [GeneratedCodeAttribute] as a means of marking generated code, as it will just slow down their builds. Currently there is no way to disable it, but I suspect that will change. (Note that my statement has nothing whatsoever to do with whether or not [GeneratedCodeAttribute] appears in code. I'm only referring to whether or not users should expect the analyzers to look at it.)

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

No branches or pull requests

4 participants