LSP icon indicating copy to clipboard operation
LSP copied to clipboard

LSP code actions on save not working

Open james2doyle opened this issue 1 year ago • 9 comments

Using the instructions for formatting on save does not seem to be working.

It works fine when called via the command palette.

image

Also seems like the action does not match the expected format?

james2doyle avatar May 31 '24 17:05 james2doyle

Quick notes

1 Issue with the JSON schema

  • the lsp_code_actions_on_save json schema would need to be relaxed a bit:
 "lsp_code_actions_on_save": {
              "type": "object",
              "additionalProperties": {
                "type": "boolean"
              },
              "propertyNames": {
                "pattern": "^source\\."
              },
              "markdownDescription": "A dictionary of code action identifiers that should be triggered on save.\n\nCode action identifiers are not officially standardized so refer to specific server's documentation on what is supported but `source.fixAll` is commonly used to apply fix-on-save code actions.\n\nThis option is also supported in syntax-specific settings and/or in the `\"settings\"` section of project files. Settings from all those places will be merged and more specific (syntax and project) settings will override less specific (from LSP or Sublime settings).\n\nOnly \"source.*\" actions are supported."
            },

2 the code action on save is never sent to Biome

Here is the initialize response for Biome:

:: [12:47:29.797] <<< LSP-biome (1) (duration: 90ms): {'capabilities': {'codeActionProvider': True, 'positionEncoding': 'utf-16', 'textDocumentSync': {'didOpen': {}, 'save': {}, 'didClose': {}, 'change': {'syncKind': 2}}}, 'serverInfo': {'name': 'biome_lsp', 'version': '1.5.3'}}

note 'codeActionProvider': True

Here is the initialize response for ESLint:

:: [12:47:29.927] <<< LSP-eslint (1) (duration: 107ms): {'capabilities': {'workspace': {'workspaceFolders': {'supported': True}}, 'executeCommandProvider': {'commands': ['eslint.applySingleFix', 'eslint.applySuggestion', 'eslint.applySameFixes', 'eslint.applyAllFixes', 'eslint.applyDisableLine', 'eslint.applyDisableFile', 'eslint.openRuleDoc']}, 'codeActionProvider': {'codeActionKinds': ['quickfix', 'source.fixAll.eslint']}, 'textDocumentSync': {'change': {'syncKind': 2}, 'didOpen': {}, 'didClose': {}, 'save': {'includeText': False}}}}

note 'codeActionProvider': {'codeActionKinds': ['quickfix', 'source.fixAll.eslint']}

This is important because when get_session_kinds is called, that function will not return anything for the Biome session.

Then, there is this peace of code that probably needs to change:

    def _get_code_actions_on_save(cls, view: sublime.View) -> dict[str, bool]:
        view_code_actions = cast(Dict[str, bool], view.settings().get('lsp_code_actions_on_save') or {})
        code_actions = userprefs().lsp_code_actions_on_save.copy()
        code_actions.update(view_code_actions)
        allowed_code_actions = dict()
        for key, value in code_actions.items():
-            if key.startswith('source.'):
+            if key.startswith('source.') or key.startswith('quickfix.'):
                allowed_code_actions[key] = value
        return allowed_code_actions

And the third thing... I tired to see how the Biome extension behaves in VS code and I could not make the quickfix.biome code action work in VS Code either...

All in all, there are things to fix on the Sublime Text LSP side.

predragnikolic avatar Jun 21 '24 11:06 predragnikolic

My impression was that only source actions should be usable from code_actions_on_save. Quick fixes normally apply to specific places in the file and are triggered manually. So I'm not sure if Biome is following the rules correctly.

I said "rules" and not "spec" because that functionality is technically not specified and we're just trying to follow what Microsoft does in its products.

I would start with creating a Biome issue to discuss that.

rchl avatar Jun 21 '24 11:06 rchl

Though if quickfix.* works in VSCode then we should probably allow that too. But I'd first check its source code to see what is exactly allowed. Maybe everything?

rchl avatar Jun 21 '24 11:06 rchl

related https://github.com/microsoft/language-server-protocol/issues/1629#issuecomment-1380267878

For code actions on save: the client will ask with a specific kind for code actions on save. The kind is CodeActionKind.SourceFixAll

Just with this sentence, it kind of looks like QuickFix should not be in the list for code actions on save.

predragnikolic avatar Jun 21 '24 11:06 predragnikolic

From this comment I understand that it does not work in VS code, meaning ST behaves the same?

I tired to see how the Biome extension behaves in VS code, but I could not make the quickfix.biome code action work in VS Code either...

LDAP avatar Jun 21 '24 11:06 LDAP

EDIT: I had disabled the biome linter in biome.json. And that is the reason why it didn't work in VS Code.


From this comment I understand that it does not work in VS code, meaning ST behaves the same?

Yes.

For more details -> In VS Code I have the following:

    "editor.defaultFormatter": "biomejs.biome",
    "biome_lsp.trace.server": "verbose",
    "editor.codeActionsOnSave": {
        "quickfix.biome": "always",
        "source.fixAll.ts": "always",
        "source.organizeImports.ts": "always",
        "source.addMissingImports.ts": "always",
    },

Triggering the Format Document command will work in VS Code. But pressing save and expecting the quickfix.biome code action to run not do anything:

VS Code req/res for the `quickfix.biome` code action
[Trace - 1:59:34 PM] Sending request 'textDocument/codeAction - (97)'.
Params: {
    "textDocument": {
        "uri": "file:///Users/predrag/Documents/sandbox/bejst/appAdmin/src/layout/Sidebar.tsx"
    },
    "range": {
        "start": {
            "line": 0,
            "character": 0
        },
        "end": {
            "line": 100,
            "character": 0
        }
    },
    "context": {
        "diagnostics": [
            {
                "range": {
                    "start": {
                        "line": 12,
                        "character": 21
                    },
                    "end": {
                        "line": 12,
                        "character": 26
                    }
                },
                "message": "Cannot find module 'foo' or its corresponding type declarations.",
                "code": 2307,
                "severity": 1,
                "source": "ts"
            },
            {
                "range": {
                    "start": {
                        "line": 12,
                        "character": 0
                    },
                    "end": {
                        "line": 12,
                        "character": 26
                    }
                },
                "message": "All imports in import declaration are unused.",
                "code": 6192,
                "severity": 4,
                "tags": [
                    1
                ],
                "source": "ts"
            }
        ],
        "only": [
            "quickfix.biome"
        ],
        "triggerKind": 2
    }
}


[Trace - 1:59:34 PM] Received response 'textDocument/codeAction - (97)' in 2ms.
Result: []

predragnikolic avatar Jun 21 '24 12:06 predragnikolic

I've been reading https://github.com/biomejs/biome/issues/1570 where people seem to be using quickfix.biome (and having unrelated problems with it).

rchl avatar Jun 21 '24 12:06 rchl

My bad.

I had linter set to false in biome.json (the project uses ESLint, so it had biome lining disabled). That is the reason why it didn't work in VS Code.

I enabled the linter and code action on save started to work in VS Code.

{
  "$schema": "https://biomejs.dev/schemas/1.5.3/schema.json",
  "linter": {
    "enabled": true, // this was `false`
    "rules": {
      "complexity": {
        "noUselessEmptyExport": "error"
      }
    }
  },
  "formatter": {
    "ignore": ["openapi.*"],
    "enabled": true,
    "indentStyle": "space",
    "lineWidth": 120
  },
  "javascript": {
    "formatter": {
      "semicolons": "asNeeded"
    }
  },
  "vcs": {
    "enabled": true,
    "clientKind": "git",
    "useIgnoreFile": true
  },
  "organizeImports": {
    "enabled": false
  }
}

predragnikolic avatar Jun 21 '24 20:06 predragnikolic

I have read the LSP spec 2 times recently and went through few LSP spec issues and the way I understood is the following -> Biome should rename the code action on save to be more spec compliant.

I've opened a ticket at the biome repo https://github.com/biomejs/biome/issues/3339

predragnikolic avatar Jul 02 '24 20:07 predragnikolic