Improve error message / feedback when a file can't be linted with ESLint when in probing mode

DanKaplanSES commented 9 months ago

Environment

Environment Info:

Node version: v18.17.1 npm version: v9.6.7 Local ESLint version: v8.50.0 (Currently used) Global ESLint version: Not found Operating System: win32 10.0.19045

What parser are you using?

Default (Espree)

What did you do?

Configuration

``` { "env": { "browser": true, "es6": true, "node": true, "jasmine": true }, "plugins": [], "extends": [ "eslint:recommended", "prettier", "plugin:prettier/recommended" ], "globals": { "Atomics": "readonly", "SharedArrayBuffer": "readonly", "State": "writable", "setup": "writable" }, "parserOptions": { "sourceType": "module" }, "rules": { "prettier/prettier": "error", "quotes": ["off"], "no-plusplus": ["off"], "curly": ["error", "all"], "prefer-const": ["warn"] } } ```

Save this file outside of your project:

// test.js
const x = function () {
console.log(
`log!`)
};

Press ctrl + o to open the file browser
Open the file you saved on step one
Press ctrl + shift + p to open the command palette
Select "Format Document"

What did you expect to happen?

I expected this JavaScript file to be formatted by ESLint.

If it doesn't work for some reason, I expected to see a helpful log message in the output tab explaining why the ESLint extension didn't/couldn't format the file.

Originally, I opened this issue in the core eslint repository by mistake. But on the bright side, I received an insightful reply from @rec0il :

I think this is happening because there is no ESLint config file relative to the test.js file or in the parent directories above it.
root/
    test.js
    project/
        index.js
        .eslintrc.json

I suspected the same reason, but in my humble opinion, the ESLint extension should error with more information on how to resolve the issue.

For example, the output tab didn't print anything when this occurred: this implies something external to the ESLint extension is causing the error. My first reaction was to check if the ESLint extension was set as the default formatter for JS files. Since that wasn't the case, I googled for this error message and some of the answers said I need to toggle some of the extension settings off and on to fix a corrupt configuration. This was not the only answer to the symptom, suggesting that 1) there are multiple reasons this could happen and 2), people are throwing stuff at the wall to see what sticks. A lot of this could be short-circuited if there was a detailed log message in the output tab.

In addition, I think that the ESLint extension should format the file based on the VS Code project's .eslintrc.json file. If there are more than two, pick a sensible default (e.g., the one closer to the project root?). Since this could mask eslint and eslint extension configuration issues, I think the output tab should also log a warning that no suitable .eslintrc.json file was found so it's defaulting to "x".

What actually happened?

I get this error message:

Pressing that configure button does not let me select eslint as an option.

Nothing is logged to the eslint output tab. I have debugging on and I have server trace on.

Link to Minimal Reproducible Example

https://github.com/DanKaplanSES/jsdom-sandbox

Participation

[X] I am willing to submit a pull request for this issue.

Additional comments

You will still have to follow the steps in the minimal reproduction example: I can't commit a file outside of its repository and that is one of the steps.

dbaeumer commented 9 months ago

I am open for a PR to improve this. However, one thing we need to keep in mind that this can't be disturbing for users that open files that can't be linted. In the past we were too eager to warn the user about this and we got pushback since people do open JS code for which no ESLint is configured.

dbaeumer commented 9 months ago

The code in question is around here: https://github.com/microsoft/vscode-eslint/blob/17e79c0f7bcd38f3033d0fbc346a5607fbb4c60f/server/src/eslint.ts#L881

Were we set silent to true if we probe support for the given file. If that files and we are in the probing run we don't print things to the suer. But I agree that a Info entry would be useful.

DanKaplanSES commented 9 months ago

I am open for a PR to improve this. However, one thing we need to keep in mind that this can't be disturbing for users that open files that can't be linted. In the past we were too eager to warn the user about this and we got pushback since people do open JS code for which no ESLint is configured.

What do you think about falling back to a reasonable default eslint config if the file is not in the project? In other words, there may not be a need for an error message if it formats regardless.

dbaeumer commented 9 months ago

Actually I am not a fan of a default. It is IMO in the longer term more confusing than helpful. The message of course should contain some links / text on how to create a config file or how to install eslint.

DanKaplanSES commented 8 months ago

Not sure which extension is responsible for this behavior or which behavior is working incorrectly (if any), but I get interesting behavior when I use this extension: buenon.scratchpads.

In this extension, when you create a scratchpad it opens a new tab and pastes your clipboard into it. Immediately after, it appears that vscode-eslint formats the scratchpad:

But the file is not in my project, it resides in: C:\Users\<user>\AppData\Roaming\Code\User\globalStorage\buenon.scratchpads\scratchpads\2598de6c6ff8511bd5e99191be2e3a80\scratch33.js

Then after that first time, Format Document will show the alert I describe in in my original post.

On one hand, troubleshooting this may take a log of effort, on the other hand, this might reveal a legitimate vscode-eslint bug. Either way, it sounds like you are saying it should never have formatted the document that first time?

dbaeumer commented 8 months ago

This actually depends on whether an ESLint instance (npm module) and configuration file is found in C:\Users\<user>\AppData\Roaming\Code\User\globalStorage\buenon.scratchpads\scratchpads\2598de6c6ff8511bd5e99191be2e3a80 or one of its parent directories.

DanKaplanSES commented 8 months ago

This actually depends on whether an ESLint instance (npm module) and configuration file is found in C:\Users\<user>\AppData\Roaming\Code\User\globalStorage\buenon.scratchpads\scratchpads\2598de6c6ff8511bd5e99191be2e3a80 or one of its parent directories.

Why would that make it format once but give me the pop up every time after? Either way, the only files in that directory are scratchpad's I have created, so I don't think that's the reason it formats: There are no NPM modules or configuration files.

dbaeumer commented 8 months ago

Could it be that you specify the ESLint formatter as default in the user settings ?

DanKaplanSES commented 8 months ago

Could it be that you specify the ESLint formatter as default in the user settings ?

I do have this in my settings.json:

  "[typescript]": {
    "editor.defaultFormatter": "dbaeumer.vscode-eslint"
  },
  "[javascript]": {
    "editor.defaultFormatter": "dbaeumer.vscode-eslint"
  },

Does that explain why it formats the first time but gives me the pop up whenever I use it on the file later?

dbaeumer commented 8 months ago

What is wired is the fact that you say you can format it the first time. When you have eslint set as the default formatter and you format a TS/JS file here is what is happening in VS Code:

VS Code will load the ESLint extension
ESLint extension will probe if eslint can be executed on that file. This can fail for various reason. Major ones are:
- ESlint it not installed in any of the parent folders of that file
- No eslint configuration can be found in any parent folder of that file
- No corresponding parser is installed for the file type (if for example you try to format a TS file)
If eslint can be executed on the file, the extension will register a format provider.
if no eslint format provider is registered for the file you want to format you see that dialog

In a setup where the probing fails you will see the dialog, however it should already appear on the first attempt of formatting that file.

So for the scratch pad files I would assume formatting always fails, even on first attempt. I tried to reproduce the scenario of not being able to execute eslint on a file and I can never format the file. Even the first attempt fails for me.

DanKaplanSES commented 8 months ago

Clarification: It doesn't format the first time I try, it formats automatically when I create the scratch pad. This extension automatically pastes what's in your clipboard into the scratch pad and it formats what it pastes automatically.

Copy some Unformatted javascript into your clipboard.
Create a javascript scratch pad.

The new scratch pad will have formatted a javascript in it.

dbaeumer commented 8 months ago

I am pretty confident that the first format is not done by ESLint. Can you disable ESLint extension and try the same to prove this.

microsoft / vscode-eslint