microsoft/vscode-eslint

ESLint server only looks at first entry in `context.only` array sent along with `textDocument/codeAction`

mrnugget opened this issue · 4 comments

At Zed we allow users to set up multiple code actions to be executed on save.

That breaks with the ESLint language server because the server only looks at the first entry of the context: { only: [] } array sent along with textDocument/codeAction requests.

Example: when we sent a request like this

{
  "jsonrpc": "2.0",
  "id": 140,
  "method": "textDocument/codeAction",
  "params": {
    // ...
    "context": {
      "diagnostics": [
        // ...
      ],
      "only": ["source.organizeImports", "source.fixAll.eslint"]
    }
  }
}

Then the ESLint server will only look at "source.organizeImports" and ignore the source.fixAll.eslint. And that has disastrous results: the server will add "disable" comments when executing organizeImports — which is not what users want most of the time.

@hmttrp tracked the behavior down to the code here:

const only: string | undefined = params.context.only !== undefined && params.context.only.length > 0 ? params.context.only[0] : undefined;

I'm not sure what exactly the intention behind the check is (I don't understand why not all entries are checked), but I think a fix would be to turn the booleans here

const isSource = only === CodeActionKind.Source;
const isSourceFixAll = (only === ESLintSourceFixAll || only === CodeActionKind.SourceFixAll);

into checks whether the array contains an item of that kind (and not just whether the first item has that kind).

This is a problem with the Zed client. The ESLint server registers for the following code action kinds

https://github.com/microsoft/vscode-eslint/blob/main/server/src/eslintServer.ts#L224

It should not be called with a code action kind of organize imports since this is nothing it has been registered for.

That's how I ended up fixing it: zed-industries/zed#14666

Ultimately your call, of course, but I think being more robust against input here and not executing different code actions would make future debugging easier.

I agree with that. But a client should not send unnecessary code actions requests for performance reasons.

Yeah, 100%! That was a bug on our side, definitely!