Contextual keyword tokens being parsed as Keyword instead of Identifier

[bradzacher]

acornjs/acorn#902

In #1487 we made a change to mark all keywords output as Keyword tokens.
I've since learned that this was incorrect for some of them.

There are keywords which are classed as "contextual keywords". These keywords are keywords when they're in a their keyword position, and identifiers when they're in an identifier position.

So we need to either:

• provide context to the token parser so it can create the correct token, or
• always parse them as Identifier tokens (this is the route that espree/acorn takes)

The list of contextual keywords:
https://github.com/microsoft/TypeScript/blob/eeff036519362513cc86a95e260a281a725295f4/src/compiler/types.ts#L255-L285

[anikethsaha]

did you mean they should be parsed as keywords?

I think currently they are being parsed as an identifier only !

cause in these lines https://github.com/typescript-eslint/typescript-eslint/blob/master/packages/typescript-estree/src/node-utils.ts#L463-L464 , the numbers for contextual keywords comes along

token.originalKeywordKind >= SyntaxKind.FirstFutureReservedWord && // 113
  token.originalKeywordKind <= SyntaxKind.LastKeyword; // 152

and I think the number range for contextual keyword will be

// contextual keywords
token.originalKeywordKind >= SyntaxKind.AbstractKeyword && // 122
  token.originalKeywordKind <= SyntaxKind.OfKeyword; // 152

Am I missing something ?

[bradzacher]

they are contextual keywords.
This means that they are keywords when they are in a keyword position, and they are identifiers when they're not.

For example, of:

const of = 1;
//    ^^ Identifier
for (const x of y) {}
//           ^^ Keyword

The parser should pass around the contextual information when constructing the Token so it can determine if the contextual keywords should be a Keyword or Identifier token

[anikethsaha]

but we cant really do this right ?

const await = 1;

or as in TS, with types we can evaluate that ?

var await: string; // identifier
var await: Promise; // eslint error ? 

[bradzacher]

Note that types have nothing to do with it - it's purely about the contextual location of the token - where it is syntactically located.

var await: string;
//  ^^^^^ Identifier
var await: Promise;
//  ^^^^^ Identifier
var of = await Promise.resolve();
//  ^^ Identifier
//       ^^^^^ Keyword

await is a contextual keyword, so you are allowed to use it as an identifier, even within an async function.
However, all parsers error on it to keep things simple and predictable, but there's nothing actually wrong with the code.

The reason parsers error on it is because you can write some really weird and unpredictable code if it's allowed:

const await = 1;
await(await); // what is this supposed to do?!?!?!?!?

[anikethsaha]

ok. got it.
I will try to fix it. not familiar with typescript-estree's code though. will take a look.
btw, typescript-estree and parser in packages both are parser ? or typescript-estree is using parser under the hood.
a bit confused as estree is spec right ?

[bradzacher]

I wouldn't worry about fixing it right now. This is very low priority, because there's currently no parsers that get this right.

typescript-estree is a package which provides tooling for:

• parsing typescript code and producing an ESTree compatible AST.
• producing a mapping from that AST back to the TypeScript types.

parser is an ESLint parser, which uses typescript-estree under the hood.

Most people just use parser, but some don't need the specific API it provides, so they directly use typescript-estree (i.e. prettier uses typescript-estree directly)

[anikethsaha]

Got it .
thanks for the input 👍

[bradzacher]

I'm going to close this - as much as I would love us to do this we'd be breaking compatibility with other parsers, which could cause unintended rule breakages in the ecosystem for no reason other than "just because it's cleaner".

Speaking to other parser owners - they can't do this easily because they don't have contextual information about the tokens when they're being created - which is why they assume Keyword always.