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

eslint.codeActionsOnSave.mode silently overrides eslint.codeActionsOnSave.rules #1388

Closed
laverdet opened this issue Dec 13, 2021 · 7 comments
Milestone

Comments

@laverdet
Copy link

Using dbaeumer.vscode-eslint v2.2.2 w/ eslint v8.4.1

.vscode/settings.json:

{
	"eslint.codeActionsOnSave.mode": "problems",
	"eslint.codeActionsOnSave.rules": [],
	"eslint.useESLintClass": true,
}

.eslintrc.yaml:

parserOptions:
  ecmaVersion: latest
  sourceType: module
rules:
  no-multiple-empty-lines: [ warn, { max: 1, maxEOF: 0 } ]
  prefer-const: warn

package.json:

{
  "name": "eslint-test",
  "version": "1.0.0",
  "dependencies": {
    "eslint": "^8.4.1"
  }
}

When saving a file all fixes are applied even though the rules are explicitly set to an empty array. It's ok that these two options are mutually exclusive but the interaction between them is surprising without any kind of runtime warning or documentation of this invariant.

Additionally the documentation around eslint.codeActionsOnSave.rules is a little confusing. It's stated "This setting is only honored if either ESLint version 8 or greater is used or ESLint version 7 is used and the setting eslint.useESLintClass is set to true." which I mentally parsed as: version >= 8 || (version == 7.x && useESLintClass). Consider: "This setting requires ESLint 7 or higher and that the eslint.useESLintClass option is enabled".

@dbaeumer
Copy link
Member

Thanks for reporting this. I will improve the documentation around this, but "eslint.codeActionsOnSave.mode": "problems" will win over "eslint.codeActionsOnSave.rules" since otherwise the whole purpose of the mode setting is superfluous.

This setting requires ESLint 7 or higher and that the eslint.useESLintClass option is enabled

This is not fully correct. If you use version >= 8 then the setting of eslint.useESLintClass is irrelevant since version 8 only ships with a ESLint Class API.

@dbaeumer dbaeumer added this to the 2.2.3 milestone Dec 14, 2021
@laverdet
Copy link
Author

This is not fully correct. If you use version >= 8 then the setting of eslint.useESLintClass is irrelevant since version 8 only ships with a ESLint Class API.

This is not the behavior I'm observing. If "eslint.useESLintClass": true is omitted from settings.json then "eslint.codeActionsOnSave.rules": [] has no effect. I'm attaching a short screen recording https://imgur.com/a/fhDqpvX

@dbaeumer
Copy link
Member

You are correct (see

if (ESLintModule.hasESLintClass(library) && useESLintClass) {
) However this is a bug and shouldn't be necessary

@dbaeumer
Copy link
Member

Actually above comment is wrong. The whole code creates a ESLint class as a default at the end.

@dbaeumer
Copy link
Member

Looking at the code I do think I fixed this, but not yet available in a new version. Do you have a repro I can clone that demos what you are seeing to ensure it works now.

@laverdet
Copy link
Author

Sure, I made a repo which demonstrates the issue clearly:
https://github.com/laverdet/eslint-test

I've tested this on a clean installation of VS Code v1.63.1 with no user settings, and vscode-eslint v2.2.2. If you open the project and save the file index.js the content should not change. If you then open .vscode/settings.json, remove the line with content "eslint.useESLintClass": true, and then save index.js once more the let will turn into a const.

@dbaeumer
Copy link
Member

dbaeumer commented Jan 3, 2022

Is fixed in main
cast

@dbaeumer dbaeumer modified the milestones: 2.2.next, 2.2.6 Jul 5, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants