[no-non-null-asserted-optional-chain] Possible Infinite Loop

[hjkcai]

• I have tried restarting my IDE and the issue persists.
• I have updated to the latest version of the packages.
• I have read the FAQ and my problem is not listed.

Repro

{
  "rules": {
    "@typescript-eslint/no-non-null-asserted-optional-chain": "error"
  }
}

let foo1: { bar: { baz: string } } | undefined;

console.log(foo1?.bar!.baz);

Expected Result

ESLint reports error.

Actual Result

Infinite Loop.

Additional Info

I am using ESLint programmatically:

import { CLIEngine, Linter } from 'eslint';
const cli = new CLIEngine({});
cli.executeOnFiles('/path/to/the/file.ts');

But the script never ended. However this problem was not present in VSCode ESLint Plugin. I have no idea about what's happening. I am not sure the above code can reproduce the problem (because it is extracted from a large codebase).

I used the debugger to pause when infinite loop occurred. It ended up here:

https://github.com/typescript-eslint/typescript-eslint/blob/master/packages/eslint-plugin/src/rules/no-non-null-asserted-optional-chain.ts#L116

There is an obvious logical error: the break only breaks the switch statement, meanwhile current.optional is truthy, meaning current is truthy. Theoretically THE LOOP WILL NEVER STOP WHEN A OPTIONAL CHAIN IS FOUND.

while (current) {
  switch (current.type) {
    case AST_NODE_TYPES.MemberExpression:
      if (current.optional) {
        // found an optional chain! stop traversing
        break;
      }

      current = current.object;
      continue;

    case AST_NODE_TYPES.CallExpression:
      if (current.optional) {
        // found an optional chain! stop traversing
        break;
      }

      current = current.callee;
      continue;

    default:
      // something that's not a ChainElement, which means this is not an optional chain we want to check
      return;
  }
}

Versions

package	version
@typescript-eslint/eslint-plugin	4.1.0
@typescript-eslint/parser	4.1.0
TypeScript	3.7.3
ESLint	6.8.0
node	12.18.2

[bradzacher]

We have this exact code as a passing test case, so I'm not sure why you're encountering an infinite loop.

typescript-eslint/packages/eslint-plugin/tests/rules/no-non-null-asserted-optional-chain.test.ts

Line 21 in 762bc99

'foo?.bar!.baz;',

The only reason I could see is because you're on an old version of TS, which triggers that code path.

Though I'm not sure why nobody else has encountered this?

Could you provide the full config you're using?

[hjkcai]

Thanks for reply. I will try to make a minimal reproduction.

BTW, the possible infinite loop is still exist in the code path. Maybe we need to figure out what triggers it.

[bradzacher]

Our tests only run on the latest TS version because nobody's spent the time to investigate / build out infra for multi-version testing.

Hence the passing test.

You're running TS 3.7, which uses a different code-path to the latest TS version (they made a breaking change to non-null asserted optional chains in TS 3.9).

Though yeah I'm not sure why nobody else as run into an infinite loop.

[hjkcai]

Yes. TS 3.7 causes the problem. I opened an individual project using:

{
  "dependencies": {
    "@typescript-eslint/eslint-plugin": "^4.1.1-alpha.7",
    "@typescript-eslint/parser": "^4.1.1-alpha.7",
    "eslint": "^7.8.1",
    "typescript": "^3.7.5"
  }
}

{
  "compilerOptions": {
    "target": "es2018",
    "strict": true
  }
}

module.exports = {
  parser: '@typescript-eslint/parser',
  parserOptions: {
    project: './tsconfig.json'
  },
  plugins: ['@typescript-eslint'],
  rules: {
    "@typescript-eslint/no-non-null-asserted-optional-chain": "error"
  }
}

// test.ts
let foo1: { bar: { baz: string } } | undefined;

console.log(foo1?.bar!.baz);
foo1?.bar!.baz

Executing eslint never stops.

npx eslint test.ts

The problem is solved after upgrading TypeScript to 3.9

I will upgrade my project to a higher version of TypeScript firstly. Please feel free to take time to find out what's broken 😄

[tadhgmister]

It may be desirable to add an internal forceUsing3.7Logic option so testing can hit the old logic. currently that whole section is untested.

[bradzacher]

I'd rather implement a full solution for testing old TS versions rather than adding per-rule hacks.
See discussion in #1752

[JoshuaKGoldberg]

#5915 is merged into #5886; closing this issue now accordingly. It'll be released as a part of the v6 major release.