Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Bug: auto-fix with eslint-plugin-prettier causes code to run with errors #15354

Closed
1 task done
await-ovo opened this issue Nov 25, 2021 · 3 comments
Closed
1 task done
Labels
3rd party plugin This is an issue related to a 3rd party plugin, config, or parser archived due to age This issue has been archived; please open a new issue for any further discussion bug ESLint is working incorrectly repro:needed
Projects

Comments

@await-ovo
Copy link

await-ovo commented Nov 25, 2021

Environment

Node version: 14.18.1
npm version: 6.14.15
Local ESLint version: v8.3.0
Operating System: macOS 10.15.6

What parser are you using?

@typescript-eslint/parser

What did you do?

Configuration
module.exports = {
  root: true,
  parser: '@typescript-eslint/parser',
  parserOptions: {
    ecmaFeatures: {
      jsx: true,
    },
    project: './tsconfig.json',
  },
  plugins: [
    "@typescript-eslint/eslint-plugin",
    "eslint-plugin-prettier"
  ],
  rules: {
    'arrow-body-style': ['error', 'never'],
    'prettier/prettier': [
      'error',
      {
        printWidth: 80,
        tabWidth: 2,
        useTabs: false,
        semi: true,
        singleQuote: true,
        trailingComma: 'all',
        bracketSpacing: true,
        jsxBracketSameLine: true,
        arrowParens: 'avoid',
      },
    ],
  }
}
const useMemo: any = () => 0;
const isLastArchiveNode = useMemo(() => {
  // comments 
  return [1, 2, 3].some(item => {
    return item > 1;
  });
}, [0]);

What did you expect to happen?

This is the code repository that can be used to reproduce: test-eslint-prettier:

  1. git clone git@github.com:await-ovo/test-eslint-prettier.git
  2. cd test-eslint-prettier
  3. yarn install
  4. yarn eslint --fix ./src/index.ts

After executing yarn eslint --fix ./src/index.ts in the root directory, the code should be formatted to look like this:

const useMemo: any = () => 0;
const isLastArchiveNode = useMemo(
  () =>
    // comments
    [1, 2, 3].some(item => item > 1),
  [0],
);

What actually happened?

In fact, the code after the automatic fix is as follows:

const useMemo: any = () => 0;
const isLastArchiveNode = useMemo(
  () =>
    // comments
    [1, 2, 3].some(item => item > 1)[0],
);

You can see that the comma before the second parameter is missing.

Participation

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

Additional comments

I found that in the loop of trying to fix the code, as soon as the range property of the next fix intersects with the previous fix, the fix is skipped and the problem is put into the remainingMessages array. Subsequent fixes that do not intersect will still be used

https://github.com/eslint/eslint/blob/main/lib/linter/source-code-fixer.js#L92

image

This can lead to problems in the figure above:

  • Replacing the code with item > 1 when the range is [112, 138].
  • When the range is [137, 139], the fix is skipped because 137 is smaller than the previous 138. Here is the code fixed by prettier, with the comma moved to the previous line.
  • When the range is [140, 145], it changes , [0] to [0], .

So the comma between the arguments of the useMemo function is lost.

I'm not sure if this is an ESLint problem or a Prettier problem, but I've found that if the fix range overlaps, just ending the fix and re-executing verify and applyFixes can avoids this problem:

image

https://github.com/eslint/eslint/blob/main/lib/linter/source-code-fixer.js#L123

I'm not sure if this is the right fix for this problem, please give me some guidance or advice, thank you for your help.

@await-ovo await-ovo added bug ESLint is working incorrectly repro:needed labels Nov 25, 2021
@eslint-github-bot eslint-github-bot bot added this to Needs Triage in Triage Nov 25, 2021
@mdjermanovic mdjermanovic moved this from Needs Triage to Triaging in Triage Nov 26, 2021
@mdjermanovic mdjermanovic added the 3rd party plugin This is an issue related to a 3rd party plugin, config, or parser label Nov 26, 2021
@mdjermanovic
Copy link
Member

Hi @await-ovo, thanks for the very detailed analysis!

This is a known bug in eslint-plugin-prettier: prettier/eslint-plugin-prettier#65

In short, each individual fix should produce a valid equivalent code when applied, but that isn't the case with fixes created by the prettier/prettier rule. The rule works on the diff between the original code and its prettier-formatted version, and then returns multiple fixes. The problem is that those fixes are not independent. As you noticed, applying only some of them (but not all) can result in a non-equivalent code.

I'm not sure if this is an ESLint problem or a Prettier problem, but I've found that if the fix range overlaps, just ending the fix and re-executing verify and applyFixes can avoids this problem

That could fix some problems, but at the same time introduce new problems of the same kind.

For example, if there are 4 fixes, and the leftmost and rightmost are prettier fixes that depend on each other, if the other two fixes in the middle are overlapping between themselves so we stop there, then we'll have the same problem - only one of two dependent prettier fixes would be applied (the leftmost).

I'm not sure if there is anything that could be done in ESLint to avoid these problems. This looks like an issue that can only be fixed in eslint-plugin-prettier.

@await-ovo
Copy link
Author

thank you for such a detailed explanation

@mdjermanovic mdjermanovic moved this from Triaging to Feedback Needed in Triage Nov 28, 2021
@snitin315
Copy link
Contributor

Closing in favor of prettier/eslint-plugin-prettier#65

Triage automation moved this from Feedback Needed to Complete Dec 12, 2021
@eslint-github-bot eslint-github-bot bot locked and limited conversation to collaborators Jun 11, 2022
@eslint-github-bot eslint-github-bot bot added the archived due to age This issue has been archived; please open a new issue for any further discussion label Jun 11, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
3rd party plugin This is an issue related to a 3rd party plugin, config, or parser archived due to age This issue has been archived; please open a new issue for any further discussion bug ESLint is working incorrectly repro:needed
Projects
Archived in project
Triage
Complete
Development

No branches or pull requests

3 participants