Merge/remove "editor.formatOnSave" and "editor.codeActionsOnSave->"source.fixAll""

[thernstig]

There are two references for this issue:

• https://code.visualstudio.com/docs/getstarted/settings
• https://code.visualstudio.com/updates/v1_23#_run-code-actions-on-save

"editor.codeActionsOnSave": {}, has the option that it can be set like this:

  "editor.codeActionsOnSave": {
    "source.fixAll": true
  },

Then this exist:

  "editor.formatOnSave": true,

I am not sure if this is a bug or not, but it is highly confusing what they both do, as their is no indication on the settings page what the first one does. Especially in comparison to the second one.

[dbaeumer]

@jrieken FYI

[thernstig]

@dbaeumer This issue actually stemmed from your execellent update to the ESLint extension and the new settings one needed to set 🥇 (Just to be clear, your update was great!).

[jrieken]

I'd say 'as designed' - we differentiate between formatting and (source) code actions and while we are aware of the lines being blurry I'd say it still makes sense. Formatting is usually around inserting and removing whitespace while a source action is also about removing unnecessary imports, unused variables etc etc.

[dbaeumer]

@jrieken I agree but had to accept the different point of view ESLint for example has on this.

[jrieken]

Sure, but they are the outliner - I don't think their point of view is enough to change this for various languages that to make this difference - like TS, Java, etc

[thernstig]

It gets a bit confusing also with this definition by Prettier, which is widly used in addition to ESLint: https://prettier.io/docs/en/comparison.html.

If you find a distinction between formatting and (source) code actions, could this at the minimum be explained in the VS Code settings docs? As is now, it is not even explained there, even though one of the biggest linters for VS Code, ESLint, uses the code actions part.

[mjbvz]

I agree with @jrieken that the distinction between format and code actions makes sense. Also see #48547 for running organize imports as part of format

@thernstig What would you expect the docs to say?

[thernstig]

I would expect source.fixAll to get mentioned on the settings page. There is no mention of it, nor is there a mention of source.organizeImports.

As ESLint does formatting, from @jrieken's explanation it sounds like ESLint should not have its config under "source.fixAll.eslint": true but I also see that @dbaeumer specifically mentions ESLint has a different view here, which they had to accept.

I am just confused since other formatters seems to rely on "editor.formatOnSave": true, while ESLint relies on "source.fixAll.eslint": true under editor.codeActionsOnSave

(I believe I am starting to repeat myself, and you are aware that the lines are blurry - but at least some other chap in the future can look at this thread for reference)

[ntotten]

We have a long-standing bug on the Prettier extension where the codeActionsOnSave and format on save causes some conflicts and undesirable flickers. It would be nice if there was some way to control this pipeline so that the file only appears to be modified once on save to avoid flickering. prettier/prettier-vscode#716

[dbaeumer]

The problem with ESLint here is that it is not only touching white space it fixes a lot of non white space things as well (e.g. wrong quites, missing brackets, ...). This is why I think it should be treated as code actions on save.

As @ntotten points out the problem is that users can enable both and the ESLint extension has no way to indicate that it does the formatter job during the code action on save hook as well. If I could when registering telling VS Code that it could skip the format call on save hence avoid the extra unnecessary work.

I could in ESLint remember some document version for the last code action on save call and then skip the format call if the document version is sitll the same. However this requires that VS Code specs the call sequence. Otherwise I might make things worse.

[thernstig]

And from a user perspective who do not know the internals, from my side source.fixAll in comparison to editor.formatOnSave is strange because I have no idea what "source" and "fixAll" means. It could mean anything to me. E.g. "convert constructor functions to ES2015 classes" could in my world fit into that, even though that is not the case now. Is source.fixAll actually doing anything else beside "formatting both whitespace and non-whitespace" characters (as in the ESLint case)? Again, it is the lack of information on what it is supposed to achieve that is confusing for me, so I have no idea what I am configuring.

And then why isn't editor.formatOnSave moved in like this to keep them all in one place?

  "editor.codeActionsOnSave": {
    "source.format": true
  },

[mjbvz]

I agree the boundary is fuzzy in cases where formatting problems (such as whitespace) are reported as errors. However in my option, there is still a important conceptual distinction:

• Format: This should only change the visual representation of the code (not change its actual structure).

• Source actions: This may change the code itself. For example, fix all could implement an interface or remove an unused parameter.

I feel extensions themselves are probably in the best position to judge how to deal with this distinction. @dbaeumer Does eslint make this distinction today?

Also, #67147 discussed unifying our on save functionality but has not seen much interest

[dbaeumer]

ESLint supports fix types however it is up to the user to specify them. I in the extension ask eslint to compute fixes on save and take the once the eslint npm module computes. I don't change them in any way.

[vscodebot]

This issue has been closed automatically because it needs more information and has not had recent activity. See also our issue reporting guidelines.

Happy Coding!

[jayqi]

Hello, I'd like some more guidance about the proper way to reconcile these different redundant settings.

I have both "editor.formatOnType": true and "editor.formatOnSave": true which work for me for formatters for other languages (Python). I'd like to keep both these settings.

What is the right way to editor.codeActionsOnSave? It sounds like this would lead to the formatting executing twice on save. Do I want to do the following then?

"editor.codeActionsOnSave": {
    "source.fixAll": false
},

Does having:

"eslint.format.enable": true,
"[javascript]": {
    "editor.defaultFormatter": "dbaeumer.vscode-eslint"
}

then still work with both "editor.formatOnType": true and "editor.formatOnSave": true?

[pluma]

Oh no :(