Skip to content

Commit

Permalink
Normalize URIs that clients send
Browse files Browse the repository at this point in the history
This ensures that the handlers receive URIs that match the URIs in the
db, including those generated by converting clj-kondo filenames into
URIs.

I think that with this change, `:encode-colons-in-path?` is always false,
and should therefore be removed.
  • Loading branch information
mainej committed Oct 17, 2022
1 parent 133e988 commit c50b616
Show file tree
Hide file tree
Showing 4 changed files with 109 additions and 22 deletions.
59 changes: 47 additions & 12 deletions cli/src/clojure_lsp/server.clj
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
[lsp4clj.liveness-probe :as lsp.liveness-probe]
[lsp4clj.lsp.requests :as lsp.requests]
[lsp4clj.server :as lsp.server]
[medley.core :as medley]
[promesa.core :as p]
[promesa.exec :as p.exec]
[taoensso.timbre :as timbre]))
Expand All @@ -38,6 +39,11 @@

(def known-files-pattern "**/*.{clj,cljs,cljc,cljd,edn,bb,clj_kondo}")

(def normalize-uri shared/normalize-uri-from-client)

(defn normalize-doc-uri [params]
(medley/update-existing-in params [:text-document :uri] normalize-uri))

(defn log! [level args fmeta]
(timbre/log! level :p args {:?line (:line fmeta)
:?file (:file fmeta)
Expand Down Expand Up @@ -165,7 +171,7 @@
;;;; clojure extra features

(defmethod lsp.server/receive-request "clojure/dependencyContents" [_ components params]
(->> params
(->> (medley/update-existing params :uri normalize-uri)
(handler/dependency-contents components)
(conform-or-log ::coercer/uri)
eventually))
Expand All @@ -185,13 +191,14 @@

(defmethod lsp.server/receive-request "clojure/cursorInfo/raw" [_ components params]
(->> params
(normalize-doc-uri)
(handler/cursor-info-raw components)
eventually))

(defmethod lsp.server/receive-notification "clojure/cursorInfo/log" [_ components params]
(eventually
(future
(try
(handler/cursor-info-log components params)
(handler/cursor-info-log components (normalize-doc-uri params))
(catch Throwable e
(logger/error e)
(throw e)))))
Expand All @@ -206,30 +213,32 @@
;; https://microsoft.github.io/language-server-protocol/specifications/lsp/3.17/specification/#textDocument_synchronization

(defmethod lsp.server/receive-notification "textDocument/didOpen" [_ components params]
(handler/did-open components params))
(handler/did-open components (normalize-doc-uri params)))

(defmethod lsp.server/receive-notification "textDocument/didChange" [_ components params]
(handler/did-change components params))
(handler/did-change components (normalize-doc-uri params)))

(defmethod lsp.server/receive-notification "textDocument/didSave" [_ components params]
(future
(try
(handler/did-save components params)
(handler/did-save components (normalize-doc-uri params))
(catch Throwable e
(logger/error e)
(throw e)))))

(defmethod lsp.server/receive-notification "textDocument/didClose" [_ components params]
(handler/did-close components params))
(handler/did-close components (normalize-doc-uri params)))

(defmethod lsp.server/receive-request "textDocument/references" [_ components params]
(->> params
(normalize-doc-uri)
(handler/references components)
(conform-or-log ::coercer/locations)
eventually))

(defmethod lsp.server/receive-request "textDocument/completion" [_ components params]
(->> params
(normalize-doc-uri)
(handler/completion components)
(conform-or-log ::coercer/completion-items)
eventually))
Expand All @@ -243,6 +252,7 @@

(defmethod lsp.server/receive-request "textDocument/prepareRename" [_ components params]
(->> params
(normalize-doc-uri)
(handler/prepare-rename components)
(conform-or-log ::coercer/prepare-rename-or-error)
after-changes))
Expand All @@ -255,90 +265,104 @@

(defmethod lsp.server/receive-request "textDocument/hover" [_ components params]
(->> params
(normalize-doc-uri)
(handler/hover components)
(conform-or-log ::coercer/hover)
eventually))

(defmethod lsp.server/receive-request "textDocument/signatureHelp" [_ components params]
(->> params
(normalize-doc-uri)
(handler/signature-help components)
(conform-or-log ::coercer/signature-help)
eventually))

(defmethod lsp.server/receive-request "textDocument/formatting" [_ components params]
(->> params
(normalize-doc-uri)
(handler/formatting components)
(conform-or-log ::coercer/edits)
eventually))

(defmethod lsp.server/receive-request "textDocument/rangeFormatting" [_this components params]
(->> params
(normalize-doc-uri)
(handler/range-formatting components)
(conform-or-log ::coercer/edits)
after-changes))

(defmethod lsp.server/receive-request "textDocument/codeAction" [_ components params]
(->> params
(normalize-doc-uri)
(handler/code-actions components)
(conform-or-log ::coercer/code-actions)
after-changes))

(defmethod lsp.server/receive-request "textDocument/codeLens" [_ components params]
(->> params
(normalize-doc-uri)
(handler/code-lens components)
(conform-or-log ::coercer/code-lenses)
after-changes))

(defmethod lsp.server/receive-request "codeLens/resolve" [_ components params]
(->> params
(defmethod lsp.server/receive-request "codeLens/resolve" [_ components code-lens]
(->> code-lens
(handler/code-lens-resolve components)
(conform-or-log ::coercer/code-lens)
eventually))

(defmethod lsp.server/receive-request "textDocument/definition" [_ components params]
(->> params
(normalize-doc-uri)
(handler/definition components)
(conform-or-log ::coercer/location)
eventually))

(defmethod lsp.server/receive-request "textDocument/declaration" [_ components params]
(->> params
(normalize-doc-uri)
(handler/declaration components)
(conform-or-log ::coercer/location)
eventually))

(defmethod lsp.server/receive-request "textDocument/implementation" [_ components params]
(->> params
(normalize-doc-uri)
(handler/implementation components)
(conform-or-log ::coercer/locations)
eventually))

(defmethod lsp.server/receive-request "textDocument/documentSymbol" [_ components params]
(->> params
(normalize-doc-uri)
(handler/document-symbol components)
(conform-or-log ::coercer/document-symbols)
after-changes))

(defmethod lsp.server/receive-request "textDocument/documentHighlight" [_ components params]
(->> params
(normalize-doc-uri)
(handler/document-highlight components)
(conform-or-log ::coercer/document-highlights)
after-changes))

(defmethod lsp.server/receive-request "textDocument/semanticTokens/full" [_ components params]
(->> params
(normalize-doc-uri)
(handler/semantic-tokens-full components)
(conform-or-log ::coercer/semantic-tokens)
after-changes))

(defmethod lsp.server/receive-request "textDocument/semanticTokens/range" [_ components params]
(->> params
(normalize-doc-uri)
(handler/semantic-tokens-range components)
(conform-or-log ::coercer/semantic-tokens)
after-changes))

(defmethod lsp.server/receive-request "textDocument/prepareCallHierarchy" [_ components params]
(->> params
(normalize-doc-uri)
(handler/prepare-call-hierarchy components)
(conform-or-log ::coercer/call-hierarchy-items)
eventually))
Expand All @@ -357,6 +381,7 @@

(defmethod lsp.server/receive-request "textDocument/linkedEditingRange" [_ components params]
(->> params
(normalize-doc-uri)
(handler/linked-editing-ranges components)
(conform-or-log ::coercer/linked-editing-ranges-or-error)
eventually))
Expand All @@ -378,7 +403,11 @@
(logger/warn params))

(defmethod lsp.server/receive-notification "workspace/didChangeWatchedFiles" [_ components params]
(->> params
(->> (medley/update-existing params
:changes (fn [changes]
(mapv (fn [change]
(medley/update-existing change :uri normalize-uri))
changes)))
(conform-or-log ::coercer/did-change-watched-files-params)
(handler/did-change-watched-files components)))

Expand All @@ -389,7 +418,13 @@
eventually))

(defmethod lsp.server/receive-request "workspace/willRenameFiles" [_ components params]
(->> params
(->> (medley/update-existing params
:files (fn [files]
(mapv (fn [file]
(-> file
(medley/update-existing :old-uri normalize-uri)
(medley/update-existing :new-uri normalize-uri)))
files)))
(handler/will-rename-files components)
(conform-or-log ::coercer/workspace-edit)
after-changes))
Expand Down Expand Up @@ -458,7 +493,7 @@
;; about which messages are sent when probably needs to be handled in lsp4clj.
;; https://microsoft.github.io/language-server-protocol/specifications/lsp/3.17/specification/#initialize
(handler/initialize components
(:root-uri params)
(normalize-uri (:root-uri params))
(:capabilities params)
(client-settings params)
(some-> params :work-done-token str))
Expand Down
16 changes: 14 additions & 2 deletions lib/src/clojure_lsp/shared.clj
Original file line number Diff line number Diff line change
Expand Up @@ -189,14 +189,16 @@
canonical (-> uri-path .toString io/file .getCanonicalPath .toString)]
canonical))

(defn ^:private jar-uri-string->jar-url-connection ^JarURLConnection [uri]
(.openConnection (URL. (unescape-uri uri))))

(defn uri->filename
"Converts a URI string into an absolute file path.
The output path representation matches that of the operating system."
[^String uri]
(if (string/starts-with? uri "jar:")
(let [unescaped-uri (unescape-uri uri)
conn ^JarURLConnection (.openConnection (URL. unescaped-uri))
(let [conn (jar-uri-string->jar-url-connection uri)
jar-file (uri-obj->filepath ^URI (.toURI ^URL (.getJarFileURL conn)))]
(str jar-file ":" (.getEntryName conn)))

Expand Down Expand Up @@ -529,3 +531,13 @@
expects Clojure style JSON, or when a map needs to be round-tripped from
clojure-lsp to the client and back without case changes."
lsp.responses/preserve-kebab-case)

(defn normalize-uri-from-client [uri]
;; Technically jar:file is not a valid URI scheme. It's a Jar URL and must be
;; treated as such.
(if (string/starts-with? uri "jar:")
(.toString (.getURL (jar-uri-string->jar-url-connection uri)))
;; unescape %3a%3a to ::
(let [uri (URI. uri)]
;; normalize scheme:/some/path to scheme:///some/path
(uri-encode (.getScheme uri) (.getPath uri)))))
5 changes: 1 addition & 4 deletions lib/src/clojure_lsp/startup.clj
Original file line number Diff line number Diff line change
Expand Up @@ -172,10 +172,7 @@
encoding-settings {:uri-format {:upper-case-drive-letter? (->> project-root-uri URI. .getPath
(re-find #"^/[A-Z]:/")
boolean)
;; We need a more reliable way to know when clients will send URIs encoded.
;; Some clients encode only `::` and not `:` from `file://` for example.
:encode-colons-in-path? (or (string/includes? project-root-uri "%3A")
(= "zipfile" (:dependency-scheme client-settings)))}}
:encode-colons-in-path? (string/includes? project-root-uri "%3A")}}
settings (shared/deep-merge encoding-settings
client-settings
project-settings
Expand Down
51 changes: 47 additions & 4 deletions lib/test/clojure_lsp/shared_test.clj
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
(:require
[clojure-lsp.shared :as shared]
[clojure-lsp.test-helper :as h]
[clojure.test :refer [deftest is testing]]
[clojure.test :refer [are deftest is testing]]
[medley.core :as medley]))

(h/reset-components-before-test)
Expand Down Expand Up @@ -46,15 +46,21 @@
(deftest filename->uri
(testing "when it is not a jar"
(h/reset-components!)
(is (= (if h/windows? "file:///C:/some%20project/foo/bar_baz.clj" "file:///some%20project/foo/bar_baz.clj")
(is (= (if h/windows?
"file:///C:/some%20project/foo/bar_baz.clj"
"file:///some%20project/foo/bar_baz.clj")
(shared/filename->uri (h/file-path "/some project/foo/bar_baz.clj") (h/db)))))
(testing "when it is a jar via zipfile"
(h/reset-components!)
(is (= (if h/windows? "zipfile:///C:/home/some/.m2/some-jar.jar::clojure/core.clj" "zipfile:///home/some/.m2/some-jar.jar::clojure/core.clj")
(is (= (if h/windows?
"zipfile:///C:/home/some/.m2/some-jar.jar::clojure/core.clj"
"zipfile:///home/some/.m2/some-jar.jar::clojure/core.clj")
(shared/filename->uri (h/file-path "/home/some/.m2/some-jar.jar:clojure/core.clj") (h/db)))))
(testing "when it is a jar via jarfile"
(swap! (h/db*) shared/deep-merge {:settings {:dependency-scheme "jar"}})
(is (= (if h/windows? "jar:file:///C:/home/some/.m2/some-jar.jar!/clojure/core.clj" "jar:file:///home/some/.m2/some-jar.jar!/clojure/core.clj")
(is (= (if h/windows?
"jar:file:///C:/home/some/.m2/some-jar.jar!/clojure/core.clj"
"jar:file:///home/some/.m2/some-jar.jar!/clojure/core.clj")
(shared/filename->uri (h/file-path "/home/some/.m2/some-jar.jar:clojure/core.clj") (h/db)))))
(testing "Windows URIs"
(h/reset-components!)
Expand Down Expand Up @@ -215,3 +221,40 @@
(is (= true (shared/class-file? "file:///foo/bar.class")))
(is (= false (shared/class-file? "jar:file:///foo/bar.jar!/some/file.clj")))
(is (= true (shared/class-file? "jar:file:///foo/bar.jar!/some/file.class"))))

(deftest normalize-uri-from-client
(testing "jar files"
(are [client-uri] (= (h/file-path "jar:file:///some/path/some.jar!/some/file.clj")
(shared/normalize-uri-from-client (h/file-path client-uri)))
;; standard
"jar:file:///some/path/some.jar!/some/file.clj"
;; Calva
"jar:file%3A///some/path/some.jar%21/some/file.clj")
;; with spaces
;; TODO: this fails because `(unescape-uri uri)` converts %20 to a space
;; character, which we don't want. But, we can't remove `(unescape-uri uri)`,
;; or else the Calva jar file test above fails. I think it's rare for jar file
;; paths to contain spaces, so I'm leaving this test commented out. Would be
;; nice to fix someday.
#_(is (= (h/file-path "jar:file:///some%20spaces/path/some.jar!/some%20spaces/file.clj")
(shared/normalize-uri-from-client (h/file-path "jar:file:///some%20spaces/path/some.jar!/some%20spaces/file.clj")))))
(testing "zipfiles"
(are [client-uri] (= (h/file-path "zipfile:///some/path/some.jar::some/file.clj")
(shared/normalize-uri-from-client (h/file-path client-uri)))
;; standard
"zipfile:///some/path/some.jar::some/file.clj"
;; coc.nvim
"zipfile:/some/path/some.jar%3a%3asome/file.clj"))
(testing "standard files"
;; standard
(is (= (h/file-path "file:///some/file.clj")
(shared/normalize-uri-from-client (h/file-path "file:///some/file.clj"))))
;; with spaces
(is (= (h/file-path "file:///some%20spaces/file%20spaces.clj")
(shared/normalize-uri-from-client (h/file-path "file:///some%20spaces/file%20spaces.clj"))))
;; Windows
(when h/windows?
(are [uri] (= "file:///c:/c.clj"
(shared/normalize-uri-from-client uri))
"file:/c:/c.clj"
"file:///c:/c.clj"))))

0 comments on commit c50b616

Please sign in to comment.