Handle nameof expressions in UnboundIdentifiersDiagnosticAnalyzerBase

[mavasani]

Addresses #23667

Ask Mode template

Customer scenario

nameof is incorrectly squiggled in a lambda with diagnostics with the following error:
Error IDE1007 The name 'nameof' does not exist in the current context.. Note that the code compiles without errors and the IDE experience is very confusing.

Bugs this fixes

#23667

Workarounds, if any

Ignore the incorrect IDE1007 errors

Risk

Low risk, we are special casing nameof expressions in the unbound identifiers analyzer

Performance impact

Low

Is this a regression from a previous update?

No

Root cause analysis

Analyzer was assuming each identifier name should bind to a symbol for successful bind. This is not true for nameof, which is represented as an IdentifierNameSyntax node in C#, but GetSymbolInfo returns no symbols for it.

How was the bug found?

Customer reported

Test documentation updated?

N/A

[jasonmalinowski]

@mavasani Looking at the terribly long conversation in that bug, why didn't we conclude that the compiler should just report this directly and we get rid of the UnboundIdentifiersDiagnosticAnalyzerBase? I have yet to meet somebody who doesn't want that thing to go away. 😄

[mavasani]

@jasonmalinowski The analyzer handles nodes within broken lambdas and incomplete members - I presume the compiler doesn't want to bind them for perf reasons, but I will ask the compiler team for their views on handling these cases.

[mavasani]

Closing this PR as per #23667 (comment) cc @jcouv

[mavasani]

@jasonmalinowski @jcouv can you please re-review this PR as per #23667 (comment)?

[CyrusNajmabadi]

Is there a reason we aren't just fixing this at the compiler layer?

[mavasani]

Is there a reason we aren't just fixing this at the compiler layer?

The compiler feature request has been stalled for quite a while now, and the underlying bug has been reported by multiple customers now. I see no reason to block the user experience on our implementation details, especially given this IDE analyzer already exists.

[CyrusNajmabadi]

I guess my take is this: given that customers are hitting this, that indicates we should un-stall the root problem. If we fix on the IDE side, that's just going to remove the motivation to ever really fix this at the root level, and we'll continue being in teh state where the IDE is simulating (poorly) that it is the compiler.

Note: we've gotten a wide range of bugs over the years with the lack of compiler support here being the root cause. Rather than fix each one in a piecemeal fashion, i'd rather the compiler just analyze these guys normally.

[mavasani]

We discussed this at the last design meeting and it was decided that we take the fix to the analyzer for 16.0, and keep the underlying issue open to track compiler support for correctly reporting errors within a lambda.

Tagging @jcouv @jasonmalinowski @dotnet/roslyn-ide for review - this PR itself is very trivial

[mavasani]

Thanks @jasonmalinowski. @jcouv can you take a quick look please?

@jinujoseph @vatsalyaagrawal for approval

[jcouv]

LGTM Thanks

[mavasani]

@jaredpar @dotnet/roslyn-infrastructure Linux_Test coreclr seems to have gotten abandoned, and multiple retry attempts lead to it getting immediately abandoned again. Known issue?