[Fix #289] Namespaced keywords vs find (local) symbol

The edge-case handling for local symbols in opt-maps wasn't using a
reader that was configured correctly and choked on a namespaced keyword.

Author: expez

CHANGELOG.md

@@ -2,6 +2,8 @@
 
 ## Unreleased
 
+### Bugs fixed
+* [#289](https://github.com/clojure-emacs/refactor-nrepl/issues/289): Fix an edge-case with involving keywords that caused find-symbol to crash.
 * [#305](https://github.com/clojure-emacs/refactor-nrepl/issues/305): Don't put `:as` or `:refer` on their own lines in the ns form, when the libspec is so long it causes the line to wrap.
 * [clojure-emacs/clj-refactor.el#459](https://github.com/clojure-emacs/clj-refactor.el/issues/459): `clean-ns` should conform to the style guide: `(:require` in the ns form should be followed by a newline.
   * You can opt out via the new `:insert-newline-after-require` configuration option.

project.clj

@@ -27,7 +27,7 @@
   :filespecs [{:type :bytes :path "refactor-nrepl/refactor-nrepl/project.clj" :bytes ~(slurp "project.clj")}]
   :profiles {;; Clojure versions matrix
              :provided {:dependencies [[cider/cider-nrepl "0.25.9"]
-                                       [org.clojure/clojure "1.8.0"]]}
+                                       [org.clojure/clojure "1.9.0"]]}
              :1.8 {:dependencies [[org.clojure/clojure "1.8.0"]
                                   [org.clojure/clojurescript "1.8.51"]
                                   [javax.xml.bind/jaxb-api "2.3.1"]]}

src/refactor_nrepl/core.clj

@@ -380,3 +380,12 @@
   [symbol-or-keyword]
   (when (prefix symbol-or-keyword)
     symbol-or-keyword))
+
+(defmacro with-clojure-version->=
+  "Guard the evaluation of `body` with a test on the current clojure version."
+  {:style/indent 1}
+  [{:keys [major minor] :as _clojure-version} & body]
+  (when (or (> (:major *clojure-version*) major)
+            (and (= (:major *clojure-version*) major)
+                 (>= (:minor *clojure-version*) minor)))
+    `(do ~@body)))

src/refactor_nrepl/find/find_symbol.clj

@@ -10,7 +10,8 @@
              [s-expressions :as sexp]]
             [refactor-nrepl.find.find-macros :refer [find-macro]]
             [refactor-nrepl.find.util :as find-util]
-            [refactor-nrepl.ns.libspecs :as libspecs])
+            [refactor-nrepl.ns.libspecs :as libspecs]
+            [clojure.tools.reader :as reader])
   (:import (java.io File)))
 
 (def ^:private symbol-regex #"[\w\.:\*\+\-_!\?]+")
@@ -140,12 +141,14 @@
 
 (defn- get&read-enclosing-sexps
   [file-content {:keys [^long line-beg ^long col-beg]}]
-  (binding [*read-eval* false]
+  (binding [*read-eval* false
+            clojure.tools.reader/*alias-map*
+            (ns-aliases (core/ns-from-string file-content))]
     (let [line (dec line-beg)
           encl-sexp-level1 (or (sexp/get-enclosing-sexp file-content line col-beg) "")
           encl-sexp-level2 (or (sexp/get-enclosing-sexp file-content line col-beg 2) "")]
-      [encl-sexp-level1 (read-string encl-sexp-level1)
-       encl-sexp-level2 (read-string encl-sexp-level2)])))
+      [encl-sexp-level1 (reader/read-string encl-sexp-level1)
+       encl-sexp-level2 (reader/read-string encl-sexp-level2)])))
 
 (defn- optmap-with-default?
   [var-name _file-content [_ [_ level1-form _ level2-form]]]

test/refactor_nrepl/integration_tests.clj

@@ -3,6 +3,7 @@
             [clojure.test :refer :all]
             [nrepl.server :as nrepl]
             [refactor-nrepl middleware
+             [analyzer :as analyzer]
              [client :refer :all]
              [core :as core]]
             [clojure.string :as str])
@@ -58,11 +59,11 @@
 (defn ns-ast-throw-error-for-five [^String content]
   (if (.contains content "com.example.five")
     (throw (IllegalThreadStateException. "FAILED!"))
-    (#'refactor-nrepl.analyzer/cachable-ast content)))
+    (#'analyzer/cachable-ast content)))
 
 (deftest test-find-two-foo-errors-ignored
   (with-testproject-on-classpath
-    (with-redefs [refactor-nrepl.analyzer/ns-ast ns-ast-throw-error-for-five]
+    (with-redefs [analyzer/ns-ast ns-ast-throw-error-for-five]
       (let [transport (connect :port 7777)
             response (find-usages :transport transport :ns 'com.example.two
                                   :file (str test-project-dir "/src/com/example/one.clj")
@@ -186,46 +187,55 @@
   (with-testproject-on-classpath
     (let [five-file (str test-project-dir "/src/com/example/five.clj")
           transport (connect :port 7777)
-          response (find-usages :transport transport :name "foo" :file five-file :line 46 :column 10)
+          response (find-usages :transport transport :name "foo" :file five-file :line 47 :column 10)
           result (remove keyword? response)]
       (is (= 3 (count result)) (format "expected 3 results but got %d" (count result))))))
 
 (deftest find-local-in-optmap-default-linebreaks
   (with-testproject-on-classpath
     (let [five-file (str test-project-dir "/src/com/example/five.clj")
           transport (connect :port 7777)
-          response (find-usages :transport transport :name "foo" :file five-file :line 49 :column 12)
+          response (find-usages :transport transport :name "foo" :file five-file :line 50 :column 12)
           result (remove keyword? response)]
       (is (= 3 (count result)) (format "expected 3 results but got %d" (count result))))))
 
 (deftest find-local-in-optmap-default-in-let
   (with-testproject-on-classpath
     (let [five-file (str test-project-dir "/src/com/example/five.clj")
           transport (connect :port 7777)
-          response (find-usages :transport transport :name "foo" :file five-file :line 59 :column 12)
+          response (find-usages :transport transport :name "foo" :file five-file :line 60 :column 12)
           result (remove keyword? response)]
       (is (= 3 (count result)) (format "expected 3 results but got %d" (count result))))))
 
+(core/with-clojure-version->= {:major 1 :minor 9}
+  (deftest find-local-in-namespaced-destructuring
+    (with-testproject-on-classpath
+      (let [five-file (str test-project-dir "/src/com/example/five.clj")
+            transport (connect :port 7777)
+            response (find-usages :transport transport :name "foo" :file five-file :line 67 :column 16)
+            result (remove keyword? response)]
+        (is (= 2 (count result)) (format "expected 3 results but got %d" (count result)))))))
+
 (deftest test-find-used-locals
   (with-testproject-on-classpath
     (let [five-file (str test-project-dir "/src/com/example/five.clj")
           transport (connect :port 7777)]
-      (is (= (find-unbound :transport transport :file five-file :line 12 :column 6)
+      (is (= (find-unbound :transport transport :file five-file :line 13 :column 6)
              '(s)))
-      (is (= (find-unbound :transport transport :file five-file :line 13 :column 13)
+      (is (= (find-unbound :transport transport :file five-file :line 14 :column 13)
              '(s sep)))
 
-      (is (= (find-unbound :transport transport :file five-file :line 20 :column 16)
+      (is (= (find-unbound :transport transport :file five-file :line 21 :column 16)
              '(p)))
-      (is (= (find-unbound :transport transport :file five-file :line 27 :column 8)
+      (is (= (find-unbound :transport transport :file five-file :line 28 :column 8)
              '(sep strings)))
 
-      (is (= (find-unbound :transport transport :file five-file :line 34 :column 8)
+      (is (= (find-unbound :transport transport :file five-file :line 35 :column 8)
              '(name)))
 
-      (is (= (find-unbound :transport transport :file five-file :line 37 :column 5)
+      (is (= (find-unbound :transport transport :file five-file :line 38 :column 5)
              '(n)))
-      (is (= (find-unbound :transport transport :file five-file :line 41 :column 4)
+      (is (= (find-unbound :transport transport :file five-file :line 42 :column 4)
              '(x y z a b c))))))
 
 (deftest find-unbound-fails-on-cljs

test/resources/testproject/src/com/example/five.clj

@@ -1,5 +1,6 @@
 (ns com.example.five
-  (:require [clojure.string :refer [join split blank? trim] :as str]))
+  (:require [clojure.string :refer [join split blank? trim] :as str]
+            [refactor-nrepl.core :as core]))
 
 ;;  remove parameters to run the tests
 (defn fn-with-unbounds [s sep]
@@ -58,6 +59,13 @@
     [:bar :foo]
     (count foo)))
 
+(core/with-clojure-version->= {:major 1 :minor 9}
+  (defn fn-with-let-with-namespaced-keyword-destructuring []
+    ;; https://github.com/clojure-emacs/refactor-nrepl/issues/289
+    (let [{::str/keys [foo bar]} (hash-map)]
+      [:bar :foo]
+      (count foo))))
+
 ;; This was causing both find-local-symbol and find-macros to blow up, for
 ;; different reasons
 ::str/bar