POC for CLJS support for find-locals

as outlined in #195, see specially #195 (comment) Take aways: - one integration test is duplicated for cljs (together with the source file) - `cljs.analyzer` used directly instead of `jvm.tools.analyzer` -- latter errored with latest cljs, did not investigate further - workarounds needed for - no `end-line`, `end-column` info in CLJS AST (also see https://dev.clojure.org/jira/browse/CLJS-2051) - no `:raw-forms` in cljs AST containing the stages of macro expansion including the original form - :op = `:binding` nodes in CLJS ASTs seems to be missing `:children` entry so the AST can not be walked properly
clojure-emacs · Apr 13, 2019 · ffcbf70 · ffcbf70
1 parent c252bd4
commit ffcbf70
Show file tree

Hide file tree

Showing 5 changed files with 222 additions and 26 deletions.
diff --git a/project.clj b/project.clj
@@ -38,10 +38,12 @@
                                    [org.clojure/clojurescript "1.10.520"]]}
              :dev {:plugins [[jonase/eastwood "0.2.0"]]
                    :global-vars {*warn-on-reflection* true}
-                   :dependencies [[org.clojure/clojurescript "1.9.946"]
+                   :dependencies [[org.clojure/clojurescript "1.10.520"]
+                                  [org.clojure/clojure "1.10.0"]
                                   [cider/piggieback "0.4.0"]
                                   [leiningen-core "2.9.0"]
-                                  [commons-io/commons-io "2.6"]]
+                                  [commons-io/commons-io "2.6"]
+                                  [javax.xml.bind/jaxb-api "2.3.1"]]
                    :repl-options {:nrepl-middleware [cider.piggieback/wrap-cljs-repl]}
                    :java-source-paths ["test/java"]
                    :resource-paths ["test/resources"

diff --git a/src/refactor_nrepl/analyzer.clj b/src/refactor_nrepl/analyzer.clj
@@ -8,20 +8,24 @@
             [clojure.tools.analyzer.jvm.utils :as ajutils]
             [clojure.tools.namespace.parse :refer [read-ns-decl]]
             [clojure.walk :as walk]
-            [refactor-nrepl
-             [config :as config]]
+            [refactor-nrepl.config :as config]
             [refactor-nrepl.ns.tracker :as tracker]
-            [clojure.string :as str])
+            [clojure.string :as str]
+            [cljs.analyzer :as cljs-ana]
+            [cljs.util :as cljs-util]
+            [cljs.compiler :as cljs-comp]
+            [cljs.env :as cljs-env]
+            [cider.nrepl.middleware.util.cljs :as cljs])
   (:import java.io.PushbackReader
            java.util.regex.Pattern))
 
 ;;; The structure here is {ns {content-hash ast}}
 (def ^:private ast-cache (atom {}))
 
 (defn get-alias [as v]
-  (cond as (first v)
+  (cond as                (first v)
         (= (first v) :as) (get-alias true (rest v))
-        :else (get-alias nil (rest v))))
+        :else             (get-alias nil (rest v))))
 
 (defn parse-ns
   "Returns tuples with the ns as the first element and
@@ -80,6 +84,64 @@
                 reader/*data-readers* *data-readers*]
         (assoc-in (aj/analyze-ns ns (aj/empty-env) opts) [0 :alias-info] aliases)))))
 
+(comment
+;; integration test: find-used-locals
+;; myast is the parsed ast of com.example.five and com.example.five-cljs respectively
+
+;; cljs
+(->> (nth myast 3) :init :methods first :body :ret :bindings first :children)
+;; returns nil, but ... :init :form) returns (trim p), strangely :init has the same children as in the clj case
+
+;; clj
+(->> (nth myast 3) :init :expr :methods first :body :bindings first :children)
+;; returns [:init] .. :init :form) returns (trim p)
+
+  )
+
+(defn- repair-binding-children
+  "Repairs cljs AST by adding `:children` entries to `:binding` AST nodes, see above comment tag."
+  []
+  (fn [env ast opts]
+    (if (= :let (:op ast))
+      (update
+       ast
+       :bindings
+       (fn [bindings]
+         (mapv #(assoc % :children [:init]) bindings)))
+      ast)))
+
+(defn cljs-analyze-ns
+  "Returns a sequence of abstract syntax trees for each form in
+  the namespace."
+  [ns]
+  (cljs-env/ensure
+    (let [f (cljs-util/ns->relpath ns)
+          res (if (re-find #"^file://" f) (java.net.URL. f) (io/resource f))]
+      (assert res (str "Can't find " f " in classpath"))
+      (binding [cljs-ana/*cljs-ns* 'cljs.user
+                cljs-ana/*cljs-file* (.getPath ^java.net.URL res)
+                cljs-ana/*passes* [cljs-ana/infer-type cljs-ana/check-invoke-arg-types cljs-ana/ns-side-effects (repair-binding-children)]]
+        (with-open [r (io/reader res)]
+          (let [env (cljs-ana/empty-env)
+                pbr (clojure.lang.LineNumberingPushbackReader. r)
+                eof (Object.)]
+            (loop [asts []
+                   r (read pbr false eof false)]
+              (let [env (assoc env :ns (cljs-ana/get-namespace cljs-ana/*cljs-ns*))]
+                (if-not (identical? eof r)
+                  (recur (conj asts (cljs-ana/analyze env r)) (read pbr false eof false))
+                  asts)))))))))
+
+(defn cljs-analyze-form [form]
+  (cljs-env/ensure
+   (binding [cljs-ana/*cljs-ns* 'cljs.user]
+     (cljs-ana/analyze (cljs-ana/empty-env) form))))
+
+(defn build-cljs-ast
+  [file-content]
+  (let [[ns aliases] (parse-ns file-content)]
+    (assoc-in (cljs-analyze-ns ns) [0 :alias-info] aliases)))
+
 (defn- cachable-ast [file-content]
   (let [[ns aliases] (parse-ns file-content)]
     (when ns
@@ -133,16 +195,16 @@
     ;; The node for ::an-ns-alias/foo, when it appeared as a toplevel form,
     ;; had nil as position info
     (and line end-line column end-column
-         (and (>= loc-line line)
-              (<= loc-line end-line)
-              (>= loc-column column)
-              (<= loc-column end-column)))))
+         (<= line loc-line end-line)
+         (<= column loc-column end-column))))
 
-(defn- normalize-anon-fn-params
+(defn normalize-anon-fn-params
   "replaces anon fn params in a read form"
   [form]
   (walk/postwalk
-   (fn [token] (if (re-matches #"p\d+__\d+#" (str token)) 'p token)) form))
+   (fn [token] (cond (re-matches #"p\d+__\d+#" (str token)) 'p
+                     (instance? java.util.regex.Pattern token) (str token)
+                     :default token)) form))
 
 (defn- read-when-sexp [form]
   (let [f-string (str form)]
@@ -155,13 +217,31 @@
   (binding [*read-eval* false]
     (let [sexp-sans-comments-and-meta (normalize-anon-fn-params (read-string sexp))
           pattern (re-pattern (Pattern/quote (str sexp-sans-comments-and-meta)))]
+      ;; (println "raw-forms" (:raw-forms node))
+      ;; (println "form "  (-> (:form node)
+      ;;                       read-when-sexp
+      ;;                       normalize-anon-fn-params))
       (if-let [forms (:raw-forms node)]
         (some #(re-find pattern %)
               (map (comp str normalize-anon-fn-params read-when-sexp) forms))
         (= sexp-sans-comments-and-meta (-> (:form node)
                                            read-when-sexp
                                            normalize-anon-fn-params))))))
 
+(defn node-for-sexp-cljs?
+  "Is NODE the ast node for SEXP for cljs?
+
+  As `:raw-forms` (stages of macro expansion, including the original form) is not available in cljs AST it does the comparison the other way around. Eg parses `sexp` with the cljs parser and compares that with the `:form` of the AST node."
+  [sexp node]
+  (binding [*read-eval* false]
+    (let [sexp-sans-comments-and-meta-form (:form (cljs-analyze-form (normalize-anon-fn-params (read-string sexp))))
+          node-form                        (-> (:form node)
+                                               read-when-sexp
+                                               normalize-anon-fn-params)]
+      ;; (println "sexp-sans-comments-and-meta" sexp-sans-comments-and-meta-form "types" (map type sexp-sans-comments-and-meta-form))
+      ;; (println "form "  node-form "types" (map type node-form))
+      (= sexp-sans-comments-and-meta-form node-form))))
+
 (defn top-level-form-index
   [line column ns-ast]
   (->> ns-ast
@@ -170,3 +250,27 @@
                                      (some (partial node-at-loc? line column)))))
        (filter #(second %))
        ffirst))
+
+(defn node-at-loc-cljs?
+  "Works around the fact that cljs AST nodes don't have end-line and end-column info in them. This cheat only works for top level forms because after a `clojure.tools.analyzer.ast/nodes` call we can't expect the nodes in the right order."
+  [^long loc-line ^long loc-column node next-node]
+  (let [{:keys [^long line ^long column]} (:env node)
+        env-next-node                     (:env next-node)
+        ^long end-column                  (:column env-next-node)
+        ^long end-line                    (:line env-next-node)]
+    ;; The node for ::an-ns-alias/foo, when it appeared as a toplevel form,
+    ;; had nil as position info
+    (and line end-line column end-column
+         (or (< line loc-line end-line)
+             (and (or (= line loc-line)
+                      (= end-line loc-line))
+                  (<= column loc-column end-column))))))
+
+(defn top-level-form-index-cljs
+  [line column ns-ast]
+  (loop [[top-level-ast & top-level-asts-rest] ns-ast
+         index 0]
+    (if (or (node-at-loc-cljs? line column top-level-ast (first top-level-asts-rest))
+            (not (first top-level-asts-rest)))
+      index
+      (recur top-level-asts-rest (inc index)))))
diff --git a/src/refactor_nrepl/find/find_locals.clj b/src/refactor_nrepl/find/find_locals.clj
@@ -1,25 +1,33 @@
 (ns refactor-nrepl.find.find-locals
   (:require [clojure.set :as set]
             [clojure.tools.analyzer.ast :refer [nodes]]
-            [refactor-nrepl
-             [analyzer :as ana]
-             [s-expressions :as sexp]
-             [core :as core]]))
+            [refactor-nrepl.analyzer :as ana]
+            [refactor-nrepl.core :as core]
+            [refactor-nrepl.s-expressions :as sexp]))
 
 (defn find-used-locals  [{:keys [file ^long line ^long column]}]
   {:pre [(number? line)
          (number? column)
          (not-empty file)]}
-  (core/throw-unless-clj-file file)
+  ;(core/throw-unless-clj-file file)
   (let [content (slurp file)
-        ast (ana/ns-ast content)
+        cljs? (core/cljs-file? file)
+        ;; fork for cljs using `cljs.analyzer` directly
+        ast (if cljs? (ana/build-cljs-ast content) (ana/ns-ast content))
         sexp (sexp/get-enclosing-sexp content (dec line) (dec column))
+        ;; work around for cljs ASTs not having end-line and end-column info
+        top-level-form-index-fn (if cljs? ana/top-level-form-index-cljs ana/top-level-form-index)
+        ;; work around the fact that cljs ASTs don't have raw-forms in them. the original form before macro expansion can not be reproduced
+        node-for-sexp-fn (if cljs? ana/node-for-sexp-cljs? ana/node-for-sexp?)
         selected-sexp-node (->> ast
-                                (ana/top-level-form-index line column)
+                                (top-level-form-index-fn line column)
                                 (nth ast)
                                 nodes
-                                (filter (partial ana/node-at-loc? line column))
-                                (filter (partial ana/node-for-sexp? sexp))
+                                ((fn [nds]
+                                   (if cljs?
+                                     nds; can't use `node-at-loc-cljs?` after `nodes`
+                                      (filter (partial ana/node-at-loc? line column) nds))))
+                                (filter (partial node-for-sexp-fn sexp))
                                 last)
         sexp-locals (->> selected-sexp-node
                          nodes

diff --git a/test/refactor_nrepl/integration_tests.clj b/test/refactor_nrepl/integration_tests.clj
@@ -228,12 +228,35 @@
       (is (= (find-unbound :transport transport :file five-file :line 41 :column 4)
              '(x y z a b c))))))
 
-(deftest find-unbound-fails-on-cljs
+(deftest test-find-used-locals-cljs
   (with-testproject-on-classpath
-    (let [cljs-file (str test-project-dir "/tmp/src/com/example/file.cljs")
+    (let [five-file (str test-project-dir "/src/com/example/five_cljs.cljs")
           transport (connect :port 7777)]
-      (is (:error (find-unbound :transport transport :file cljs-file
-                                :line 12 :column 6))))))
+      (is (= (find-unbound :transport transport :file five-file :line 12 :column 6)
+             '(s)))
+      ;; maybe fails because of a bug in `refactor-nrepl.s-expressions/get-enclosing-sexp`?! which is covered up by the fact that we can prefilter AST nodes with `nodes-at-loc` for clj but for cljs?!
+      ;; (is (= (find-unbound :transport transport :file five-file :line 13 :column 13)
+      ;;        '(s sep)))
+
+      (is (= (find-unbound :transport transport :file five-file :line 20 :column 16)
+             '(p)))
+      (is (= (find-unbound :transport transport :file five-file :line 27 :column 8)
+             '(sep strings)))
+
+      (is (= (find-unbound :transport transport :file five-file :line 34 :column 8)
+             '(name)))
+
+      (is (= (find-unbound :transport transport :file five-file :line 37 :column 5)
+             '(n)))
+      (is (= (find-unbound :transport transport :file five-file :line 41 :column 4)
+             '(x y z a b c))))))
+
+;; (deftest find-unbound-fails-on-cljs
+;;   (with-testproject-on-classpath
+;;     (let [cljs-file (str test-project-dir "/tmp/src/com/example/file.cljs")
+;;           transport (connect :port 7777)]
+;;       (is (:error (find-unbound :transport transport :file cljs-file
+;;                                 :line 12 :column 6))))))
 
 (deftest test-version
   (is (= (str (core/version))

diff --git a/test/resources/testproject/src/com/example/five_cljs.cljs b/test/resources/testproject/src/com/example/five_cljs.cljs
@@ -0,0 +1,59 @@
+(ns com.example.five-cljs
+  (:require [clojure.string :refer [join split blank? trim] :as str]))
+
+;;  remove parameters to run the tests
+(defn fn-with-unbounds [s sep]
+  (when-not (blank? s)
+    (-> s (split #" ")
+        (join sep)
+        trim)))
+
+(defn orig-fn [s]
+  (let [sep ";"]
+    (when-not (blank? s)
+      (-> s
+          (split #" ")
+          ((partial join sep))
+          trim))))
+
+(defn find-in-let [s p]
+  (let [z (trim p)]
+    (assoc {:s s
+            :p p
+            :z z} :k "foobar")))
+
+(defn threading-macro [strings]
+  (let [sep ","]
+    (->> strings
+         flatten
+         (join sep))))
+
+(defn repeated-sexp []
+  (map name [:a :b :c])
+  (let [name #(str "myname" %)]
+    (map name [:a :b :c])))
+
+(defn sexp-with-anon-fn [n]
+  (let [g 5]
+    (#(+ g %) n)))
+
+(defn many-params [x y z a b c]
+  (* x y z a b c))
+
+(defn fn-with-default-optmap
+  [{:keys [foo bar] :or {foo "foo"}}]
+  [:bar :foo]
+  (count foo))
+
+(defn fn-with-default-optmap-linebreak
+  [{:keys [foo
+           bar]
+    :or {foo
+         "foo"}}]
+  [:bar :foo]
+  (count foo))
+
+(defn fn-with-let-default-optmap []
+  (let [{:keys [foo bar] :or {foo "foo"}} (hash-map)]
+    [:bar :foo]
+    (count foo)))