no-manual-cleanup: Cannot read property 'match' of undefined

[AriPerkkio]

Hello, no-manual-cleanup rule seems to crash in certain cases. This issue was spotted by automated CI run - it is not blocking my development or anything. ESlint rules should not crash in any condition since this makes all valid linting problems disappear. If this is a false flag please let me know.

https://github.com/AriPerkkio/eslint-remote-tester/runs/1551964310?check_suite_focus=true

Complete list of dependencies and .eslintrc is available at CI runs logs, steps Run yarn list | grep eslint and Run yarn log --config ./plugin-configs/eslint-plugin-testing-library.config.js.

eslint-plugin-testing-library@3.10.1

rules: {
    'testing-library/no-manual-cleanup': 'error',
}

This minimal repro doesn't exactly look like a complete case. I think this should still be considered as a bug. Users will run into this while having the rule enabled and typing new import statements above function declaration. It is not desired to have linter crashing while typing.

Minimal repro:

// Start typing new import statement above function declaration
import
function App() {}

// Start typing new import statement above function declaration
import A from
function App() {}

Crash reports from real projects

Rule: no-manual-cleanup

• Message: Cannot read property 'match' of undefined Occurred while linting <text>:3
• Path: wesleyclzns/Perdita/perdita-app/src/componentes/App.js
• Link

import React from 'react';
import Pidex from './Pidex'
import
function App() {
  return (
    <div className="App">
      <header className="App-header">
       <h1>Pertida Index</h1>

TypeError: Cannot read property 'match' of undefined Occurred while linting <text>:3
    at ImportDeclaration(/home/runner/work/eslint-remote-tester/eslint-remote-tester/ci/node_modules/eslint-plugin-testing-library/rules/no-manual-cleanup.js:47:59)
    at /home/runner/work/eslint-remote-tester/eslint-remote-tester/ci/node_modules/eslint/lib/linter/safe-emitter.js:45:58
    at Array.forEach (<anonymous>)
    at Object.emit (/home/runner/work/eslint-remote-tester/eslint-remote-tester/ci/node_modules/eslint/lib/linter/safe-emitter.js:45:38)
    at NodeEventGenerator.applySelector(/home/runner/work/eslint-remote-tester/eslint-remote-tester/ci/node_modules/eslint/lib/linter/node-event-generator.js:254:26)
    at NodeEventGenerator.applySelectors(/home/runner/work/eslint-remote-tester/eslint-remote-tester/ci/node_modules/eslint/lib/linter/node-event-generator.js:283:22)
    at NodeEventGenerator.enterNode(/home/runner/work/eslint-remote-tester/eslint-remote-tester/ci/node_modules/eslint/lib/linter/node-event-generator.js:297:14)
    at CodePathAnalyzer.enterNode(/home/runner/work/eslint-remote-tester/eslint-remote-tester/ci/node_modules/eslint/lib/linter/code-path-analysis/code-path-analyzer.js:711:23)
    at /home/runner/work/eslint-remote-tester/eslint-remote-tester/ci/node_modules/eslint/lib/linter/linter.js:952:32
    at Array.forEach (<anonymous>)

[Belco90]

Hi again Ari! It's really useful you are finding those edge cases with eslint-remote-tester 😮

Same thing as the previous issue you created before: It's causing issues on v3 but solved on v4 since all rules are refactorred and unexpected code is handled much better:

I'm curious about these scenarios tho: are they even valid from a syntax point of view? My bad, you already mentioned these are not valid but represent something while user is still writing code.

Anyway, you can see they are fixed on v4, but happy to keep checking others to address as much problems as we can before the release! I was planning to use eslint-remote-tester when we have all the rules refactored, it's a great tool. In case you are interested, you can see progress on last v4 bit on #198

[AriPerkkio]

I'm curious about these scenarios tho: are they even valid from a syntax point of view? My bad, you already mentioned these are not valid but represent something while user is still writing code.

I couldn't just leave this without debugging further. 😄

Turns out other ESLint parser such as the default espree don't output AST for this code block. They detect it as syntax error: Unexpected token function. This is easily reproduced in https://astexplorer.net/.

There is an open issue at typescript-eslint related to this. typescript-eslint/typescript-eslint#1852
The typescript parser doesn't mark this as syntax error and just outputs AST which doesn't follow ESTree spec.

I would still recommend to handle this in the rule implementation. Make sure to check v4 works with typescript parser as well.

[Belco90]

Whoa, interesting! I've suscribed to that typescript-eslint issue. And yes, making sure v4 works with typescript parser as well is a fantastic idea, I'll add it to my lost of things when all rules are rewritten. Thank you for your help! I'll write here if I find anything else about this.

[Belco90]

This should be fixed on v4.0.0

The release is available on:

• npm package (@latest dist-tag)
• GitHub release