Aspnetcore: FullyQualifyNamespace light bulb does not appear in certain contexts

[TanayParikh]

This issue is a branch off of the work in #19449 however it addresses a seperate underlying issue highlighted by @ajaybhargavb (link).

Reproduce:

1. dotnet new blazorserver
2. Open Counter.razor
3. In the IncrementCount() method add the following below the currentCount++; line: RenderTree.

Invoke the FullyQualifyNamespace light bulb
Expected:

    private void IncrementCount()
    {
        currentCount++;
        RenderTree // code action for Microsoft.AspNetCore.Components.RenderTree provided
    }

Actual:

    private void IncrementCount()
    {
        currentCount++;
        RenderTree // code action for Microsoft.AspNetCore.Components.RenderTree (erroneously) not provided
    }

[TanayParikh]

I've isolated this issue and have been able to reproduce it in a vanilla MVC project. Inspecting the omnisharp-vscode codebase, we don't seem to manipulate the context.diagnostics which contain the compiler errors.

https://github.com/OmniSharp/omnisharp-vscode/blob/master/src/features/codeActionProvider.ts#L29

This indicates dotnet/roslyn itself isn't providing the underlying CS0246 compiler error in certain situations. I've created an issue there with repro-repo and demo video.

https://github.com/dotnet/roslyn/issues/42974

[TanayParikh]

https://github.com/dotnet/roslyn/issues/42974#issuecomment-607031161

Given this, we can either keep the behavior as is, or remove the filtering we do for the code actions shown to the user:

https://github.com/dotnet/aspnetcore-tooling/blob/master/src/Razor/src/Microsoft.AspNetCore.Razor.VSCode/src/CodeActions/RazorFullyQualifiedCodeActionTranslator.ts#L29-L32

Are there any preferences for this? @ryanbrandenburg @NTaylorMullen @ajaybhargavb

[NTaylorMullen]

Are there any preferences for this? @ryanbrandenburg @NTaylorMullen @ajaybhargavb

Really thorough investigation, nice job Tanay! We need to keep the filtering as-is because we don't support every light bulb diagnostic. Given this is "by design" from Roslyn's perspective we want to treat it the same as well. I'd close this issue as by-design.

[ryanbrandenburg]

Secret option number 3 might be to just add this new CS-code that we've verified to the list of allowable codes on that Translator. That way this scenario works as expected but we also aren't allowing untested CodeActions through.

[TanayParikh]

@ryanbrandenburg we'd have to allow either:

CS0103 "The name 'RenderTree' does not exist in the current context [blazorserver]"
OR
CS1002 ; expected [blazorserver]"

in addition to the existing CS0246 missing namespace compiler error. In my opinion both CS0103, CS1002 may be a bit too general and we could end up with code actions in unanticipated situations. This is under the assumption the suggested code action also passes the secondary filtering logic for fully qualified namespace:

https://github.com/dotnet/aspnetcore-tooling/blob/master/src/Razor/src/Microsoft.AspNetCore.Razor.VSCode/src/CodeActions/RazorFullyQualifiedCodeActionTranslator.ts#L42-L51

I think it may be fine to leave our logic as is, to maintain feature parity with Roslyn if you're okay with that?

[ryanbrandenburg]

Sounds totally reasonable, just wanted to give it as an options.

[TanayParikh]

Thanks for the feedback! Closing out as "by design" in Roslyn https://github.com/dotnet/roslyn/issues/42974#issuecomment-607031161.