emacs-lsp / lsp-ui

UI integrations for lsp-mode
https://emacs-lsp.github.io/lsp-ui
GNU General Public License v3.0
1.04k stars 141 forks source link

lsp getting confused after just a few edits: typescript-language-server/web-mode/tsx #238

Closed cwhatley closed 5 years ago

cwhatley commented 5 years ago

Describe the bug

lsp-ui/lsp 6.0

After a few edits to a file, there is some problem with communicating with the language server which results in a nonsensical flycheck error list. Restarting the server fixes the problem for a bit.

I have lsp-ui on top with flycheck instead of flymake.

To Reproduce

Edit any sizable TSX file under web-mode with typescript-language-server installed in your path.

I can usually get it to happen when I go to a line, add some garbage lines plus some garbage at beginning of existing line. Then delete garbage lines and finally clean up the existing line I changed.

My only customization:

(setq-default lsp-prefer-flymake nil)

Expected behavior The flycheck errors should go away.

Which Language Server did you use typescript-language-server

OS Which os do you use? MacOS with emacs-mac and emacs-plus brew installs. Ubuntu 18.04 with emacs 26.1.

Error callstack There is no error callstack.

screen shot 2019-02-04 at 10 12 08 pm

The LSP log is here. Note that the specific 2304 typescript error doesn't show up in the log at all, but perhaps it scrolled off the log.

lsp-log.txt

yyoncho commented 5 years ago

Possible dup of https://github.com/emacs-lsp/lsp-ui/issues/237 ?

cwhatley commented 5 years ago

Definitely seems similar, but in the case I'm experiencing, I can navigate around the file and the overlay with the incorrect info is properly hidden and replaced with content from other lines. When I return to the line, I get the incorrect info again.

There are also often bits of underlines left around on top the code where errors once were and the flymake error list buffer when shown always shows the incorrect info until the LSP session is restarted.

One thing I noticed that seemed kind of odd is that when I run (lsp-diagnostics) and look at the hash entry for the file in question, I see a list that contains a few diag structs and then as its final element there's another list containing a copy of the preceding elements of the main diag list. E.g., like (diag1 diag2 (diag1 diag2)). Not obvious to me from a scan of the code that that is intentional.

yyoncho commented 5 years ago

Can you check lsp-ui-flycheck-list and flycheck-buffer content? I want to diagnose whether the issue is in lsp-mode failing to handle diagnostics properly or lsp-ui failing to cleanup cache.

E.g., like (diag1 diag2 (diag1 diag2)). Not obvious to me from a scan of the code that that is intentional

This is probably caused by the fact that each diagnostic contains its original value (the diagnostic that is comming from lsp server).

cwhatley commented 5 years ago

For this error:

image

Flycheck:

image

The output of M-: RET (lsp-ui-flycheck-list) appears totally unrelated to what's shown:

image

Attached below is a pretty minimal init.el where I can still make the problem happen:

issue-238-init.el.txt

Also, FYI, I have the latest typescript-language-server 0.3.7.

cwhatley commented 5 years ago

Hmm. I just tried removing the typescript-language-server and adding javascript-typescript-langserver and now I am not seeing the problem anymore.

yyoncho commented 5 years ago

I am not js/ts user so do you have a sample project which I can use to try to reproduce the issue? From what I can see the js-ts language server messes up and does not clear the errors but I cannot determine that since the log is truncated( you may change the number of stored lines via lsp-log-max ).

Also, FYI, I have the latest typescript-language-server 0.3.7.

Do you have the latest typescript package as well? typescript-language-server is using it.

cwhatley commented 5 years ago

Recipe:

npx create-react-app lsp-ts-fu-test --typescript

Replace src/index.tsx in the project with the attachment here:

index.tsx.zip

Visit src/index.tsx and go to line 25 and put your cursor on the opening quote of "maths"

Run this macro:

(fset 'typescript-fubar
   "pojawfpojawe\C-mfawe\C-mf\C-mawef\C-mawe\C-mef\C-mawe\C-mf\C-mawef\C-mw\C-m\C-m\C-[f\C-f\C-e\C-m\C-mawef\C-maw\C-mef\C-mawe\C-mfaw\C-mef\C-mwaef\C-m\C-a\C-p\C-p\C-p\C-p\C-p\C-k\C-k\C-k\C-k\C-k\C-k\C-k\C-k\C-a\C-p\C-p\C-p\C-p\C-n\C-k\C-k\C-k\C-k\C-k\C-k\C-k\C-i\C-a\C-?\C-a\C-p\C-p\C-n\C-nwefwef\C-p\C-a\C-@\C-p\C-p\C-p\C-p\C-p\C-p\C-w\C-p\C-p\C-p\C-p\C-p\C-p\C-p\C-p\C-n\C-n\C-n\C-n\C-n\C-p\C-@\C-n\C-n\C-n\C-n\C-w\C-d\C-[d\C-d\C-i")

You should see something like this:

image

Also, I have switched over to javascript-typescript-langserver instead of typescript-language-server and have had no issues. Maybe that should be the recommended typescript server on the lsp-mode readme.

yyoncho commented 5 years ago

we need to fix https://github.com/emacs-lsp/lsp-mode/issues/588#issuecomment-450737726 before recommending this server.

I had some troubles replying the macro - might be caused by my heavily customized environment. I will try it tomorrow with emacs -q and using your setup(hopefully it will reproduce) and emacs 26.1.

yyoncho commented 5 years ago

Also, FYI, I have the latest typescript-language-server 0.3.7.

Do you use the latest version of typescript (tsserver) as well?

cwhatley commented 5 years ago

The sample project above created by create-react-app will use typescript 3.3.3 which is the latest. The language server launches the tsserver from within the project (at least I'm pretty sure it does b/c I don't have it installed globally).

yyoncho commented 5 years ago

@cwhatley I reproduced it on my side, it seems like either lsp-mode is sending incorrect text edits, or typescript language server handles them wrongly.

yyoncho commented 5 years ago

@cwhatley I am on to it - the issue is that typescript messes up I am still trying to figure out what is the reason for that.

I added logs in document.js from the typescript-language-server and here it is the output: https://gist.github.com/yyoncho/fdb06e4909d60010cab5fab0c57f7be7

yyoncho commented 5 years ago

@cwhatley can you verify that (setq indent-line-function nil) fixes the issue on your side. It seems like the changes performed by this function are not reported as changes and lsp-mode is missing them. This explains why I was unable to reproduce the issue locally (I was using js2-mode) .

cwhatley commented 5 years ago

Yes. I set indent-line-function to nil and I have been unable to recreate the problem since.

yyoncho commented 5 years ago

Given the lower impact/the amount of open issues/the presence of a workaround I propose to close this bug as won't fix/workaround and document it in lsp-mode FAQ section.

cwhatley commented 5 years ago

Is the workaround the nullifying of indent-line-function or using the other langserver? Nullifying indent-line-function would be pretty detrimental to the use of web-mode.

Do you have any idea what web-mode-indent-line is doing that causes the change to not get picked up by LSP?

yyoncho commented 5 years ago

@cwhatley

Is the workaround the nullifying of indent-line-function

Nullifying the indent-line-function.

Do you have any idea what web-mode-indent-line is doing that causes the change to not get picked up by LSP?

It inserts the indentation in a way that breaks the change hook execution. (lsp-on-change does not receive the proper start/end/length ). (or I believe so).

cwhatley commented 5 years ago

Ah, so I think I see what the issue is. For whatever reason, web-mode-indent-line has a let that setsinhibit-modification-hooks to t. This prevents the lsp-on-change from getting fired in the after-change-functions hook.

I just posted an issue for web-mode here: https://github.com/fxbois/web-mode/issues/1039

yyoncho commented 5 years ago

The fix in web-mode is in, thus closing the PR.