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.73k stars 864 forks source link

Missing highlight for errors in java projects #3326

Open vspinu opened 2 years ago

vspinu commented 2 years ago

Bug description

I don't see highlights in java even though the errors are clearly there.

image

Steps to reproduce

Start lsp plain and navigate to fixtures/org-mode/java-project/.../App.java and make some errors.

Expected behavior

see flymake underlying for errors

Which Language Server did you use?

eclipse.jdt.ls/plugins/org.eclipse.equinox.launcher_1.6.400.v20210924-0641.jar

OS

Linux

Error callstack

I do see the diagnostic message coming back:

[Trace - 10:45:31 pm] Received notification 'textDocument/publishDiagnostics'.
Params: {
  "uri": "file:///home/vspinu/Dropbox/dev/lsp-mode/test/fixtures/org-mode/java-project/src/main/java/temp/App.java",
  "diagnostics": [
    {
      "range": {
        "start": {
          "line": 8,
          "character": 21
        },
        "end": {
          "line": 8,
          "character": 31
        }
      },
      "severity": 1,
      "code": "67108964",
      "source": "Java",
      "message": "The method getClassss() is undefined for the type Class<System>"
    }
  ]
}

But nothing else that would involve that region.

yyoncho commented 2 years ago

which emacs version do you use? Can you try installing the latest flymake from elpa? I tested with emacs 29 + lsp-start-plain.el (after removing flycheck) and it works fine.

vspinu commented 2 years ago

It's 28 and 29 that I have tested. Will pick from melpa and try again. How do I go about debugging? This flymake backend is a good start I presume? https://github.com/emacs-lsp/lsp-mode/blob/6ddba8532c43b2e3453ade063107b4519ca9e87d/lsp-diagnostics.el#L278

yyoncho commented 2 years ago

I would test this one in the scope of the current buffer.

(gethash (lsp--fix-path-casing buffer-file-name) (lsp-diagnostics t))
vspinu commented 2 years ago

Right on spot! It's the same problem as #3325. I have the project simlinked. Diagnostic table is with truenames, the query is with the symlink path :thinking:

yyoncho commented 2 years ago

that is pretty much the core of the problem. If we call file-truename when receiving diagnostics it will pretty much make emacs unusable in certain cases. The longer the paths, the slower file-truename becomes. And some servers tend to refresh diagnostics in all files very often.

vspinu commented 2 years ago

To me it looks that the solution is to move the other way around - never use truename in the first place. The buffername and emacs navigation operates on filenames as they are, which makes a lot of sense and this is what users expect.

I am constantly seeing messages of the kind "file X and Y are the same file" because of this mismatch.

yyoncho commented 2 years ago

this is what we are trying to do. But when the server has a different notion of what is the file name is we get issues like that.

vspinu commented 2 years ago

Some of that comes from lsp-mode itself. Initialization and workspace/didChangeWatchedFiles do send the truename. Other requests use symlinks from what I see.

yyoncho commented 2 years ago

Initialization and workspace/didChangeWatchedFiles do send the truename. Other requests use symlinks from what I see.

That is needed to handle circular symlinks.

vspinu commented 2 years ago

So no plans to change that? Circular symlinks sound like a user problem, not lsp-mode to deal with.

yyoncho commented 2 years ago

@vspinu IMHO it is a valid use-case and we have to handle it. I mean it is perfectly valid to have a link under /a/c/ to /a. We had a bug report for that.

IMO there are several options:

  1. Core emacs exposes an async file-truename that does not run on UI thread.
  2. What we have right now. ATM it is the responsibility of the user to organize symlinks and workspace folder roots in a way that it works. Not sure that we handle all cases. I think that if
  3. Cache file-truename.
vspinu commented 2 years ago

Do you have an example of an issue where file truename is really needed? To my mind if a server returns a true name instead of the originally requested symlink, it's a bug in the server and that should be addresed upstream.

yyoncho commented 2 years ago

Check find-buffer-visiting. Note also that there are systems that are not case sensitive, and also afaik MacOS also might return different casing for the volumes. Also, note that the server might return info about a file that we haven't opened yet.

yyoncho commented 2 years ago

To elaborate a bit more - the issue is how to find the buffer corresponding to URI on all OS-es fast enough.