Better handle eslint-disable comments when "eslint.codeActionsOnSave.rules" are used.

[pfoedermayr]

When a file with one or more eslint-disable comments is saved while there are other lint errors present the comments are automatically removed. I was able to reproduce it with a fresh project with just eslint and a javascript file. Only the extension dbaeumer.vscode-eslint in version 3.0.10 being enabled in VSCode.

eslint.config.mjs

export default [
  {
    linterOptions: {
      reportUnusedDisableDirectives: 'error'
    }
  },
  {
    rules: {
      'no-console': 'error',
      'quotes': ['error', 'single']
    }
  },
];

.vscode/settings.json

{
    "editor.codeActionsOnSave": {
      "source.fixAll.eslint": "explicit"
    },
    "eslint.codeActionsOnSave.rules": [
        "quotes"
    ]
}

When I save a file that looks like this for example

// eslint-disable-next-line no-console
console.log('test');

var a = "a"; // Lint error here

it results in:

console.log('test');

var a = 'a'; // Lint error here

Running just ESLint: Fix all auto-fixable Problems does fix the lint error in line 4 and keeps the eslint-disable comment in line 1.

[dbaeumer]

@pfoedermayr could you please provide me with a GitHub repository I can clone. That ensure we are using the identical setup when I try to reproduce this.

[pfoedermayr]

Hope this helps, let me know if I should change something or if you need anything else

https://github.com/pfoedermayr/vscode-eslint-1938

[dbaeumer]

Thanks for the repository.

What is strange is that if you remove

    "eslint.codeActionsOnSave.rules": [
        "quotes"
    ]

everything works as expected.

[dbaeumer]

This is a setup issue that is hard to fix (e.g. I don't have an idea how). The problem is as follows:

    "eslint.codeActionsOnSave.rules": [
        "quotes"
    ]

tells ESLint to only fix the "quotes" rule. When computing the fixes the console.log is not detected as a problem hence the disable directive is unnecessary and is fixed by removing it. The unnecessary directive detection can't be customized per rule. I could disable it in your case but then all unnecessary directives would stay.

All I can recommend is to turn the eslint.codeActionsOnSave.rules setting into a negative pattern for those rules that are expensive to validate and very likely don't produce a fix. That is actually the intended use of that setting.

[dbaeumer]

Can you explain why you customize the eslint.codeActionsOnSave.rules in that way?

[pfoedermayr]

Can you explain why you customize the eslint.codeActionsOnSave.rules in that way?

It's basically a list of rules that we came up with over time. As it was easy to just add a rule, when we thought that it should be "fast" and we have the benefit of not having to to think about e.g. writing " instead of '.

All I can recommend is to turn the eslint.codeActionsOnSave.rules setting into a negative pattern for those rules that are expensive to validate and very likely don't produce a fix.

Will definitely have a look at that, but in my head I would still have the same problem when an error is detected for the expensive rules right?

[pfoedermayr]

Thanks for explaining why this is happening, it definitely makes a lot more sense now. Digging into this, I saw that there is also some discussion going on in the eslint repository about how this can/should be configured etc.

As a workaround we set reportUnusedDisableDirectives to off when using the vscode-eslint extension, which is not optimal, but a better experience than having all comments removed.

[dbaeumer]

Makes sense. As said I have no better solution right now either.

[JavaScriptBach]

I have a similar use case at my company.

I can't run my full ESLint config on save because it's too expensive. (It includes a number of @typescript-eslint rules that need type information and are therefore quite slow.)

I want to use eslint.codeActionsOnSave.rules to restrict to a known subset of fast rules. But doing so will cause ESLint v9 to remove unused disables.

In the CLI I can customize this using https://eslint.org/docs/latest/use/command-line-interface#--fix-type. But I don't see a way to configure this in the VSCode extension. Could we add an extension option that allows us to forward --fix-type to the CLI? Seems like that would be the easiest way to solve this.

[dbaeumer]

The setting is eslint.options. It allows you to pass in the options specified here: https://eslint.org/docs/latest/integrate/nodejs-api#-new-eslintoptions

And the eslint.codeActionsOnSave.rules supports negating. The idea is that you basically remove the once that are slow since they most of the time don't have fixes anyways.

[JavaScriptBach]

What I'd love to have is eslint.codeActionsOnSave.options, i.e. the ability to pass an entirely different config to ESLint on save. The idea is that I still want to inline highlighting to run the full lint, but that autofix-on-save runs a subset (with different parsing options, such as not using typescript-eslint's projectService) for better performance.

[dbaeumer]

eslint.codeActionsOnSave.options that is a good idea. Would you be willing to work on a PR for this?

[JavaScriptBach]

I can give it a try if you can point me to where to start.

[dbaeumer]

Great. Here are some pointers:

On the client side we need to add a setting eslint.codeActionsOnSave.options in the package.json somewhere here: https://github.com/microsoft/vscode-eslint/blob/main/package.json#L445 That should have the same shape as eslint.options

Then we need to have the setting in TypeScript here: https://github.com/microsoft/vscode-eslint/blob/main/%24shared/settings.ts#L67 and fill in the value here: https://github.com/microsoft/vscode-eslint/blob/main/client/src/client.ts#L715

On the server side the save rule config is computed here:

https://github.com/microsoft/vscode-eslint/blob/main/server/src/eslint.ts#L485

We should add the options to the SaveRuleConfigItem and then when we create the ESLint class here https://github.com/microsoft/vscode-eslint/blob/main/server/src/eslintServer.ts#L781 honor the options.

[joshkel]

I ran across this today. Instead of, or in addition to, eslint.codeActionsOnSave.options, what about updating vscode-eslint to automatically omit directive from fixTypes if eslint.codeActionsOnSave.rules is present? Leaving directives untouched if using an incomplete set of rules seems like a good default, and the current behavior seems incorrect.