sveltejs / eslint-plugin-svelte

ESLint plugin for Svelte using AST
https://sveltejs.github.io/eslint-plugin-svelte/
MIT License
282 stars 30 forks source link

Invalid `ExpressionStatement` node when using a custom actions with parameters #583

Open mcous opened 10 months ago

mcous commented 10 months ago

Before You File a Bug Report Please Confirm You Have Done The Following...

What version of ESLint are you using?

8.49.0

What version of eslint-plugin-svelte are you using?

2.33.1

What did you do?

I encountered an interesting ESLint crash that appears to be an bad interaction between eslint-plugin-svelte, eslint-plugin-sonarjs, and @typescript-eslint/parser. Since the crash is primarily triggered by passing an argument to a Svelte action, I decided to file the issue here, though the actual cause may lie elsewhere!

With the sonarjs/no-unused-collection rule enabled, if the following conditions are met, ESLint will crash:

  1. You have a Svelte component
  2. Using lang="ts"
  3. That import's a custom action
  4. And attaches that action to an element
  5. And passes an argument to that action
<script lang="ts">
  import { customAction } from "./action.js";
</script>

<p use:customAction={"hello"}>hello world</p>

The particular SonarJS rule above (to paraphrase) runs:

if (
  node.type === 'ExpressionStatement' &&
  node.expression.type === 'AssignmentExpression'
) {
  // ...
}

However, the following node gets passed to the rule:

Statement: {
  type: 'ExpressionStatement',
  directive: undefined,
  expression: null,
  range: [ 96, 145, [length]: 2 ],
  loc: { start: { line: 5, column: 20 }, end: { line: 6, column: 23 } },
  parent: {
    type: 'Program',
    // ...
  }
}

Since expression is null, expression.type crashes ESLint with TypeError: Cannot read properties of null (reading 'type'). I do not believe expression is allowed to be null, but I cannot figure out what exactly is producing this node.

What did you expect to happen?

Passing an argument to an imported custom Svelte action should not cause an ESLint crash, regardless of the plugins and rules I'm using.

What actually happened?

Oops! Something went wrong! :(

ESLint: 8.49.0

TypeError: Cannot read properties of null (reading 'type')
Occurred while linting /Users/mc/sandbox/no-empty-collection/component.svelte:1
Rule: "sonarjs/no-unused-collection"
    at isElementWrite (/Users/mc/sandbox/no-empty-collection/node_modules/.pnpm/eslint-plugin-sonarjs@0.21.0_eslint@8.49.0/node_modules/eslint-plugin-sonarjs/lib/rules/no-unused-collection.js:149:30)
    at isRead (/Users/mc/sandbox/no-empty-collection/node_modules/.pnpm/eslint-plugin-sonarjs@0.21.0_eslint@8.49.0/node_modules/eslint-plugin-sonarjs/lib/rules/no-unused-collection.js:118:18)
    at isUnusedCollection (/Users/mc/sandbox/no-empty-collection/node_modules/.pnpm/eslint-plugin-sonarjs@0.21.0_eslint@8.49.0/node_modules/eslint-plugin-sonarjs/lib/rules/no-unused-collection.js:84:18)
    at Array.filter (<anonymous>)
    at collectUnusedCollections (/Users/mc/sandbox/no-empty-collection/node_modules/.pnpm/eslint-plugin-sonarjs@0.21.0_eslint@8.49.0/node_modules/eslint-plugin-sonarjs/lib/rules/no-unused-collection.js:54:25)
    at /Users/mc/sandbox/no-empty-collection/node_modules/.pnpm/eslint-plugin-sonarjs@0.21.0_eslint@8.49.0/node_modules/eslint-plugin-sonarjs/lib/rules/no-unused-collection.js:59:9
    at Array.forEach (<anonymous>)
    at collectUnusedCollections (/Users/mc/sandbox/no-empty-collection/node_modules/.pnpm/eslint-plugin-sonarjs@0.21.0_eslint@8.49.0/node_modules/eslint-plugin-sonarjs/lib/rules/no-unused-collection.js:58:23)
    at Program:exit (/Users/mc/sandbox/no-empty-collection/node_modules/.pnpm/eslint-plugin-sonarjs@0.21.0_eslint@8.49.0/node_modules/eslint-plugin-sonarjs/lib/rules/no-unused-collection.js:41:17)
    at ruleErrorHandler (/Users/mc/sandbox/no-empty-collection/node_modules/.pnpm/eslint@8.49.0/node_modules/eslint/lib/linter/linter.js:1051:28)

Link to GitHub Repo with Minimal Reproducible Example

https://github.com/mcous/eslint-svelte-null-expression-repro

Additional comments

I'd be interested in narrowing this reproduction down further if it was helpful (e.g. with svelte-eslint-parser directly), but I'm having trouble figuring out how the Svelte parser interacts with @typescript-eslint/parser

ota-meshi commented 10 months ago

Thanks for the bug report. I think the parser is probably creating the variable scope incorrectly. The parser tries to edit the scope manager generated by typescript-eslint to make it understand Svelte scopes, but I think something is probably wrong there.