Bug: no-restricted-exports: Cannot read property 'name' of null

[AriPerkkio]

Environment

Node version: v14.17.4 and 16 LTS
npm version: 6.14.14, yarn 1.22.10
Local ESLint version: 8.3.0
Global ESLint version: none
Operating System: Debian 10

What parser are you using?

@typescript-eslint/parser

What did you do?

Configuration

<!-- Paste your configuration here -->
{
  "root": true,
  "parser": "@typescript-eslint/parser",
  "parserOptions": {
    "ecmaVersion": 2020,
    "sourceType": "module"
  },
  "rules": {
    "no-restricted-exports": "error"
  }
}

{
  "scripts": {
    "lint": "eslint test.js"
  },
  "devDependencies": {
    "@typescript-eslint/parser": "5.4.0",
    "eslint": "8.3.0",
    "typescript": "4.5.2"
  }
}

<!-- Paste your code here -->
class Example {}
export class extends Example {}

What did you expect to happen?

The export class extends should not crash the rule. It should rather report the rule if any problems are detected.

What actually happened?

$ eslint test.js

Oops! Something went wrong! :(

ESLint: 8.3.0

TypeError: Cannot read property 'name' of null
Occurred while linting /workspaces/eslint-demo-repro/test.js:3
Rule: "no-restricted-exports"
    at checkExportedName (/workspaces/eslint-demo-repro/node_modules/eslint/lib/rules/no-restricted-exports.js:51:31)
    at ExportNamedDeclaration (/workspaces/eslint-demo-repro/node_modules/eslint/lib/rules/no-restricted-exports.js:74:25)
    at ruleErrorHandler (/workspaces/eslint-demo-repro/node_modules/eslint/lib/linter/linter.js:966:28)
    at /workspaces/eslint-demo-repro/node_modules/eslint/lib/linter/safe-emitter.js:45:58
    at Array.forEach (<anonymous>)
    at Object.emit (/workspaces/eslint-demo-repro/node_modules/eslint/lib/linter/safe-emitter.js:45:38)
    at NodeEventGenerator.applySelector (/workspaces/eslint-demo-repro/node_modules/eslint/lib/linter/node-event-generator.js:297:26)
    at NodeEventGenerator.applySelectors (/workspaces/eslint-demo-repro/node_modules/eslint/lib/linter/node-event-generator.js:326:22)
    at NodeEventGenerator.enterNode (/workspaces/eslint-demo-repro/node_modules/eslint/lib/linter/node-event-generator.js:340:14)
    at CodePathAnalyzer.enterNode (/workspaces/eslint-demo-repro/node_modules/eslint/lib/linter/code-path-analysis/code-path-analyzer.js:795:23)
error Command failed with exit code 2.
info Visit https://yarnpkg.com/en/docs/cli/run for documentation about this command.

Participation

• I am willing to submit a pull request for this issue.

Additional comments

Crash report: AriPerkkio/eslint-remote-tester#315 (comment)

This does not happen with default espree parser. This is likely caused by typescript-eslint/typescript-eslint#1852 but should be handled by the rule.

[nzakas]

The JavaScript code is invalid. You can’t have “export class extends” because exports require a name. I’ve tested Acorn, Babel, and Espree, and they all throw a syntax error on your example code.

typescript-eslint should be throwing a parsing error before it ever gets to the rule, but it actually looks like TypeScript itself is not throwing an error, so typescript-eslint is just converting what it gets back. The true source of the error appears to be TypeScript.

@bradzacher

[bradzacher]

we currently throw almost no errors during parsing - typescript-eslint/typescript-eslint#1852

typescript doesn't throw error because its parser is intentionally "forgiving" so that it can work in an IDE where you need a parser that can recover to provide a good UX (eg so a parser error earlier doesn't break all autocomplete, etc for the entire file).

[nzakas]

Yikes okay, that seems like a problematic behavior. The problem from the ESLint side is that we assume a valid AST is presented. If we need to start assuming that the AST may be invalid, that will affect a ton of rules and become a maintenance nightmare. We just can’t programmatically check for validity in each rule without adding a lot of overhead and headaches.

Would it make sense for typescript-eslint to throw during transforming the TypeScript AST into ESTree format?

[bradzacher]

Would it make sense for typescript-eslint to throw during transforming the TypeScript AST into ESTree format?

Yes, see the issue I mentioned. It's just going to take time and effort to build that - two things we're short on.

If we need to start assuming that the AST may be invalid, that will affect a ton of rules and become a maintenance nightmare.

It's worth noting that this is how it has always been since the parser's creation (the code you originally wrote back in 2015!).

This is not a problem in the real world because these "invalid" states are transient - people write broken code and then they correct it straight away. People don't report issues for these sorts of crashes because they fix themselves when they fix their broken code. In particular for TS - they won't be able to build their code without fixing it, so people have to fix it!

In this instance @AriPerkkio maintains a tool which is sort of "crowd-sourced" testing of ESLint rules on real-world code. https://github.com/AriPerkkio/eslint-remote-tester

This "bug" report is essentially telling you that there is code out there on github that is broken, and running ESLint over that broken code produces a crash in a lint rule.

---

TL;DR - you can close this issue. It's not actually a problem you need to worry about.

[AriPerkkio]

[..] people write broken code and then they correct it straight away. People don't report issues for these sorts of crashes because they fix themselves when they fix their broken code.

In similar previous reports I've seen people complaining how IDE's linter integrations keep crashing while a new line is being written. However these were pretty old comments and such IDE plugins have improved a lot since. I think these are the only cases these crashes will affect.

If we need to start assuming that the AST may be invalid, that will affect a ton of rules and become a maintenance nightmare. We just can’t programmatically check for validity in each rule without adding a lot of overhead and headaches.

I was expecting this to be fixed on a rule level but this is valid point. Let's close this issue as everything works as expected from ESLint's side.