Introduce wrap-ignore-errors

Fixes #291

CHANGELOG.md

@@ -2,6 +2,18 @@
 
 ## Unreleased
 
+#### Changes
+* The `:ignore-errors` option will be honored in more places, making refactor-nrepl more robust in face of files not particularly meant to be part of the AST corpus
+  * Examples: WIP files, scripts.
+  * Closes https://github.com/clojure-emacs/refactor-nrepl/issues/291
+* Upgrade Orchard
+  * Worth emphasizing: now the following options are available https://github.com/clojure-emacs/orchard/tree/v0.7.0#configuration-options
+  * They can make the refactor-nrepl experience simpler / more robust.
+* Reliabilty 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)
+
 ### 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.

eastwood.clj

@@ -0,0 +1,4 @@
+(disable-warning
+ {:linter                      :unused-ret-vals-in-try
+  :if-inside-macroexpansion-of #{'clojure.test/deftest
+                                 'clojure.core/assert}})

project.clj

@@ -66,12 +66,13 @@
                                           with-debug-bindings [[:inner 0]]
                                           merge-meta [[:inner 0]]
                                           try-if-let [[:block 1]]}}}]
-             :eastwood {:plugins         [[jonase/eastwood "0.7.0"]]
+             :eastwood {:plugins         [[jonase/eastwood "0.7.1"]]
                         :eastwood {;; vendored - shouldn't be tweaked for satisfying linters:
                                    :exclude-namespaces [refactor-nrepl.ns.slam.hound.regrow]
                                    ;; :implicit-dependencies would fail spuriously when the CI matrix runs for Clojure < 1.10,
                                    ;; because :implicit-dependencies can only work for a certain corner case starting from 1.10.
-                                   :exclude-linters [:implicit-dependencies]}}
+                                   :exclude-linters [:implicit-dependencies]
+                                   :config-files ["eastwood.clj"]}}
              :clj-kondo [:test
                          {:dependencies [[clj-kondo "2021.06.18"]]}]}
 

src/refactor_nrepl/analyzer.clj

@@ -132,7 +132,7 @@
                              "OK"))))))
 
 (defn warm-ast-cache []
-  (doseq [f (tracker/project-files-in-topo-order)]
+  (doseq [f (tracker/project-files-in-topo-order true)]
     (try
       (ns-ast (slurp f))
       (catch Throwable th

src/refactor_nrepl/find/find_macros.clj

@@ -81,7 +81,8 @@
 (defn- find-macro-definitions-in-project
   "Finds all macros that are defined in the project."
   [ignore-errors?]
-  (->> (core/find-in-project (some-fn core/cljc-file? core/clj-file?))
+  (->> (core/find-in-project (util/wrap-ignore-errors (some-fn core/cljc-file? core/clj-file?)
+                                                      ignore-errors?))
        (mapcat #(try
                   (get-macro-definitions-in-file-with-caching %)
                   (catch Exception e
@@ -215,7 +216,8 @@
   (when (fully-qualified-name? fully-qualified-name)
     (let [all-defs (find-macro-definitions-in-project ignore-errors?)
           macro-def (first (filter #(= (:name %) fully-qualified-name) all-defs))
-          tracker (tracker/build-tracker)
+          tracker (tracker/build-tracker (util/wrap-ignore-errors tracker/default-predicate
+                                                                  ignore-errors?))
           origin-ns (symbol (core/prefix fully-qualified-name))
           dependents (tracker/get-dependents tracker origin-ns)]
       (some->> macro-def

src/refactor_nrepl/find/find_symbol.clj

@@ -135,20 +135,21 @@
         fully-qualified-name (if (= namespace "clojure.core")
                                var-name
                                (str/join "/" [namespace var-name]))
-        referred-syms (libspecs/referred-syms-by-file&fullname)]
+        referred-syms (libspecs/referred-syms-by-file&fullname ignore-errors)]
     (->> (core/dirs-on-classpath)
-         (mapcat (partial core/find-in-dir (every-pred (some-fn core/clj-file? core/cljc-file?)
-                                                       (fn [f]
-                                                         (try
-                                                           (let [n (some-> f
-                                                                           core/read-ns-form
-                                                                           parse/name-from-ns-decl)]
-                                                             (if-not n
-                                                               false
-                                                               (not (self-referential? n))))
-                                                           (catch Exception e
-                                                             (util/maybe-log-exception e)
-                                                             false))))))
+         (mapcat (partial core/find-in-dir (util/wrap-ignore-errors (every-pred (some-fn core/clj-file? core/cljc-file?)
+                                                                                (fn [f]
+                                                                                  (try
+                                                                                    (let [n (some-> f
+                                                                                                    core/read-ns-form
+                                                                                                    parse/name-from-ns-decl)]
+                                                                                      (if-not n
+                                                                                        false
+                                                                                        (not (self-referential? n))))
+                                                                                    (catch Exception e
+                                                                                      (util/maybe-log-exception e)
+                                                                                      false))))
+                                                                    ignore-errors)))
          (mapcat (partial find-symbol-in-file fully-qualified-name ignore-errors referred-syms)))))
 
 (defn- get&read-enclosing-sexps

src/refactor_nrepl/ns/libspecs.clj

@@ -1,6 +1,7 @@
 (ns refactor-nrepl.ns.libspecs
   (:require [refactor-nrepl.core :as core]
-            [refactor-nrepl.ns.ns-parser :as ns-parser])
+            [refactor-nrepl.ns.ns-parser :as ns-parser]
+            [refactor-nrepl.util :as util])
   (:import [java.io File]))
 
 ;; The structure here is {path {lang [timestamp value]}}
@@ -46,13 +47,18 @@
 
   {:clj {util com.acme.util str clojure.string
    :cljs {gstr goog.str}}}"
-  []
-  {:clj (->> (core/find-in-project (some-fn core/clj-file? core/cljc-file?))
-             (map (partial get-libspec-from-file-with-caching :clj))
-             aliases-by-frequencies)
-   :cljs (->> (core/find-in-project (some-fn core/cljs-file? core/cljc-file?))
-              (map (partial get-libspec-from-file-with-caching :cljs))
-              aliases-by-frequencies)})
+  ([]
+   (namespace-aliases false))
+
+  ([ignore-errors?]
+   {:clj  (->> (core/find-in-project (util/wrap-ignore-errors (some-fn core/clj-file? core/cljc-file?)
+                                                              ignore-errors?))
+               (map (partial get-libspec-from-file-with-caching :clj))
+               aliases-by-frequencies)
+    :cljs (->> (core/find-in-project (util/wrap-ignore-errors (some-fn core/cljs-file? core/cljc-file?)
+                                                              ignore-errors?))
+               (map (partial get-libspec-from-file-with-caching :cljs))
+               aliases-by-frequencies)}))
 
 (defn- unwrap-refer
   [file {:keys [ns refer]}]
@@ -75,10 +81,15 @@
    Example:
    {:clj  {\"/home/someuser/projects/some.clj\" [\"example.com/foobar\" foobar]}
     :cljs}"
-  []
-  {:clj (->> (core/find-in-project (some-fn core/clj-file? core/cljc-file?))
-             (map (juxt identity (partial get-libspec-from-file-with-caching :clj)))
-             sym-by-file&fullname)
-   :cljs (->> (core/find-in-project (some-fn core/cljs-file? core/cljc-file?))
-              (map (juxt identity (partial get-libspec-from-file-with-caching :cljs)))
-              sym-by-file&fullname)})
+  ([]
+   (referred-syms-by-file&fullname false))
+
+  ([ignore-errors?]
+   {:clj  (->> (core/find-in-project (util/wrap-ignore-errors (some-fn core/clj-file? core/cljc-file?)
+                                                              ignore-errors?))
+               (map (juxt identity (partial get-libspec-from-file-with-caching :clj)))
+               sym-by-file&fullname)
+    :cljs (->> (core/find-in-project (util/wrap-ignore-errors (some-fn core/cljs-file? core/cljc-file?)
+                                                              ignore-errors?))
+               (map (juxt identity (partial get-libspec-from-file-with-caching :cljs)))
+               sym-by-file&fullname)}))

src/refactor_nrepl/ns/tracker.clj

@@ -7,6 +7,7 @@
              [repl :refer [refresh-dirs]]
              [track :as tracker]]
             [refactor-nrepl.core :as core]
+            [refactor-nrepl.util :as util]
             [refactor-nrepl.ns.ns-parser :as ns-parser])
   (:import [java.io File]))
 
@@ -36,13 +37,16 @@
       ;; corner case - use the mranderson-ized refresh-dirs (for supporting this project's test suite):
       refresh-dirs))
 
+(def default-predicate (every-pred core/source-file?
+                                   safe-for-clojure-tools-namespace?))
+
 (defn build-tracker
   "Build a tracker for the project.
 
   If file-predicate is provided, use that instead of `core/source-file?`"
   ([]
-   (build-tracker #(and (core/source-file? %)
-                        (safe-for-clojure-tools-namespace? %))))
+   (build-tracker default-predicate))
+
   ([file-predicate]
    (file/add-files (tracker/tracker) (core/find-in-project file-predicate))))
 
@@ -74,11 +78,16 @@
       (boolean (some #(str/starts-with? % file-as-absolute-paths)
                      refresh-dirs-as-absolute-paths)))))
 
-(defn project-files-in-topo-order []
-  (let [tracker (build-tracker (every-pred (partial in-refresh-dirs? (user-refresh-dirs))
-                                           core/clj-file?))
-        nses (dep/topo-sort (:clojure.tools.namespace.track/deps tracker))
-        filemap (:clojure.tools.namespace.file/filemap tracker)
-        ns2file (zipmap (vals filemap) (keys filemap))]
-    (->> (map ns2file nses)
-         (remove nil?))))
+(defn project-files-in-topo-order
+  ([]
+   (project-files-in-topo-order false))
+
+  ([ignore-errors?]
+   (let [tracker (build-tracker (util/wrap-ignore-errors (every-pred (partial in-refresh-dirs? (user-refresh-dirs))
+                                                                     core/clj-file?)
+                                                         ignore-errors?))
+         nses (dep/topo-sort (:clojure.tools.namespace.track/deps tracker))
+         filemap (:clojure.tools.namespace.file/filemap tracker)
+         ns2file (zipmap (vals filemap) (keys filemap))]
+     (->> (map ns2file nses)
+          (remove nil?)))))

src/refactor_nrepl/rename_file_or_dir.clj

@@ -197,7 +197,8 @@
   [old-path new-path]
   (let [old-ns (core/ns-from-string (slurp old-path))
         new-ns (path->ns new-path)
-        tracker (tracker/build-tracker)
+        tracker (tracker/build-tracker (util/wrap-ignore-errors tracker/default-predicate
+                                                                :ignore-errors))
         dependents (tracker/get-dependents tracker old-ns)
         new-dependents (atom {})]
     (doseq [^File f dependents]

src/refactor_nrepl/util.clj

@@ -72,3 +72,14 @@
 (defn maybe-log-exception [^Throwable e]
   (when (System/getProperty "refactor-nrepl.internal.log-exceptions")
     (.printStackTrace e)))
+
+(defn wrap-ignore-errors [f ignore-errors?]
+  (if-not ignore-errors?
+    f
+    (fn [x]
+      (try
+        (f x)
+        (catch Exception e
+          (maybe-log-exception e)
+          ;; return false, because `wrap-ignore-errors` is oriented for predicate usage:
+          false)))))

test-resources/readable_file_incorrect_aliases.clj

@@ -0,0 +1,2 @@
+;; An incorrect alias usage (because `a` is not declared in the `ns` form):
+::a/foo

test-resources/readable_file_incorrect_data_readers.clj

@@ -0,0 +1,2 @@
+;; An incorrect data readers usage (because there's no such data reader declared anywhere):
+#totally.made.up/foo []

test-resources/unreadable_file.clj

@@ -0,0 +1 @@
+(ns unreadable-file

test/refactor_nrepl/extract_definition_test.clj

@@ -1,16 +1,18 @@
 (ns refactor-nrepl.extract-definition-test
   (:require [clojure.java.io :as io]
             [clojure.test :refer :all]
-            [refactor-nrepl.extract-definition :refer :all]))
+            [refactor-nrepl.extract-definition :as sut]
+            [refactor-nrepl.unreadable-files :refer [ignore-errors-str]]))
 
 (defn- -extract-definition
   [name line col]
-  (get-in (extract-definition
+  (get-in (sut/extract-definition
            {:file (.getAbsolutePath (io/file "test-resources/extract_definition.clj"))
             :ns "extract-definition"
             :line line
             :column col
             :name name
+            :ignore-errors ignore-errors-str
             :dir "test-resources"})
           [:definition :definition]))
 
@@ -72,11 +74,12 @@
          "(+ 11 17)")))
 
 (deftest returns-meta-data
-  (let [res (extract-definition
+  (let [res (sut/extract-definition
              {:file (.getAbsolutePath (io/file "test-resources/extract_definition.clj"))
               :ns "extract-definition"
               :line 44
               :column 14
+              :ignore-errors ignore-errors-str
               :name "if-let-bound"
               :dir "."})]
     (is (= (count (:occurrences res)) 1))

test/refactor_nrepl/find/find_macros_test.clj

@@ -1,12 +1,13 @@
 (ns refactor-nrepl.find.find-macros-test
   (:require [clojure.test :refer :all]
-            [refactor-nrepl.find.find-macros :refer [find-macro]]))
+            [refactor-nrepl.find.find-macros :refer [find-macro]]
+            [refactor-nrepl.unreadable-files :refer [ignore-errors?]]))
 
 (defn- found? [regexp occurrences]
   (first (filter #(re-find regexp (:match %)) occurrences)))
 
 (deftest find-macro-test
-  (let [occurrences (find-macro "com.example.macro-def/my-macro" false)
+  (let [occurrences (find-macro "com.example.macro-def/my-macro" ignore-errors?)
         {:keys [line-beg col-beg ^String file match]}
         (first (filter #(.contains ^String (:match %) "defmacro") occurrences))]
     (testing "finds the macro definition"
@@ -29,27 +30,27 @@
       (is (.endsWith file "macro_def.clj")))))
 
 (deftest find-regular-symbol-test
-  (is (nil? (find-macro "sym" false))))
+  (is (nil? (find-macro "sym" ignore-errors?))))
 
 (deftest find-fully-qualified-random-name
-  (is (nil? (find-macro "asdf" false))))
+  (is (nil? (find-macro "asdf" ignore-errors?))))
 
 (deftest find-fully-qualified-fn
-  (is (nil? (find-macro "refactor-nrepl.find.find-macros/find-macro" false))))
+  (is (nil? (find-macro "refactor-nrepl.find.find-macros/find-macro" ignore-errors?))))
 
 (deftest finds-macro-defined-in-cljc-file
   (is (found? #"defmacro cljc-macro"
-              (find-macro "com.example.macro-def-cljc/cljc-macro" false))))
+              (find-macro "com.example.macro-def-cljc/cljc-macro" ignore-errors?))))
 
 (deftest finds-macro-defined-in-cljc-file-and-used-in-clj-file
   (is (found? #"(com.example.macro-def-cljc/cljc-macro :fully-qualified)"
-              (find-macro "com.example.macro-def-cljc/cljc-macro" false))))
+              (find-macro "com.example.macro-def-cljc/cljc-macro" ignore-errors?))))
 
 (deftest macro-definitions-are-cached
-  (find-macro "com.example.macro-def/my-macro" false)
+  (find-macro "com.example.macro-def/my-macro" ignore-errors?)
   (with-redefs [refactor-nrepl.find.find-macros/put-cached-macro-definitions
                 (fn [& _] (throw (ex-info "Cache miss!" {})))]
-    (is (found? #"defmacro my-macro" (find-macro "com.example.macro-def/my-macro" false))))
+    (is (found? #"defmacro my-macro" (find-macro "com.example.macro-def/my-macro" ignore-errors?))))
   (reset! @#'refactor-nrepl.find.find-macros/macro-defs-cache {})
   (with-redefs [refactor-nrepl.find.find-macros/put-cached-macro-definitions
                 (fn [& _] (throw (Exception. "Expected!")))]

test/refactor_nrepl/ns/namespace_aliases_test.clj

@@ -2,10 +2,11 @@
   (:require [clojure.test :refer [deftest is]]
             [refactor-nrepl.core :as core]
             [refactor-nrepl.ns.libspecs :as sut]
-            [refactor-nrepl.util :as util]))
+            [refactor-nrepl.util :as util]
+            [refactor-nrepl.unreadable-files :refer [ignore-errors?]]))
 
 (defn finds [selector alias libspec]
-  (let [aliases (selector (sut/namespace-aliases))]
+  (let [aliases (selector (sut/namespace-aliases ignore-errors?))]
     (some (fn [[k vs]]
             (and (= k alias)
                  (some #{libspec} vs)))
@@ -24,18 +25,18 @@
   (is (finds :cljs 'gstr 'goog.string)))
 
 (deftest sorts-by-frequencies
-  (let [aliases (:clj (sut/namespace-aliases))
+  (let [aliases (:clj (sut/namespace-aliases ignore-errors?))
         _ (core/ns-form-from-string "(ns foo)")
         utils (get (util/filter-map #(= (first %) 'util) aliases) 'util)]
     (is (= (first utils) 'refactor-nrepl.util))))
 
 (deftest libspecs-are-cached
-  (sut/namespace-aliases)
+  (sut/namespace-aliases ignore-errors?)
   (with-redefs [refactor-nrepl.ns.libspecs/put-cached-libspec
                 (fn [& _] (throw (ex-info "Cache miss!" {})))]
-    (is (sut/namespace-aliases)))
+    (is (sut/namespace-aliases ignore-errors?)))
   (reset! @#'sut/cache {})
   (with-redefs [refactor-nrepl.ns.libspecs/put-cached-libspec
                 (fn [& _] (throw (Exception. "Expected!")))]
     (is (thrown-with-msg? Exception #"Expected!"
-                          (sut/namespace-aliases)))))
+                          (sut/namespace-aliases false)))))

test/refactor_nrepl/ns/resolve_missing_caching_test.clj

@@ -9,7 +9,7 @@
 (use-fixtures :each session-fixture)
 
 (defn message [arg-map]
-  (let [{:keys [error] :as response}
+  (let [{:keys [^String error] :as response}
         (refactor-nrepl.ns.resolve-missing-test/message arg-map)]
     (when error
       (throw (RuntimeException. error)))

test/refactor_nrepl/ns/resolve_missing_test.clj

@@ -69,7 +69,7 @@
 
 (t/deftest resolve-missing-test
   (t/testing "Finds functions is regular namespaces"
-    (let [{:keys [error] :as response} (message {:op :resolve-missing :symbol 'print-doc})
+    (let [{:keys [^String error] :as response} (message {:op :resolve-missing :symbol 'print-doc})
           {:keys [name type]} (first (edn/read-string (:candidates response)))]
       (when error
         (println error)