fix: Cannot read property 'range' of null error with indent

[WaldoJeffers]

What is the purpose of this pull request? (put an "X" next to item)

• Documentation update
• Bug fix (template)
• New rule (template)
• Changes an existing rule (template)
• Add autofixing to a rule
• Add a CLI option
• Add something to the core
• Other, please explain:

Tell us about your environment

• ESLint Version: 5.16.0
• Node Version: 10
• npm Version: 6

What parser (default, Babel-ESLint, etc.) are you using?
@typescript-eslint/parser

Please show your full configuration:

Configuration

const { specifiedRules: allGraphQLValidators } = require('graphql');
const { compose, map, prop, without } = require('ramda');
// We need to disable these validators because it's how Apollo works
// See https://github.com/apollographql/eslint-plugin-graphql#selecting-validation-rules
const allGraphQLValidatorNames = compose(
  without(['KnownDirectives', 'NoUnusedFragments', 'KnownFragmentNames']),
  map(prop('name')),
)(allGraphQLValidators);

module.exports = {
  parser: '@typescript-eslint/parser',
  extends: [
    'plugin:@typescript-eslint/recommended',
    'airbnb',
    'plugin:jest/recommended',
    'plugin:cypress/recommended',
    'prettier',
    'prettier/react',
    'prettier/@typescript-eslint',
  ],

  env: {
    browser: true,
    node: true,
    es6: true,
    jasmine: true,
    'jest/globals': true,
    'cypress/globals': true,
  },

  plugins: [
    'react',
    '@shinetools/eslint-plugin-shine',
    '@typescript-eslint',
    'graphql',
    'jest',
    'cypress',
  ],

  globals: {
    cancelAnimationFrame: true,
    requestAnimationFrame: true,
    fetch: false,
    element: true,
    by: true,
    device: true,
    waitFor: true,
  },

  rules: {
    'react/prop-types': 'off',
    'import/no-unresolved': 'error',
    indent: 2,
    'eslint/no-unused-vars': 0,
    '@typescript-eslint/no-unused-vars': [
      'error',
      { ignoreRestSiblings: true },
    ],
    '@typescript-eslint/indent': ['error', 2],
    '@typescript-eslint/explicit-function-return-type': [
      'error',
      {
        allowTypedFunctionExpressions: true,
      },
    ],
    'jest/no-disabled-tests': 2,
    'react/no-unused-state': 2,
    camelcase: 0,
    'react/jsx-filename-extension': [
      1,
      {
        extensions: ['.tsx'],
      },
    ],
    'global-require': 0,
    'jsx-a11y/accessible-emoji': 0,
    '@shinetools/shine/no-native-text-component': 2,
    '@shinetools/shine/no-several-button-styles': 2,
    'react/sort-comp': 0,
    'react/jsx-sort-props': [
      2,
      {
        callbacksLast: false,
        shorthandFirst: false,
        shorthandLast: false,
        ignoreCase: true,
        noSortAlphabetically: false,
        reservedFirst: false,
      },
    ],
    'react/jsx-sort-default-props': [
      2,
      {
        ignoreCase: true,
      },
    ],
    'react/sort-prop-types': 2,
    'import/prefer-default-export': 0,
    'import/named': 'error',
    'import/default': 'error',
    'no-underscore-dangle': 0,
    'import/no-extraneous-dependencies': [
      'error',
      {
        devDependencies: [
          'scripts/**',
          '__e2e__/**',
          'jest/**',
          '**/*.test.ts?',
          'jest.setup.js',
          'webpack/**',
        ],
      },
    ],
    'graphql/template-strings': [
      'error',
      {
        env: 'literal',
        validators: allGraphQLValidatorNames,
      },
    ],
    'graphql/no-deprecated-fields': [
      'error',
      {
        env: 'literal',
      },
    ],
    'graphql/capitalized-type-name': [
      'error',
      {
        env: 'literal',
      },
    ],
    'graphql/named-operations': [
      'error',
      {
        env: 'literal',
      },
    ],
  },

  settings: {
    'import/resolver': {
      // use <root>/tsconfig.json
      typescript: {},

      // use <root>/path/to/folder/tsconfig.json
      typescript: {
        directory: '.',
      },
    },
    react: {
      version: 'detect',
    },
  },
};

What did you do? Please include the actual source code causing the issue.
Run prettier-eslint --fix on a file containing a TypeScript casting using the as keyword:

const a = 'hello' as string;

Very interestingly, manually running Prettier and then ESLint does not trigger the bug, I couldn't understand exactly why.

What did you expect to happen?
I expected ESLint to fix any errors I might have, after the file has been prettified.

What actually happened? Please include the actual, raw output from ESLint.
ESLint crashed with the following error:

prettier-eslint [ERROR]: eslint fix failed due to an eslint error
prettier-eslint-cli [ERROR]: There was an error formatting "index.tsx": 
    TypeError: Cannot read property 'range' of null
    Occurred while linting /index.tsx:1
        at SourceCode.getTokenAfter (/node_modules/eslint/lib/token-store/index.js:320:18)
        at Object.BinaryExpression, LogicalExpression [as listener] (/node_modules/eslint/lib/rules/indent.js:1081:55)
        at Program:exit.listenerCallQueue.filter.forEach.nodeInfo (/node_modules/eslint/lib/rules/indent.js:1554:55)
        at Array.forEach (<anonymous>)
        at Program:exit (/eslint/lib/rules/indent.js:1554:26)
        at listeners.(anonymous function).forEach.listener (/eslint/lib/util/safe-emitter.js:45:58)
        at Array.forEach (<anonymous>)
        at Object.emit (/eslint/lib/util/safe-emitter.js:45:38)
        at NodeEventGenerator.applySelector (/eslint/lib/util/node-event-generator.js:251:26)
        at NodeEventGenerator.applySelectors (/eslint/lib/util/node-event-generator.js:280:22)
failure formatting 1 file with prettier-eslint
error Command failed with exit code 1.

The issue has already been reported in different places, but it's hard to fully understand what causes the error. For example, babel/babel-eslint#530 (comment)

This apparently has already been fixed for some other rules (eg. #10938)

What changes did you make? (Give an overview)
When parsing an expression containing as, it's identifed as a BinaryExpression, but the operator is null. If that's the case, shortcut the process as it will lead to errors.

Is there anything you'd like reviewers to focus on?

[jsf-clabot]

All committers have signed the CLA.

[kaicataldo]

I know we're still evaluating whether we should land this or not, but we are unable to accept PRs that change behavior without corresponding tests. Do you mind adding some?

Using @typescript-eslint/parser in ASTExplorer, it looks like this should be parsed as a TSAsExpression node. Do you know why this code would affect that?

Edit: To clarify, looking at the spec, it doesn't look like a BinaryExpression without an operator is a valid node, and I want to make sure we're making changes in the right place!

[kaicataldo]

This comment is a bit confusing now because it looks like it's referring to this line.

[WaldoJeffers]

Hello @kaicataldo ,

Thanks for the time you took reviewing this.

I know we're still evaluating whether we should land this or not, but we are unable to accept PRs that change behavior without corresponding tests. Do you mind adding some?

I completely understand, I tried adding some, but didn't how to configure the parser to be @typescript-eslint/parser, so all my tests failed miserably :'(

Using @typescript-eslint/parser in ASTExplorer, it looks like this should be parsed as a TSAsExpression node. Do you know why this code would affect that?
Edit: To clarify, looking at the spec, it doesn't look like a BinaryExpression without an operator is a valid node, and I want to make sure we're making changes in the right place!
It definitely makes sense. My fix definitely feels like a sweep it under the rug fix, and we must make sure we are addressing the correct issue.

I tried configuring the live version of ASTExplorer, but couldn't manage to configure prettier to use typescript as a parser, although it seems to be an accepted value.

So I dug a bit more:
Here's the AST I get when running prettier and then eslint separately:

// prettier + eslint

{ type: 'BinaryExpression',
  operator: 'as',
  left:
   { type: 'Literal',
     raw: '\'hello\'',
     value: 'hello',
     range: [ 17, 24 ],
     loc: { start: [Object], end: [Object] },
     parent:
      { type: 'TSAsExpression',
        expression: [Circular],
        typeAnnotation: [Object],
        range: [Array],
        loc: [Object],
        parent: [Object] } },
  right:
   { type: 'TSStringKeyword',
     range: [ 28, 34 ],
     loc: { start: [Object], end: [Object] },
     parent:
      { type: 'TSAsExpression',
        expression: [Object],
        typeAnnotation: [Circular],
        range: [Array],
        loc: [Object],
        parent: [Object] } },
  parent:
   { type: 'VariableDeclarator',
     id:
      { type: 'Identifier',
        name: 'a',
        range: [Array],
        loc: [Object],
        parent: [Circular] },
     init:
      { type: 'TSAsExpression',
        expression: [Object],
        typeAnnotation: [Object],
        range: [Array],
        loc: [Object],
        parent: [Circular] },
     range: [ 13, 34 ],
     loc: { start: [Object], end: [Object] },
     parent:
      { type: 'VariableDeclaration',
        declarations: [Array],
        kind: 'const',
        range: [Array],
        loc: [Object],
        parent: [Object] } },
  range: [ 17, 34 ],
  loc:
   { start: { line: 1, column: 17 }, end: { line: 1, column: 34 } } }

Here's the AST I get when running prettier-eslint

// prettier-eslint

{ type: 'BinaryExpression',
  operator: 'as',
  left:
   { type: 'Literal',
     range: [ 17, 24 ],
     loc: { start: [Object], end: [Object] },
     raw: '\'hello\'',
     value: 'hello',
     parent:
      { type: 'TSAsExpression',
        range: [Array],
        loc: [Object],
        transformFlags: undefined,
        expression: [Circular],
        typeAnnotation: [Object],
        parent: [Object] } },
  right:
   { type: 'TSTypeAnnotation',
     loc: { start: [Object], end: [Object] },
     range: [ 26, 34 ],
     typeAnnotation:
      { type: 'TSStringKeyword',
        range: [Array],
        loc: [Object],
        parent: [Circular] },
     parent:
      { type: 'TSAsExpression',
        range: [Array],
        loc: [Object],
        transformFlags: undefined,
        expression: [Object],
        typeAnnotation: [Circular],
        parent: [Object] } },
  parent:
   { type: 'VariableDeclarator',
     range: [ 13, 34 ],
     loc: { start: [Object], end: [Object] },
     id:
      { type: 'Identifier',
        range: [Array],
        loc: [Object],
        name: 'a',
        parent: [Circular] },
     init:
      { type: 'TSAsExpression',
        range: [Array],
        loc: [Object],
        transformFlags: undefined,
        expression: [Object],
        typeAnnotation: [Object],
        parent: [Circular] },
     parent:
      { type: 'VariableDeclaration',
        range: [Array],
        loc: [Object],
        declarations: [Array],
        kind: 'const',
        parent: [Object] } },
  range: [ 17, 34 ],
  loc:
   { start: { line: 1, column: 17 }, end: { line: 1, column: 34 } } }

The difference seems to be how the right part is interpreted. I couldn't figure out exactly why, or what was wrong. But while I was adding console.log, I noticed the following line in prettier-eslint's source code

It intrigued me since:
1/ It seemed to override any parser I would have configured in my ESLint config
2/ I thought the TS parser was called @typescript-eslint/parser and not typescript-eslint-parser

So it seems typescript-eslint-parser has been deprecated and should not be used anymore.

In addition, they apparently fixed the brutal parser overriding in prettier-eslint. The fix was made 5 months ago, but is only available in prettier-eslint since v9+, which was released four days ago.

I tried it, and 🎉 : no more errors!

So I guess the problem came from typescript-eslint-parser. Now, I don't know if even with a faulty parser, it's intended that ESLint crashes with this quite confusing error, or if it should throw a more clear error message, or if it should fail silently (which can be dangerous, and is roughly what this PR does). I don't think this PR is necessary anymore, because it might mask other errors, but I am not sure about it, my understanding of ESLint is too limited. What do you think @kaicataldo ?

[kaicataldo]

Thanks for the detailed response. All that information is super helpful!

Since ESLint allows for custom parsers (and those parsers can do anything), we can’t guarantee that all core rules will work with them. I believe we have made changes in the past to prevent a rule from crashing with @typescript-eslint/parser, but since this is no longer happening in the newest version, I agree with you that we shouldn’t make any changes at this time.

Thanks for digging into this!

[WaldoJeffers]

Alright, I'm closing this now :)
For anyone else coming here from a Google search,
TL;DR, update prettier-eslint to v9+, it might solve your issue :)

Thanks again @kaicataldo