fix(typescript-estree): fix identifier tokens typed as Keyword

[lydell]

As in #681 and #750, the of keyword wasn't correctly handled in the parser with respect to variable names, so the token it created was typed as Keyword instead of Identifier.

Fixes lydell/eslint-plugin-simple-import-sort#33

[typescript-eslint]

Thanks for the PR, @lydell!

typescript-eslint is a 100% community driven project, and we are incredibly grateful that you are contributing to that community.

The core maintainers work on this in their personal time, so please understand that it may not be possible for them to review your work immediately.

Thanks again!

---

🙏 Please, if you or your company is finding typescript-eslint valuable, help us sustain the project by sponsoring it transparently on https://opencollective.com/typescript-eslint. As a thank you, your profile/company logo will be added to our main README which receives thousands of unique visitors per day.

[bradzacher]

LGTM.

Could you please also double check all of the other SyntaxKind.**Keyword types while you're at it?
Might as well make sure we don't need any more follow up PRs.

Looking at the TS source code; this is the list of keywords
(https://github.com/microsoft/TypeScript/blob/eeff036519362513cc86a95e260a281a725295f4/src/compiler/types.ts#L120-L535)

I think that these are the only ones we need to worry about:

        // Contextual keywords
        AbstractKeyword,
        AsKeyword,
        AssertsKeyword,
        AnyKeyword,
        AwaitKeyword,
        BooleanKeyword,
        ConstructorKeyword,
        DeclareKeyword,
        InferKeyword,
        KeyOfKeyword,
        NamespaceKeyword,
        NeverKeyword,
        ReadonlyKeyword,
        RequireKeyword,
        NumberKeyword,
        ObjectKeyword,
        StringKeyword,
        SymbolKeyword,
        UndefinedKeyword,
        UniqueKeyword,
        UnknownKeyword,
        FromKeyword,
        GlobalKeyword,
        BigIntKeyword,

Looking at the enum, there are also these, but I think that using these as identifiers results in syntax errors:

        // Reserved words
        BreakKeyword,
        CaseKeyword,
        CatchKeyword,
        ClassKeyword,
        ConstKeyword,
        ContinueKeyword,
        DebuggerKeyword,
        DefaultKeyword,
        DeleteKeyword,
        DoKeyword,
        ElseKeyword,
        EnumKeyword,
        ExportKeyword,
        ExtendsKeyword,
        FalseKeyword,
        FinallyKeyword,
        ForKeyword,
        FunctionKeyword,
        IfKeyword,
        ImportKeyword,
        InKeyword,
        InstanceOfKeyword,
        NewKeyword,
        NullKeyword,
        ReturnKeyword,
        SuperKeyword,
        SwitchKeyword,
        ThisKeyword,
        ThrowKeyword,
        TrueKeyword,
        TryKeyword,
        TypeOfKeyword,
        VarKeyword,
        VoidKeyword,
        WhileKeyword,
        WithKeyword,

        // Strict mode reserved words
        ImplementsKeyword,
        InterfaceKeyword,
        LetKeyword,
        PackageKeyword,
        PrivateKeyword,
        ProtectedKeyword,
        PublicKeyword,
        StaticKeyword,
        YieldKeyword,

[armano2]

in SyntaxKind there are special values that represents boundaries of node types that can be used to do range check

for example i think in this case we should do something like this instead (i'm not sure tho)

  if ('originalKeywordKind' in token && token.originalKeywordKind) {
    if (token.originalKeywordKind === SyntaxKind.NullKeyword) {
      return AST_TOKEN_TYPES.Null;
    } else if (
      token.originalKeywordKind > SyntaxKind.LastFutureReservedWord &&
      token.originalKeywordKind <= SyntaxKind.LastKeyword
    ) {
      return AST_TOKEN_TYPES.Identifier;
    }
    return AST_TOKEN_TYPES.Keyword;
  }

this will mark 30 not reserved keywords as Identifiers if used as such

[bradzacher]

This is the list of keywords which typescript allows as identifiers without throwing an error:

        // Strict mode reserved words
        implements,
        interface,
        let,
        package,
        private,
        protected,
        public,
        static,
        yield,
        // Contextual keywords
        abstract,
        as,
        asserts,
        any,
        async,
        await,
        boolean,
        constructor,
        declare,
        get,
        infer,
        is,
        keyof,
        module,
        namespace,
        never,
        readonly,
        require,
        number,
        object,
        set,
        string,
        symbol,
        type,
        undefined,
        unique,
        unknown,
        from,
        global,
        bigint,
        of,

repl

Which suggests the range you'd want to check instead is SyntaxKind.FirstFutureReservedWord to SyntaxKind.LastKeyword

I believe the check we want is this:

  if ('originalKeywordKind' in token && token.originalKeywordKind) {
    if (token.originalKeywordKind === SyntaxKind.NullKeyword) {
      return AST_TOKEN_TYPES.Null;
    } else if (
      token.originalKeywordKind >= SyntaxKind.FirstFutureReservedWord &&
      token.originalKeywordKind <= SyntaxKind.LastKeyword
    ) {
      return AST_TOKEN_TYPES.Identifier;
    }
    return AST_TOKEN_TYPES.Keyword;
  }

[lydell]

Pushed an update. Hopefully I got it right. Let me know what you think.

[bradzacher]

The code you've got LGTM.

---

Now that you've got me thinking about this:

const foo = { get bar() { return 1 } };
//            ^^^ should this be an Identifier token or a Keyword token?

Both babel-eslint and espree both label the above get as an Identifier token, but shouldn't it be a keyword? Super strange - I'm okay with us going with the consensus there.

[bradzacher]

I've raised eslint/espree#428 and I'll chat with the eslint team there, and we can follow up to correct the logic in a later PR if needs be.
Otherwise - this LGTM.

[armano2]

LGTM