Semantic tokens mode issue in lua-mode

[drcxd]

Thank you for the bug report

• [X] I am using the latest version of lsp-mode related packages.
• [X] I checked FAQ and Troubleshooting sections
• [X] You may also try reproduce the issue using clean environment using the following command: M-x lsp-start-plain

Bug description

As you can see in the picture, m and test are rendered using one color before the long comment, but after the long comment, the same tokens are rendered using different colors.

Using describe-char to check the properties of test, I have found that those before the long comment have text properties "property", while those after the long comment don't.

I first raise the issue under the language server I used, that is lua-language-server. (Link to the issue https://github.com/sumneko/lua-language-server/issues/1753). However, people using VSCode and NeoVim all report all the four test in my example code are labeled as "property". Thus, I doubt this might be a lsp-mode issue?

Steps to reproduce

I am using Emacs 29.0.50 on Windows. lsp-mode version is 20221228.2109. lua-mode version is 20221218.605. lua-language-server version is 3.6.4. I have used a clean configuration and reproduce the bug as the picture descripted. The only configuration code I have is to set package-archives, lsp-clients-lua-language-server-bin and lsp-clients-lua-language-server-main-location. No other packages are installed. Just open a lua file and enable lsp-mode and lsp-semantic-tokens-mode, the bug could be reproduced.

Expected behavior

Long comment should not affect the behavior of semantic tokens highlighting.

Which Language Server did you use?

lua-language-server

OS

Windows

Error callstack

No response

Anything else?

No response

[yyoncho]

You have lsp-semantic-tokens-enable set to t, right?

[drcxd]

@yyoncho No, it is nil. I execute lsp-semantic-tokens-mode to enable it. Setting it to t does not change the behavior.

[yyoncho]

@drcxd you should change that before running M-x lsp for the first time.

[drcxd]

@yyoncho I have done what you said.

(setq lsp-clients-lua-language-server-bin "e:/lls/bin/lua-language-server.exe"
      lsp-clients-lua-language-server-main-location "e:/lls/main.lua"
      lsp-semantic-tokens-enable t)

Here is the only code in my clean configuration. After I execute lsp, the result is still the same.

[yyoncho]

Ok, I will check it out. (You may test also doing M-x lsp-workspace-restart just in case lsp has run after setting the variables).

[yyoncho]

However, people using VSCode and NeoVim all report all the four test in my example code are labeled as "property". Thus, I doubt this might be a lsp-mode issue?

I tested it locally and I doubt that - it looks like a server bug. Note that VScode/Neovim might have better default color and although you see that it works fine in Neovim/VScode it might come from the fact that their coloring is better without the language server.

cc @sebastiansturm @ericdallo since they are more familiar with semantics code.

[drcxd]

I tested it locally and I doubt that - it looks like a server bug. Note that VScode/Neovim might have better default color and although you see that it works fine in Neovim/VScode it might come from the fact that their coloring is better without the language server.

See this guy's reply https://github.com/sumneko/lua-language-server/issues/1753#issuecomment-1357929194

I just tested this in Neovim, and all four test are correctly labeled as @property.

If I didn't get it wrong, then "labeled as @property" means that the server detects those tokens as "property". If that is true, I think the server might be working properly?

[yyoncho]

If I didn't get it wrong, then "labeled as @property" means that the server detects those tokens as "property". If that is true, I think the server might be working properly?

They might be right - but it is easy to confuse that it works fine if you just look it in the editor. I will let @sebastiansturm / @ericdallo check it out because they are more familiar with that part of the code.

[ericdallo]

If server is really returning the exactly same semantic tokens types and modifiers to all those m.test, then, it's probably a lsp-mode bug, I suggest starting debugging that if that is not already clear.

There is a handy (lsp-semantic-tokens-enable-log) + (lsp-semantic-tokens-export-log) that help understand the fortification, maybe it could help here.

Also, at clojure-lsp server, we have a custom command lsp-clojure-cursor-info that brings information about the current cursor, like what semantic tokens types and modifiers are using, which help understand easily if it's a server or client bug, would be nice to have that for other servers.

[yyoncho]

semantic-token-snapshots.tar.gz

Here is a sample output.

[sebastiansturm]

the issue here seems to be that the lua language server ascribes long token lengths (9997 and 9999) to the comment lines, which according to the spec is valid (If multiline tokens are not supported and a tokens length takes it past the end of the line, it should be treated as if the token ends at the end of the line and will not wrap onto the next line.). So we should probably truncate tokens to end-of-line when not advertising multiline token support, and advertise multiline token support in general (in which case we should still make sure not to extend past (point-max), as that will cause text property creation to fail, which is what causes lsp-semantic-tokens--fontify to exit and thus leave the remaining tokens unfontified. You should be able to try this on your end by replacing lsp-semantic-tokens.el:547 (setq text-property-end (+ text-property-beg (aref data (+ i 2)))) by (setq text-property-end (+ text-property-beg (min (aref data (+ i 2)) (- (pos-eol) text-property-beg)))) (at least on Emacs 29, on earlier versions you may have to use (point-at-eol) in place of (pos-at-eol)). Will do a small PR for this

[sebastiansturm]

@drcxd could you please try pr #3886?

[drcxd]

@sebastiansturm Applying this patch does fix the problem, at least in my environment.