bug#58790: Eglot URI parsing bug when using clojure-lsp server

[Danny Freeman via "Bug reports for GNU Emacs, the Swiss army knife of text editors" <bug-gnu-emacs@gnu.org>]

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

I have a new patch attached that does what João suggested in the 5 point 
list,
which I will respond to again

 > 2. Eglot converts the URI X into a some file designator Y, using
 >    eglot--uri-to-path.  Y may or may not be === X, as long as there is
 >    exactly one X for every Y.  Eglot makes xref-item objects from it
 >    using xref-make-match and xref-make-file-location.  The designator Y
 >    goes into the 'file' slot of the xref-file-location object.

The patch leaves X === Y for `jar` type URIs. My updated packages now 
deals with
the full jar URIs. No conversion are done. My new package can be found here:
https://git.sr.ht/~dannyfreeman/jarchive/tree/9148ed7ada03ff2516f6e4ede20c453974d6da19/item/jarchive.el

 > 4. B2 can be setup in a way so that project-current returns the same
 >    object which is returned in B.  If this is true,
 >    eglot--current-server discovers the same connection which also
 >    manages B1 and Eglot adds buffer B2 to the list of managed buffers in
 >    that connection.
 >
 >    However, if eglot-extend-to-xref is non-nil, eglot--current-server
 >    should also discover the correct connection.  This is less ideal than
 >    making project-current know that the buffers of source code inside
 >    the jar are also in the same project, but it works.  I can explain
 >    the downsides, but it's not very relevant right now.

My newly updated package does nothing with `project-current`, as I have no
obvious way to access the previous project when extracting files from 
the jar.
(Extracting now happens in `insert-file-contents` instead of 
`get-file-buffer`)

BUT eglot-extend-to-xref works as it should with this, and I think I 
prefer that.
It allows people who don't want that behavior to disable 
`eglot-extend-to-xref`
and have eglot start up a new lsp server when visiting the file.
Idk why anyone would want that but you never know.

 > 5. Upon findin the "file", Eglot transmits a :textDocument/didOpen
 >    notification to the server.  This requires eglot--path-to-uri, the
 >    reciprocal of eglot--uri-to-path to convert the path Y to URI X
 >    again.  Again, maybe this conversion is just #'identity.
 >
 >    Eventually, the LSP server knows that the client is now working on a
 >    textDocument whose relationship to other opened documents in the
 >    workspace it understands (which may or may not be read-only).

Sending the full jar uri back to the lsp server worked exactly as 
intended here.
I am honestly surprised I didn't have to change anything in clojure-lsp. 
It can
do it's analysis on the file without issue.

 > 6. xref-find-definition on any part of the B2 should now work similarly
 >    to this whole flow.

It does!!!!!

As for this question

 > I don't know what happens if another server also points to the same file.
 > Probably nothing very bad, but there may be some suprising behavior: I
 > haven't tested.

The same file continues to work with respect to the original lsp server
that was used to open it. The second lsp server is not conidered. I
guess because eglot can only work with one lsp server at a time. If you 
want to
associate the un-jar'd file with the second server, close it and
xref-find-defintions to it from a file in the second server. That seems 
like a
fine workaround to me, and its not something I've ever encountered in my 
routine
development workflow.


So, what do yall think about the patch? Is that something you might 
consider for
eglot? I believe I could also accomplish something similar with "advice" 
in my
package and leave eglot alone, but I would be poking at eglot internals 
which
seems not ideal.

[-- Attachment #2: 0001-Do-not-parse-jar-type-URIs-provided-by-lsp-servers-i.patch --]
[-- Type: text/x-patch, Size: 4931 bytes --]

From 671d3117bd9a810616dd5bbdfffa18e91a4d9896 Mon Sep 17 00:00:00 2001
From: dannyfreeman <danny@dfreeman.email>
Date: Mon, 31 Oct 2022 10:01:21 -0400
Subject: [PATCH] Do not parse jar type URIs provided by lsp servers in eglot

Clojure-lsp and possibly other lsp servers for JVM languages, can
provide file locations as jar scheme URIs for dependencies located
inside of jar archives. These URIs contain nested URIs that Emacs out of
the box does not know how to work with. Example:
  jar:file:///path/to/lib.jar!/path/inside/jar/source.ext

Even if they are parsed twice, the locations are of the format
  /path/to/lib.jar!/path/inside/jar/source.ext
which Emacs also does not know how to handle.

This commit introduces a var `eglot-preserve-jar-uri` that, when true,
will pass along jar URIs without parsing them. This allows other
packages to handle them through the `file-name-handler-alist` mechanism.
Additionally, any jars sent BACK to the lsp server by eglot will also
remain in the `jar` format, which at least clojure-lsp knows how to
interpret.

Given the correct file-name-handler-alist implementation, this change
allows xref to navigate to a file within a jar. When
`eglot-extend-to-xref` option is enabled, eglot can correctly connect to
the previous lsp server and continue to work in the newly opened file.

To ensure clojure-lsp servers always use the "jar" URI scheme for these
types of dependencies, an initialization option is set. The default
option, the "zipfile" scheme, is very similar to the jar
scheme. However, the "zipfile" scheme is something that the clojure-lsp
maintainers want to move away from and eventually remove.
---
 lisp/progmodes/eglot.el | 37 +++++++++++++++++++++++++------------
 1 file changed, 25 insertions(+), 12 deletions(-)

diff --git a/lisp/progmodes/eglot.el b/lisp/progmodes/eglot.el
index c587061837..b8f50e3cd8 100644
--- a/lisp/progmodes/eglot.el
+++ b/lisp/progmodes/eglot.el
@@ -231,7 +231,7 @@ eglot-server-programs
                                 (html-mode . ,(eglot-alternatives '(("vscode-html-language-server" "--stdio") ("html-languageserver" "--stdio"))))
                                 (dockerfile-mode . ("docker-langserver" "--stdio"))
                                 ((clojure-mode clojurescript-mode clojurec-mode)
-                                 . ("clojure-lsp"))
+                                 . ("clojure-lsp" :initializationOptions (:dependency-scheme "jar")))
                                 (csharp-mode . ("omnisharp" "-lsp"))
                                 (purescript-mode . ("purescript-language-server" "--stdio"))
                                 (perl-mode . ("perl" "-MPerl::LanguageServer" "-e" "Perl::LanguageServer::run"))
@@ -1492,25 +1492,38 @@ eglot--uri-path-allowed-chars
     vec)
   "Like `url-path-allows-chars' but more restrictive.")
 
+(defvar eglot-preserve-jar-uri nil
+  "If non-nil, jar: type URIs will not be converted to paths.
+This means they will be provided to xref as URIs and not file paths.")
+
 (defun eglot--path-to-uri (path)
   "URIfy PATH."
-  (let ((truepath (file-truename path)))
-    (concat "file://"
-            ;; Add a leading "/" for local MS Windows-style paths.
-            (if (and (eq system-type 'windows-nt)
-                     (not (file-remote-p truepath)))
-                "/")
-            (url-hexify-string
-             ;; Again watch out for trampy paths.
-             (directory-file-name (file-local-name truepath))
-             eglot--uri-path-allowed-chars))))
+  (if (and eglot-preserve-jar-uri (equal "jar" (url-type (url-generic-parse-url path))))
+      path
+    (let ((truepath (file-truename path)))
+      (concat "file://"
+              ;; Add a leading "/" for local MS Windows-style paths.
+              (if (and (eq system-type 'windows-nt)
+                       (not (file-remote-p truepath)))
+                  "/")
+              (url-hexify-string
+               ;; Again watch out for trampy paths.
+               (directory-file-name (file-local-name truepath))
+               eglot--uri-path-allowed-chars)))))
+
+(defun eglot--parse-uri (uri)
+  "Try to parse a URI."
+  (let ((url (url-generic-parse-url uri)))
+    (if (and eglot-preserve-jar-uri (string= "jar" (url-type url)))
+        uri
+      (url-unhex-string (url-filename url)))))
 
 (defun eglot--uri-to-path (uri)
   "Convert URI to file path, helped by `eglot--current-server'."
   (when (keywordp uri) (setq uri (substring (symbol-name uri) 1)))
   (let* ((server (eglot-current-server))
          (remote-prefix (and server (eglot--trampish-p server)))
-         (retval (url-unhex-string (url-filename (url-generic-parse-url uri))))
+         (retval (eglot--parse-uri uri))
          ;; Remove the leading "/" for local MS Windows-style paths.
          (normalized (if (and (not remote-prefix)
                               (eq system-type 'windows-nt)
-- 
2.37.3