emacs-lsp / lsp-mode

Emacs client/library for the Language Server Protocol
https://emacs-lsp.github.io/lsp-mode
GNU General Public License v3.0
4.79k stars 890 forks source link

Wrong or Inconsistent Semantic highlighting #3265

Open asmodeus812 opened 2 years ago

asmodeus812 commented 2 years ago

Thank you for the bug report

Bug description

I am facing issues with semantic highlighting, am using the plain file with only the following customization added on top of it. I am not 100% sure that this is all I need for the highlighting to work (I also tried with different permutations of the settings below)

(setq  lsp-enable-semantic-highlighting t
       lsp-semantic-tokens-enable t
       lsp-semantic-tokens-warn-on-missing-face t)

Here is the result that i am getting. As you can see here the colouring is very inconsistent and plain incorrect . Member variables are once coloured green, when declared, once coloured light blue (in the constructor). The types are also green. On the dispatch method, both parameters have different colouring for their type (Object and String are both different colour). The method invocations getUsers and forEach are again different colours. Annotation is also the same color as member variable usage (light blue/green/cyan whatever that is) image

Steps to reproduce

Using the following command to start emacs -q -l plain.el

(require 'package)

(setq debug-on-error t
      no-byte-compile t
      package-archives '(("melpa" . "https://melpa.org/packages/")
                         ("gnu" . "https://elpa.gnu.org/packages/"))
      package-user-dir (expand-file-name (make-temp-name "lsp-tmp-elpa")
                                         user-emacs-directory)
      custom-file (expand-file-name "custom.el" package-user-dir))

(let* ((pkg-list '(lsp-mode lsp-ui yasnippet lsp-java lsp-python-ms lsp-haskell helm-lsp lsp-treemacs dap-mode lsp-origami lsp-dart company flycheck lsp-pyright
                            rust-mode php-mode scala-mode dart-mode clojure-mode typescript-mode)))

  (package-initialize)
  (package-refresh-contents)

  (mapc (lambda (pkg)
          (unless (package-installed-p pkg)
            (package-install pkg))
          (require pkg))
        pkg-list)

  (yas-global-mode)
  (add-hook 'prog-mode-hook 'lsp)
  )

(setq  lsp-enable-semantic-highlighting t
       lsp-semantic-tokens-enable t
       lsp-semantic-tokens-warn-on-missing-face t)
(provide 'lsp-start-plain)

Expected behavior

Consistent colouring and no missing faces in the logs

Which Language Server did you use?

The Java language server - JDTLS. With lsp-java

OS

Linux

Error callstack


Here are the logs from `lsp-semantic-tokens-warn-on-missing-face t`. 

Warning (lsp-mode): No face has been associated to the semantic token ’modifier’: consider adding a corresponding definition to lsp-semantic-token-faces
Warning (lsp-mode): No face has been associated to the semantic token ’annotation’: consider adding a corresponding definition to lsp-semantic-token-faces
Warning (lsp-mode): No face has been associated to the semantic token ’annotationMember’: consider adding a corresponding definition to lsp-semantic-token-faces
Warning (lsp-mode): No face has been associated to the semantic token ’record’: consider adding a corresponding definition to lsp-semantic-token-faces
Warning (lsp-mode): No face has been associated to the semantic token ’recordComponent’: consider adding a corresponding definition to lsp-semantic-token-faces
Warning (lsp-mode): No face has been associated to the semantic token modifier ’public’: consider adding a corresponding definition to lsp-semantic-token-modifier-faces
Warning (lsp-mode): No face has been associated to the semantic token modifier ’private’: consider adding a corresponding definition to lsp-semantic-token-modifier-faces
Warning (lsp-mode): No face has been associated to the semantic token modifier ’protected’: consider adding a corresponding definition to lsp-semantic-token-modifier-faces
Warning (lsp-mode): No face has been associated to the semantic token modifier ’native’: consider adding a corresponding definition to lsp-semantic-token-modifier-faces
Warning (lsp-mode): No face has been associated to the semantic token modifier ’generic’: consider adding a corresponding definition to lsp-semantic-token-modifier-faces
Warning (lsp-mode): No face has been associated to the semantic token modifier ’typeArgument’: consider adding a corresponding definition to lsp-semantic-token-modifier-faces
Warning (lsp-mode): No face has been associated to the semantic token modifier ’importDeclaration’: consider adding a corresponding definition to lsp-semantic-token-modifier-faces
Warning (lsp-mode): No face has been associated to the semantic token modifier ’constructor’: consider adding a corresponding definition to lsp-semantic-token-modifier-faces
Warning (lsp-mode): No face has been associated to the semantic token ’modifier’: consider adding a corresponding definition to lsp-semantic-token-faces
Warning (lsp-mode): No face has been associated to the semantic token ’annotation’: consider adding a corresponding definition to lsp-semantic-token-faces
Warning (lsp-mode): No face has been associated to the semantic token ’annotationMember’: consider adding a corresponding definition to lsp-semantic-token-faces
Warning (lsp-mode): No face has been associated to the semantic token ’record’: consider adding a corresponding definition to lsp-semantic-token-faces
Warning (lsp-mode): No face has been associated to the semantic token ’recordComponent’: consider adding a corresponding definition to lsp-semantic-token-faces
Warning (lsp-mode): No face has been associated to the semantic token modifier ’public’: consider adding a corresponding definition to lsp-semantic-token-modifier-faces
Warning (lsp-mode): No face has been associated to the semantic token modifier ’private’: consider adding a corresponding definition to lsp-semantic-token-modifier-faces
Warning (lsp-mode): No face has been associated to the semantic token modifier ’protected’: consider adding a corresponding definition to lsp-semantic-token-modifier-faces
Warning (lsp-mode): No face has been associated to the semantic token modifier ’native’: consider adding a corresponding definition to lsp-semantic-token-modifier-faces
Warning (lsp-mode): No face has been associated to the semantic token modifier ’generic’: consider adding a corresponding definition to lsp-semantic-token-modifier-faces
Warning (lsp-mode): No face has been associated to the semantic token modifier ’typeArgument’: consider adding a corresponding definition to lsp-semantic-token-modifier-faces
Warning (lsp-mode): No face has been associated to the semantic token modifier ’importDeclaration’: consider adding a corresponding definition to lsp-semantic-token-modifier-faces
Warning (lsp-mode): No face has been associated to the semantic token modifier ’constructor’: consider adding a corresponding definition to lsp-semantic-token-modifier-faces
Warning (lsp-mode): No face has been associated to the semantic token ’modifier’: consider adding a corresponding definition to lsp-semantic-token-faces
Warning (lsp-mode): No face has been associated to the semantic token ’annotation’: consider adding a corresponding definition to lsp-semantic-token-faces
Warning (lsp-mode): No face has been associated to the semantic token ’annotationMember’: consider adding a corresponding definition to lsp-semantic-token-faces
Warning (lsp-mode): No face has been associated to the semantic token ’record’: consider adding a corresponding definition to lsp-semantic-token-faces
Warning (lsp-mode): No face has been associated to the semantic token ’recordComponent’: consider adding a corresponding definition to lsp-semantic-token-faces
Warning (lsp-mode): No face has been associated to the semantic token modifier ’public’: consider adding a corresponding definition to lsp-semantic-token-modifier-faces
Warning (lsp-mode): No face has been associated to the semantic token modifier ’private’: consider adding a corresponding definition to lsp-semantic-token-modifier-faces
Warning (lsp-mode): No face has been associated to the semantic token modifier ’protected’: consider adding a corresponding definition to lsp-semantic-token-modifier-faces
Warning (lsp-mode): No face has been associated to the semantic token modifier ’native’: consider adding a corresponding definition to lsp-semantic-token-modifier-faces
Warning (lsp-mode): No face has been associated to the semantic token modifier ’generic’: consider adding a corresponding definition to lsp-semantic-token-modifier-faces
Warning (lsp-mode): No face has been associated to the semantic token modifier ’typeArgument’: consider adding a corresponding definition to lsp-semantic-token-modifier-faces
Warning (lsp-mode): No face has been associated to the semantic token modifier ’importDeclaration’: consider adding a corresponding definition to lsp-semantic-token-modifier-faces
Warning (lsp-mode): No face has been associated to the semantic token modifier ’constructor’: consider adding a corresponding definition to lsp-semantic-token-modifier-faces
Warning (lsp-mode): No face has been associated to the semantic token ’modifier’: consider adding a corresponding definition to lsp-semantic-token-faces
Warning (lsp-mode): No face has been associated to the semantic token ’annotation’: consider adding a corresponding definition to lsp-semantic-token-faces
Warning (lsp-mode): No face has been associated to the semantic token ’annotationMember’: consider adding a corresponding definition to lsp-semantic-token-faces
Warning (lsp-mode): No face has been associated to the semantic token ’record’: consider adding a corresponding definition to lsp-semantic-token-faces
Warning (lsp-mode): No face has been associated to the semantic token ’recordComponent’: consider adding a corresponding definition to lsp-semantic-token-faces
Warning (lsp-mode): No face has been associated to the semantic token modifier ’public’: consider adding a corresponding definition to lsp-semantic-token-modifier-faces
Warning (lsp-mode): No face has been associated to the semantic token modifier ’private’: consider adding a corresponding definition to lsp-semantic-token-modifier-faces
Warning (lsp-mode): No face has been associated to the semantic token modifier ’protected’: consider adding a corresponding definition to lsp-semantic-token-modifier-faces
Warning (lsp-mode): No face has been associated to the semantic token modifier ’native’: consider adding a corresponding definition to lsp-semantic-token-modifier-faces
Warning (lsp-mode): No face has been associated to the semantic token modifier ’generic’: consider adding a corresponding definition to lsp-semantic-token-modifier-faces
Warning (lsp-mode): No face has been associated to the semantic token modifier ’typeArgument’: consider adding a corresponding definition to lsp-semantic-token-modifier-faces
Warning (lsp-mode): No face has been associated to the semantic token modifier ’importDeclaration’: consider adding a corresponding definition to lsp-semantic-token-modifier-faces
Warning (lsp-mode): No face has been associated to the semantic token modifier ’constructor’: consider adding a corresponding definition to lsp-semantic-token-modifier-faces
Warning (lsp-mode): No face has been associated to the semantic token ’modifier’: consider adding a corresponding definition to lsp-semantic-token-faces
Warning (lsp-mode): No face has been associated to the semantic token ’annotation’: consider adding a corresponding definition to lsp-semantic-token-faces
Warning (lsp-mode): No face has been associated to the semantic token ’annotationMember’: consider adding a corresponding definition to lsp-semantic-token-faces
Warning (lsp-mode): No face has been associated to the semantic token ’record’: consider adding a corresponding definition to lsp-semantic-token-faces
Warning (lsp-mode): No face has been associated to the semantic token ’recordComponent’: consider adding a corresponding definition to lsp-semantic-token-faces
Warning (lsp-mode): No face has been associated to the semantic token modifier ’public’: consider adding a corresponding definition to lsp-semantic-token-modifier-faces
Warning (lsp-mode): No face has been associated to the semantic token modifier ’private’: consider adding a corresponding definition to lsp-semantic-token-modifier-faces
Warning (lsp-mode): No face has been associated to the semantic token modifier ’protected’: consider adding a corresponding definition to lsp-semantic-token-modifier-faces
Warning (lsp-mode): No face has been associated to the semantic token modifier ’native’: consider adding a corresponding definition to lsp-semantic-token-modifier-faces
Warning (lsp-mode): No face has been associated to the semantic token modifier ’generic’: consider adding a corresponding definition to lsp-semantic-token-modifier-faces
Warning (lsp-mode): No face has been associated to the semantic token modifier ’typeArgument’: consider adding a corresponding definition to lsp-semantic-token-modifier-faces
Warning (lsp-mode): No face has been associated to the semantic token modifier ’importDeclaration’: consider adding a corresponding definition to lsp-semantic-token-modifier-faces
Warning (lsp-mode): No face has been associated to the semantic token modifier ’constructor’: consider adding a corresponding definition to lsp-semantic-token-modifier-faces
Warning (lsp-mode): No face has been associated to the semantic token ’modifier’: consider adding a corresponding definition to lsp-semantic-token-faces
Warning (lsp-mode): No face has been associated to the semantic token ’annotation’: consider adding a corresponding definition to lsp-semantic-token-faces
Warning (lsp-mode): No face has been associated to the semantic token ’annotationMember’: consider adding a corresponding definition to lsp-semantic-token-faces
Warning (lsp-mode): No face has been associated to the semantic token ’record’: consider adding a corresponding definition to lsp-semantic-token-faces
Warning (lsp-mode): No face has been associated to the semantic token ’recordComponent’: consider adding a corresponding definition to lsp-semantic-token-faces
Warning (lsp-mode): No face has been associated to the semantic token modifier ’public’: consider adding a corresponding definition to lsp-semantic-token-modifier-faces
Warning (lsp-mode): No face has been associated to the semantic token modifier ’private’: consider adding a corresponding definition to lsp-semantic-token-modifier-faces
Warning (lsp-mode): No face has been associated to the semantic token modifier ’protected’: consider adding a corresponding definition to lsp-semantic-token-modifier-faces
Warning (lsp-mode): No face has been associated to the semantic token modifier ’native’: consider adding a corresponding definition to lsp-semantic-token-modifier-faces
Warning (lsp-mode): No face has been associated to the semantic token modifier ’generic’: consider adding a corresponding definition to lsp-semantic-token-modifier-faces
Warning (lsp-mode): No face has been associated to the semantic token modifier ’typeArgument’: consider adding a corresponding definition to lsp-semantic-token-modifier-faces
Warning (lsp-mode): No face has been associated to the semantic token modifier ’importDeclaration’: consider adding a corresponding definition to lsp-semantic-token-modifier-faces
Warning (lsp-mode): No face has been associated to the semantic token modifier ’constructor’: consider adding a corresponding definition to lsp-semantic-token-modifier-faces
Warning (lsp-mode): No face has been associated to the semantic token ’modifier’: consider adding a corresponding definition to lsp-semantic-token-faces
Warning (lsp-mode): No face has been associated to the semantic token ’annotation’: consider adding a corresponding definition to lsp-semantic-token-faces
Warning (lsp-mode): No face has been associated to the semantic token ’annotationMember’: consider adding a corresponding definition to lsp-semantic-token-faces
Warning (lsp-mode): No face has been associated to the semantic token ’record’: consider adding a corresponding definition to lsp-semantic-token-faces
Warning (lsp-mode): No face has been associated to the semantic token ’recordComponent’: consider adding a corresponding definition to lsp-semantic-token-faces
Warning (lsp-mode): No face has been associated to the semantic token modifier ’public’: consider adding a corresponding definition to lsp-semantic-token-modifier-faces
Warning (lsp-mode): No face has been associated to the semantic token modifier ’private’: consider adding a corresponding definition to lsp-semantic-token-modifier-faces
Warning (lsp-mode): No face has been associated to the semantic token modifier ’protected’: consider adding a corresponding definition to lsp-semantic-token-modifier-faces
Warning (lsp-mode): No face has been associated to the semantic token modifier ’native’: consider adding a corresponding definition to lsp-semantic-token-modifier-faces
Warning (lsp-mode): No face has been associated to the semantic token modifier ’generic’: consider adding a corresponding definition to lsp-semantic-token-modifier-faces
Warning (lsp-mode): No face has been associated to the semantic token modifier ’typeArgument’: consider adding a corresponding definition to lsp-semantic-token-modifier-faces
Warning (lsp-mode): No face has been associated to the semantic token modifier ’importDeclaration’: consider adding a corresponding definition to lsp-semantic-token-modifier-faces
Warning (lsp-mode): No face has been associated to the semantic token modifier ’constructor’: consider adding a corresponding definition to lsp-semantic-token-modifier-faces
Warning (lsp-mode): No face has been associated to the semantic token ’modifier’: consider adding a corresponding definition to lsp-semantic-token-faces
Warning (lsp-mode): No face has been associated to the semantic token ’annotation’: consider adding a corresponding definition to lsp-semantic-token-faces
Warning (lsp-mode): No face has been associated to the semantic token ’annotationMember’: consider adding a corresponding definition to lsp-semantic-token-faces
Warning (lsp-mode): No face has been associated to the semantic token ’record’: consider adding a corresponding definition to lsp-semantic-token-faces
Warning (lsp-mode): No face has been associated to the semantic token ’recordComponent’: consider adding a corresponding definition to lsp-semantic-token-faces
Warning (lsp-mode): No face has been associated to the semantic token modifier ’public’: consider adding a corresponding definition to lsp-semantic-token-modifier-faces
Warning (lsp-mode): No face has been associated to the semantic token modifier ’private’: consider adding a corresponding definition to lsp-semantic-token-modifier-faces
Warning (lsp-mode): No face has been associated to the semantic token modifier ’protected’: consider adding a corresponding definition to lsp-semantic-token-modifier-faces
Warning (lsp-mode): No face has been associated to the semantic token modifier ’native’: consider adding a corresponding definition to lsp-semantic-token-modifier-faces
Warning (lsp-mode): No face has been associated to the semantic token modifier ’generic’: consider adding a corresponding definition to lsp-semantic-token-modifier-faces
Warning (lsp-mode): No face has been associated to the semantic token modifier ’typeArgument’: consider adding a corresponding definition to lsp-semantic-token-modifier-faces
Warning (lsp-mode): No face has been associated to the semantic token modifier ’importDeclaration’: consider adding a corresponding definition to lsp-semantic-token-modifier-faces
Warning (lsp-mode): No face has been associated to the semantic token modifier ’constructor’: consider adding a corresponding definition to lsp-semantic-token-modifier-faces


### Anything else?

_No response_
sebastiansturm commented 2 years ago

thanks for the bug report, I'll have a look this evening or on the weekend. Not sure if it's just missing face definitions or if tokens are really applied incorrectly; I should probably add some way to query semantic tokens at point

ericdallo commented 2 years ago

@sebastiansturm I suspect it's not a bug on the semantic tokens logic, but misconfiguration for java language or for that theme itself, maybe double check if we need to do to java what we did for clojure and it's working good since then

asmodeus812 commented 2 years ago

@ericdallo @sebastiansturm Thanks for the swift response. I also haven't validated for other languages such as C, C++, TS, JS etc. I will try to verify if i face the same issues with them too.

sebastiansturm commented 2 years ago

I haven't tried the Java server myself, as I'm not sure what code base you're using it on. @ericdallo is probably right though, our default set of faces may not cover the ones defined by the Java server very well. If you want to contribute some new definitions, the following helper function (inefficient but good enough for the purpose I hope) may be of some use:

(defun lsp-semantic-tokens--annotate (tokens) ()
       "Turn TOKENS into a list of debugging-friendly plists."
       (let* ((line-number 1)
              (column 0)
              (line-delta)
              (token-info (-some #'lsp--semantic-tokens-as-defined-by-workspace lsp--buffer-workspaces))
              (token-faces (plist-get token-info :token-types))
              (token-modifiers (plist-get token-info :token-modifiers)))
         (cl-loop for i from 0 to (1- (length tokens)) by 5 do
                  (setq line-delta (aref tokens i))
                  (setq line-number (+ line-number line-delta))
                  (unless (= line-delta 0) (setq column 0))
                  (setq column (+ column (aref tokens (1+ i))))
                  collect
                  `(:line-number ,line-number :column-beg ,column
                    :column-end ,(+ column (aref tokens (+ i 2)))
                    :face ,(aref token-faces (aref tokens (+ i 3)))
                    :modifiers ,(cl-loop for j from 0 to (1- (length token-modifiers))
                                         when (> (logand (aref tokens (+ i 4)) (lsh 1 j)) 0)
                                         collect (aref token-modifiers j))))))

(defun lsp-semantic-tokens-at-point () (interactive)
       "Show tokens and modifiers at point."
       (let* ((annotated-tokens
               (lsp-semantic-tokens--annotate
                (plist-get (lsp-request
                            "textDocument/semanticTokens/full"
                            `(:textDocument, (lsp--text-document-identifier))) :data)))
              (cur-line (line-number-at-pos))
              (column (- (point) (line-beginning-position)))
              (relevant-tokens (--filter (and (= cur-line (plist-get it :line-number))
                                              (>= column (plist-get it :column-beg))
                                              (<= column (plist-get it :column-end)))
                                         annotated-tokens))
              (token-strings (--map (format "face: %s\nmodifiers: %s"
                                            (plist-get it :face)
                                            (s-join ", " (plist-get it :modifiers)))
                                    relevant-tokens)))
         (message (s-join "\n\n" token-strings))))
asmodeus812 commented 2 years ago

Hi, @sebastiansturm thanks for the feedback. I am using it on a small test java project, to verify the configuration, nothing special. I am not terribly familiar with elisp or lsp's code base, so I am in no position to make any viable contribution. I wanted to make sure this problem is properly reported, as I assume other people will be affected as well.

asmodeus812 commented 2 years ago

Hi @sebastiansturm any update on this issue ?

asmodeus812 commented 2 years ago

Is Lsp planned on supporting semantic highlighting ? We have to resort to treesitter which is not that stable or even working okay with emacs these days.

yyoncho commented 2 years ago

Is Lsp planned on supporting semantic highlighting ? We have to resort to treesitter which is not that stable or even working okay with emacs these days.

We do our best to support as much functionality as possible. People here are working on this project in their free time, don't expect immediate results.

HaraldKi commented 8 months ago

I am looking at the semantic highlighting for Java too and tried to adjust or improve the coloring and I think I understand a bit of the problems:

The server provides two types of information for a token:

  1. the type of the token
  2. zero or more modifier bits

The possible values can be seen in the language server specification.

Both, type and modifiers are translated/mapped to faces independently and separately and all resulting faces are then applied to a token. To the faces applied to a token, put the cursor on it and run M-x describe-char. You'll find that the completely green constructor line is the result of the interface modifier, if I recall correctly. You could customize-group lsp-semantic-tokens and tweak the faces, taking into account that faces for modifiers should probably not apply a color, but only a face-modification like underline, italic, etc.

I customized (lsp-face-semhl-interface ((t nil))) for a bit of relief, but there are more problems. For example the @param in a javadoc comment is marked as keyword with modifier documentation. This is logically correct, but I would like to avoid having param in the same color as for, if, public and the like. But neither would I like to have it in the same color as all of documentation. If faces would "merge" colors, that would be something, but I don't think this is possible.

What is sorely needed is a combined mapping of (type,modifier1, modifier2, etc) -> face. Though this would require a complete rewrite, afaict, of the type/modifier to face mapping. :cry: