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