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 do not work intermittently #2482

Closed
sricks opened this issue May 24, 2024 · 8 comments
Closed

lsp_code_actions_on_save do not work intermittently #2482

sricks opened this issue May 24, 2024 · 8 comments
Labels

Comments

@sricks
Copy link

sricks commented May 24, 2024

Describe the bug

I'm using LSP-typescript in sublimetext4 on Linux.

With the following LSP configuration, the addMissingImports action doesn't work, intermittently.

	"lsp_code_actions_on_save": {
		"source.fixAll.ts": true,
		"source.organizeImports.ts": true,
		"source.addMissingImports.ts": true,
	},

I think it's possibly an overload of the typescript server. When I use only source.addMissingImports.ts without organize or fix, it works more often.

This is a simple example, it's a very basic react app created with vite, in App.tsx

// missing import: import { useState } from "react";

function App() {
	const [foo, bar] = useState(0);
	return "foo";}

export default App;

If I edit the file at all, eg insert a new line, am typing a comment, or anything, and save immediately after my last keystroke: the import is not added back.

But if I finish typing and wait about 1 second, and then save, the import is added back. So it seems like typescript is still "processing" my new additions and therefore doesn't respect the action.

Expected behavior

Import should always be added regardless of ongoing "processing" by the typescript server.

Logs
This log is me adding an "f" to the end of a comment and saving immediately, when it didn't work.

:: [13:23:58.656]  -> LSP-typescript textDocument/didChange: {'textDocument': {'uri': 'file:///home/d/react/first-app/vite-project/src/App.tsx', 'version': 1313}, 'contentChanges': [{'range': {'start': {'line': 1, 'character': 22}, 'end': {'line': 1, 'character': 22}}, 'rangeLength': 0, 'text': 'f'}]}
:: [13:23:58.656] --> LSP-typescript textDocument/codeAction (17): {'textDocument': {'uri': 'file:///home/d/react/first-app/vite-project/src/App.tsx'}, 'range': {'start': {'line': 0, 'character': 0}, 'end': {'line': 7, 'character': 0}}, 'context': {'diagnostics': [], 'triggerKind': <CodeActionTriggerKind.Automatic: 2>, 'only': ['source.addMissingImports.ts', 'source.organizeImports.ts']}}
:: [13:23:58.665] <<< LSP-typescript (17) (duration: 8ms): []
:: [13:23:58.898]  -> LSP-typescript textDocument/didSave: {'textDocument': {'uri': 'file:///home/d/react/first-app/vite-project/src/App.tsx'}}
:: [13:23:58.899] --> LSP-typescript textDocument/codeLens (18): {'textDocument': {'uri': 'file:///home/d/react/first-app/vite-project/src/App.tsx'}}
:: [13:23:58.900] --> LSP-typescript textDocument/documentHighlight (19): {'textDocument': {'uri': 'file:///home/d/react/first-app/vite-project/src/App.tsx'}, 'position': {'line': 1, 'character': 23}}
:: [13:23:58.900] <<< LSP-typescript (18) (duration: 0ms): []
:: [13:23:58.902] <<< LSP-typescript (19) (duration: 2ms): []
:: [13:23:59.012] <-  LSP-typescript textDocument/publishDiagnostics: {'uri': 'file:///home/d/react/first-app/vite-project/src/App.tsx', 'diagnostics': [{'range': {'start': {'line': 2, 'character': 8}, 'end': {'line': 2, 'character': 11}}, 'message': "'foo' is declared but its value is never read.", 'severity': 1, 'code': 6133, 'source': 'typescript', 'tags': [1]}, {'range': {'start': {'line': 2, 'character': 13}, 'end': {'line': 2, 'character': 16}}, 'message': "'bar' is declared but its value is never read.", 'severity': 1, 'code': 6133, 'source': 'typescript', 'tags': [1]}, {'range': {'start': {'line': 2, 'character': 20}, 'end': {'line': 2, 'character': 28}}, 'message': "Cannot find name 'useState'.", 'severity': 1, 'code': 2304, 'source': 'typescript', 'tags': []}]}
:: [13:23:59.015] --> LSP-typescript textDocument/codeAction (20): {'textDocument': {'uri': 'file:///home/d/react/first-app/vite-project/src/App.tsx'}, 'range': {'start': {'line': 1, 'character': 23}, 'end': {'line': 1, 'character': 23}}, 'context': {'diagnostics': [], 'triggerKind': <CodeActionTriggerKind.Automatic: 2>}}
:: [13:23:59.019] <<< LSP-typescript (20) (duration: 3ms): [{'title': 'Move to a new file', 'kind': 'refactor.move', 'command': {'title': 'Move to a new file', 'command': '_typescript.applyRefactoring', 'arguments': [{'file': '/home/d/react/first-app/vite-project/src/App.tsx', 'startLine': 2, 'startOffset': 24, 'endLine': 2, 'endOffset': 24, 'refactor': 'Move to a new file', 'action': 'Move to a new file'}]}}]
:: [13:23:59.032] <-  LSP-typescript textDocument/publishDiagnostics: {'uri': 'buffer:246', 'diagnostics': [{'range': {'start': {'line': 0, 'character': 13}, 'end': {'line': 0, 'character': 22}}, 'message': "'someparam' is declared but its value is never read.", 'severity': 4, 'code': 6133, 'source': 'typescript', 'tags': [1]}]}

Environment (please complete the following information):

  • OS: Ubuntu 24.04
  • Sublime Text version: 4152
  • LSP version: 2.1.0
  • Language servers used: typescript

Possibly related to these issues:
#2421
microsoft/vscode#164876

@sricks sricks changed the title lsp_code_actions_on_save do not work intermittently lsp_code_actions_on_save do not work intermittently May 24, 2024
@sricks
Copy link
Author

sricks commented May 24, 2024

Also noticed something else. If I have source.removeUnused enabled, it only removes some bad imports, in weird ways. In the same App.tsx file:

import { useCallback, useState } from "react";
import reactLogo from "./assets/react.svg";
import viteLogo from "/vite.svg";

useCallback, reactLogo and viteLogo are not needed. I experience the same problem of save-after-typing, even when removeUnused is the only action, but also:

If I use removeUnused AND organizeImports together on save, nothing will get removed at all, no matter what. UNLESS I manually delete the reactLogo and viteLogo import lines; then, the useCallback will get deleted appropriately.

With all of these issues, order of the actions does not seem to matter.

@sricks
Copy link
Author

sricks commented May 28, 2024

I'm not sure if something changed, but I've discovered that addMissingImports never works as long as organizeImports is enabled. I think it might be because the changes from both of these actions are batched; they affect the same lines, so, somehow the end state is that nothing gets changed?

Logs:

:: [19:53:49.360] --> LSP-typescript textDocument/codeAction (17): {'textDocument': {'uri': 'file:///home/d/react/todo/src/App.tsx'}, 'range': {'start': {'line': 0, 'character': 0}, 'end': {'line': 87, 'character': 0}}, 'context': {'diagnostics': [{'range': {'start': {'line': 48, 'character': 4}, 'end': {'line': 48, 'character': 12}}, 'message': "Cannot find name 'TodoList'.", 'severity': 1, 'code': 2304, 'source': 'typescript', 'tags': []}], 'triggerKind': <CodeActionTriggerKind.Automatic: 2>, 'only': ['source.fixAll.ts', 'source.addMissingImports.ts', 'source.organizeImports.ts']}}
:: [19:53:49.360] --> LSP-eslint textDocument/codeAction (2): {'textDocument': {'uri': 'file:///home/d/react/todo/src/App.tsx'}, 'range': {'start': {'line': 0, 'character': 0}, 'end': {'line': 87, 'character': 0}}, 'context': {'diagnostics': [], 'triggerKind': <CodeActionTriggerKind.Automatic: 2>, 'only': ['source.fixAll.eslint']}}
:: [19:53:49.371] <<< LSP-eslint (2) (duration: 10ms): []
:: [19:53:49.411] <<< LSP-typescript (17) (duration: 51ms): [{'title': 'Organize Imports', 'edit': {'documentChanges': [{'textDocument': {'uri': 'file:///home/d/react/todo/src/App.tsx', 'version': 311}, 'edits': [{'range': {'start': {'line': 0, 'character': 0}, 'end': {'line': 1, 'character': 0}}, 'newText': 'import "@/App.css";\nimport Button from "@/components/Button";\nimport FilterButton from "@/components/FilterButton";\nimport SearchBox from "@/components/SearchBox";\nimport TodoFooter from "@/components/TodoFooter";\nimport { Filter, Todo } from "@/types";\nimport { useState } from "react";\n'}, {'range': {'start': {'line': 1, 'character': 0}, 'end': {'line': 2, 'character': 0}}, 'newText': ''}, {'range': {'start': {'line': 2, 'character': 0}, 'end': {'line': 3, 'character': 0}}, 'newText': ''}, {'range': {'start': {'line': 3, 'character': 0}, 'end': {'line': 4, 'character': 0}}, 'newText': ''}, {'range': {'start': {'line': 4, 'character': 0}, 'end': {'line': 5, 'character': 0}}, 'newText': ''}, {'range': {'start': {'line': 5, 'character': 0}, 'end': {'line': 6, 'character': 0}}, 'newText': ''}, {'range': {'start': {'line': 6, 'character': 0}, 'end': {'line': 7, 'character': 0}}, 'newText': ''}]}]}, 'kind': 'source.organizeImports.ts'}, {'title': 'Add all missing imports', 'edit': {'documentChanges': [{'textDocument': {'uri': 'file:///home/d/react/todo/src/App.tsx', 'version': 311}, 'edits': [{'range': {'start': {'line': 5, 'character': 0}, 'end': {'line': 5, 'character': 0}}, 'newText': 'import TodoList from "@/components/TodoList";\n'}]}]}, 'kind': 'source.addMissingImports.ts'}]

@sricks
Copy link
Author

sricks commented May 28, 2024

By the way, the workaround I'm using is

  • Disable organizeImports on save
  • Use the Sublime command pallete to organize imports on-demand
  • Always wait ~1 second after editing before saving to ensure imports are added on save

@jwortmann
Copy link
Member

That looks like a missed bug to me still from the Python 3.8 update. LSP tries to send the enum value object for the triggerKind instead of just the plain value. I guess there is a corresponding error message in the console that it's not JSON-serializable?

@sricks
Copy link
Author

sricks commented May 28, 2024

I enabled "log_debug": true in my LSP settings but the only thing I see in my sublime console on save is:

LSP: ignoring edit due to non-matching document version
LSP: ignoring edit due to non-matching document version

If I can help debug this in some way, please let me know and I'd be glad to help.

@predragnikolic
Copy link
Member

It is a bug.

We trigger multiple lsp_apply_document_edit commands at the same time, here ->

LSP/plugin/code_actions.py

Lines 261 to 265 in cd7cb68

for config_name, code_actions in responses:
session = self._task_runner.session_by_name(config_name, 'codeActionProvider')
if session:
tasks.extend([
session.run_code_action_async(action, progress=False, view=view) for action in code_actions

So when the edits for the first one is applied,
the second one will fail with the error LSP: ignoring edit due to non-matching document version ,
because the view.count changed.

if required_view_version is not None and required_view_version != view_version:

Request
{
  "context": {
    "diagnostics": [
      {
        "code": 2304,
        "message": "Cannot find name 'useState'.",
        "range": {"end": {"character": 20, "line": 18 }, "start": {"character": 12, "line": 18 } },
        "severity": 1,
        "source": "typescript",
        "tags": []
      },
      {
        "code": 6133,
        "message": "'d' is declared but its value is never read.",
        "range": {"end": {"character": 9, "line": 18 }, "start": {"character": 8, "line": 18 } },
        "severity": 4,
        "source": "typescript",
        "tags": [1]
      }
    ],
    "only": [
      "source.addMissingImports.ts",
      "source.organizeImports.ts"
    ],
    "triggerKind": 2
  },
  "range": {"end": {"character": 0, "line": 113 }, "start": {"character": 0, "line": 0 } },
  "textDocument": {
    "uri": "file:///Users/predrag/Documents/work/pravoslavnaknjizara/appAdmin/src/pages/OrdersPage/OrdersPage.tsx"
  }
}
Response
[
  {
    "edit": {
      "documentChanges": [
        {
          "edits": [
            {
              "newText": "import { DataNotFound } from \"@/components/NotDataFound/NotDataFound\"\nimport { SomethingBadHappen } from \"@/components/SomethingBadHappen/SomethingBadHappen\"\nimport { TopBar } from \"@/components/TopBar/TopBar\"\nimport { fets } from \"@/http\"\nimport { orderStatusToLabel } from \"@/utils/orderStatusLabels\"\nimport { useScrollToTop } from \"@/utils/useScrollToTop\"\nimport { Input, Pagination, ScrollShadow, Tab, Tabs } from \"@nextui-org/react\"\nimport { OrderStatus } from \"@shared/utils/applicationWideConstants\"\nimport { useDebounce } from \"@shared/utils/hooks/useDebounce\"\nimport { useNoInitialEffect } from \"@shared/utils/hooks/useNoInitialEffect\"\nimport { usePagination } from \"@shared/utils/hooks/usePagination\"\nimport { useQueryParam } from \"@shared/utils/hooks/useQueryParams\"\nimport { keepPreviousData, useQuery } from \"@tanstack/react-query\"\nimport { AnimatePresence, motion } from \"framer-motion\"\nimport { Search } from \"lucide-react\"\nimport { OrderItem } from \"./components/OrderItem\"\n",
              "range": {
                "end": {
                  "character": 0,
                  "line": 1
                },
                "start": {
                  "character": 0,
                  "line": 0
                }
              }
            },
            {
              "newText": "",
              "range": {"end": {"character": 0, "line": 2 }, "start": {"character": 0, "line": 1 } } },
            {
              "newText": "",
              "range": {"end": {"character": 0, "line": 3 }, "start": {"character": 0, "line": 2 }
              }
            },
            {
              "newText": "",
              "range": {"end": {"character": 0, "line": 4 }, "start": {"character": 0, "line": 3 } }
            },
            {
              "newText": "",
              "range": {"end": {"character": 0, "line": 5 }, "start": {"character": 0, "line": 4 }
              }
            },
            {
              "newText": "",
              "range": {"end": {"character": 0, "line": 6 }, "start": {"character": 0, "line": 5 }
              }
            },
            {
              "newText": "",
              "range": {"end": {"character": 0, "line": 7 }, "start": {"character": 0, "line": 6 } }
            },
            {
              "newText": "",
              "range": {"end": {"character": 0, "line": 8 }, "start": {"character": 0, "line": 7 } }
            },
            {
              "newText": "",
              "range": {"end": {"character": 0, "line": 9 }, "start": {"character": 0, "line": 8 } }
            },
            {
              "newText": "",
              "range": {"end": {"character": 0, "line": 10 }, "start": {"character": 0, "line": 9 } }
            },
            {
              "newText": "",
              "range": {"end": {"character": 0, "line": 11 }, "start": {"character": 0, "line": 10 } }
            },
            {
              "newText": "",
              "range": {"end": {"character": 0, "line": 12 }, "start": {"character": 0, "line": 11 } }
            },
            {
              "newText": "",
              "range": {"end": {"character": 0, "line": 13 }, "start": {"character": 0, "line": 12 } }
            },
            {
              "newText": "",
              "range": {"end": {"character": 0, "line": 14 }, "start": {"character": 0, "line": 13 } }
            },
            {
              "newText": "",
              "range": {"end": {"character": 0, "line": 15 }, "start": {"character": 0, "line": 14 } }
            },
            {
              "newText": "",
              "range": {"end": {"character": 0, "line": 16 }, "start": {"character": 0, "line": 15 } }
            }
          ],
          "textDocument": {
            "uri": "file:///Users/predrag/Documents/work/pravoslavnaknjizara/appAdmin/src/pages/OrdersPage/OrdersPage.tsx",
            "version": 192
          }
        }
      ]
    },
    "kind": "source.organizeImports.ts",
    "title": "Organize Imports"
  },
  {
    "edit": {
      "documentChanges": [
        {
          "edits": [
            {
              "newText": "import { useState } from \"react\"\n",
              "range": {"end": {"character": 0, "line": 16 }, "start": {"character": 0, "line": 16 } }
            }
          ],
          "textDocument": {
            "uri": "file:///Users/predrag/Documents/work/pravoslavnaknjizara/appAdmin/src/pages/OrdersPage/OrdersPage.tsx",
            "version": 192
          }
        }
      ]
    },
    "kind": "source.addMissingImports.ts",
    "title": "Add all missing imports"
  }
]

@rchl
Copy link
Member

rchl commented May 31, 2024

I believe I said somewhere that vscode only sends at most one code action in the only list per request. If so then the way to fix it should be to trigger as many requests as there are "fix on save code actions" specified.

@predragnikolic
Copy link
Member

closing as duplicate of #2421

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