Skip to content

Commit

Permalink
Make encode work for zipfile deps (#1327)
Browse files Browse the repository at this point in the history 
* Make encode work for zipfile deps

* Pass db, not db*, through call-hierarchy

* Normalize URIs that clients send

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.

* Try to fix tests for Windows

* Try again to fix tests for Windows

* Replace set with with-redefs

* Add find-definition to integration test

* Fix tests

Co-authored-by: Jacob Maine <jacob.maine@gmail.com>
  • Loading branch information
@ericdallo @mainej
ericdallo and mainej committed Oct 18, 2022
1 parent fe2165f commit 2b07890
Show file tree
Hide file tree
Showing 10 changed files with 186 additions and 39 deletions.
48 changes: 39 additions & 9 deletions cli/integration-test/integration/definition_test.clj
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,50 +21,50 @@
{:uri (h/source-path->uri "definition/a.clj")
:range {:start {:line 8 :character 16}
:end {:line 8 :character 33}}}
(lsp/request! (fixture/definition-request "definition/a.clj" 11 2))))
(lsp/request! (fixture/definition-request (h/source-path->uri "definition/a.clj") 11 2))))

(testing "find definition of local var"
(h/assert-submap
{:uri (h/source-path->uri "definition/a.clj")
:range {:start {:line 3 :character 5}
:end {:line 3 :character 13}}}
(lsp/request! (fixture/definition-request "definition/a.clj" 9 9))))
(lsp/request! (fixture/definition-request (h/source-path->uri "definition/a.clj") 9 9))))

(testing "find definition of public var"
(h/assert-submap
{:uri (h/source-path->uri "definition/a.clj")
:range {:start {:line 5 :character 6}
:end {:line 5 :character 22}}}
(lsp/request! (fixture/definition-request "definition/b.clj" 3 4)))))
(lsp/request! (fixture/definition-request (h/source-path->uri "definition/b.clj") 3 4)))))

(testing "keywords"
(testing "definition of local simple keyword is the keyword itself"
(h/assert-submap
{:uri (h/source-path->uri "definition/b.clj")
:range {:start {:line 5 :character 0}
:end {:line 5 :character 17}}}
(lsp/request! (fixture/definition-request "definition/b.clj" 5 2))))
(lsp/request! (fixture/definition-request (h/source-path->uri "definition/b.clj") 5 2))))

(testing "definition of invalid local namespaced keyword is the keyword itself"
(h/assert-submap
{:uri (h/source-path->uri "definition/b.clj")
:range {:start {:line 7 :character 0}
:end {:line 7 :character 8}}}
(lsp/request! (fixture/definition-request "definition/b.clj" 7 2))))
(lsp/request! (fixture/definition-request (h/source-path->uri "definition/b.clj") 7 2))))

(testing "find definition of valid other ns namespaced keyword"
(h/assert-submap
{:uri (h/source-path->uri "definition/a.clj")
:range {:start {:line 13 :character 7}
:end {:line 13 :character 15}}}
(lsp/request! (fixture/definition-request "definition/b.clj" 9 2))))
(lsp/request! (fixture/definition-request (h/source-path->uri "definition/b.clj") 9 2))))

(testing "find definition of valid other aliased keyword"
(h/assert-submap
{:uri (h/source-path->uri "definition/a.clj")
:range {:start {:line 13 :character 7}
:end {:line 13 :character 15}}}
(lsp/request! (fixture/definition-request "definition/b.clj" 11 2))))))
(lsp/request! (fixture/definition-request (h/source-path->uri "definition/b.clj") 11 2))))))

(deftest definition-external-dependency-jar-scheme
(h/delete-project-file "../../.lsp/.cache/")
Expand All @@ -74,7 +74,7 @@
(lsp/notify! (fixture/initialized-notification))
(lsp/notify! (fixture/did-open-source-path-notification "definition/a.clj"))

(let [{:keys [uri]} (lsp/request! (fixture/definition-request "definition/a.clj" 15 2))]
(let [{:keys [uri]} (lsp/request! (fixture/definition-request (h/source-path->uri "definition/a.clj") 15 2))]
(lsp/notify! (fixture/did-open-external-path-notification uri (slurp uri)))

(testing "LSP features work on external clojure opened files"
Expand All @@ -92,7 +92,7 @@
(lsp/notify! (fixture/initialized-notification))
(lsp/notify! (fixture/did-open-source-path-notification "definition/a.clj"))

(let [{:keys [uri]} (lsp/request! (fixture/definition-request "definition/a.clj" 15 2))]
(let [{:keys [uri]} (lsp/request! (fixture/definition-request (h/source-path->uri "definition/a.clj") 15 2))]
(lsp/notify! (fixture/did-open-external-path-notification uri (-> uri
(string/replace "zipfile:" "jar:file:")
(string/replace "::" "!/")
Expand All @@ -105,3 +105,33 @@
(-> (lsp/request! (fixture/hover-external-uri-request uri 7612 5))
:contents
(get 1))))))

(deftest definition-external-dependency-zipfile-scheme-escaped-uris
(h/delete-project-file "../../.lsp/.cache/")
(with-redefs [h/*escape-uris?* true]
(lsp/start-process!)
(lsp/request! (fixture/initialize-request))
(lsp/notify! (fixture/initialized-notification))
(lsp/notify! (fixture/did-open-source-path-notification "definition/a.clj"))

(let [{:keys [uri]} (lsp/request! (fixture/definition-request (h/source-path->uri "definition/a.clj") 15 2))]
(lsp/notify! (fixture/did-open-external-path-notification (h/escape-uri uri)
(-> (h/unescape-uri uri)
(string/replace "zipfile:" "jar:file:")
(string/replace "::" "!/")
slurp)))

(testing "LSP hover feature works on external clojure opened files"
(h/assert-submap
{:language "clojure"
:value "[x]\n[x message]"}
(-> (lsp/request! (fixture/hover-external-uri-request (h/escape-uri uri) 7612 5))
:contents
(get 1))))

(testing "LSP definition from external clojure opened file works"
(h/assert-submap
{:uri uri
:range {:start {:line 4840 :character 10}
:end {:line 4840 :character 16}}}
(lsp/request! (fixture/definition-request (h/escape-uri uri) 7612 5)))))))
4 changes: 2 additions & 2 deletions cli/integration-test/integration/fixture.clj
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,9 +23,9 @@
{:textDocument {:uri (h/source-path->uri path)}
:position {:line line :character character}}])

(defn definition-request [path line character]
(defn definition-request [uri line character]
[:textDocument/definition
{:textDocument {:uri (h/source-path->uri path)}
{:textDocument {:uri uri}
:position {:line line :character character}}])

(defn declaration-request [path line character]
Expand Down
24 changes: 22 additions & 2 deletions cli/integration-test/integration/helper.clj
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,12 +4,18 @@
[clojure.java.io :as io]
[clojure.pprint :as pprint]
[clojure.string :as string]
[clojure.test :refer [is]]))
[clojure.test :refer [is]])
(:import
[java.net
URLDecoder]
[java.nio.charset StandardCharsets]))

(def windows?
"Whether is running on MS-Windows."
(string/starts-with? (System/getProperty "os.name") "Windows"))

(def ^:dynamic *escape-uris?* false)

(def root-project-path
(-> (io/file *file*)
.getParentFile
Expand Down Expand Up @@ -45,8 +51,22 @@
rest
(->> (apply str)))))

(defn unescape-uri
[^String uri]
(try
(URLDecoder/decode uri (.name StandardCharsets/UTF_8)) ;; compatible with Java 1.8 too!
(catch IllegalArgumentException _
uri)))

(defn escape-uri [uri]
;; Do a better escape considering more chars
(string/replace uri "::" "%3A%3A"))

(defn file->uri [file]
(-> file fs/canonicalize .toUri .toString))
(let [uri (-> file fs/canonicalize .toUri .toString)]
(if *escape-uris?*
(escape-uri uri)
uri)))

(defn string=
"Like `clojure.core/=` applied on STRING1 and STRING2, but treats
Expand Down
6 changes: 3 additions & 3 deletions cli/integration-test/integration/java_interop_test.clj
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@
{:uri (h/source-path->uri "java_interop/SampleClass.java")
:range {:start {:line 0 :character 0}
:end {:line 0 :character 0}}}
(lsp/request! (fixture/definition-request "java_interop/a.clj" 7 5)))))
(lsp/request! (fixture/definition-request (h/source-path->uri "java_interop/a.clj") 7 5)))))

(deftest find-definition-of-java-class-when-source-does-not-exists-for-jar-scheme
(h/delete-project-file "../../.lsp/.cache")
Expand All @@ -34,7 +34,7 @@
(lsp/notify! (fixture/initialized-notification))
(lsp/notify! (fixture/did-open-source-path-notification "java_interop/a.clj"))

(let [result (lsp/request! (fixture/definition-request "java_interop/a.clj" 8 5))]
(let [result (lsp/request! (fixture/definition-request (h/source-path->uri "java_interop/a.clj") 8 5))]
(testing "We find java compiled class first"
(h/assert-submap
{:uri (-> "integration-test/sample-test/.lsp/.cache/java/decompiled/clojure/lang/PersistentVector.java"
Expand Down Expand Up @@ -63,7 +63,7 @@
(lsp/notify! (fixture/initialized-notification))
(lsp/notify! (fixture/did-open-source-path-notification "java_interop/a.clj"))

(let [result (lsp/request! (fixture/definition-request "java_interop/a.clj" 8 5))]
(let [result (lsp/request! (fixture/definition-request (h/source-path->uri "java_interop/a.clj") 8 5))]
(testing "We find java compiled class first"
(h/assert-submap
{:uri (-> "integration-test/sample-test/.lsp/.cache/java/decompiled/clojure/lang/PersistentVector.java"
Expand Down

0 comments on commit 2b07890

Please sign in to comment.