vscode-eslint: 'Fix all auto-fixable problems' doesn't fix as many issues as possible

Wanted to chime in on this, first reported here (marked as duplicate): https://github.com/Microsoft/vscode-eslint/issues/575

Lots of online resources start to recommend to use eslint+prettier. Basically using eslint for its awesome code-quality rules, but prettier for its awesome formatting rules: https://prettier.io/docs/en/comparison.html

If someone uses VS Code and eslint, they would start by installing the extension vscode-eslint. The npm packages installed for the repository you work on would be eslint, prettier, eslint-config-prettier, eslint-plugin-prettier.

Prettier best practices integration with eslint tells you to use "extends": ["plugin:prettier/recommended"]. This will make eslint --fix also run the prettier formatter. If I run this on command line like so node_modules/.bin/eslint --fix server FILE it formats everything perfectly. This has already been established in this thread.

In VS code I’ve set:

"editor.formatOnSave": true
"eslint.autoFixOnSave": true,
"javascript.format.enable": false,

When I then save the same FILE in VS Code, I have to do three consecutive saves in order for the formatting to end up in its final state, equal to as when I ran it from command line. This is obviously not good and has also been established in this thread.

I then turn to install Prettier in VS Code. This section about integrating VS Code Eslint+VS Code Prettier reads:

prettier-eslint and prettier-tslint are included with the installation of this extension. There is no need for a separate local or global install of either for functionality.

The recommended VS Code configuration to integrate these two is to set the configuration "prettier.eslintIntegration" true and remove "eslint.autoFixOnSave": true and "javascript.format.enable": false

The purpose of prettier-eslint is to run prettier, followed by eslint --fix. If one has set the ESLint configuration ("extends": ["plugin:prettier/recommended"]), then prettier will already be run with eslint --fix. In practice what will happen then is:

1. prettier is run.
2. eslint --fix is run. a. eslint --fix runs the the prettier plugin and the rule called prettier, which again formats according in the same way as 1.

This is a strong reason as to why possible the VS Code ESLint extension and the command ESLint: Fix all auto-fixable Problems should instead issue a eslint --fix FILE instead of what it currently does. I would then not have to rely on the VS Code Prettier extension to format the document in a subpar way.

I released 2.0.4 today which addresses this. Please note that for large file you need to increase the time budget to compute the fixes using editor.codeActionsOnSaveTimeout

@alexeagle The following is just my opinion

1. The built in VS Code formatter leaves a lot to be desired. It is not very configurable and you end up in a situation where the formatter actually creates linter errors.
2. Using a separate formatter extension creates an additional dependency where you now have to duplicate your style in both the formatter configuration file and the .eslintrc file
3. The auto-fix can detect and fix problems that a formatter would typically not involve itself with, such as replacing var with const. A formatter could be made to do this, but then the feature creep is going in the other direction and that defeats the purpose of separating formatting and linting anyway.
4. You will never get people to agree on one objectively correct way to format code, so I don’t think it’s fair to assume the formatter author knows best.

One of the issues is with large files in VSCode. Basically the after-save hook only gives you a couple hundred millis to create patches, and ESLint will sometimes need longer than that, even with only one pass.

Today, this can happen in files > 2k SLOC

If we run multiple ESLint passes, who knows what the “maximum” filesize you can format will be, 800 SLOC? That’s why it’s important to set up determinant e2e tests that actually run in VSCode and will surface these issues/information.

Yes, I’m still working on it 👍

This issue can be solved by registering ESLint as a formatter rather than a command.

I tried playing around with vscode-eslint’s source code and modified it so that it takes the final output from eslint --fix and send the whole formatted code back to VS Code (rather than sending individual fixes as separate edit).

As you can see here, when I run the autofix command, the cursor, which used to be on line 2 on the word arguments, immediately jumps down to the bottom, since the whole editor’s content is replaced.

For comparison, here’s the original behavior — I had to run it several times, but here the cursor should stay at the word arguments and not jump down to the bottom of the file.

However, by registering ESLint as a document formatter (activated via “Format document” command instead), we can simply send the final code to VS Code, and it will diff the editor’s contents with the formatted code to compute the minimal changes to apply. As far as I know, this function is not exposed via the extension API anywhere except through the “Format document” command. I mentioned this about a year ago in this issue.

Here, I updated the extension to register itself as a code formatter, and by calling the “Format document” command, the cursor stays at the correct position:

I’d be fine with it executing eslint --fix on the current file.

I’ll take a stab at fixing this later this week, should be able to batch fixes from multiple eslint passes before resolving the handler for textDocument.onWillSaveTextDocument

Doesn’t help that there are currently zero tests for this project, though

PR to register vscode-eslint as document formatter submitted — https://github.com/microsoft/vscode-eslint/pull/766

I was working on adding tests for existing code (Using same approach as the vscode-prettier project). As this extension has >20M installs and auto-updates, I would hate to introduce a regression. Still planning on fixing this bug after that

I can’t believe that after 3 years since reporting this issue is still not fixed. Any ideas will it be ever be resolved at all?

@tcg I think you’re still going to see weird behavior. With the suggested “ESLint with prettier config”, you’ll need to save multiple times to get all your fixes run. (That’s what this issue is about.)

Calling eslint --fix File is actually not an option due to the fact that the eslint extension validates the buffer and not the file on disk. Calling the command would change the file on disk and it might collide with edits the user does in the editor. In addition you would loose all markers (like breakpoints) in the document since for VS Code it will look like the whole buffer has changed.

The underlying problem is that ESlint itself does not do one pass when fixing problems. It runs n times until not further changes are detected. This needs to be mirrored in the ESLint plugin.

We already have autofix on precommit, but there’s still a hefty amount of frustration caused by the lack of confidence in the editor display - is a line red because of how an import is ordered, or because the imported file doesn’t exist? Hence why they save multiple times in a row, even though they could fix it through the CLI.

VS Code’s source code has this function computeMoreMinimalEdits which basically takes a list of TextEdit operations and computes a minimal one.

This allows the whole document text to be replaced while keeping markers and stuff. However, I am not sure how this API could be invoked from the extension process.

Works so well! My life is completely changed! No more Cmd+S, Cmd+S, Cmd+S, Cmd+S 😂 ❤️

A new version with the fix is not released yet. You can run download and run this extension from source code in the meantime.

I write a command eslint.executeAutofixThoroughly to walk around this. You can get the code from here . Replace the extension.js and the package.json in your local extension folder with the files in the gist to add this command.

This is what I added:

function tryFix() {
    let textEditor = vscode_1.window.activeTextEditor;
    if (!textEditor) {
        return;
    }
    let textDocument = {
        uri: textEditor.document.uri.toString(),
        version: textEditor.document.version
    };
    let params = {
        command: 'eslint.applyAutoFix',
        arguments: [textDocument]
    };
    return client.sendRequest(vscode_languageclient_1.ExecuteCommandRequest.type, params).then(() => textEditor.document.getText(), () => {
        vscode_1.window.showErrorMessage('Failed to apply ESLint fixes to the document. Please consider opening an issue with steps to reproduce.');
    });
}

let preCode;
let times = 0;
tryFix().then(onFixed);

function onFixed(code) {
    times++;
    if (code !== preCode) {
        if (times < 10) {
            tryFix().then(onFixed);
        } else {
            vscode_1.window.showErrorMessage('Failed to apply ESLint fixes thoroughly.');
        }
    }

    preCode = code;
}

@dbaeumer Is this solution acceptable to you?

I write a command eslint.executeAutofixThoroughly to walk around this. You can get the code from here . Replace the extension.js and the package.json in your local extension folder with the files in the gist to add this command.

This is what I added:

function tryFix() {
    let textEditor = vscode_1.window.activeTextEditor;
    if (!textEditor) {
        return;
    }
    let textDocument = {
        uri: textEditor.document.uri.toString(),
        version: textEditor.document.version
    };
    let params = {
        command: 'eslint.applyAutoFix',
        arguments: [textDocument]
    };
    return client.sendRequest(vscode_languageclient_1.ExecuteCommandRequest.type, params).then(() => textEditor.document.getText(), () => {
        vscode_1.window.showErrorMessage('Failed to apply ESLint fixes to the document. Please consider opening an issue with steps to reproduce.');
    });
}

let preCode;
let times = 0;
tryFix().then(onFixed);

function onFixed(code) {
    times++;
    if (code !== preCode) {
        if (times < 10) {
            tryFix().then(onFixed);
        } else {
            vscode_1.window.showErrorMessage('Failed to apply ESLint fixes thoroughly.');
        }
    }

    preCode = code;
}

@dbaeumer Is this solution acceptable to you?

Are there any downsides to using this solution?

The problem is that the npm eslint module computes all the fixes by running in a loop until no more fixes are computed (see https://github.com/eslint/eslint/issues/5329 for the details). So for a given state S

• it computes fixes F and applied them to S moving S to S’
• it then computes fixes F’ on S’ and applies them and move S’ to S’’

To implement that in the extension we need to:

• implement the above loop
• and then compute the delta between S and S’’ since the VS Code editor still sits on version S
• when happening on save this all has to happen in < 200ms since that is the budget VS Code gives an extension to particiapte in save.

Trying to understand what’s the core issue here - can someone ELI5 this?

@PavelPolyakov thanks for your willingness to look into this.

The problem is that the result of the fixes need to be applied on the client side since the client owns the document content since it is open in the editor. After the first round is applied the eslint server receives a document change event with the change / new content and the parses the document again to compute any ESLint errors and their outstanding fixes.

The only way to build the recursive on the server side is doing the following:

• get the fixes on willSave
• apply them locally to a document content in memory
• ask ESLint if there are more fixes with the document content in memory
• if yes loop
• if not we can send the edits back to the client if only one loop was made. If more than one loop happend we need to compute the edits for moving the document from its original state to the state of the last loop. This can either be done with moving edits or by diffing the two contents and computing a whole new edit tree.
• in general we should not loop more than 100 - 200 ms to not fall out of the will save event.

We are also discussing if we can extend the API so that will save provider can ask for more time. But nothing concrete yet.

Is anyone still looking into this ? if not i might take a stab at it in the next few days

The issue still persists. 😦

Also, there are no other formatters installed, and my settings file looks like this:

“eslint.autoFixOnSave”: true, “editor.formatOnSave”: false

@raix running fix multiple times behind the scenes (until there are no more fixable problems left) which is what ESLint does via the cli.

trying to get a fix in PR this weekend

edit: closer to*

@dbaeumer do you know if this is an issue in the vs code extension or is it the CLIEngine?

Does anyone of you know why eslint needs n rounds to fix up these things. Doing n rounds in the extension needs quite some coding since it is a pre save hook and all the modifications need to be described on the version of the document. The way how I understand the round concept in eslint is that fixes are applied like this fix(fix(fix(fix(n) which moves the document to version n + 4. So I can’t simply take the fixes compute the rounds 2, 3, 4 and apply them to the document version n. Moreover I suspect that the edits might be overlapping and therefore a merge might not be possible. So the only save answer I see here is to run the fix rounds until I reach a fix point and then use a compare algorithm to compute a minimal edit set 😦