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

[darkred]

I have ESLint extension 1.0.7 with VSCode 1.6.0 in win10 x64
and ESLint 3.7.1 globally installed.

First of all, please note that since ESLint 2.9.0:

when you use the --fix option, ESLint will make multiple passes over your code in an attempt to fix as many issues as possible.

Let's say I have the following .eslintrc.js in C:\Users\Kostas:

module.exports = {
    "env": {
        "browser": true,
        "es6": true,
        "greasemonkey": true,
        "jquery": true
    },
    "extends": "eslint:recommended",
    "rules": {
        "indent": ["error","tab"],       
    }
};

Now, I have this Javascript file:

var localTimezone;

    function convertDates(f) {
        if (f === 'f1') {
            format = 'DD.MM.YYYY HH:mm';
        } else {
            format = 'MMMM DD, YYYY';
        }
    }

If I do eslint --fix myfile.js via CLI (a single time) it will be fixed into:

var localTimezone;

function convertDates(f) {
    if (f === 'f1') {
        format = 'DD.MM.YYYY HH:mm';
    } else {
        format = 'MMMM DD, YYYY';
    }
}

But, in VSCode the `Eslint: Fix all auto-fixable problems command doesn't fix all indentation at once:
executing it the first time will only fix indentation on line 3,
a second time fixes lines 4 + 9,
and finally a 3rd time fixes lines 5-8,
i.e. it requires multiple (3) executions.

_For reference, in Atom with linter 1.11.18 and linter-eslint 8.0.0 using my global ESLint installation as well,
with the relevant command (Linter eslint: Fix File) all indentation is fixed with a single execution._

[ivancuric]

Can confirm. It only does a single pass.

[egamma]

The above example is handled fine by the JS formatter and we now support format on save in 1.6.

Are there other examples than formatting of fixes that require multiple passes?

[carlin-q-scott]

Maybe the extension could run the formatter instead of autofix from the context menu if it handles this better. I haven't had this issue outside of dealing with white-space.

[darkred]

@egamma

The above example is handled fine by the JS formatter and we now support format on save in 1.6.

Format on save requires that you have installed formatter extensions, which will run on each save.
First of all, I don't want running a formatter every time I save a file - I prefer to do that manually.
Also, JSCS and ESlint have become a single team since April.
ESLint provides lots of stylistic rules with auto-fix already,
and I'm sure the "gap" between JSCS and ESLInt will soon be bridged. (see issue 5856 of ESLint for more).
Plus, a formatter will beautify the code, but it doesn't fix all stylistic problems that ESLint (incl. JSCS) detects.

Are there other examples than formatting of fixes that require multiple passes?

Yes. when the errors are both stylistic and non-stylistic.

Example:

/*eslint quotes: ["error", "single"], yoda: "error"*/

var color;

if ("red" === color) {
    // ...
}

Here line 5 has 2 errors: a yoda and a quote one:
the first pass fixes the quote error,
and an extra pass is needed for the yoda error.

And, in general, I can't rule out that there aren't other cases other than formatting that require multiple passes.

The Fix all auto-fixable problems command currently only fixes those errors that it displays,
i.e. it only does a single pass.

For reference, what the --fix option of ESLint offers (1, 2) is that:

This option instructs ESLint to try to fix as many issues as possible. The fixes are made to the actual files themselves and only the remaining unfixed issues are output.

So, my suggestion is to offer a user setting with which to implement ESLint '--fix' functionality as it is in CLI:
i.e. to replace the current file with one having as many issues as possible fixed ,
and then report only which unfixed issues remain.

[egamma]

@darkred Frankly, I cannot get warm for implementing formatting using a multi-pass linting approach. Formatters are great at formatting and I don't want to maintain separate settings for formatting and linting. See also https://medium.com/@Jakeherringbone/why-linting-makes-me-yawn-cadbd9a51ca9#.9tw56pje0 from @alexeagle by https://github.com/Microsoft/vscode-tslint/issues/84#issuecomment-246158200.

For VS Code we have a commit hook that doesn't allow a check-in if there are formatting violations. This implemented using a formatter and not a linter.

[darkred]

Frankly, I cannot get warm for implementing formatting using a multi-pass linting approach.

I'm afraid that might have been a misunderstanding here. To clarify:
the current command is [ESLint] Fix all auto-fixable problems.
My suggestion is to rename it to [ESLint] Fix file
and make it so that it runs eslint --fix myfile.js in the background
and replace the current file with the output it.

[egamma]

@darkred sorry, my comment was less about implementation. I've just reacted to the 'identity crisis' of linters vs formatters. I'll calm down now 😄

My suggestion is to rename it to [ESLint] Fix file and make it so that it runs eslint --fix myfile.js in the background and replace the current file with the output it.

This would destroy markers like break points. The right way to do this is to diff the output with the current version and generate separate edits to create a minimal update, but @dbaeumer knows all about this...

[dsbert]

This behavior causes problems when performing a very common set of actions. If I have a code block nested inside another code block and delete the outer code block, then the inner block is now improperly indented. When I save the document, only the first level of the code block is moved to the correct indentation. Now any code within that block is incorrectly indented.

[dbaeumer]

As @egamma pointed out the fix should only provide a minimal edit set to keep as many markers in the file as possible. In addition the time a save participant has in VS Code is very limited. All participant have one second and if you are the guy who causes other to not run 3 times you will be disabled. So everything we do here in save participation must keep performance in mind.

[alexeagle]

Per the discussion of preferring a formatter to do formatting:
What is the ultimate reason these users don't use a formatter extension for VSCode instead? Maybe this problem would be less acute if we had the right advertising somewhere, to say "it looks like you're trying to format your file, here are two extensions that do that for you". Or do they have some objection to using a formatter (eg. not sufficiently configurable, which IMO is not a big deal, any consistent formatting is better than forcing your personal tastes on others)

[dsbert]

@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.

[joshfttb]

I have to agree with @dsbert on this. I just deleted an outmost code block on a mocha file and had to run fix five times to fix my indenting down the chain.

I could install and configure a separate formatter... but on the other hand eslint --fix does the job just fine.

[dbaeumer]

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 :-(

[darkred]

@dbaeumer check https://github.com/eslint/eslint/issues/5329#issue-134715189,
https://github.com/eslint/eslint/issues/5329#issuecomment-187899308 and
https://github.com/eslint/eslint/issues/5329#issuecomment-210146684
_(issue 5329 is the one referenced in the 2.9.0 changelog of ESLint in which this behavior was implemented)._

[dbaeumer]

@darkred thanks for the pointers. Reading through it makes it clear that the n round approach is here to stay. So I will look into how we can best adopt this. One thing that is already clear is that I need to time budget this, given that a save participant can only run for so many ms. So it still might be the case that not all auto fixes are applied in one save.

[joshfttb]

Personally I'm less worried about on save than when I run the command manually. 

Josh F

Sent from
https://polymail.io/

On Wed, Nov 02, 2016 at 08:58 "Dirk Bäumer"

<
mailto:

wrote:

a, pre, code, a:link, body { word-wrap: break-word !important; }

https://github.com/darkred
thanks for the pointers. Reading through it makes it clear that the n round approach is here to stay. So I will look into how we can best adopt this. One thing that is already clear is that I need to time budget this, given that a save participant can only run for so many ms. So it still might be the case that not all auto fixes are applied in one save.

—

You are receiving this because you are subscribed to this thread.

Reply to this email directly,
https://github.com/Microsoft/vscode-eslint/issues/154#issuecomment-257856735
, or
https://github.com/notifications/unsubscribe-auth/ACeafmXRMBGTHbUJuArE7mR7UDb4JIVaks5q6IjngaJpZM4KT2vS
.

[gcazaciuc]

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

[dbaeumer]

A pull request is highly appreciated here.

[leandono]

Any update about this issue?

[dbaeumer]

No update. PR is still welcome.

[gcazaciuc]

@dbaeumer I've began taking a look at it. However, it seems we are doing loads of processing with the ranges reported by Eslint and it's really hard to figure out what's going on.
My question is why we can't use the outputFixes() CLI method on Eslint and remove all that range translation logic which is error prone ?
Similar to how Atom does it( https://github.com/AtomLinter/linter-eslint/blob/master/src/worker.js):

function fixJob({ cliEngineOptions, contents, eslint, filePath }) {
  const report = lintJob({ cliEngineOptions, contents, eslint, filePath })

  eslint.CLIEngine.outputFixes(report)

  if (!report.results.length || !report.results[0].messages.length) {
    return 'Linter-ESLint: Fix complete.'
  }
  return 'Linter-ESLint: Fix attempt complete, but linting errors remain.'
}

Basically just run validate on the file, get the report and pass it to outputFixes() cli method.
I know there might be some issues with unsaved files but I believe any conflicts , if they occur , should be harmless and recoverable from.

Thoughts ?

[dbaeumer]

The problem is that eslint.CLIEngine.outputFixes(report) writes to the file to my knowledge. Since the file is open in the editor conflicts are IMO not harmless since they might result in UI interaction in the editor. The fixes and possible typings might even be conflicting which will show a merge editor.

In addition we support to apply all fixes per action even to an unsaved file.

The range translation logic is actually converting from an offset, length based model to a line, column based model and we use well proofed helpers for this.

[blackmiaool]

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?

[dbaeumer]

@blackmiaool interesting approach. How would that work with fix on save which a lot of people want to work with multi round as well.

[iddan]

I suspect that because of vscode's paste mechanism indent don't work well cause when I use eslint cli it indents well

[ma53]

Is there any chance this issue will receive any attention? There are no formatters available that can take the place of eslint's autoFix functionality, and to be required to run it multiple times is a major pain point.

[alexeagle]

@ma53 I'm curious, what more do you want a formatter to fix?

On Angular we use clang-format which does things like sort imports alphabetically. That's a feature I know prettier is missing.

[ma53]

I mentioned the lack of formatters that could take the place of ESLint to contest the argument @egamma made. It seems to me he's dismissing this issue because he feels that this is a piece of responsibility that should be delegated to a formatter. I disagree for a number of reasons, but even if I granted that point it's still the case that there is no such formatter that can fill the shoes of ESLint. Further, his argument seems to imply that anyone who is asking for this feature to function this way is doing so out of a misguided desire to use ESLint in place of a formatter. That's not my use case. I want this functionality because that's the way it should be (indeed, the way it is in many implementations). It's true that ESLint can do a lot of things that can be done and done better by a formatter, but it is not the case that a formatter can replace ESLint. Further, ESLint gives you such granular control over what rules to enforce and how...the formatters I'm familiar with are all very rigid and are designed to enforce their own format rather than let you define what to enforce.

Perhaps I'm among a very small number of users who feel this way. If that's the case and it's not worth the time to implement this feature I won't be upset -- I asked the question so that I would know whether I need to look for another solution.

[alexeagle]

@ma53 I asked out of sincere curiosity, if there is something you think a formatter could be doing.

I think that everyone on the thread agrees that this should be fixed, however based on the design discussion it seems to be hard. That means trading off this issue against other things. Making that tradeoff requires looking at the alternatives. So if using a formatter mostly ameliorates the need for a single pass that fixes everything, then that's good to know.

For what it's worth, I'm very active in the --fix feature for TSLint, and have observed that the desire to use it as a formatter restricts it to only very safe --fix behavior - for example the discussion on https://github.com/palantir/tslint/pull/2907 - autoFix for double-equals could break a program. So I don't see that many things that can be auto-fixed by a linter which neither overlap with a linter, nor have any potential to change the program's semantics.

[ma53]

@alexeagle I appreciate that you have taken the time to engage with me on this. Let me see if I can give you a satisfactory answer.

I think that everyone on the thread agrees that this should be fixed

I did not get that impression reading through it. This statement seems to directly contradict your earlier comment (which, I admit, was over a year ago at this point, so please correct me if your thoughts on this have changed):

Per the discussion of preferring a formatter to do formatting:
What is the ultimate reason these users don't use a formatter extension for VSCode instead? Maybe this problem would be less acute if we had the right advertising somewhere, to say "it looks like you're trying to format your file, here are two extensions that do that for you".

You seem to be suggesting here that time and effort ought to be invested in redirecting users to formatters rather than addressing the issue. First, this does nothing to address my use case, which I think I can sum up thusly: "as a user of the ESLint plugin for VSCode, I would like it if the 'fix all auto-fixable problems' function would run multiple iterations until no auto-fixable problems remained, as it does in most ESLint implementations." I honestly think that this is an issue that ought to be addressed regardless of where one draws the line between responsibilities of linters and formatters. If it's possible that fixing a linting problem could create another linting problem, the linter will need to run repeatedly in order to really satisfy its requirements. Consider a formatter with a congruent issue: by fixing one formatting problem, it could create a different formatting problem. Clearly this ought to be addressed regardless of whether or not the formatting issue could be handled by a linter.

If it is the case that everyone in this thread is in agreement that this issue should be fixed, then all I'd like to know is whether or not it _will_ be fixed. As you said, fixing this issue requires an investment of time that could otherwise be spent elsewhere. Is this issue, then, something that will just never be worth the investment to fix? Or is it on the roadmap somewhere and might just take some time?

Otherwise, there are a couple other points you made that I find interesting and would like to address:

I don't see that many things that can be auto-fixed by a linter which neither overlap with a linter, nor have any potential to change the program's semantics.

I think that drawing the line of responsibility between linters and formatters based on semantics is a compelling idea. Linters can help you with semantics while formatters take care of the rest. If this is the case, though, then _ESLint is a formatter_. Looking at their documentation, over half of the rules are either stylistic or "best practices" which don't impact semantics. If this is the case, then I can grant you the point that these stylistic issues should be handled by formatters rather than linters and still present my use case as relevant: I'm using ESLint as my formatter (which is only natural since _most of what it does is formatting_), and it's behaving in an unexpected and undesired way that I think ought to be fixed.

do they have some objection to using a formatter (eg. not sufficiently configurable, which IMO is not a big deal, any consistent formatting is better than forcing your personal tastes on others)

If it is the case that any consistent formatting is better than forcing your personal tastes on others, then supplying a formatter that is not sufficiently configurable is an _incredibly_ big deal because _that formatter is forcing its personal tastes on others_. The whole reason there are arguments over semicolons and brace styles is that _there isn't clearly right answer_ and each method has different virtues. Which style you go with will ultimately be a personal decision. A formatter that satisfies this virtue you expressed of not forcing its views on others must then be extremely configurable. Fortunately, such a tool exists: ESLint.

Even if we decide that it's only _my_ personal tastes that shouldn't be forced on others, I still have a use case if _no one else is using my code_. If I'm the only person responsible for the development and maintenance of my code base, then there's no way for me to force my tastes on others. Code should be formatted in a way that facilitates the efforts of everyone who interacts with it. If I'm the only person interacting with it, then the code should be formatted in the way that is easiest for me to read and interact with. Even if I get hit by a bus and someone inherits my responsibilities who finds my chosen format illegible, it should be a fairly simple process for them to use _their_ formatter customized with _their_ preferred rules to format the code in such a way as to suit themselves.

I want to be clear that I don't consider myself an expert on linter or formatter implementations. I honestly don't know how much effort would be required to fix this issue. I'm not trying to suggest that this issue is of trivial complexity or massive importance...only that the use case exists and is defensible. It may well be the case that this issue can't be addressed quickly or even at all due to the relative scarcity of time available to work on it -- that's fine. I'm not here to make demands, and I don't think I'm smarter or more qualified than anyone. I'd just like to know if it's an issue that will be addressed or not, and I don't want to see it swept under the rug by delegating responsibility to another tool.

Again, I sincerely appreciate that you have taken the time and made the effort to reply and discuss this issue with me. Conversations like these can be treacherous when conducted through purely text-based communications...have I interpreted your views and arguments correctly? Is there another tool available that satisfies these requirements that I've overlooked?

[leandono]

Hi guys. We had this problem about six months ago, using Standardjs with the autoFixOnSave config enabled (that use ESlint behind). Our workaround was use Prettier to format the code (with some custom configuration works similar to Standard) and keep ESlint to only lint the code.

[mudrz]

Our workaround was use Prettier to format the code (with some custom configuration works similar to Standard) and keep ESlint to only lint the code.

I started using Prettier because of this too, but it turns out it is better at formatting than ESlint anyways

[PavelPolyakov]

Decided to give a try to vs code and found that eslint doesn't fix everything in one go. That was disappointing.

I think it's obvious bug, because after the first run eslint actually knows that some part of the code remained to be incorrect, why not to try to fix it if another fix all auto-fixable problems run will do so. If this is a feature the name of the command is misleading, should be fix all auto-fixable problems, but only for this state of the file.

Let's do it recursive, please!

@dbaeumer
Let's imagine that onSave is out of discussion.

I've tried to try to fix it myself, found this piece of code:
https://github.com/Microsoft/vscode-eslint/blob/master/server/src/server.ts#L1091

and thought that we can apply another round of edits after 1st/2nd or 3rd is done. But I haven't found any option to receive the next version of the document after applyEdit. In the [documentation
(https://code.visualstudio.com/docs/extensionAPI/vscode-api#applyEdit) it's also said that it resolves in case applying edits is possible. Which does not equal that they are applied. Is there any way to build it recursive on the server side?

Maybe if you push me in the write direction I can shape up PR.
I also have no clue where the actual edits calculation takes place. As I see only codeActions.get(uri) which seems to take edits from the map, but who calculates the edits ?

Help is appreciated.

[dbaeumer]

@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.

[OneOfOne]

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

[dtinth]

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.

[jondot]

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

[dbaeumer]

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.

[arndeash]

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?

[dbaeumer]

@ahennie thanks for looking into this. Unfortunately this only fixes it for running the action manually. However a lot of reports came in for auto fix on save as well which then would lead to a total different solution. To not have two code paths here I would still like to see the addressed in a way that it works for the save case as well.

[didrip]

+1 @ahennie for this (temporary) solution.

[thernstig]

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.

[dbaeumer]

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.

[arcanis]

Is there a recommended workaround at the moment? From what I gather we can either:

• Press ^S multiple times
• Run eslint --fix

Could an extension rebind ctrl+S / cmd+S to continuously run a save until the buffer becomes stable (since the rebind would be outside of the "real" save event I guess it would not be subject to the 200ms threshold)? That would preserve the markers and work from within the editor.

[thernstig]

@arcanis Using Prettier with ESLint is a workaround that works as it will do all formatting once on save. See https://github.com/Microsoft/vscode-eslint/issues/154#issuecomment-438995360.

[kg-currenxie]

Not sure what's the point of the plugin if we have to hit cmd+S multiple times 🤦‍♂
Not only is it annoying, if you have an auto-reload listener set up for your browser, it will reload (sometimes with an error), then reload again, etc

[Bkucera]

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

[kurtextrem]

@bkucera any news?

[Bkucera]

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

[mdelano]

Looking forward to this fix. Any updates?

[FOr3st]

With autofix set up on every save I need even ___3!___ saves.
Consider an example:

Initial code:

return (<ApprovalIcon color="#ffffff" opacity={1} height="16px" width="16px"></ApprovalIcon>);

After 1st save:

return (
  <ApprovalIcon
    color="#ffffff"
    opacity={1}
    height="16px"
    width="16px"
  ></ApprovalIcon>
);

After 2nd save:

return (
  <ApprovalIcon
    color="#ffffff"
    opacity={1}
    height="16px"
    width="16px"
   />
);

After 3rd save:

return (
  <ApprovalIcon color="#ffffff" opacity={1} height="16px" width="16px" />
);

However, npm run lint -- --fix does the job in a single run.

[Bkucera]

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 :+1:

[milost1982]

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?

[raix]

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

[Bkucera]

trying to get a fix in PR this weekend

edit: closer to*

[arcanis]

Fwiw and as one data point, our engineers are reporting significant productivity loss due to this bug (we recently added a few new rules in our configuration that causes this problem to appear more than before). If VSCode's stance is to not fix this bug because it could lead to slower auto-save, consider that the alternative is for engineers to press ^S up to four times, which is hardly more efficient 😕

To workaround this, I'll have to write merged rules that will apply multiple fixes in the same pass ... not something that should have to be done imo.

[nunogois]

I still believe this should be fixed but, if it helps anyone, the way I mitigate this problem is to add fix: true to my webpack options:

extendWebpack (cfg) {
  cfg.module.rules.push({
    enforce: 'pre',
    test: /\.(js|vue)$/,
    loader: 'eslint-loader',
    exclude: /node_modules/,
    options: {
      fix: true
    }
  })
}

For reference, my .vscodesettings.json are like so:

"eslint.autoFixOnSave": true,
"eslint.packageManager": "yarn",
"eslint.validate": [
  "javascript",
  "javascriptreact",
  {
    "language": "vue",
    "autoFix": true
  }
]

This way I ensure it always runs --fix before building, which it does recursively (i.e. the right way). For any other use, remember you can always integrate your eslint --fix command in some way, like your package.json scripts.

Looking forward to seeing native VSCode improvements on this.

[arcanis]

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.

[thernstig]

@arcanis and @nunogois it is already possible to fix this with VS Code ESLint + Prettier extension as I explained here in this post https://github.com/microsoft/vscode-eslint/issues/154#issuecomment-438995360 It is a workaround but it works for now. Ideally one should not use the Prettier extension just to use the ESLint extension properly. Which is what this issue is about to fix.

[Bkucera]

@arcanis also, potential performance is not the blocker here, it's the merging fixes logic mentioned above. I just mentioned performance since that should also be considered

[zfeher]

I've experienced this multiple save or exec of ESLint: Fix all auto-fixable Problems command to fix/format a file completely recently.

Can be reproduced on this repo: https://github.com/octref/veturpack Clone it, install deps, and open Counter.vue and try issuing the ESLint: Fix all auto-fixable Problems a couple of times it needs 3-4 round.

Hope it can be fixed one day, really annoying and makes manual or auto eslint fix unusable :(

[raix]

@Bkucera are you still working on a pr? and is it solving the actual issue or just running fix a couple of times behind the scenes? (I would imagine that basing fixes on a list of rules might not be enough - so eslint should be able to provide the fix?)

[Bkucera]

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

[dtinth]

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:

[SpencerCarstens]

Brilliant!

On Fri, Sep 20, 2019 at 9:38 AM Thai Pangsakulyanont <
[email protected]> wrote:

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.

[image: after1]
https://user-images.githubusercontent.com/193136/65342933-ff6c3080-dbfd-11e9-80ea-dcaf8f5eb3f8.gif

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.

[image: before]
https://user-images.githubusercontent.com/193136/65343552-71914500-dbff-11e9-98ed-5512036c1c45.gif

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
https://github.com/microsoft/vscode/blob/66dc7a11b709c9353d6bb2fd144c7e088ad35ecf/src/vs/editor/common/services/editorSimpleWorker.ts#L445.
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
https://github.com/microsoft/vscode-eslint/issues/154#issuecomment-385344629
.

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:

[image: after]
https://user-images.githubusercontent.com/193136/65343294-d8fac500-dbfe-11e9-8a7f-20aa7fe4187c.gif

—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
https://github.com/microsoft/vscode-eslint/issues/154?email_source=notifications&email_token=AAMT4BCJH2XHGF7LWV3BTWTQKT36RA5CNFSM4CSPNPJKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD7HH2CQ#issuecomment-533626122,
or mute the thread
https://github.com/notifications/unsubscribe-auth/AAMT4BGSL5PRZJMPGE7NJ4DQKT36RANCNFSM4CSPNPJA
.

>

Best,

Spencer Carstens

[dtinth]

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

[tcg]

I don't want to pile on, just wondering: Is this related to the "toggling" behavior, where multiple saves will result in content switching among 2 or more formatted states?

(I'm researching solutions for issue shown in the gifs here: https://github.com/tcg/vscode-eslint-prettier-autoformat-glitch)

[moltar]

@tcg afaik this issue is unrelated to what you are experiencing.

Your issue is a conflict of different formatters fighting for the right format. Just use ESLint with prettier config, and disable prettier itself. I.e. get ESLint CLI config to work properly with prettier and disable prettier ext.

[tcg]

Thanks @moltar. Unfortunately I already “just” did that. I think. I’m reading your last two sentences as recommending opposite things.

So , ESLint w/ eslint-prettier plugin and no Prettier extension (my current setup) ??

Or ESLint and Prettier extensions, somehow configured not to interfere w/ each other’s rules?

[ThomasKientz]

@tcg no prettier extension, but running eslint --fix at save by your IDE or webpack. https://prettier.io/docs/en/integrating-with-linters.html

[tcg]

@ThomasKientz Thanks for that clarification.

Sorry to hijack!

I started everything from scratch, and followed those directions (yesterday, and again this morning) and still seeing some weird behavior, but I now think it's specific to rules/code I'm looking at, and not across the board.

I really appreciate the extra info! 👋

[matthewwithanm]

@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.)

[SikoSoft]

The issue still persists. :(

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

"eslint.autoFixOnSave": true,
"editor.formatOnSave": false

[dtinth]

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

[dbaeumer]

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

[kg-currenxie]

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

[Tenemo]

The fix is amazing, thank you!