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.75k stars 869 forks source link

Invalid face reference: lsp-flycheck-info-unnecessary #2255

Closed leungbk closed 2 years ago

leungbk commented 3 years ago
def hello():
    def abc(args):
        print(42)

    a = 1
    print("hello world")
    a += 1
    b = 5
    c = 10
    a = [1, 2]

    d = 5

    print("hello ")
    abc(1)

hello()
print("print statement")

Using lsp-pyright with the above Python file, there are numerous Flycheck errors classified as lsp-flycheck-info-unnecessary. When point is on a line with one such error, Invalid face reference: lsp-flycheck-info-unnecessary appears in the Messages buffer. This message seems to be called by merge_face_ref in the xfaces.c file in core Emacs.

The following seems to make the message go away:

---
 lsp-diagnostics.el | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/lsp-diagnostics.el b/lsp-diagnostics.el
index b7aa5daa..ea6d2113 100644
--- a/lsp-diagnostics.el
+++ b/lsp-diagnostics.el
@@ -93,7 +93,7 @@ g. `error', `warning') and list of LSP TAGS."
                       flycheck-level
                       (mapconcat #'symbol-name tags "-"))))
     (or (intern-soft name)
-        (let* ((face (--doto (intern (format "lsp-%s-face" name))
+        (let* ((face (--doto (intern name)
                        (copy-face (-> flycheck-level
                                       (get 'flycheck-overlay-category)
                                       (get 'face))
@@ -102,7 +102,7 @@ g. `error', `warning') and list of LSP TAGS."
                                (apply #'set-face-attribute it nil
                                       (cl-rest (assoc tag lsp-diagnostics-attributes))))
                              tags)))
-               (category (--doto (intern (format "lsp-%s-category" name))
+               (category (--doto (intern (format "%s-category" name))
                            (setf (get it 'face) face
                                  (get it 'priority) 100)))
                (new-level (intern name))
-- 
2.28.0

Apparently, making the face name match name does the trick, but I'm not sure why. The error levels that Flycheck defines don't look like this. It seems weird to define some symbol that has not only the properties assigned to it by flycheck-define-error-level but also is the name of a face, so we should probably find a cleaner solution.

I am not able to build Emacs from source on my machine for some reason, so I can't go into xfaces.c to debug this.

yyoncho commented 3 years ago

Willing to turn that into PR?

indigoviolet commented 2 years ago

@yyoncho https://github.com/emacs-lsp/lsp-mode/pull/3611

SnichOne commented 1 year ago

I see the exact issue with up-to-date flycheck: 20221112.1552 version. With both pyright and pylsp.

Should the issue be re-opened?

usq commented 1 year ago

I see the same issue

indigoviolet commented 1 year ago

@ericdallo @yyoncho any suggestions? I still see this as well. Please reopen if that's the right thing to do?

ericdallo commented 1 year ago

This issue was fized 9 months ago, the complains started 2months ago, so I suggest opening a new issue with detailed repro and link to this issue, it may be something different that is causing the same issue to final user

yyoncho commented 1 year ago

@ericdallo @yyoncho any suggestions? I still see this as well. Please reopen if that's the right thing to do?

@indigoviolet are you using desktop mode or something like that? Also, before loading lsp-mode can you check if it is defined?

davidhaley commented 1 year ago

I'm seeing the same issue.

image

image

It seems to come up as I'm typing an import for a component in SvelteKit. I have Web mode enabled, and Copilot enabled as well.

Flycheck:

    Version: 20230306.414
     Commit: 5f2ef177cb21ae8b73714575802beef04abd0f5e

Doom:

generated  Apr 27, 2023 10:16:13
system     Ubuntu 22.04.2 LTS Linux 5.19.0-41-generic x86_64
emacs      28.2 ~/.emacs.d/
doom       3.0.0-pre PROFILE=_@0 HEAD -> master 4e105a95 2023-03-22
       18:29:38 -0400 ~/.doom.d/
shell      /bin/bash
features   ACL CAIRO DBUS FREETYPE GIF GLIB GMP GNUTLS GPM GSETTINGS
       HARFBUZZ JPEG JSON LCMS2 LIBOTF LIBSELINUX LIBSYSTEMD LIBXML2
       M17N_FLT MODULES NATIVE_COMP NOTIFY INOTIFY PDUMPER PNG RSVG
       SECCOMP SOUND THREADS TIFF TOOLKIT_SCROLL_BARS X11 XDBE XIM
       XPM GTK3 ZLIB
traits     batch server-running envvar-file custom-file
modules    :config use-package :completion (company +childframe) vertico
       :ui doom doom-dashboard doom-quit hl-todo modeline nav-flash
       ophints (popup +defaults) treemacs (vc-gutter +pretty)
       vi-tilde-fringe window-select workspaces zen :editor (evil
       +everywhere) file-templates fold format multiple-cursors
       snippets :emacs dired electric (undo +tree) vc :term vterm
       :checkers syntax :tools (debugger +lsp) (eval +overlay)
       lookup lsp magit pdf :lang emacs-lisp (go +lsp) json
       javascript lua markdown (org +pretty +roam2 +pandoc +jupyter)
       (python +poetry +lsp +pyright +virtualenv) sh (web +html +css
       +lsp) yaml :config (default +bindings +smartparens)
packages   (deno-fmt) (jupyter) (svelte-mode) (focus) (transpose-frame)
       (copilot :recipe (:host github :repo zerolfx/copilot.el
       :files (*.el dist))) (org-fragtog)
pixel-ant commented 9 months ago

These are my findings.

Current situation

This is what lsp-mode does:

For each error of some level and tagged with some tags that LSP server yields:

  1. Generate a generated flycheck level, a generated category and a generated face named after level and tags.
    • E.g: lsp-flycheck-info-unnecessary, lsp-flycheck-info-unnecessary-category and lsp-flycheck-info-unnecessary-face respectively.
  2. Set property face of generated category to generated face.
    • E.g: lsp-flycheck-info-unnecessary-category.face <- lsp-flycheck-info-unnecessary-face.
  3. Create a symbol overlay to highlight the erroneous symbol.
    • E.g: symbol-overlay-1.
  4. Set category property of symbol overlay to generated category.
    • E.g: symbol-overlay-1.category <- lsp-flycheck-info-unnecessary-category.
  5. Create a sideline overlay to display error message near the erroneous line.
    • E.g: sideline-overlay-1.
  6. Set face property of error message to generated flycheck level. BUG?????
    • E.g: "very bad done".face <- lsp-flycheck-info-unnecessary.

Consequences

Emacs can't find the face named as generated flycheck level as many times as characters in error message? So many error messages like

   Invalid face reference: lsp-flycheck-info-unnecessary [38 times]

are emitted.

If authors pretended to use the same face for symbol overlay and sideline overlay, both face properties should be set to generated face.

Transient solution

To avoid changing the original source code, the referenced face can be defined in init file. E.g:

(defface lsp-flycheck-info-unnecessary
  '((t))
  "Face which apply to side line for symbols not used.
Possibly erroneously redundant of lsp-flycheck-info-unnecessary-face."
  :group 'lsp-ui-sideline)

Then it can be themed or customized. Remember this is the face for sideline overlay. This has stopped all error messages for me.

Permanent solution

I prefer different faces for symbol overlay and sideline overlay. But that of sideline overlay should be named lsp-ui-sideline-{level}-{tags}-face. If a generated flycheck level like lsp-flycheck-info-unnecessary is made a face name, it confuses users when looking for a face name related to sidelines.

rdiaz02 commented 5 months ago

This issue was fized 9 months ago, the complains started 2months ago, so I suggest opening a new issue with detailed repro and link to this issue, it may be something different that is causing the same issue to final user

Just added a simple to reproduce example with M-x lsp-start-plain in https://github.com/emacs-lsp/lsp-ui/issues/761#issuecomment-1992186359

CeleritasCelery commented 3 days ago

The transient solution by @pixel-ant triggers the issue https://github.com/syl20bnr/spacemacs/issues/16224 for me. Having this warning is much better than the error in lsp--in-idle.