Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

LSP code actions on save not working #2495

Open
james2doyle opened this issue May 31, 2024 · 9 comments
Open

LSP code actions on save not working #2495

james2doyle opened this issue May 31, 2024 · 9 comments
Labels

Comments

@james2doyle
Copy link
Contributor

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?

@predragnikolic predragnikolic transferred this issue from sublimelsp/LSP-biome Jun 21, 2024
@predragnikolic
Copy link
Member

predragnikolic commented Jun 21, 2024

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.

@rchl
Copy link
Member

rchl commented Jun 21, 2024

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
Copy link
Member

rchl commented Jun 21, 2024

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?

@predragnikolic
Copy link
Member

related microsoft/language-server-protocol#1629 (comment)

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.

@LDAP
Copy link
Contributor

LDAP commented Jun 21, 2024

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

@predragnikolic
Copy link
Member

predragnikolic commented Jun 21, 2024

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: []

@rchl
Copy link
Member

rchl commented Jun 21, 2024

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

@predragnikolic
Copy link
Member

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
Copy link
Member

predragnikolic commented Jul 2, 2024

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 biomejs/biome#3339

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

4 participants