Strengthen resolve-missing

CHANGELOG.md

@@ -20,6 +20,7 @@
 
 ### Bugs fixed
 
+* [#335](https://github.com/clojure-emacs/refactor-nrepl/issues/335): Strengthen `resolve-missing` against various edge cases.
 * [#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.

project.clj

@@ -67,9 +67,7 @@
                                           merge-meta [[:inner 0]]
                                           try-if-let [[:block 1]]}}}]
              :eastwood {:plugins         [[jonase/eastwood "0.9.9"]]
-                        :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,
+                        :eastwood {;; :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]
                                    :add-linters [:performance :boxed-math]

src/refactor_nrepl/ns/slam/hound/search.clj → src/refactor_nrepl/ns/class_search.clj

@@ -1,39 +1,36 @@
 ;;;; Copied from slamhound 1.5.5
 ;;;; Copyright © 2011-2012 Phil Hagelberg and contributors
 ;;;; Distributed under the Eclipse Public License, the same as Clojure.
-(ns refactor-nrepl.ns.slam.hound.search
-  "Search the classpath for vars and classes."
-  (:require [orchard.java.classpath :as cp]
-            [clojure.java.io :refer [file]]
-            [clojure.string :as string])
+(ns refactor-nrepl.ns.class-search
+  "Search the classpath for classes.
+
+  Formerly known as `refactor-nrepl.ns.slam.hound.search`."
+  (:require
+   [clojure.java.io :refer [file]]
+   [clojure.string :as string]
+   [refactor-nrepl.util :as util])
   (:import
-   [java.io File FilenameFilter]
-   [java.util.jar JarFile JarEntry]
-   java.util.regex.Pattern
-   java.util.StringTokenizer))
-
-;;; Mostly taken from leiningen.util.ns and swank.util.class-browse.
-
-;; TODO: replace with bultitude? but that doesn't do classes
-
-;;; Clojure namespaces
+   (java.io File FilenameFilter)
+   (java.util StringTokenizer)
+   (java.util.jar JarEntry JarFile)
+   (java.util.regex Pattern)))
 
 (defn jar? [^File f]
   (and (.isFile f) (.endsWith (.getName f) ".jar")))
 
 (defn class-file? [^String path]
   (.endsWith path ".class"))
 
-(defn clojure-fn-file? [f]
-  (re-find #"\$.*__\d+\.class" f))
+(defn clojure-fn-file? [^String file]
+  ;; originally this logic was: (re-find #"\$.*__\d+\.class" f)
+  ;; however that doesn't cover e.g. "clojure/spec/alpha$double_in.class"
+  ;; so we mimic the logic that e.g. Compliment has:
+  (or (.contains file "__")
+      (.contains file "$")))
 
 (defn clojure-ns-file? [^String path]
   (.endsWith path "__init.class"))
 
-;;; Java classes
-
-;; could probably be simplified
-
 (def jar-filter
   (proxy [FilenameFilter] []
     (accept [d n] (jar? (file n)))))
@@ -47,24 +44,22 @@
       (.. f getParentFile (list jar-filter))
       [f])))
 
-(defn class-or-ns-name
-  "Returns the Java class or Clojure namespace name for a class relative path."
+(def ^String resource-separator
+  "Please do not use File/separator see e.g. https://git.io/Jzig3"
+  "/")
+
+(defn class-name
   [^String path]
-  (-> (if (clojure-ns-file? path)
-        (-> path (.replace "__init.class" "") (.replace "_" "-"))
-        (.replace path ".class" ""))
-      (.replace File/separator ".")))
+  (-> path
+      (.replace ".class" "")
+      (.replace resource-separator ".")))
 
 (defmulti path-class-files
-  "Returns a list of classes found on the specified path location
-  (jar or directory), each comprised of a map with the following keys:
-    :name  Java class or Clojure namespace name
-    :loc   Classpath entry (directory or jar) on which the class is located
-    :file  Path of the class file, relative to :loc"
-  (fn [^File f _]
-    (cond (.isDirectory f) :dir
-          (jar? f) :jar
-          (class-file? (.getName f)) :class)))
+  (fn [^File f _loc]
+    (cond
+      (.isDirectory f) :dir
+      (jar? f) :jar
+      (class-file? (.getName f)) :class)))
 
 (defmethod path-class-files :default [& _] [])
 
@@ -77,9 +72,13 @@
             (comp
              (map #(.getName ^JarEntry %))
              (filter class-file?)
-             (map class-or-ns-name))
+             (remove clojure-fn-file?)
+             (map class-name))
             (enumeration-seq (.entries (JarFile. f))))
-      (catch Exception _e []))))          ; fail gracefully if jar is unreadable
+      (catch Exception e
+        (util/maybe-log-exception e)
+        ;; fail gracefully if jar is unreadable:
+        []))))
 
 (defmethod path-class-files :dir
   ;; Dispatch directories and files (excluding jars) recursively.
@@ -93,46 +92,45 @@
   ;; Build class info using file path relative to parent classpath entry
   ;; location. Make sure it decends; a class can't be on classpath directly.
   [^File f ^File loc]
-  (let [fp (str f), lp (str loc)
-        loc-pattern (re-pattern (Pattern/quote (str "^" loc)))]
-    (if (re-find loc-pattern fp)                 ; must be descendent of loc
+  (let [fp (str f)
+        lp (str loc)]
+    (if (re-find (re-pattern (Pattern/quote (str "^" loc))) fp) ; must be descendent of loc
       (let [fpr (.substring fp (inc (count lp)))]
-        [(class-or-ns-name fpr)])
+        [(class-name fpr)])
       [])))
 
 (defn path-entries-seq
   "Split a string on the 'path separator', i.e. ':'. Used for splitting multiple
   classpath entries."
   [path-str]
-  (enumeration-seq
-   (StringTokenizer. path-str File/pathSeparator)))
-
-(defn all-classpath-entries []
-  (mapcat cp/classpath-seq (cp/classpath)))
+  (-> path-str
+      (StringTokenizer. File/pathSeparator)
+      enumeration-seq))
 
 (defn- get-available-classes []
   (into ()
-        (comp (mapcat path-entries-seq)
-              (mapcat expand-wildcard)
-              (mapcat #(path-class-files % %))
+        (comp (mapcat expand-wildcard)
+              (mapcat (fn [file]
+                        (path-class-files file file)))
               (remove clojure-fn-file?)
               (distinct)
               (map symbol))
-        (all-classpath-entries)))
+        ;; We use `(System/getProperty "java.class.path")` (at least for the time being) because
+        ;; This code was originally written to handle that string, not a list
+        ;; (this code was broken for a while as `orchard.java.classpath` was being incompatibly used instead)
+        (path-entries-seq (System/getProperty "java.class.path"))))
 
 (def available-classes
   (delay (get-available-classes)))
 
-(defn- get-available-classes-by-last-segment
-  []
-  (delay
-   (group-by #(symbol (peek (string/split (str %) #"\."))) @available-classes)))
+(defn- get-available-classes-by-last-segment []
+  (group-by #(symbol (peek (string/split (str %) #"\."))) @available-classes))
 
 (def available-classes-by-last-segment
   (delay (get-available-classes-by-last-segment)))
 
 (defn reset
   "Reset the cache of classes"
   []
-  (alter-var-root #'available-classes (constantly (get-available-classes)))
-  (alter-var-root #'available-classes-by-last-segment (constantly (get-available-classes-by-last-segment))))
+  (alter-var-root #'available-classes (constantly (delay (get-available-classes))))
+  (alter-var-root #'available-classes-by-last-segment (constantly (delay (get-available-classes-by-last-segment)))))

src/refactor_nrepl/ns/imports_and_refers_analysis.clj

@@ -0,0 +1,85 @@
+;;;; Copied from slamhound 1.5.5
+;;;; Copyright © 2011-2012 Phil Hagelberg and contributors
+;;;; Distributed under the Eclipse Public License, the same as Clojure.
+(ns refactor-nrepl.ns.imports-and-refers-analysis
+  "Formerly known as `refactor-nrepl.ns.slam.hound.regrow`."
+  (:require
+   [nrepl.middleware.interruptible-eval :refer [*msg*]]
+   [refactor-nrepl.ns.class-search :as class-search]))
+
+(def ^:dynamic *cache* (atom {}))
+(def ^:dynamic *dirty-ns* (atom #{}))
+
+(defn wrap-clojure-repl [f]
+  (fn [& args]
+    (when-let [ns (some-> *msg* :ns symbol find-ns)]
+      (swap! *dirty-ns* conj ns))
+    (apply f args)))
+
+;; XXX remove this if possible, we shouldn't be this sort of stuff.
+(alter-var-root #'clojure.main/repl wrap-clojure-repl)
+
+(defn cache-with-dirty-tracking
+  "The function to be cached, f, should have two signatures. A zero-operand
+  signature which computes the result for all namespaces, and a two-operand
+  version which takes the previously computed result and a list of dirty
+  namespaces, and returns an updated result."
+  [k f]
+  (if *cache*
+    (if-let [cached (get @*cache* k)]
+      (if-let [dirty (seq @*dirty-ns*)]
+        (k (swap! *cache* assoc k (f cached dirty)))
+        cached)
+      (k (swap! *cache* assoc k (f))))
+    (f)))
+
+(defn clear-cache! []
+  (when *cache*
+    (reset! *cache* {})
+    (reset! *dirty-ns* #{})))
+
+(defn- all-ns-imports*
+  ([]
+   (all-ns-imports* {} (all-ns)))
+  ([init namespaces]
+   (reduce (fn [imports ns]
+             (assoc imports ns (ns-imports ns)))
+           init namespaces)))
+
+(defn- all-ns-imports []
+  (cache-with-dirty-tracking :all-ns-imports all-ns-imports*))
+
+(defn- symbols->ns-syms*
+  ([]
+   (symbols->ns-syms* {} (all-ns)))
+  ([init namespaces]
+   (reduce
+    (fn [m ns] (let [ns-sym (ns-name ns)]
+                 (reduce
+                  (fn [m k]
+                    (assoc m k (conj (or (m k) #{}) ns-sym)))
+                  m (keys (ns-publics ns)))))
+    init namespaces)))
+
+(defn- symbols->ns-syms []
+  (cache-with-dirty-tracking :symbols->ns-syms symbols->ns-syms*))
+
+(defn- ns-import-candidates
+  "Search (all-ns) for imports that match missing-sym, returning a set of
+  class symbols. This is slower than scanning through the list of static
+  package names, but will successfully find dynamically created classes such
+  as those created by deftype, defrecord, and definterface."
+  [missing-sym]
+  (reduce (fn [s imports]
+            (if-let [^Class cls (get imports missing-sym)]
+              (conj s (symbol (.getCanonicalName cls)))
+              s))
+          #{} (vals (all-ns-imports))))
+
+(defn candidates
+  "Return a set of class or ns symbols that match the given constraints."
+  [type missing _body _old-ns-map]
+  (case type
+    :import (into (ns-import-candidates missing)
+                  (get @class-search/available-classes-by-last-segment missing))
+    :refer (get (symbols->ns-syms) missing)))

src/refactor_nrepl/ns/resolve_missing.clj

@@ -6,14 +6,14 @@
             [clojure.string :as str]
             [refactor-nrepl.core :refer [prefix suffix]]
             [refactor-nrepl.util :refer [self-referential?]]
-            [refactor-nrepl.ns.slam.hound.regrow :as slamhound]))
+            [refactor-nrepl.ns.imports-and-refers-analysis :as imports-and-refers-analysis]))
 
 (defn- candidates [sym]
   (reduce into
           [(when-let [p (prefix sym)]
-             (slamhound/candidates :import (symbol p) [] {}))
-           (slamhound/candidates :import (symbol (suffix sym)) [] {})
-           (slamhound/candidates :refer (symbol (suffix sym)) [] {})]))
+             (imports-and-refers-analysis/candidates :import (symbol p) [] {}))
+           (imports-and-refers-analysis/candidates :import (symbol (suffix sym)) [] {})
+           (imports-and-refers-analysis/candidates :refer (symbol (suffix sym)) [] {})]))
 
 (defn- get-type [sym]
   (let [info (info 'user sym)]