bug#59826: flymake-show-project-diagnotics not updating (eglot for Java with jdtls)

bug#59826: flymake-show-project-diagnotics not updating (eglot for Java with jdtls)

[David Ventimiglia]

[-- Attachment #1: Type: text/plain, Size: 1520 bytes --]

Hello,

When I run flymake-show-project-diagnostics some warnings are listed for my
Java project. As I jump to those locations and fix the warnings, the
diagnostics buffer doesn't update. One problem I have is that I don't even
know where to begin to look in order to troubleshoot this. It could be a
problem with eglot, jdtls, flymake, or some combination thereof. I started
a GitHub discussion for the eglot project here:

https://github.com/joaotavora/eglot/discussions/1131#discussion-4626934

In summary, for Java if I add say an unused import to a file, jdtls
publishes a diagnostic event for this and the flymake buffer shows a
warning for the unused import.  If I correct the issue by deleting the
unused import, jdtls again publishes a correct diagnostic report for the
file, but the flymake diagnostics buffer doesn't update.

After some correspondence with its maintainer, I believe we ruled out the
jdtls server, since the events buffer seems to show an accurate diagnostics
report being delivered from the server to Emacs/eglot/flymake.  We're
wondering if perhaps the problem lies therefore in eglot and/or flymake.  I
should also say that I'm using the "stock" eglot and flymake built into
Emacs, as I'm using emacs-snapshot.  Here's my Emacs version info:

GNU Emacs 30.0.50 (build 1, x86_64-pc-linux-gnu, GTK+ Version 3.24.33,
cairo version 1.16.0) of 2022-12-03

I grant that this may not be a bug and could be a mis-configuration, but I
am trying to track that down.  Thanks!

Best,
David Ventimiglia

[-- Attachment #2: Type: text/html, Size: 1854 bytes --]

bug#59824: flymake-show-project-diagnotics not updating (eglot for Java with jdtls)

[João Távora]

[-- Attachment #1: Type: text/plain, Size: 1825 bytes --]

Thanks David. Please also repost here the transcripts of the events buffer
that you collected and shared in the original discussion, for convenience.

On Sun, Dec 4, 2022, 20:39 David Ventimiglia <
davidaventimiglia@neptunestation.com> wrote:

> Hello,
>
> When I run flymake-show-project-diagnostics some warnings are listed for
> my Java project. As I jump to those locations and fix the warnings, the
> diagnostics buffer doesn't update. One problem I have is that I don't even
> know where to begin to look in order to troubleshoot this. It could be a
> problem with eglot, jdtls, flymake, or some combination thereof. I started
> a GitHub discussion for the eglot project here:
>
> https://github.com/joaotavora/eglot/discussions/1131#discussion-4626934
>
> In summary, for Java if I add say an unused import to a file, jdtls
> publishes a diagnostic event for this and the flymake buffer shows a
> warning for the unused import.  If I correct the issue by deleting the
> unused import, jdtls again publishes a correct diagnostic report for the
> file, but the flymake diagnostics buffer doesn't update.
>
> After some correspondence with its maintainer, I believe we ruled out the
> jdtls server, since the events buffer seems to show an accurate diagnostics
> report being delivered from the server to Emacs/eglot/flymake.  We're
> wondering if perhaps the problem lies therefore in eglot and/or flymake.  I
> should also say that I'm using the "stock" eglot and flymake built into
> Emacs, as I'm using emacs-snapshot.  Here's my Emacs version info:
>
> GNU Emacs 30.0.50 (build 1, x86_64-pc-linux-gnu, GTK+ Version 3.24.33,
> cairo version 1.16.0) of 2022-12-03
>
> I grant that this may not be a bug and could be a mis-configuration, but I
> am trying to track that down.  Thanks!
>
> Best,
> David Ventimiglia
>
>

[-- Attachment #2: Type: text/html, Size: 2408 bytes --]

bug#59824: flymake-show-project-diagnotics not updating (eglot for Java with jdtls)

[João Távora]

David Ventimiglia <davidaventimiglia@neptunestation.com> writes:

> I grant that this may not be a bug and could be a mis-configuration,
> but I am trying to track that down.  Thanks!

I may have found the possibly culprit in Eglot.  I wish you could try
the patch after my sig, untested by me.  Also, if you find that the
project listing still doesn't update automatically, please go to its
buffer and type 'g' (and report back here that you needed this extra
step).

João

diff --git a/lisp/progmodes/eglot.el b/lisp/progmodes/eglot.el
index e057b12e0ee..3d0e97bba8c 100644
--- a/lisp/progmodes/eglot.el
+++ b/lisp/progmodes/eglot.el
@@ -2048,9 +2048,11 @@ eglot-handle-notification
                     (t          'eglot-note)))
             (mess (source code message)
               (concat source (and code (format " [%s]" code)) ": " message)))
-    (if-let ((buffer (find-buffer-visiting (eglot--uri-to-path uri))))
+    (if-let* ((path (expand-file-name (eglot--uri-to-path uri)))
+              (buffer (find-buffer-visiting path)))
         (with-current-buffer buffer
           (cl-loop
+           initially (assoc-delete-all path flymake-list-only-diagnostics #'string=)
            for diag-spec across diagnostics
            collect (eglot--dbind ((Diagnostic) range code message severity source tags)
                        diag-spec
@@ -2093,7 +2095,6 @@ eglot-handle-notification
                          (t
                           (setq eglot--diagnostics diags)))))
       (cl-loop
-       with path = (expand-file-name (eglot--uri-to-path uri))
        for diag-spec across diagnostics
        collect (eglot--dbind ((Diagnostic) code range message severity source) diag-spec
                  (setq message (mess source code message))

bug#59824: flymake-show-project-diagnotics not updating (eglot for Java with jdtls)

[David Ventimiglia]

[-- Attachment #1: Type: text/plain, Size: 2823 bytes --]

That seems to have fixed it.  Can you explain what's going on here?  To my
untutored eye, it looks we're doing the following:

   1. In addition to defining the "buffer" variable from the "uri", we're
   now also defining "path" from the "uri".
   2. Using the value of "path" to purge all entries from
   "flymake-list-only-diagnostics" whose key is string= to the "path".
   3. Further down in the function, no longer setting "path" while using
   cl-loop to loop over "diagnostics", presumably because that's redundant now
   that "path" has been defined above.

Broadly, it looks like diagnostics were sorta *cached* and if the eglot
backed publishes an empty set of diagnostics for a file (i.e., the file has
been corrected) then we make sure to purge the stale cache.  Or something
like that.  Am I close?

Thanks!
David

On Sun, Dec 4, 2022 at 1:01 PM João Távora <joaotavora@gmail.com> wrote:

> David Ventimiglia <davidaventimiglia@neptunestation.com> writes:
>
> > I grant that this may not be a bug and could be a mis-configuration,
> > but I am trying to track that down.  Thanks!
>
> I may have found the possibly culprit in Eglot.  I wish you could try
> the patch after my sig, untested by me.  Also, if you find that the
> project listing still doesn't update automatically, please go to its
> buffer and type 'g' (and report back here that you needed this extra
> step).
>
> João
>
> diff --git a/lisp/progmodes/eglot.el b/lisp/progmodes/eglot.el
> index e057b12e0ee..3d0e97bba8c 100644
> --- a/lisp/progmodes/eglot.el
> +++ b/lisp/progmodes/eglot.el
> @@ -2048,9 +2048,11 @@ eglot-handle-notification
>                      (t          'eglot-note)))
>              (mess (source code message)
>                (concat source (and code (format " [%s]" code)) ": "
> message)))
> -    (if-let ((buffer (find-buffer-visiting (eglot--uri-to-path uri))))
> +    (if-let* ((path (expand-file-name (eglot--uri-to-path uri)))
> +              (buffer (find-buffer-visiting path)))
>          (with-current-buffer buffer
>            (cl-loop
> +           initially (assoc-delete-all path flymake-list-only-diagnostics
> #'string=)
>             for diag-spec across diagnostics
>             collect (eglot--dbind ((Diagnostic) range code message
> severity source tags)
>                         diag-spec
> @@ -2093,7 +2095,6 @@ eglot-handle-notification
>                           (t
>                            (setq eglot--diagnostics diags)))))
>        (cl-loop
> -       with path = (expand-file-name (eglot--uri-to-path uri))
>         for diag-spec across diagnostics
>         collect (eglot--dbind ((Diagnostic) code range message severity
> source) diag-spec
>                   (setq message (mess source code message))
>
>
>

[-- Attachment #2: Type: text/html, Size: 3593 bytes --]

bug#59824: flymake-show-project-diagnotics not updating (eglot for Java with jdtls)

[João Távora]

[-- Attachment #1: Type: text/plain, Size: 1335 bytes --]

On Sun, Dec 4, 2022 at 11:23 PM David Ventimiglia <
davidaventimiglia@neptunestation.com> wrote:
>
> That seems to have fixed it.  Can you explain what's going on here?

What's going on is "a lot" or "not much" depending on the level of detail
you're after.

There are two main types of project-wide diagnostic, as described in the
manual,
depending on whether the file that the diagnostic refers to is or isn't
visited by emacs
with Flymake enabled.

As [1] explains, the unvisited type can be handled by "foreign" or
"list-only" diagnostics.
Eglot uses the latter.  When Eglot is activated in a buffer visiting one of
the files that
were in 'flymake-list-only-diagnostics' it needs to remember to remove that
file from
that list, as it is no longer needed there.  This wasn't happening and my
patch fixes it.
The project listing is then refreshed automatically (at least it seems it
is, according to
your report).

Hope this helps.

> To my untutored  eye, it looks we're doing the following. [...] Am I
close?

To be honest, I don't think so. At least I don't recognize in your
description the design
I put forth some time ago (and which I had since forgotten until this bug).

João

[1]:
https://www.gnu.org/software/emacs/manual/html_node/flymake/Foreign-and-list_002donly-diagnostics.html

[-- Attachment #2: Type: text/html, Size: 1911 bytes --]

bug#59824: flymake-show-project-diagnotics not updating (eglot for Java with jdtls)

[David Ventimiglia]

[-- Attachment #1: Type: text/plain, Size: 1600 bytes --]

No worries.  I just updated the bug with the additional info.  I trust you
can take it from here.  Thanks!

On Mon, Dec 5, 2022 at 3:31 AM João Távora <joaotavora@gmail.com> wrote:

> On Sun, Dec 4, 2022 at 11:23 PM David Ventimiglia <
> davidaventimiglia@neptunestation.com> wrote:
> >
> > That seems to have fixed it.  Can you explain what's going on here?
>
> What's going on is "a lot" or "not much" depending on the level of detail
> you're after.
>
> There are two main types of project-wide diagnostic, as described in the
> manual,
> depending on whether the file that the diagnostic refers to is or isn't
> visited by emacs
> with Flymake enabled.
>
> As [1] explains, the unvisited type can be handled by "foreign" or
> "list-only" diagnostics.
> Eglot uses the latter.  When Eglot is activated in a buffer visiting one
> of the files that
> were in 'flymake-list-only-diagnostics' it needs to remember to remove
> that file from
> that list, as it is no longer needed there.  This wasn't happening and my
> patch fixes it.
> The project listing is then refreshed automatically (at least it seems it
> is, according to
> your report).
>
> Hope this helps.
>
> > To my untutored  eye, it looks we're doing the following. [...] Am I
> close?
>
> To be honest, I don't think so. At least I don't recognize in your
> description the design
> I put forth some time ago (and which I had since forgotten until this bug).
>
> João
>
> [1]:
> https://www.gnu.org/software/emacs/manual/html_node/flymake/Foreign-and-list_002donly-diagnostics.html
>
>

[-- Attachment #2: Type: text/html, Size: 2410 bytes --]

bug#59824: flymake-show-project-diagnotics not updating (eglot for Java with jdtls)

[João Távora]

[-- Attachment #1: Type: text/plain, Size: 100 bytes --]

I've now pushed a fix for this in the emacs-29 branch.

Marking this bug done.

Thanks,
João

[-- Attachment #2: Type: text/html, Size: 214 bytes --]

bug#59826: additional info

[David Ventimiglia]

[-- Attachment #1.1: Type: text/plain, Size: 2043 bytes --]

Taking an example Java servlet file ProtonServlet.java, when I add an
unused import there's an event like this:

[server-notification] Sun Dec  4 12:01:33 2022:
(:jsonrpc "2.0" :method "textDocument/publishDiagnostics" :params
 (:uri
"file:///home/neptunestationorg/Work/GitHub/ProtonChamber/src/main/java/org/protonchamber/ProtonServlet.java"
:diagnostics
[(:range
 (:start
  (:line 3 :character 7)
  :end
  (:line 3 :character 15))
 :severity 2 :code "268435844" :source "Java" :message "The import java.sql
is never used" :tags
 [1])]))


That makes sense. When I delete the unused import, there's an event like
this:

That also makes sense, as it seems to be that the server is publishing an
empty list of diagnostics for this file. Nevertheless, the
flymake-show-project-diagnostics buffer still has this entry:

ProtonServlet.java            4   8 warning  n        Java [268435844]: The
import java.sql is never used


Also, I see that sometimes warnings are supplied by one back-end, but then
when I fix the issue the same warning is then provided by a different
back-end. Here's what I mean. I have another file with an unused import for
java.sql. When I do, the diagnostics buffer shows an entry from the e-f-b
back-end, which I take to be the "eglot-flymake-backend":

SQLServlet.java               4   7 warning  e-f-b    Java [268435844]: The
import java.sql is never used


If I delete the offending line, the diagnostics buffer replaces that with
this:

SQLServlet.java               4   8 warning  n        Java [268435844]: The
import java.sql is never used


I don't know what the n back-end is, but evidently it's publishing its own
diagnostic for this code 268435844 which somehow is shadowed by the same
diagnostic for the same code from the e-f-b back-end. When the e-f-b
back-end correctly publishes an empty diagnostic report for this file, the
diagnostic report from the n back-end becomes revealed. Or something like
that.

Finally, I was sent a patch, which is attached.  When applied, that seemed
to fix the issue.

[-- Attachment #1.2: Type: text/html, Size: 3225 bytes --]

[-- Attachment #2: eglot.patch --]
[-- Type: text/x-patch, Size: 1189 bytes --]

--- a/lisp/progmodes/eglot.el
+++ b/lisp/progmodes/eglot.el
@@ -2048,9 +2048,11 @@ eglot-handle-notification
                     (t          'eglot-note)))
             (mess (source code message)
               (concat source (and code (format " [%s]" code)) ": " message)))
-    (if-let ((buffer (find-buffer-visiting (eglot--uri-to-path uri))))
+    (if-let* ((path (expand-file-name (eglot--uri-to-path uri)))
+              (buffer (find-buffer-visiting path)))
         (with-current-buffer buffer
           (cl-loop
+           initially (assoc-delete-all path flymake-list-only-diagnostics #'string=)
            for diag-spec across diagnostics
            collect (eglot--dbind ((Diagnostic) range code message severity source tags)
                        diag-spec
@@ -2093,7 +2095,6 @@ eglot-handle-notification
                          (t
                           (setq eglot--diagnostics diags)))))
       (cl-loop
-       with path = (expand-file-name (eglot--uri-to-path uri))
        for diag-spec across diagnostics
        collect (eglot--dbind ((Diagnostic) code range message severity source) diag-spec
                  (setq message (mess source code message))