UnboundIdentifiers: 'nameof' does not exist in the current context.

[vsfeedback]

In a lambda expression. If I write a #warning directive, the nameof will be error with

CS0103 The name 'nameof' does not exist in the current context.

Test codes:

Action action = () =>
{
    var x = nameof(System);
#warning xxx
};

But there is an interesting phenomenon, the codes can be compile successed and run correctly.

In my test, this bug is appears in both of Visual Studio 2015 and Visual Studio 2017.

This issue has been moved from https://developercommunity.visualstudio.com/content/problem/71314/nameof-does-not-exist-in-the-current-context.html
VSTS ticketId: 454314
These are the original issue comments:

彭闽 on 6/21/2017, 11:43 PM (169 days ago):

Action action = () => {

var x = nameof(System);

#warning xxx

};

Dan J on 8/30/2017, 11:06 AM (99 days ago): I also have this happen in 15.3.3, does not prevent build from suceeding, but constantly having 20 errors makes it hard to find what is actually boken
These are the original issue solutions:
(no solutions)

[PMExtra]

Test Code:

Action action = () => {
    var x = nameof(System);
#warning xxx
};

[jcouv]

@heejaechang @mavasani
I can see that this diagnostic (ERR_NameNotInContext) is produced when binding the identifier nameof by itself (as opposed to the invocation nameof(System)) when the IDE call the semantic model on it (GetSymbolInfo on the identifier). But the semantic model logic properly discards that diagnostic.

Any idea where or how this diagnostic might surface to the IDE?
One thing I noticed is that this diagnostic only appears in the IDE if there is another (real) error in the code (such as the #warning or some other bad code).

Note this also repros in sharplab.

        [Fact]
        [WorkItem(23667, "https://github.com/dotnet/roslyn/issues/23667")]
        public void NameofNotFound()
        {

            var source = @"
class Program
{
    public static void Main()
    {
        System.Action action = () => {
            var x = nameof(System);
            System.Console.Write(x);
#warning xxx
        };
    }
}";
            var comp = CreateStandardCompilation(source);

            var tree = comp.SyntaxTrees.Single();
            var model = comp.GetSemanticModel(tree, ignoreAccessibility: false);
            var nameof = tree.GetRoot().DescendantNodes().OfType<InvocationExpressionSyntax>().ElementAt(0).Expression;

            var symbol = model.GetSymbolInfo(nameof);
// stepping through this code I can see that the diagnostic is produced, but it gets discarded (not sure how it reaches the IDE)
        }

[heejaechang]

@jcouv do you have some external plug in installed? we won't show blue squiggle by default. or do you have build error as blue squiggle internal option on? that is off by default, and hidden, you need to explicitly edit reg key to turn it on, did you do that?

[heejaechang]

@jcouv you can set break points here (all analyzeXXX methods)
http://source.roslyn.io/#Microsoft.CodeAnalysis/DiagnosticAnalyzer/CompilerDiagnosticAnalyzer.CompilationAnalyzer.cs,30

and then open the file that shows the error and check the diagnostics returned by those method. one of them should contain that error if it surface to IDE as long as it is us who report it not third party analyzers.

[jcouv]

I think the blue squiggles are a red herring. They do not appear on my machine, where I'm still able to repro the problem.

Thanks, I'll try debugging the method you mentioned.

[jcouv]

@heejaechang Thanks for the tip.
Unfortunately, I was not able to intercept this squiggle through CompilerDiagnosticAnalyzer. When I set breakpoints in the various Analyze methods and also log calls to the ReportDiagnostics method, I only see the other two diagnostics (shown as green squiggles), but not the error that produces red squiggles on nameof (ERR_NameNotInContext).

Let me know if you could take a look at it with me. I have a repro set up. Thanks

[heejaechang]

@jcouv let me take a look. I will get back to you tonight or tomorrow.

[heejaechang]

@jcouv the error is reported by "Microsoft.CodeAnalysis.CSharp.Diagnostics.CSharpUnboundIdentifiersDiagnosticAnalyzer" (http://source.roslyn.io/#Microsoft.CodeAnalysis.CSharp.Features/Diagnostics/Analyzers/CSharpUnboundIdentifiersDiagnosticAnalyzer.cs,23)

[jcouv]

Thanks a bunch @heejaechang .

I thought I had searched for "CS0103" in the entire codebase (because I remembered another instance of a similar confusion). I must have messed up :-(

Analyzers that produce compiler error codes are evil. What do you think of requiring analyzers to produce their own error codes?
I'll change this analyzer and try to put a guard against this happening again.

[heejaechang]

tagging @jinujoseph @Pilchie for "What do you think of requiring analyzers to produce their own error codes?"

[jinujoseph]

cc @mavasani did not you had a some thing similar in analyzer repo

[Pilchie]

I think that's what we generally expect and desire, but I'm not sure how well we can enforce it. At best we can enforce that a Workspace doesn't have multiple providers that produce the same error code.

On the other hand, one could imagine an author deliberating separating different reporting of the same "error" into multiple providers (a fast one and slow one, etc).

[heejaechang]

I think for this particular case, IDE intentionally produced diagnostic with compiler error code to handle some corner error case I believe. question is, can we just use IDE own error code and change fixer to look for that error as well? rather than pretend error is from compiler?
...

by the way, currently, what "CSharpUnboundIdentifiersDiagnosticAnalyzer" does is supported scenario. we even designed for it (diagnostic analyzer and fixer being loosely coupled). but in real life, if a diagnostic analyzer produces a diagnostic at random location with an error code existing fixer claims they can handle, most likely, the fixer will fail (throw exception). so this loosely coupled thing doesn't work as much as we hoped for. so reasoning to share same error code is not as good as we thought it would be.

[mavasani]

I think following are guidelines:

1. Different analyzers in same compilation shouldn't use duplicate IDs - that is very like a bug.
2. Different analyzers in different compilation may use the same ID - this may be a bug, but could also very likely be a case where different analyzers report similar issues for different constructs OR have different implementation of similar diagnostic. We can probably consider reporting a host diagnostic when we see different analyzers in different packages reporting same diagnostic.
3. Analyzers reporting CS or BC diagnostics - this is normally done to augment compiler diagnostics where analyzer feels compiler is missing reporting some cases and the author wants FixAll to fix all the compiler and augmented diagnostics. @ivanbasov faced a similar issue where we compiler was not reporting unused local diagnostics for local functions. We can consider prohibiting this by writing an analyzer in Microsoft.CodeAnalysis.Analyzers that flags any analyzer reporting diagnostic with CS or BC prefix. User will have to perform two FixAll operations to fix compiler and analyzer diagnostics separately, but that seems acceptable to me.

[mavasani]

We also recently added an analyzer diagnostic in Microsoft.CodeAnalysis.Analyzers for case 1. above: https://github.com/dotnet/roslyn-analyzers/blob/26fe065161a871718c717da1821d82c4decbc8e9/src/Microsoft.CodeAnalysis.Analyzers/Core/CodeAnalysisDiagnosticsResources.resx#L296

  <data name="UseUniqueDiagnosticIdMessage" xml:space="preserve">
    <value>Diagnostic Id '{0}' is already used by analyzer '{1}'. Please use a different diagnostic ID.</value>
  </data>

[heejaechang]

@mavasani hmmm this (#23667 (comment)) is not actually the way we originally designed it for. and coded for.

probably in real life, what you wrote it true though. so, I think we need to make up our mind on this issue

tagging @jinujoseph @Pilchie @dotnet/roslyn-analysis

[heejaechang]

"We can consider prohibiting this by writing an analyzer in Microsoft.CodeAnalysis.Analyzers that flags any analyzer reporting diagnostic with CS or BC prefix. User will have to perform two FixAll operations to fix compiler and analyzer diagnostics separately, but that seems acceptable to me."

this is interesting. for fix all, do we only care about equivalent key or (error code + equivalent key)?
..
just looked code. fix all can handle multiple diagnostics ID at the same time as long as it can be fixed by 1 fixer and code action returns same equivalent key. so, I think fix all will work as well.

[jcouv]

For preventing analyzers from producing "CS..." diagnostics, I was thinking of adding a debug assertion in the `AnalyzerExecutor.IsSupportedDiagnostic". It already has logic to distinguish the compiler analyzer from the rest.

[mavasani]

For preventing analyzers from producing "CS..." diagnostics, I was thinking of adding a debug assertion in the `AnalyzerExecutor.IsSupportedDiagnostic".

Nice idea @jcouv! I would recommend going a step further and instead of adding a debug assert, we throw an ArgumentException with invalid diagnostic ID, just like we do for supported diagnostics with invalid identifier as an ID (see here). We probably want a slightly different message then CodeAnalysisResources.InvalidDiagnosticIdReported

[CyrusNajmabadi]

Ideally CSharpUnboundIdentifiersDiagnosticAnalyzer wouldn't even exist. It's a hacky workaround to deal with the issue of the compiler not reporting errors in some cases. I tried to remove it a while back, but ran into several scenarios (i think around lambdas) where we wouldn't get appropriate compiler errors.

[CyrusNajmabadi]

I think a better solution, in the future, woudl be to have the compiler emit these diagnostics, but convey that they dont' need to be shown by default in a UI (like an error list, console, or squiggle). They would still convey to fixers that the issue is there, but they wouldn't cause cascading error-hell in the UI.

[SylwesterZarebski]

Error is still in 15.8.4. Is there any roadmap to fix it?

[jaredpar]

@SylwesterZarebski this bug is currently targeted at 16.0 for the earliest fix.

[SylwesterZarebski]

Thanks for information. :-)

[nnpcYvIVl]

This is still an issue in Visual Studio 2019 Preview 1.

[mavasani]

@jcouv Should we consider taking #24071 as a workaround until we make progress on this issue? No reason to block the user experience on implementation details, especially given this IDE analyzer already exists.

[jcouv]

Yes, it sounds like a good idea at this point.
You can link the IDE change to this issue if it needs to be undone once the compiler layer is fixed.

[jasonmalinowski]

How much work does it just take to fix the compiler layer?

[mavasani]

How much work does it just take to fix the compiler layer?

@jcouv can provide the cost. However, given that this bug was filed precisely a year ago, with multiple customer complaints since then, IMO we should take the fix to the existing IDE analyzer in Preview2 unless the compiler can fix this in Preview2.

[CyrusNajmabadi]

However, given that this bug was filed precisely a year ago, with multiple customer complaints since then,

I don't see how the age of the bug is relevant here :D. We have had several bugs over hte years that stem from the same root cause. Some of them are likely 5+ years old. We've been ok in that time accepting our current behavior, so i'd actually argue that that indicates we can accept the status quo. If we were to invest time into this, i'd rather us invest at the right level, and deal with all the issues at once, instead of continually trying to paper over the root cause with complex and wonky solutions at the IDE level :)

--

In other words, this hasn't been significant enogh to have to deal with up till now. So why did it become critical for preview 2 to have a solution now?

[CyrusNajmabadi]

Enh. I've changed my mind. If it's cheap and easy, i'm ok moving forward. I do really wish that we'd just fix the underlying compiler issue though. This is annoying and has been problematic for quite a while. And it just doesn't seem that difficult to address in a reasonable fashion.

[mavasani]

If we were to invest time into this, i'd rather us invest at the right level

The reason I was proposing taking the fix to the IDE analyzer is that we already have a PR for the fix, so there is no further investment for it from us, but with noticeable customer benefit. I agree with your concerns about the core issue slipping further down the radar with the workaround, but that does not justify continuous pain for the customers :-)

[CyrusNajmabadi]

@mavasani Sounds good to me :) A little bummed that we keep dragging this code along. But i agree that in terms of costs and outcomes, it's a very reasonable approach to take now.

[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 this issue open to track compiler support for correctly reporting errors within a lambda.

[jcouv]

@mavasani This issue got close automatically. It would probably be good to file a new issue for the compiler (possibly with a new repro?).

[mavasani]

Thanks @jcouv. Filed #32628 to track the compiler feature request and #32629 to follow it up deleting this IDE analyzer.