sveltejs / eslint-plugin-svelte3

An ESLint plugin for Svelte v3 components.
MIT License
373 stars 43 forks source link

Destructuring Causes Parse Error With `--fix` #19

Closed ghost closed 5 years ago

ghost commented 5 years ago

Environment

Steps

  1. index.svelte

    <script>
    const { number } = { number: 0 };
    </script>
  2. $ eslint index.svelte --fix

Expected

index.svelte

<script>
  const {
    number
  } = {
    number: 0
  };
</script>

$ 2:2 error 'number' is assigned a value but never used no-unused-vars

Actual

index.svelte

<script>
  const {
 number 
} = {
 number: 0 

<script>
  const { number } = { number: 0 };
</script>

$ 8:3 error Unexpected token ParseError

Conduitry commented 5 years ago

What does your eslintrc look like? With object-curly-newline what I'm getting is:

<script>
  const {
 number 
} = {
 number: 0 
};
</script>

which isn't great but at least is syntactically correct.

ghost commented 5 years ago
module.exports = {
  parserOptions: {
    ecmaVersion: 2019,
    sourceType: 'module',
  },
  plugins: ['svelte3'],
  overrides: [
    {
      files: '*.svelte',
      processor: 'svelte3/svelte3'
    },
  ],
  env: {
    browser: true,
  },
  rules: {
    'eol-last': ['error', 'always'],
    'object-curly-newline': ['error', 'always'],
  }
};
Conduitry commented 5 years ago

Thanks, I can reproduce this now.

I think the main part of the solution to this though is simply going to be to ignore eol-last errors in Svelte components, even if the user has them enabled in their config. There are a few errors that this plugin is currently ignoring - and I think it makes sense to add eol-last to that list.

ghost commented 5 years ago

I think the main part of the solution

What do you mean by ”main part”? Is there another?

The solution to this though is simply going to be to ignore eol-last error

That's the workaround. The solution is to fix the bug, even if it involves filing an issue with and contributing to ESLint.

There are a few errors that this plugin is currently ignoring

Two of which are unavoidable. The other two, as you know, are workarounds for ESLint shortcomings: no-labels and no-unused-labels. Both lack granularity: a way to exclude certain labels. We need to file an issue with ESLint stating our case for making these rules more granular. Then we should add a comment to the code linking to the issues.

Conduitry commented 5 years ago

When I was working on this issue, I was able to get the eol-last fix to stop corrupting the file, but I realized that, as it's written, there's not really much good the eol-last rule would be able to do. It's only going to be getting the JS string that's getting constructed from the Svelte component and passed along to ESLint. The last line in a Svelte component is going to be in its template, not in a script. I don't see a reasonable addition to ESLint core that would help here. If we wanted to more properly support that, it would be entirely manually. That's not a precedent I'm too anxious to start.

For the label rules, there is actually something that we can do without further support from ESLint, it just wasn't happening yet. As of 2.1.0, warnings about labels are only suppressed if the label is $. (Or $ followed by non-ascii unicode characters, which I think is a completely acceptable quirk.)

Chasing down ESLint maintainers, championing and keeping tabs on new ESLint plugin features, or becoming familiar enough with the ESLint codebase to contribute these myself - none of these are things I'm particularly interested in spending my time doing. ESLint has a lot of things to worry about. If someone manages to push some helpful plugin enhancements to the front of the queue, I'll happily use them in this project, but they're not something I'm generally going to be actively pursuing.

ghost commented 5 years ago

The last line in a Svelte component is going to be in its template, not in a script.

Ah, that's right.

As of 2.1.0, warnings about labels are only suppressed if the label is $.

Nice.

Chasing down ESLint maintainers, championing and keeping tabs on new ESLint plugin features, or becoming familiar enough with the ESLint codebase to contribute these myself - none of these are things I'm particularly interested in spending my time doing.

Noted.

ghost commented 5 years ago

I was going to PR case 'eol-last': return false;, but I see you've already done it. Thank you.