diff --git a/CHANGELOG.md b/CHANGELOG.md index b2843d15..e778a51a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -15,6 +15,7 @@ * 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, `require`, and ruling out certain namespaces like refactor-nrepl itself) +* Improve `namespace-aliases` performance and make it return more accurate results. * Honor internal `future-cancel` calls, improving overall responsiveness and stability. ### Bugs fixed diff --git a/src/refactor_nrepl/core.clj b/src/refactor_nrepl/core.clj index 1569674c..8260e1a3 100644 --- a/src/refactor_nrepl/core.clj +++ b/src/refactor_nrepl/core.clj @@ -1,15 +1,17 @@ (ns refactor-nrepl.core - (:require [clojure.java.io :as io] - [clojure.string :as str] - [clojure.tools.namespace.parse :as parse] - [clojure.tools.reader.reader-types :as readers] - [orchard.java.classpath :as cp] - [orchard.misc :as misc] - [me.raynes.fs :as fs] - [refactor-nrepl.util :as util :refer [normalize-to-unix-path]] - [refactor-nrepl.s-expressions :as sexp] - [refactor-nrepl.config :as config]) - (:import [java.io File FileReader PushbackReader StringReader])) + (:require + [clojure.java.io :as io] + [clojure.string :as str] + [clojure.tools.namespace.parse :as parse] + [clojure.tools.reader.reader-types :as readers] + [me.raynes.fs :as fs] + [orchard.java.classpath :as cp] + [orchard.misc :as misc] + [refactor-nrepl.config :as config] + [refactor-nrepl.s-expressions :as sexp] + [refactor-nrepl.util :as util :refer [normalize-to-unix-path]]) + (:import + (java.io File FileReader PushbackReader StringReader))) (defn version [] (let [v (-> (or (io/resource "refactor-nrepl/refactor-nrepl/project.clj") @@ -57,6 +59,18 @@ (when (.isDirectory ^File f) f))) (remove (comp ignore-dir-on-classpath? str)))) +(defn source-dirs-on-classpath + "Like `#'dirs-on-classpath`, but restricted to dirs that look like + (interesting) source/test dirs." + [] + (->> (dirs-on-classpath) + (remove (fn [^File f] + (let [s (-> f .toString)] + (or (-> s (.contains "resources")) + (-> s (.contains "target")) + (-> s (.contains ".gitlibs")))))) + (remove util/dir-outside-root-dir?))) + (defn project-root "Return the project root directory. @@ -141,34 +155,32 @@ {:read-cond :allow :features #{dialect}}) (catch Exception _ nil)))))) -(defn- data-file? - "True of f is named like a clj file but represents data. - - E.g. true for data_readers.clj" - [path-or-file] - (let [path (.getPath (io/file path-or-file)) - data-files #{"data_readers.clj" "project.clj" "boot.clj"}] - (reduce (fn [acc data-file] (or acc (.endsWith path data-file))) - false - data-files))) +(defn cljc-extension? [^String path] + (.endsWith path ".cljc")) (defn cljc-file? [path-or-file] (let [path (.getPath (io/file path-or-file))] - (and (.endsWith path ".cljc") + (and (cljc-extension? path) (read-ns-form path)))) +(defn cljs-extension? [^String path] + (.endsWith path ".cljs")) + (defn cljs-file? [path-or-file] (let [path (.getPath (io/file path-or-file))] - (and (.endsWith path ".cljs") + (and (cljs-extension? path) (read-ns-form path)))) +(defn clj-extension? [^String path] + (.endsWith path ".clj")) + (defn clj-file? [path-or-file] (let [path (.getPath (io/file path-or-file))] - (and (not (data-file? path-or-file)) - (.endsWith path ".clj") + (and (not (util/data-file? path-or-file)) + (clj-extension? path) (read-ns-form path)))) (defn source-file? @@ -194,11 +206,29 @@ (defn find-in-project "Return the files in the project satisfying (pred ^File file)." - [pred] - (->> (dirs-on-classpath) - (pmap (partial find-in-dir pred)) - (apply concat) - distinct)) + ([pred] + (find-in-project pred (dirs-on-classpath))) + ([pred dirs] + (->> dirs + (pmap (partial find-in-dir pred)) + (apply concat) + distinct))) + +(defn source-files-with-clj-like-extension + "Finds files with .clj* extension in the project, without inspecting them. + + Meant as a particularly fast operation (as it doesn't slurp files)." + ([ignore-errors?] + (source-files-with-clj-like-extension ignore-errors? (source-dirs-on-classpath))) + ([ignore-errors? dirs] + (find-in-project (util/with-suppressed-errors + (comp (some-fn clj-extension? + cljc-extension? + cljs-extension?) + (fn [^File f] + (.getPath f))) + ignore-errors?) + dirs))) (defn throw-unless-clj-file [file-path] (when-not (re-matches #".+\.clj$" file-path) diff --git a/src/refactor_nrepl/ns/libspecs.clj b/src/refactor_nrepl/ns/libspecs.clj index 496a59be..00cd40c0 100644 --- a/src/refactor_nrepl/ns/libspecs.clj +++ b/src/refactor_nrepl/ns/libspecs.clj @@ -1,8 +1,10 @@ (ns refactor-nrepl.ns.libspecs - (:require [refactor-nrepl.core :as core] - [refactor-nrepl.ns.ns-parser :as ns-parser] - [refactor-nrepl.util :as util]) - (:import [java.io File])) + (:require + [refactor-nrepl.core :as core] + [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]}} ;; where lang is either :clj or :cljs @@ -43,23 +45,34 @@ (put-cached-libspec f lang))) (defn namespace-aliases - "Return a map of file type to a map of aliases to namespaces + "Returns a map of file type to a map of aliases to namespaces {:clj {util com.acme.util str clojure.string :cljs {gstr goog.str}}}" ([] (namespace-aliases false)) ([ignore-errors?] - {:clj (->> (core/find-in-project (util/with-suppressed-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/with-suppressed-errors - (some-fn core/cljs-file? core/cljc-file?) - ignore-errors?)) - (map (partial get-libspec-from-file-with-caching :cljs)) - aliases-by-frequencies)})) + (namespace-aliases ignore-errors? (core/source-dirs-on-classpath))) + ([ignore-errors? dirs] + (let [;; fetch the file list just once (as opposed to traversing the project once for each dialect) + files (core/source-files-with-clj-like-extension ignore-errors? dirs) + ;; pmap parallelizes a couple things: + ;; - `pred`, which is IO-intentive + ;; - `aliases-by-frequencies`, which is moderately CPU-intensive + [clj-files cljs-files] (pmap (fn [[dialect pred] corpus] + (->> corpus + (filter pred) + (map (partial get-libspec-from-file-with-caching dialect)) + aliases-by-frequencies)) + [[:clj (util/with-suppressed-errors + (some-fn core/clj-file? core/cljc-file?) + ignore-errors?)] + [:cljs (util/with-suppressed-errors + (some-fn core/cljs-file? core/cljc-file?) + ignore-errors?)]] + (repeat files))] + {:clj clj-files + :cljs cljs-files}))) (defn- unwrap-refer [file {:keys [ns refer]}] diff --git a/src/refactor_nrepl/util.clj b/src/refactor_nrepl/util.clj index 9c313f3b..df994e18 100644 --- a/src/refactor_nrepl/util.clj +++ b/src/refactor_nrepl/util.clj @@ -1,6 +1,10 @@ (ns refactor-nrepl.util - (:require [clojure.string :as string]) - (:import java.util.regex.Pattern)) + (:require + [clojure.java.io :as io] + [clojure.string :as string]) + (:import + (java.io File) + (java.util.regex Pattern))) (defn normalize-to-unix-path "Replace use / as separator and lower-case." @@ -99,3 +103,34 @@ ;; in other places that already are using `some-fn`, `every-pred`, etc ([_] (.isInterrupted (Thread/currentThread)))) + +(defn dir-outside-root-dir? + "Dirs outside the root dir often represent uninteresting dirs to examine, e.g. + Lein checkouts, .gitlibs from deps.edn, or other extraneous source paths + which aren't directly related to the project. + + By excluding them, one gets better and more accurate performance." + [^File f] + {:pre [(-> f .isDirectory)]} + (let [f (-> f .getCanonicalPath File.) + root-dir (File. (System/getProperty "user.dir")) + parent-dirs (->> f + (iterate (fn [^File f] + (some-> f .getCanonicalPath File. .getParent File.))) + (take-while some?) + (set))] + (not (parent-dirs root-dir)))) + +(defn data-file? + "True of f is named like a clj file but represents data. + + E.g. true for data_readers.clj" + [path-or-file] + (let [path (.getPath (io/file path-or-file)) + data-files #{"data_readers.clj" "project.clj" "boot.clj"}] + (reduce (fn [acc data-file] + (let [v (or acc (.endsWith path data-file))] + (cond-> v + v reduced))) + false + data-files))) diff --git a/test/refactor_nrepl/core_test.clj b/test/refactor_nrepl/core_test.clj index 199a74fc..ee5b861e 100644 --- a/test/refactor_nrepl/core_test.clj +++ b/test/refactor_nrepl/core_test.clj @@ -2,14 +2,14 @@ (:require [clojure.test :refer [are deftest is testing]] [refactor-nrepl.config :as config] - [refactor-nrepl.core :refer [ignore-dir-on-classpath? read-ns-form]]) + [refactor-nrepl.core :as sut]) (:import (java.io File))) (defmacro assert-ignored-paths [paths pred] `(doseq [p# ~paths] - (is (~pred (ignore-dir-on-classpath? p#))))) + (is (~pred (sut/ignore-dir-on-classpath? p#))))) (deftest test-ignore-dir-on-classpath? (let [not-ignored ["/home/user/project/test" @@ -32,9 +32,16 @@ (are [input expected] (testing input (assert (-> input File. .exists)) (is (= expected - (read-ns-form input))) + (sut/read-ns-form input))) true) "test-resources/readable_file_incorrect_aliases.clj" nil "testproject/src/com/example/one.clj" '(ns com.example.one (:require [com.example.two :as two :refer [foo]] [com.example.four :as four])))) + +(deftest source-files-with-clj-like-extension-test + (let [result (sut/source-files-with-clj-like-extension true)] + (doseq [extension [".clj" ".cljs" ".cljc"]] + (is (pos? (count (filter (fn [^File f] + (-> f .getPath (.endsWith extension))) + result))))))) diff --git a/test/refactor_nrepl/ns/namespace_aliases_test.clj b/test/refactor_nrepl/ns/namespace_aliases_test.clj index 70eca52f..5e59bc55 100644 --- a/test/refactor_nrepl/ns/namespace_aliases_test.clj +++ b/test/refactor_nrepl/ns/namespace_aliases_test.clj @@ -1,12 +1,13 @@ (ns refactor-nrepl.ns.namespace-aliases-test - (:require [clojure.test :refer [deftest is]] - [refactor-nrepl.core :as core] - [refactor-nrepl.ns.libspecs :as sut] - [refactor-nrepl.util :as util] - [refactor-nrepl.unreadable-files :refer [ignore-errors?]])) + (:require + [clojure.test :refer [deftest is]] + [refactor-nrepl.core :as core] + [refactor-nrepl.ns.libspecs :as sut] + [refactor-nrepl.unreadable-files :refer [ignore-errors?]] + [refactor-nrepl.util :as util])) (defn finds [selector alias libspec] - (let [aliases (selector (sut/namespace-aliases ignore-errors?))] + (let [aliases (selector (sut/namespace-aliases ignore-errors? (core/dirs-on-classpath)))] (some (fn [[k vs]] (and (= k alias) (some #{libspec} vs))) diff --git a/test/refactor_nrepl/util_test.clj b/test/refactor_nrepl/util_test.clj index 4e1c5572..1766d59a 100644 --- a/test/refactor_nrepl/util_test.clj +++ b/test/refactor_nrepl/util_test.clj @@ -1,6 +1,9 @@ (ns refactor-nrepl.util-test - (:require [clojure.test :refer [deftest is]] - [refactor-nrepl.util :as sut])) + (:require + [clojure.test :refer [are deftest is]] + [refactor-nrepl.util :as sut]) + (:import + (java.io File))) (deftest with-additional-ex-data-test (try @@ -10,3 +13,22 @@ (catch clojure.lang.ExceptionInfo e (let [{:keys [foo]} (ex-data e)] (is (= foo :bar)))))) + +(deftest dir-outside-root-dir?-test + (are [input expected] (= expected + (sut/dir-outside-root-dir? input)) + (File. (System/getProperty "user.dir")) false + (File. ".") false + (File. "src") false + (File. "/") true)) + +(deftest data-file?-test + (are [input expected] (= expected + (sut/data-file? input)) + "project.clj" true + "boot.clj" true + "data_readers.clj" true + "project.cljs" false + "boot.cljs" false + "data_readers.cljs" false + "a.clj" false))