Always perform a require prior to analyzing a ns (#321)

Part of #245

Author: vemv

CHANGELOG.md

@@ -11,7 +11,7 @@
 * Reliability improvement: try using `require` prior to `find-ns`
   * This increases the chances that a namespace will be found, which in turns makes refactor-nrepl more complete/accurate.
 * Replace Cheshire with `clojure.data.json`
-* Build ASTs more robustly (by using locks and ruling out certain namespaces like refactor-nrepl itself)
+* Build ASTs more robustly (by using locks, `require`, and ruling out certain namespaces like refactor-nrepl itself)
 * Honor internal `future-cancel` calls, improving overall responsiveness and stability.
 
 ### Bugs fixed

project.clj

@@ -48,6 +48,7 @@
              :test {:dependencies [[print-foo "1.0.2"]]}
              :dev {:global-vars {*warn-on-reflection* true}
                    :dependencies [[org.clojure/clojurescript "1.10.520"]
+                                  [org.clojure/core.async "1.3.618" :exclusions [org.clojure/clojure org.clojure/tools.reader]]
                                   [cider/piggieback "0.5.2"]
                                   [commons-io/commons-io "2.8.0"]]
                    :repl-options {:nrepl-middleware [cider.piggieback/wrap-cljs-repl]}

src/refactor_nrepl/analyzer.clj

@@ -77,7 +77,13 @@
              (not (util/self-referential? ns)))
     ;; Use `locking`, because AST analysis can perform arbitrary evaluation.
     ;; Parallel analysis is not safe, especially as it can perform `require` calls.
-    (locking core/require-lock
+    (locking core/require-lock ;; for both `require` and `aj/analyze-ns`
+
+      ;; Performing this `require` makes it more likely that t.ana will succeed.
+      ;; I believe it's because `require` will also require other namespaces recursively.
+      ;; t.ana does so in theory as well, but it's slightly more rigid,
+      ;; and/or does not always do the same exact thing the Clojure compiler would.
+      (require ns)
       (let [opts {:passes-opts
                   {:validate/unresolvable-symbol-handler shadow-unresolvable-symbol-handler
                    :validate/throw-on-arity-mismatch     false

test-resources/core_async_usage.clj

@@ -0,0 +1,4 @@
+(ns core-async-usage
+  "Analyzing this ns used to be problematic, see git.io/Jcd7W"
+  (:require
+   [clojure.core.async :refer [go-loop]]))

test/refactor_nrepl/analyzer_test.clj

@@ -0,0 +1,10 @@
+(ns refactor-nrepl.analyzer-test
+  (:require
+   [clojure.java.io :as io]
+   [refactor-nrepl.analyzer :as sut]
+   [clojure.test :refer [deftest is]]))
+
+(deftest ns-ast-test
+  (doseq [f ["core_async_usage.clj"]
+          :let [c (-> f io/resource slurp)]]
+    (is (some? (sut/ns-ast c)))))