roslyn: SyntaxGenerator.NameOfExpression generates node with incorrect contextual kind

That is expected behavior

I do not agree, and I am not sure why that is an expected behavior. Are you suggesting that any syntax node generated out of SyntaxGenerator needs the entire source file to be reparsed to ensure no errors? I donot believe that was the original goal of SyntaxGenerator.

Additionally, I don’t understand how SyntaxGenerator.NameOfExpression generating an identifier token with incorrect syntax kind (SyntaxKind.Identifier instead of SyntaxKind.NameOfKeyword) is by design.

The same situation applies for var.

Note: ‘var’ is not a contextual keyword. The parser does not produce any special nodes for it (same with ‘dynamic)’. They are true identifiers that just have special meaning during binding if they didn’t bind to a preexisting symbol. That’s not the same with things like “nameof” where the lexer will do the following:

                        else if (SyntaxFacts.IsContextualKeyword(info.Kind))
                        {
                            info.ContextualKind = info.Kind;
                            info.Kind = SyntaxKind.IdentifierToken;
                        }

While reparsing is truly necessary to understand the real result that will happen from making the change, many parts of our system do just do a series of tree transformations, knowing that they should get reasonable intermediary results. For example, that’s how simplification and what-not work.

In this case, as this is SyntaxGenerator, and it’s literally bein asked to make a NameOfExpression, it really should make a NameOfExpression that is equivalent to what the parser woudl make.

Also, in a perfect world, there would be no SyntaxKind.NameOfKeyword (like there is no SyntaxKind.VarKeyword/DynamicKeyword). However, that would be a breaking change for certain. But we could put documentation on this that says “this was a mistake. you won’t ever get this. our bad.”

This feels super confusing!

Here’s the important thing: ContextualKind is really an internal detail. We actually removed it from the public surface area a long time ago as it was too confusing to have both. really, it’s just used to allow the compiler to do fast checks of identifiers without needing to do string checks.

However, we also wanted a way for people to ask if something was a keyword or not, and we wanted to separate out contextual keywords as the language understand them to be a different concept. So we have that IsContextualKeyword helper.

–

Now, nameof is in a special position. It really is just an identifier. but the compiler wanted that fast check. So it abuses the fact that the internal .ContextualKind is NameofKeyword. This is extremely weird as the user can never really observe a token whose kind is NameofKeyword!!.

Honestly, the most appropriate thing to do would be to just have the compiler check for the literal text “nameof”, like it looks for the literal text ‘var’ or ‘dynamic’:

        internal static bool IsVar(this Syntax.InternalSyntax.SyntaxToken node)
        {
            return node.Kind == SyntaxKind.IdentifierToken && node.ValueText == "var";
        }

i would also approve such a change at the binder level. It really shouldn’t affect perf. And it would be much more in line with how we do everything else.

@sharwell This is most certainly a bug. Our nameof fixer uses this API here to generate a nameof(x) node in place of string literal "x", where x is a valid parameter name, and this causes the new compilation with the code fix application in the unit test framework to generate CS0103 The name 'nameof' does not exist in the current context.. Note that the code fix works fine in IDE because the IDE will reparse the fixed text, at which point the compiler will use the correct syntax kind (SyntaxKind.NameOfKeyword). See https://github.com/dotnet/roslyn-analyzers/issues/1364 for details.