Allow overriding an analyzer's opt-out of analyzing generated source code

[stephentoub]

I'm a source generator author. I want the code my source generator emits to be as clean as possible, so in my tests for the source generator and the code it emits, I turn C# compiler warnings/errors up to the max, and I want a large number of analyzers to examine the generated code to ensure it's up to snuff. The problem, however, is that most analyzers opt-out of examining generated code, which means all of that analysis that could help ensure my source generator is producing good code isn't available to me, and there's no mechanism for overriding the analyzer's preference. I would like in my tests to be able to tell analyzers to examine my source generated code even if they opt-out of examining generated code.

[CyrusNajmabadi]

Honestly, it's unclear to me why we automatically opt things out here.

If the SG wants to be opted out, it could always emit // <auto-generated/> (or whatever that comment is).

If the SG wants to be analyzed, it doesn't emit that, and it's treated like a normal file.

It seems like a super weird policy we took to invent a new concept here instead of using the existing well understood one.

[stephentoub]

Is the complete heuristic used for this documented anywhere? My understanding is it's not just // <auto-generated>, but also things like .g.cs file extension. And even if it is, changing a source generator's implementation to behave differently when under test is itself a strange thing to do.

[CyrusNajmabadi]

Looks like this is the code:

        private static bool IsGeneratedCodeFile([NotNullWhen(returnValue: true)] string? filePath)
        {
            if (!RoslynString.IsNullOrEmpty(filePath))
            {
                var fileName = PathUtilities.GetFileName(filePath);
                if (fileName.StartsWith("TemporaryGeneratedFile_", StringComparison.OrdinalIgnoreCase))
                {
                    return true;
                }

                var extension = PathUtilities.GetExtension(fileName);
                if (!string.IsNullOrEmpty(extension))
                {
                    var fileNameWithoutExtension = PathUtilities.GetFileName(filePath, includeExtension: false);
                    if (fileNameWithoutExtension.EndsWith(".designer", StringComparison.OrdinalIgnoreCase) ||
                        fileNameWithoutExtension.EndsWith(".generated", StringComparison.OrdinalIgnoreCase) ||
                        fileNameWithoutExtension.EndsWith(".g", StringComparison.OrdinalIgnoreCase) ||
                        fileNameWithoutExtension.EndsWith(".g.i", StringComparison.OrdinalIgnoreCase))
                    {
                        return true;
                    }
                }
            }

            return false;
        }

and

        private static readonly string[] s_autoGeneratedStrings = new[] { "<autogenerated", "<auto-generated" };

        private static bool BeginsWithAutoGeneratedComment(
            SyntaxTree tree, Func<SyntaxTrivia, bool> isComment, CancellationToken cancellationToken)
        {
            var root = tree.GetRoot(cancellationToken);
            if (root.HasLeadingTrivia)
            {
                var leadingTrivia = root.GetLeadingTrivia();

                foreach (var trivia in leadingTrivia)
                {
                    if (!isComment(trivia))
                    {
                        continue;
                    }

                    var text = trivia.ToString();

                    // Check to see if the text of the comment contains an auto generated comment.
                    foreach (var autoGenerated in s_autoGeneratedStrings)
                    {
                        if (text.Contains(autoGenerated))
                        {
                            return true;
                        }
                    }
                }
            }

            return false;
        }

[CyrusNajmabadi]

And even if it is, changing a source generator's implementation to behave differently when under test is itself a strange thing to do.

I agree. Basically my point is: i don't know why we invented something new for source-generators, instead of just having SGs opt in/out of this behavior by using the existing mechanism that we've already had for tools generating C# code.

[stephentoub]

Thanks, though I'm still not clear on the feedback and what you're recommending I do. I don't want to force my source generator's generated code to trigger analyzer warnings for anyone beyond my own tests. What is it I should do?

[mavasani]

@CyrusNajmabadi Currently, only an analyzer controls whether or not it runs on generated code. Most of the analyzers that opt-out of generated code analysis do so to reduce noise for the end users, not because the analyzer is not capable of analyzing generated code. I believe @stephentoub's request here is to allow an analyzer to state that by default it wants to opt-out of generated code analysis, but if an end user explicitly wants to run it on generated code (say via some editorconfig option or an MSBuild property), then it should get analysis callbacks for generated code. IMO, that sounds like a reasonable feature request.

One of the ways to implement such a request would be to add a new flag to the below Flags enum: https://github.com/dotnet/roslyn/blob/173d2c4219928c54dc8c02307669126219f80f86/src/Compilers/Core/Portable/DiagnosticAnalyzer/DiagnosticAnalysisContext.cs#L247-L271

If an analyzer specifies the new flag when invoking ConfigureGeneratedCodeAnalysis, then for generated code, the analyzer driver would first check if the user has specified the corresponding editorconfig/MSBuild option, and if so, make callbacks to the analyzer for generated code.

[mavasani]

@jaredpar @chsienki for triage.

[mavasani]

@sharwell is going to follow up with @stephentoub on couple of suggestions he has to enable this testing without requiring a new analyzer feature.

[Eli-Black-Work]

In our case, we'd like to be able to enable warnings in generated code for non-testing scenarios 🙂