Light bulb does not invoke code fix provider for diagnostics created in compilation end action

[jnm2]

Repros in Visual Studio 3.9.0 (and 3.8.6) both as a NuGet package and a VSIX. Diagnostics are reported on all declared types.

If I report diagnostics from the compilation end action, they show up properly, but my code fix is never invoked when I bring up the light bulb. If I report the same diagnostics from the symbol action, they show up the same, and my code fix is invoked when I bring up the lightbulb. Am I abusing the API, or is there a Roslyn bug?

Repro steps:

1. Paste the code below into an analyzer project and either use Ctrl+F5 on a VSIX project to deploy the analyzer, or create a NuGet package and reference it in a simple throwaway project.
2. Wait for EXAMPLE diagnostics to appear on all types in the project you open.
3. Press Ctrl+. to invoke the light bulb on one of the EXAMPLE diagnostics. Nothing happens. RegisterCodeFixesAsync was not called.
4. Recompile the analyzer, but cut and paste the two lines that report a diagnostic into the RegisterSymbolAction callback as indicated by the code comment.
5. Redeploy the VSIX or NuGet package and repeat the above steps by invoking the light bulb when the diagnostics appear.
6. You should instantly see that Debugger.Launch() was called this time. RegisterCodeFixesAsync was called.

Minimal repro is below. If you want to know why I'm using this pattern, the motivating source is at https://gist.github.com/jnm2/3b45cd503e28fc3341e9e2453c148f5f.

using System.Collections.Generic;
using System.Collections.Immutable;
using System.Composition;
using System.Linq;
using System.Threading.Tasks;
using Microsoft.CodeAnalysis;
using Microsoft.CodeAnalysis.CodeFixes;
using Microsoft.CodeAnalysis.Diagnostics;

public sealed class DiagnosticDescriptors
{
    public static DiagnosticDescriptor ExampleDiagnostic { get; } = new(
        id: "EXAMPLE",
        title: "Example diagnostic",
        messageFormat: "Example diagnostic",
        "CodeQuality",
        defaultSeverity: DiagnosticSeverity.Warning,
        isEnabledByDefault: true);
}

[DiagnosticAnalyzer(LanguageNames.CSharp)]
public sealed class ClassModifierAnalyzer : DiagnosticAnalyzer
{
    public override ImmutableArray<DiagnosticDescriptor> SupportedDiagnostics { get; } = ImmutableArray.Create(
        DiagnosticDescriptors.ExampleDiagnostic);

    public override void Initialize(AnalysisContext context)
    {
        context.ConfigureGeneratedCodeAnalysis(GeneratedCodeAnalysisFlags.Analyze | GeneratedCodeAnalysisFlags.ReportDiagnostics);
        context.EnableConcurrentExecution();

        context.RegisterCompilationStartAction(OnCompilationStart);
    }

    private static void OnCompilationStart(CompilationStartAnalysisContext context)
    {
        var suggestionCandidates = new List<INamedTypeSymbol>();

        context.RegisterSymbolAction(context =>
        {
            var symbol = (INamedTypeSymbol)context.Symbol;
            lock (suggestionCandidates) suggestionCandidates.Add(symbol);
        }, SymbolKind.NamedType);

        context.RegisterCompilationEndAction(context =>
        {
            foreach (var symbol in suggestionCandidates)
            {
                // Move these two lines into the symbol callback, and the light bulb starts invoking the code fix provider defined below
                var syntax = symbol.DeclaringSyntaxReferences.FirstOrDefault()?.GetSyntax(context.CancellationToken);
                context.ReportDiagnostic(Diagnostic.Create(DiagnosticDescriptors.ExampleDiagnostic, syntax?.GetLocation()));
            }
        });
    }
}

[ExportCodeFixProvider(LanguageNames.CSharp), Shared]
public sealed class ClassModifierCodeFixProvider : CodeFixProvider
{
    public override ImmutableArray<string> FixableDiagnosticIds { get; } = ImmutableArray.Create(
        DiagnosticDescriptors.ExampleDiagnostic.Id);

    public override async Task RegisterCodeFixesAsync(CodeFixContext context)
    {
        System.Diagnostics.Debugger.Launch();
    }
}

[jnm2]

In fact, any diagnostics I report in the compilation end action behave this way:

context.RegisterCompilationEndAction(context =>
{
    foreach (var tree in context.Compilation.SyntaxTrees)
    {
        context.ReportDiagnostic(Diagnostic.Create(
            DiagnosticDescriptors.ExampleDiagnostic,
            Location.Create(tree, new(0, tree.Length))));
    }
});

While the same diagnostics reported in the symbol action work properly:

var didReport = 0;

context.RegisterSymbolAction(context =>
{
    if (Interlocked.Exchange(ref didReport, 1) == 1) return;

    foreach (var tree in context.Compilation.SyntaxTrees)
    {
        context.ReportDiagnostic(
            Diagnostic.Create(DiagnosticDescriptors.ExampleDiagnostic,
            Location.Create(tree, new(0, tree.Length))));
    }
}, SymbolKind.NamedType);

[jnm2]

Let me know if there's any trouble reproducing the behavior, and I can create a full solution or take traces.

[CyrusNajmabadi]

Tagging @mavasani . But i'm fairly certain this is a known limitation we have. Compilation-end analyzers are too expensive for us to run in the IDE, so they are just not run. This leads to this unfortunate outcome.

[jnm2]

Does that mean my only option (besides giving up) is to eagerly walk the entire compilation inside one of the other callbacks, just so I can report the diagnostic inside a callback where there can actually be a light bulb fix?

[jnm2]

But the fact is that diagnostics reported by the end action are shown in source and in the error list, so even if they are only gotten OOP, the IDE should be able to pass them to the code fix provider in-process.

[mavasani]

But i'm fairly certain this is a known limitation we have. Compilation-end analyzers are too expensive for us to run in the IDE, so they are just not run. This leads to this unfortunate outcome.

Yes, that is correct. Build-only/compilation end diagnostics only contribute to the build, not the code fix process. Otherwise, pressing Ctrl + Dot after every edit will have to analyze the whole project instead of just the current file. This was experimented in the past, and turned out to be un-dogfoodable.

[jnm2]

@mavasani Why is it necessary to run an analyzer to invoke the code fix provider, given that the diagnostic is already being shown in the source file and the light bulb was invoked on it?

[mavasani]

@jnm2 All analyzer diagnostics are potentially stale as soon as the user edits the code. Code fixes always get the up-to-date diagnostics for the latest source snapshot.

[jnm2]

I have two asks:

A) Can rerunning the analyzer be skipped before calling the code fix provider if the source text is identical to what was originally analyzed?

B) In the meantime, it looks like I'll have to analyze all operations in the compilation during some callback other than the compilation end action. I get how bad that sounds, but what's the least bad way to implement that now that I can't really use the compilation end action? Maybe I can cache information about source texts myself from analyzing previous versions of the compilation?

[mavasani]

Can rerunning the analyzer be skipped before calling the code fix provider if the source text is identical to what was originally analyzed?

Unfortunately, it cannot be skipped unless we can ensure that all of the files in the compilation and all of the references are identical, as any of this can cause compilation end diagnostics to change.

[jnm2]

What's the hardest part about tracking whether any source text or reference has changed?

Also, I've been coding my code fixes to assume diagnostics may be stale and recheck everything as part of the process of finding the pieces needed to do the fix. Is that not a responsibility that falls to code fix providers already? If VS is sort of helping them out in case they don't do proper staleness checks, what if VS only looks for changes in the source text that contains the diagnostic before deciding whether to rerun the analyzer?

[mavasani]

I've been coding my code fixes to assume diagnostics may be stale and recheck everything as part of the process of finding the pieces needed to do the fix. Is that not a responsibility that falls to code fix providers already?

No, the design has always been that code fixes will get up-to-date diagnostics for the document/solution snapshot handed to it.

What's the hardest part about tracking whether any source text or reference has changed?

Its probably not hard, but attempting to re-use diagnostics from previous snapshots has always been a non-goal. Compilation-end diagnostics have always been assumed to be build-only diagnostics, similar to compiler's unused field warning, it was never designed for full-fledged IDE support due to performance concerns.

[jnm2]

I see. Could you provide an API that lets my code fix provider say "I handle stale diagnostics, please don't rerun the analyzer before invoking me"? This could open up an opportunity for performance optimizations, too.

It's funny you mention the unused field warning. That must explain why there's always this duplication; it's how you are able to provide a light bulb fix. This doesn't seem like an ideal thing.

[jmarolf]

@mavasani could we have the engine instead be lazy? essentially dismiss compilation end diagnostics whenever the solution snapshot they come from is not the current one? That would mean the lightbulb item would go away as soonn as things are not up-to-date, but a user could still apply codefixes from the ide. As it stands today the only way someone could apply a codefix that does compilation-end-analysis is to use dotnet-format.

[CyrusNajmabadi]

I see. Could you provide an API that lets my code fix provider say "I handle stale diagnostics, please don't rerun the analyzer before invoking me"? This could open up an opportunity for performance optimizations, too.

This is interesting. I would be curious on your thoughts here Manish. Because a code fix always needs to reanalyze the code to recreate the state needed for a diagnostic, it seems interesting to say: i will try to recreate my state, knowing that that might fail in teh current snapshot of hte world, using teh older diag.

[jnm2]

FYI, Stephen Toub is proposing that dotnet/runtime ships exactly the same analyzer and fixer that I created which suffers from not being fixable due to the issue I reported above: dotnet/runtime#49944

[CyrusNajmabadi]

Design meeting notes 8/30/21:

1. If we want to enable FSA we'd need a full UX design on how to do this in a comprehensible way for users.
2. We could have a new concept of 'project scope' for running analyzers, where we'd run all analysis (including compilation-end) on the projects that have documents open.
3. Would likely need our UI to let user know this is substantially more expensive.
4. If we did this, would we have a new option on by default?
5. If it makes things worse for users, how would they know?
6. We're going to followup with platform to see what sort of perf notification/center they are working on. Any work we did here would likely need to tie into that for user visibility

--

We will continue talking to the platform team about how we ensure good visibility/discoverability around potentially high-impact perf changes.

[sharwell]

Duplicate of #1535