yyoncho
commented
3 years ago @kuba-orlik I am not js/ts expert - do you know what might be the reason not to get the warning for missing return type in the first place?
Closed kuba-orlik closed 3 years ago
yyoncho
commented
3 years ago @kuba-orlik I am not js/ts expert - do you know what might be the reason not to get the warning for missing return type in the first place?
kuba-orlik
commented
3 years ago I don't think there's any such thing. It looks like the lsp server receives something else than what the buffer shows. That would especially explain the error syntax that gets shown even though the code is exactly the same as it was a few edits ago.
The lsp-ts server log show the following error:
[Trace - 07:21:07 pm] Received response 'textDocument/documentSymbol - (105)' in 10ms.
Result: {
"message": "Request textDocument/documentSymbol failed with message: Error processing request. Cannot read property 'charCount' of undefined\nTypeError: Cannot read property 'charCount' of undefined\n at LineNode.walk (/home/kuba/.npm-global/lib/node_modules/typescript/lib/tsserver.js:155664:68)\n at LineIndex.edit (/home/kuba/.npm-global/lib/node_modules/typescript/lib/tsserver.js:155545:31)\n at ScriptVersionCache._getSnapshot (/home/kuba/.npm-global/lib/node_modules/typescript/lib/tsserver.js:155335:47)\n at ScriptVersionCache.getSnapshot (/home/kuba/.npm-global/lib/node_modules/typescript/lib/tsserver.js:155328:82)\n at TextStorage.getSnapshot (/home/kuba/.npm-global/lib/node_modules/typescript/lib/tsserver.js:146897:32)\n at ScriptInfo.getSnapshot (/home/kuba/.npm-global/lib/node_modules/typescript/lib/tsserver.js:147059:41)\n at InferredProject.Project.getScriptSnapshot (/home/kuba/.npm-global/lib/node_modules/typescript/lib/tsserver.js:147715:39)\n at SyntaxTreeCache.getCurrentSourceFile (/home/kuba/.npm-global/lib/node_modules/typescript/lib/tsserver.js:143045:44)\n at Object.getNavigationTree (/home/kuba/.npm-global/lib/node_modules/typescript/lib/tsserver.js:143704:71)\n at IOSession.Session.getNavigationTree (/home/kuba/.npm-global/lib/node_modules/typescript/lib/tsserver.js:154516:44)\n at Session.handlers.ts.Map.ts.getEntries._a.<computed> (/home/kuba/.npm-global/lib/node_modules/typescript/lib/tsserver.js:153310:61)\n at /home/kuba/.npm-global/lib/node_modules/typescript/lib/tsserver.js:154962:88\n at IOSession.Session.executeWithRequestId (/home/kuba/.npm-global/lib/node_modules/typescript/lib/tsserver.js:154953:28)\n at IOSession.Session.executeCommand (/home/kuba/.npm-global/lib/node_modules/typescript/lib/tsserver.js:154962:33)\n at IOSession.Session.onMessage (/home/kuba/.npm-global/lib/node_modules/typescript/lib/tsserver.js:154986:35)\n at Interface.<anonymous> (/home/kuba/.npm-global/lib/node_modules/typescript/lib/tsserver.js:157202:27)\n at Interface.emit (node:events:365:28)\n at Interface._onLine (node:readline:452:10)\n at Interface._normalWrite (node:readline:606:12)\n at Socket.ondata (node:readline:252:10)\n at Socket.emit (node:events:365:28)\n at addChunk (node:internal/streams/readable:314:12)\n at readableAddChunk (node:internal/streams/readable:289:9)\n at Socket.Readable.push (node:internal/streams/readable:228:10)\n at Pipe.onStreamRead (node:internal/stream_base_commons:190:23)",
"code": -32603
}But there are no errors in eslint server logs. Maybe I'm not doing enough to have them logged? I set up lsp-log-io to ;t
willnevillain
commented
3 years ago Hello, I'm also encountering this issue - eslint continues to report outdated diagnostic data after making a change via a code action.
I captured some log groups from with lsp-log-io and lsp-eslint-trace-server set to t. I don't see anything unusually besides the fact that an invalid diagnostic (in my case, one for an unused import that has already been removed from the source file via a code action) is still being reported.
Happy to provide any logs or additional details to help troubleshooting, not sure how code actions interact with the diagnostics of the running server(s).
kuba-orlik
commented
3 years ago My current fix is to run lsp every time I run lsp-execute-code-action:
(advice-add 'lsp-execute-code-action :after (lambda (r) (call-interactively 'lsp)))
sin-ack
commented
3 years ago Also seeing this issue. Correct me if I'm wrong, but I think what happens is that when the Typescript language server makes a change to the buffer via the code action, this change is not reflected back to the ESLint language server, and when you start entering code in the buffer, you will be entering invalid code for ESLint (which has the buffer state before the code action was executed), causing those syntax errors. Restarting the ESLint language server sends the entire buffer contents to it again, fixing the problems. I'm not quite sure about LSP internals, but is it possible to just make the other language servers re-read the updated buffer when a code action is executed? Or maybe there is a way to reflect the exact operations a code action performs on the buffer to other language servers.
yyoncho
commented
3 years ago @sin-ack this is not how it works - the changes are applied client-side and then sent to the server(s).
sin-ack
commented
3 years ago I see, then something else is the issue. I'll see whether I can dig further into it.
calbrecht
commented
3 years ago @sin-ack, your thoughts weren't that wrong, though.
With customized lsp-log-io . t, we are able to inspect the communication between lsp-mode and the language servers in the buffers *lsp-log: ts-ls:XXXXXXX* and *lsp-log: eslint:XXXXXXX*
There we can see, the action "Infer function return type" is a refactor code action provided by typescript-language-server, whereas the diagnostics stem from lsp-eslint.
When comparing the communication flow of lsp-mode with that from vscode, interestingly (but not surprisingly, because, i guess, the typescript language server is tsserver directly) there are some differences.
In vscode (several messages left out for brevity)
# Gui: move point to f_unction
<- eslint: textDocument/publishDiagnostics # Payload: diagnostics including warnings
# Gui: C-S-r
-> typescript: getApplicableRefactors # Payload: (point)
<- typescript: getApplicableRefactors # Payload: refactoring including "Infer type..."
# Gui: apply refactoring "Infer type..."
-> typescript: getEditsForRefactor # Payload (point)
<- typescript: getEditsForRefactor # Payload textChanges [...] ": number"
# Editor: applies textChanges
-> eslint: textDocument/didChange # Payload textChanges [...] ": number"
-> typescript: updateOpen # Payload textChanges
<- typescript: updateOpen # ACK
<- eslint: textDocument/publishDiagnostics # Payload diagnostics []In lsp-mode (several messages left out for brevity)
# Gui: move point to f_unction
<- eslint: textDocument/publishDiagnostics # Payload: diagnostics including warnings
# Gui: <Prefix> a a
-> typescript: textDocument/codeAction # Payload: (point)
<- typescript: textDocument/codeAction # Payload: refactoring including "Infer type..."
-> eslint: textDocument/codeAction # Payload: (point)
<- eslint: textDocument/codeAction # Payload: quickfix "Disable rules..."
# Gui: apply refactoring "Infer type..."
?? -> typescript: textDocument/didChange # Payload contentChanges [...] ": number"
-> typescript: workspace/executeCommand # Payload: _typescript.applyRefactoring "Infer type.."
<- typescript: workspace/applyEdit # Payload: changes [...] ": number"
-> eslint: workspace/executeCommand # Payload: _typescript.applyRefactoring "Infer type.."
<- eslint: workspace/executeCommand # Payload: {}
# Editor: applies textChanges
-> typescript: workspace/applyEdit # Payload: applied: trueAs we see, the log entry textDocument/didChange is confusing.
It is logged out of order before the textDocument/applyEdit instructions are even received from typescript-language-server.
And it is in the wrong log file, namely that of ts-ls and does not occur in the eslint log.
My early conclusion, based on observations, would be that the textDocument/didChange is not send to eslint and therefore it does not know about the changes.
calbrecht
commented
3 years ago Here is a script to be run with emacs --load ./test-2901.el --visit index.ts
for generating log files lsp-eslint.log and lsp-ts-ls.log.
;;; package --- Log generator for #2901
;;; Commentary:
;;;
;;; Run with "emacs --load ./test-2901.el --visit index.ts", after
;;; cloning the reproduction repository and running "npm install".
;;;
;;; This script will *kill* Emacs at the end *without* confirmation
;;; and *without* asking to save buffers.
;;;
;;; Find logs generated in "./lsp-ts-ls.log" and "./lsp-eslint.log"
;;;
;;; Code:
(eval-when-compile
(require 'lsp-mode))
(with-current-buffer (switch-to-buffer (find-file-noselect "index.ts"))
(goto-char 9)
(defun on-invalid-error (err)
(remove-hook 'flycheck-process-error-functions #'on-invalid-error t)
(mapcar
(lambda (buffer)
(when (string-match-p "lsp-log: ts-ls" (buffer-name buffer))
(with-current-buffer buffer (write-file "lsp-ts-ls.log") (save-buffer 0)))
(when (string-match-p "lsp-log: eslint" (buffer-name buffer))
(with-current-buffer buffer (write-file "lsp-eslint.log") (save-buffer 0)))
nil)
(buffer-list))
(setq kill-emacs-hook nil)
(kill-emacs 0)
nil)
(defun on-outdated-error (err)
(remove-hook 'flycheck-process-error-functions #'on-outdated-error t)
(ignore-errors (undo))
(add-hook 'flycheck-process-error-functions #'on-invalid-error nil t)
nil)
(defun on-valid-error (err)
(remove-hook 'flycheck-process-error-functions #'on-valid-error t)
(lsp-execute-code-action
(lsp-seq-first
(seq-filter
(lambda (action)
(equal "Infer function return type" (gethash "title" action)))
(lsp-code-actions-at-point "refactor"))))
(add-hook 'flycheck-process-error-functions #'on-outdated-error nil t)
nil)
(add-hook 'flycheck-process-error-functions #'on-valid-error nil t)
nil)
(provide 'test-2901)
;;; test-2901.el ends here
JordanAnthonyKing
commented
3 years ago Has there been any progress on this issue? It's been driving me up the wall :L
yyoncho
commented
3 years ago I am testing with the provided repo using lsp-gitpod and it seems like the provided steps work fine.
Can someone provide other steps and eventually test with lsp-gitpod?
JordanAnthonyKing
commented
3 years ago Thanks for responding. I have set up this repro for reproduction: https://github.com/JordanAnthonyKing/lsp-eslint-test
This is a basic angular project set up with ng new [project name]. All I have added are some configs for eslint and prettier. You can see these changes in the .prettierrc, .prettierignore, .eslintrc.json, .eslintignore, and then the new packages added to package.json that contain eslint in their name compared to a default project.
To reproduce:
npm update -gnpm i in the repo to install all packagests-ls, angular-ls and eslint-ls installedapp.component.ts. It should look like this with two errors. The first on line 8 is a typical angular error that an interface (OnInit) has not been implemented. The second on line 9 is an eslint error from the prettier config, that double quotes are being used in place of single quotes (sometimes diagnostics aren't applied to the first buffer the servers are started on, you may need to open another file, close app.component.ts, then reopen it or some other dance):
In order to restore the buffer to a working state lsp-workspace-restart must be executed for the eslint language server:
Note: You should be able to fix these errors without issue. My initial assumption was just that since these fixes do not shift lines they don't cause any problem, however I have had this bug when converting concatenated strings to interpolated strings, despite this not shifting the lines at all.
Thanks, and please let me know if you need further info or any help reproducing this.
Thank you for the bug report
lsp-moderelated packages.where
lsp-start-plain.elcan be downloaded here. Alternatively, it will be great if you can reproduce the issue using lsp-docker which provides the minimal configurations forlsp-modeand ships with most of the language servers.Bug description
After executing a code action with
lsp-execute-code-action, the reported diagnostics stop making sense. They report issues that do not exist, or report critical syntax errors when there are none.You can see the bug in action here:
https://asciinema.org/a/eYwnQDtlgyGpdbT5sM21bjfE9
Steps to reproduce
M-x lspM-x lsp-execute-code-actionExpected behavior
The error highlight should work properly after executing a code action
Which Language Server did you use?
ts-ls, eslint
OS
Linux
Error callstack
No response
Anything else?
No response