From b78e3545bc51208747cab4617f1672c0d265cde8 Mon Sep 17 00:00:00 2001 From: vemv Date: Thu, 8 Jul 2021 17:03:13 +0200 Subject: [PATCH] Introduce `with-suppressed-errors` (#317) Fixes https://github.com/clojure-emacs/refactor-nrepl/issues/291 --- CHANGELOG.md | 11 +++++ README.md | 9 ++++ eastwood.clj | 4 ++ project.clj | 5 ++- src/refactor_nrepl/analyzer.clj | 2 +- src/refactor_nrepl/config.clj | 9 ++++ src/refactor_nrepl/find/find_macros.clj | 6 ++- src/refactor_nrepl/find/find_symbol.clj | 28 ++++++------ src/refactor_nrepl/middleware.clj | 4 +- src/refactor_nrepl/ns/libspecs.clj | 43 ++++++++++++------- src/refactor_nrepl/ns/tracker.clj | 28 +++++++----- src/refactor_nrepl/rename_file_or_dir.clj | 28 +++++++----- src/refactor_nrepl/util.clj | 16 +++++++ .../readable_file_incorrect_aliases.clj | 2 + .../readable_file_incorrect_data_readers.clj | 2 + test-resources/unreadable_file.clj | 1 + .../extract_definition_test.clj | 9 ++-- test/refactor_nrepl/find/find_macros_test.clj | 19 ++++---- .../ns/namespace_aliases_test.clj | 13 +++--- .../ns/resolve_missing_caching_test.clj | 2 +- .../ns/resolve_missing_test.clj | 2 +- .../rename_file_or_dir_test.clj | 39 +++++++++-------- test/refactor_nrepl/unreadable_files.clj | 30 +++++++++++++ 23 files changed, 217 insertions(+), 95 deletions(-) create mode 100644 eastwood.clj create mode 100644 test-resources/readable_file_incorrect_aliases.clj create mode 100644 test-resources/readable_file_incorrect_data_readers.clj create mode 100644 test-resources/unreadable_file.clj create mode 100644 test/refactor_nrepl/unreadable_files.clj diff --git a/CHANGELOG.md b/CHANGELOG.md index bf44a6b1..9a69bec6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,17 @@ ## Unreleased +#### Changes +* [#291](https://github.com/clojure-emacs/refactor-nrepl/issues/291): 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, Moustache template files, scripts. +* 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. +* 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) + ### 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. diff --git a/README.md b/README.md index 53bc327f..2c144061 100644 --- a/README.md +++ b/README.md @@ -81,6 +81,15 @@ Configuration settings are passed along with each msg, currently the recognized :debug false + ;; if `true`: + ;; * files that can't be `read`, `require`d or analyzed (with `tools.analyzer`) will be ignored, + ;; instead of aborting the early phases of refactor-nrepl execution. + ;; * ops like `find-symbol` will carry on even if there is broken namespace which we can not build AST for. + ;; Setting `false` will be more strict, yielding possibly more correct usage, + ;; but it also needs that `:ignore-paths` is correctly set, that all namespaces are valid, + ;; that tools.analyzer is able to analyze all of them, etc + :ignore-errors true + ;; When true `clean-ns` will remove unused symbols, otherwise just ;; sort etc :prune-ns-form true diff --git a/eastwood.clj b/eastwood.clj new file mode 100644 index 00000000..8cfb30f4 --- /dev/null +++ b/eastwood.clj @@ -0,0 +1,4 @@ +(disable-warning + {:linter :unused-ret-vals-in-try + :if-inside-macroexpansion-of #{'clojure.test/deftest + 'clojure.core/assert}}) diff --git a/project.clj b/project.clj index 000f7bd7..21f668c7 100644 --- a/project.clj +++ b/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"]]}]} diff --git a/src/refactor_nrepl/analyzer.clj b/src/refactor_nrepl/analyzer.clj index 7daff9e6..b371143b 100644 --- a/src/refactor_nrepl/analyzer.clj +++ b/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 diff --git a/src/refactor_nrepl/config.clj b/src/refactor_nrepl/config.clj index 6fb6cc2a..a16fd796 100644 --- a/src/refactor_nrepl/config.clj +++ b/src/refactor_nrepl/config.clj @@ -8,6 +8,15 @@ :debug false + ;; if `true`: + ;; * files that can't be `read`, `require`d or analyzed (with `tools.analyzer`) will be ignored, + ;; instead of aborting the early phases of refactor-nrepl execution. + ;; * ops like `find-symbol` will carry on even if there is broken namespace which we can not build AST for. + ;; Setting `false` will be more strict, yielding possibly more correct usage, + ;; but it also needs that `:ignore-paths` is correctly set, that all namespaces are valid, + ;; that tools.analyzer is able to analyze all of them, etc + :ignore-errors true + ;; When true `clean-ns` will remove unused symbols, otherwise just ;; sort etc :prune-ns-form true diff --git a/src/refactor_nrepl/find/find_macros.clj b/src/refactor_nrepl/find/find_macros.clj index 164fa6e1..78f7140b 100644 --- a/src/refactor_nrepl/find/find_macros.clj +++ b/src/refactor_nrepl/find/find_macros.clj @@ -81,7 +81,9 @@ (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/with-suppressed-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 +217,7 @@ (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/with-suppressed-errors tracker/default-file-filter-predicate ignore-errors?)) origin-ns (symbol (core/prefix fully-qualified-name)) dependents (tracker/get-dependents tracker origin-ns)] (some->> macro-def diff --git a/src/refactor_nrepl/find/find_symbol.clj b/src/refactor_nrepl/find/find_symbol.clj index 90c0d21b..be25c8fc 100644 --- a/src/refactor_nrepl/find/find_symbol.clj +++ b/src/refactor_nrepl/find/find_symbol.clj @@ -135,20 +135,22 @@ 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/with-suppressed-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 diff --git a/src/refactor_nrepl/middleware.clj b/src/refactor_nrepl/middleware.clj index d2970e33..29a48b60 100644 --- a/src/refactor_nrepl/middleware.clj +++ b/src/refactor_nrepl/middleware.clj @@ -157,8 +157,8 @@ (delay (require-and-resolve 'refactor-nrepl.rename-file-or-dir/rename-file-or-dir))) -(defn- rename-file-or-dir-reply [{:keys [transport old-path new-path] :as msg}] - (reply transport msg :touched (@rename-file-or-dir old-path new-path) +(defn- rename-file-or-dir-reply [{:keys [transport old-path new-path ignore-errors] :as msg}] + (reply transport msg :touched (@rename-file-or-dir old-path new-path (= ignore-errors "true")) :status :done)) (defn- namespace-aliases-reply [{:keys [transport] :as msg}] diff --git a/src/refactor_nrepl/ns/libspecs.clj b/src/refactor_nrepl/ns/libspecs.clj index e40cd90c..68568ce4 100644 --- a/src/refactor_nrepl/ns/libspecs.clj +++ b/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,19 @@ {: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/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)})) (defn- unwrap-refer [file {:keys [ns refer]}] @@ -75,10 +82,16 @@ 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/with-suppressed-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/with-suppressed-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)})) diff --git a/src/refactor_nrepl/ns/tracker.clj b/src/refactor_nrepl/ns/tracker.clj index 5c7f301a..fac58dd5 100644 --- a/src/refactor_nrepl/ns/tracker.clj +++ b/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,15 @@ ;; corner case - use the mranderson-ized refresh-dirs (for supporting this project's test suite): refresh-dirs)) +(def default-file-filter-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-file-filter-predicate)) ([file-predicate] (file/add-files (tracker/tracker) (core/find-in-project file-predicate)))) @@ -74,11 +77,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/with-suppressed-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?))))) diff --git a/src/refactor_nrepl/rename_file_or_dir.clj b/src/refactor_nrepl/rename_file_or_dir.clj index dc301d24..be9e58e2 100644 --- a/src/refactor_nrepl/rename_file_or_dir.clj +++ b/src/refactor_nrepl/rename_file_or_dir.clj @@ -194,10 +194,10 @@ (defn- rename-source-file "Move file from old to new, updating any dependents." - [old-path new-path] + [old-path new-path ignore-errors?] (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/with-suppressed-errors tracker/default-file-filter-predicate ignore-errors?)) dependents (tracker/get-dependents tracker old-ns) new-dependents (atom {})] (doseq [^File f dependents] @@ -214,7 +214,7 @@ [path old-parent new-parent] (str/replace path old-parent new-parent)) -(defn- rename-dir [old-path new-path] +(defn- rename-dir [old-path new-path ignore-errors?] (let [old-path (util/normalize-to-unix-path old-path) new-path (util/normalize-to-unix-path new-path) old-path (if (.endsWith old-path "/") old-path (str old-path "/")) @@ -222,7 +222,9 @@ (flatten (for [^File f (file-seq (File. old-path)) :when (not (fs/directory? f)) :let [path (util/normalize-to-unix-path (.getAbsolutePath f))]] - (-rename-file-or-dir path (merge-paths path old-path new-path)))))) + (-rename-file-or-dir path + (merge-paths path old-path new-path) + ignore-errors?))))) (defn- file-or-symlink-exists? [^String path] (let [f (File. path)] @@ -234,12 +236,12 @@ (when (.. target toFile exists) path))))))) -(defn- -rename-file-or-dir [^String old-path new-path] +(defn- -rename-file-or-dir [^String old-path new-path ignore-errors?] (let [affected-files (if (fs/directory? old-path) - (rename-dir old-path new-path) + (rename-dir old-path new-path ignore-errors?) (if ((some-fn core/clj-file? core/cljs-file?) (File. old-path)) - (rename-source-file old-path new-path) + (rename-source-file old-path new-path ignore-errors?) (rename-file! old-path new-path)))] (->> affected-files flatten @@ -271,7 +273,11 @@ old-path and new-path are expected to be aboslute paths. Returns a list of all files that were affected." - [old-path new-path] - (assert-friendly old-path new-path) - (binding [*print-length* nil] - (-rename-file-or-dir old-path new-path))) + + ([old-path new-path] + (rename-file-or-dir old-path new-path false)) + + ([old-path new-path ignore-errors?] + (assert-friendly old-path new-path) + (binding [*print-length* nil] + (-rename-file-or-dir old-path new-path ignore-errors?)))) diff --git a/src/refactor_nrepl/util.clj b/src/refactor_nrepl/util.clj index 3b949977..747b2412 100644 --- a/src/refactor_nrepl/util.clj +++ b/src/refactor_nrepl/util.clj @@ -72,3 +72,19 @@ (defn maybe-log-exception [^Throwable e] (when (System/getProperty "refactor-nrepl.internal.log-exceptions") (.printStackTrace e))) + +(defn with-suppressed-errors + "Wraps a predicate `f` in a try-catch, depending on `ignore-errors?`. + + Typically used for making ingestion of data (`read`/`require`/`analyze`) more robust + in face of unconfigured or otherwise problematic source paths." + [f ignore-errors?] + (if-not ignore-errors? + f + (fn [x] + (try + (f x) + (catch Exception e + (maybe-log-exception e) + ;; return false, because `with-suppressed-errors` is oriented for predicate usage + false))))) diff --git a/test-resources/readable_file_incorrect_aliases.clj b/test-resources/readable_file_incorrect_aliases.clj new file mode 100644 index 00000000..0750de54 --- /dev/null +++ b/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 diff --git a/test-resources/readable_file_incorrect_data_readers.clj b/test-resources/readable_file_incorrect_data_readers.clj new file mode 100644 index 00000000..4be155d7 --- /dev/null +++ b/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 [] diff --git a/test-resources/unreadable_file.clj b/test-resources/unreadable_file.clj new file mode 100644 index 00000000..f33a8486 --- /dev/null +++ b/test-resources/unreadable_file.clj @@ -0,0 +1 @@ +(ns unreadable-file diff --git a/test/refactor_nrepl/extract_definition_test.clj b/test/refactor_nrepl/extract_definition_test.clj index 5336b677..6f77306b 100644 --- a/test/refactor_nrepl/extract_definition_test.clj +++ b/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)) diff --git a/test/refactor_nrepl/find/find_macros_test.clj b/test/refactor_nrepl/find/find_macros_test.clj index 5131366a..7e5ca446 100644 --- a/test/refactor_nrepl/find/find_macros_test.clj +++ b/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!")))] diff --git a/test/refactor_nrepl/ns/namespace_aliases_test.clj b/test/refactor_nrepl/ns/namespace_aliases_test.clj index 345968b5..70eca52f 100644 --- a/test/refactor_nrepl/ns/namespace_aliases_test.clj +++ b/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))))) diff --git a/test/refactor_nrepl/ns/resolve_missing_caching_test.clj b/test/refactor_nrepl/ns/resolve_missing_caching_test.clj index d8855254..3d60d6af 100644 --- a/test/refactor_nrepl/ns/resolve_missing_caching_test.clj +++ b/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))) diff --git a/test/refactor_nrepl/ns/resolve_missing_test.clj b/test/refactor_nrepl/ns/resolve_missing_test.clj index b96db8b0..2c096343 100644 --- a/test/refactor_nrepl/ns/resolve_missing_test.clj +++ b/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) diff --git a/test/refactor_nrepl/rename_file_or_dir_test.clj b/test/refactor_nrepl/rename_file_or_dir_test.clj index 8f6a2b6f..5dd0e4dd 100644 --- a/test/refactor_nrepl/rename_file_or_dir_test.clj +++ b/test/refactor_nrepl/rename_file_or_dir_test.clj @@ -3,7 +3,8 @@ [refactor-nrepl.core :refer [get-ns-component ns-form-from-string]] - [refactor-nrepl.rename-file-or-dir :refer :all]) + [refactor-nrepl.unreadable-files :refer [ignore-errors?]] + [refactor-nrepl.rename-file-or-dir :as sut]) (:import java.io.File)) (def from-file-path (.getAbsolutePath (File. "testproject/src/com/move/ns_to_be_moved.clj"))) @@ -25,7 +26,7 @@ refactor-nrepl.rename-file-or-dir/update-ns! (fn [path old-ns]) refactor-nrepl.rename-file-or-dir/update-dependents! (fn [dependents]) refactor-nrepl.rename-file-or-dir/file-or-symlink-exists? (constantly true)] - (let [res (rename-file-or-dir from-file-path to-file-path)] + (let [res (sut/rename-file-or-dir from-file-path to-file-path ignore-errors?)] (is (or (list? res) (instance? clojure.lang.Cons res))) (is (= 4 (count res))))));; currently not tracking :require-macros!! @@ -38,7 +39,7 @@ (doseq [[^String path content] deps] (when (.endsWith path "clj") (swap! dependents conj content))))] - (rename-file-or-dir from-file-path to-file-path) + (sut/rename-file-or-dir from-file-path to-file-path ignore-errors?) (doseq [content @dependents :let [ns-form (ns-form-from-string content) require-form (get-ns-component ns-form :require) @@ -57,7 +58,7 @@ (fn [deps] (doseq [[f content] deps] (swap! dependents conj [f content])))] - (rename-file-or-dir from-file-path to-file-path) + (sut/rename-file-or-dir from-file-path to-file-path ignore-errors?) (doseq [[^String f ^String content] @dependents :when (.endsWith f "ns2.clj")] (is (.contains content new-ns-ref)) @@ -73,7 +74,7 @@ (is (= new to-file-path))) refactor-nrepl.rename-file-or-dir/update-ns! (fn [path old-ns]) refactor-nrepl.rename-file-or-dir/update-dependents! (fn [deps])] - (rename-file-or-dir from-file-path to-file-path))) + (sut/rename-file-or-dir from-file-path to-file-path ignore-errors?))) (deftest replaces-ns-references-in-dependendents-when-moving-dirs (let [dependents (atom [])] @@ -85,7 +86,7 @@ (doseq [[^String path content] deps] (when (.endsWith path ".clj") (swap! dependents conj content))))] - (rename-file-or-dir from-dir-path to-dir-path) + (sut/rename-file-or-dir from-dir-path to-dir-path ignore-errors?) (doseq [content @dependents :let [ns-form (ns-form-from-string content) require-form (get-ns-component ns-form :require) @@ -101,7 +102,7 @@ refactor-nrepl.rename-file-or-dir/update-ns! (fn [path old-ns]) refactor-nrepl.rename-file-or-dir/update-dependents! (fn [dependents]) refactor-nrepl.rename-file-or-dir/file-or-symlink-exists? (constantly true)] - (let [res (rename-file-or-dir from-dir-path to-dir-path)] + (let [res (sut/rename-file-or-dir from-dir-path to-dir-path ignore-errors?)] (is (seq? res)) (is (= 8 (count res)))))) @@ -113,7 +114,7 @@ (fn [deps] (doseq [[f content] deps] (swap! dependents conj [f content])))] - (rename-file-or-dir from-dir-path to-dir-path) + (sut/rename-file-or-dir from-dir-path to-dir-path ignore-errors?) (doseq [[^String f ^String content] @dependents :when (.endsWith f "ns2.clj")] (is (.contains content new-ns-ref-dir)) @@ -129,7 +130,7 @@ (swap! files conj [old new])) refactor-nrepl.rename-file-or-dir/update-ns! (fn [path old-ns]) refactor-nrepl.rename-file-or-dir/update-dependents! (fn [deps])] - (rename-file-or-dir from-dir-path to-dir-path) + (sut/rename-file-or-dir from-dir-path to-dir-path ignore-errors?) (is (= (count @files) 9)) (doseq [[^String old ^String new] @files] (is (.contains old "/move/")) @@ -142,7 +143,7 @@ (swap! files conj new)) refactor-nrepl.rename-file-or-dir/update-ns! (fn [path old-ns]) refactor-nrepl.rename-file-or-dir/update-dependents! (fn [deps])] - (rename-file-or-dir from-dir-path to-dir-path) + (sut/rename-file-or-dir from-dir-path to-dir-path ignore-errors?) (is (some #(.endsWith ^String % "non_clj_file") @files)) (is (= 4 (count (filter #(.endsWith ^String % ".cljs") @files))))))) @@ -158,7 +159,7 @@ refactor-nrepl.rename-file-or-dir/update-ns! (fn [path old-ns]) refactor-nrepl.rename-file-or-dir/update-dependents! (fn [dependents]) refactor-nrepl.rename-file-or-dir/file-or-symlink-exists? (constantly true)] - (let [res (rename-file-or-dir from-file-path-cljs to-file-path-cljs)] + (let [res (sut/rename-file-or-dir from-file-path-cljs to-file-path-cljs ignore-errors?)] (is (or (list? res) (instance? clojure.lang.Cons res))) (is (= 4 (count res)))))) @@ -170,7 +171,7 @@ (fn [deps] (doseq [[f content] deps] (swap! dependents conj content)))] - (rename-file-or-dir from-file-path-cljs to-file-path-cljs) + (sut/rename-file-or-dir from-file-path-cljs to-file-path-cljs ignore-errors?) (doseq [content @dependents :let [ns-form (ns-form-from-string content) require-form (get-ns-component ns-form :require) @@ -186,7 +187,7 @@ (fn [deps] (doseq [[f content] deps] (swap! dependents conj [f content])))] - (rename-file-or-dir from-file-path-cljs to-file-path-cljs) + (sut/rename-file-or-dir from-file-path-cljs to-file-path-cljs ignore-errors?) (doseq [[^String f ^String content] @dependents :when (.endsWith f "ns2.cljs")] (is (.contains content new-ns-ref)) @@ -202,7 +203,7 @@ (is (= new to-file-path-cljs))) refactor-nrepl.rename-file-or-dir/update-ns! (fn [path old-ns]) refactor-nrepl.rename-file-or-dir/update-dependents! (fn [deps])] - (rename-file-or-dir from-file-path-cljs to-file-path-cljs))) + (sut/rename-file-or-dir from-file-path-cljs to-file-path-cljs ignore-errors?))) (deftest replaces-ns-references-in-dependendents-when-moving-dirs-for-cljs (let [dependents (atom [])] @@ -214,7 +215,7 @@ (doseq [[^String path content] deps] (when (.endsWith path ".cljs") (swap! dependents conj content))))] - (rename-file-or-dir from-dir-path to-dir-path) + (sut/rename-file-or-dir from-dir-path to-dir-path ignore-errors?) (doseq [content @dependents :let [ns-form (ns-form-from-string content) require-form (get-ns-component ns-form :require) @@ -233,7 +234,7 @@ refactor-nrepl.rename-file-or-dir/update-ns! (fn [path old-ns]) refactor-nrepl.rename-file-or-dir/update-dependents! (fn [dependents]) refactor-nrepl.rename-file-or-dir/file-or-symlink-exists? (constantly true)] - (let [res (rename-file-or-dir from-dir-path to-dir-path)] + (let [res (sut/rename-file-or-dir from-dir-path to-dir-path ignore-errors?)] (is (seq? res)) (is (= 8 (count res)))))) @@ -245,7 +246,7 @@ (fn [deps] (doseq [[f content] deps] (swap! dependents conj [f content])))] - (rename-file-or-dir from-dir-path to-dir-path) + (sut/rename-file-or-dir from-dir-path to-dir-path ignore-errors?) (doseq [[^String f ^String content] @dependents :when (.endsWith f "ns2.cljs")] (is (.contains content new-ns-ref-dir)) @@ -261,7 +262,7 @@ (swap! files conj [old new])) refactor-nrepl.rename-file-or-dir/update-ns! (fn [path old-ns]) refactor-nrepl.rename-file-or-dir/update-dependents! (fn [deps])] - (rename-file-or-dir from-dir-path to-dir-path) + (sut/rename-file-or-dir from-dir-path to-dir-path ignore-errors?) (is (= (count @files) 9)) (doseq [[^String old ^String new] @files] (is (.contains old "/move/")) @@ -274,6 +275,6 @@ (swap! files conj new)) refactor-nrepl.rename-file-or-dir/update-ns! (fn [path old-ns]) refactor-nrepl.rename-file-or-dir/update-dependents! (fn [deps])] - (rename-file-or-dir from-dir-path to-dir-path) + (sut/rename-file-or-dir from-dir-path to-dir-path ignore-errors?) (is (some #(.endsWith ^String % "non_clj_file") @files)) (is (= 4 (count (filter #(.endsWith ^String % ".clj") @files))))))) diff --git a/test/refactor_nrepl/unreadable_files.clj b/test/refactor_nrepl/unreadable_files.clj new file mode 100644 index 00000000..b83c9681 --- /dev/null +++ b/test/refactor_nrepl/unreadable_files.clj @@ -0,0 +1,30 @@ +(ns refactor-nrepl.unreadable-files + "Helpers and a self-assertion for dealing with unreadable files in a given codebase." + (:require + [clojure.java.io :as io] + [clojure.test :refer [deftest is]])) + +(def ignore-errors? + "Currently errors while analysing the classpath (most relevantly our `test-resources` dir) are ignored. + + This is because some files in that dir are intentionally invalid, which intentionally represents a use case + (e.g. users can have WIP code, files not meant to be vanilla namespaces, etc)." + true) + +(def ignore-errors-str + (pr-str ignore-errors?)) + +;; This is more of an assertion about the test suite (namely that certain files exist in the claspath and are problematic) +;; than an actual unit test (as there's no SUT) +(deftest readable-file? + (doseq [file (->> ["unreadable_file.clj" + "readable_file_incorrect_aliases.clj" + "readable_file_incorrect_data_readers.clj"] + (map (comp io/as-file io/resource)))] + (is (try + (-> file slurp read-string) + false + (catch Exception _ + true)) + "All these files are unreadable. And yet the refactor-nrepl test suite will pass +(it would have failed prior to the introduction of `#'refactor-nrepl.util/with-suppressed-errors`).")))