From c05e692593b70f3efb475566b1bc53c1fd675ec9 Mon Sep 17 00:00:00 2001
From: vemv <vemv@users.noreply.github.com>
Date: Fri, 9 Jul 2021 16:08:28 +0200
Subject: [PATCH] Always perform a `require` prior to analyzing a ns (#321)

Part of https://github.com/clojure-emacs/refactor-nrepl/issues/245
---
 CHANGELOG.md                          |  2 +-
 project.clj                           |  1 +
 src/refactor_nrepl/analyzer.clj       |  8 +++++++-
 test-resources/core_async_usage.clj   |  4 ++++
 test/refactor_nrepl/analyzer_test.clj | 10 ++++++++++
 5 files changed, 23 insertions(+), 2 deletions(-)
 create mode 100644 test-resources/core_async_usage.clj
 create mode 100644 test/refactor_nrepl/analyzer_test.clj

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 9c0de116..cc4e9af5 100644
--- a/CHANGELOG.md
+++ b/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
diff --git a/project.clj b/project.clj
index 21f668c7..aced53d3 100644
--- a/project.clj
+++ b/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]}
diff --git a/src/refactor_nrepl/analyzer.clj b/src/refactor_nrepl/analyzer.clj
index b371143b..a3fcaf2f 100644
--- a/src/refactor_nrepl/analyzer.clj
+++ b/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
diff --git a/test-resources/core_async_usage.clj b/test-resources/core_async_usage.clj
new file mode 100644
index 00000000..01fe2413
--- /dev/null
+++ b/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]]))
diff --git a/test/refactor_nrepl/analyzer_test.clj b/test/refactor_nrepl/analyzer_test.clj
new file mode 100644
index 00000000..8cdbf522
--- /dev/null
+++ b/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)))))