Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

clean-ns: fall back to :relative-path, when given #251

Merged
merged 2 commits into from
Apr 22, 2019
Merged

clean-ns: fall back to :relative-path, when given #251

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
916b554
clean-ns: fall back to :relative-path, when given
plexus Apr 18, 2019
d56ff99
Address test failure, fix some reflection warnings.
plexus Apr 19, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
40 changes: 22 additions & 18 deletions src/refactor_nrepl/ns/clean_ns.clj
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,8 @@
[ns-parser :as ns-parser]
[prune-dependencies :refer [prune-dependencies]]
[rebuild :refer [rebuild-ns-form]]]
[clojure.string :as str]))
[clojure.string :as str]
[clojure.java.io :as io]))

(defn- assert-no-exclude-clause
[use-form]
Expand All @@ -35,20 +36,23 @@
(assert-no-exclude-clause (core/get-ns-component ns-form :use))
ns-form)

(defn clean-ns [{:keys [path]}]
{:pre [(seq path) (string? path) (core/source-file? path)]}
;; Prefix notation not supported in cljs.
;; We also turn it off for cljc for reasons of symmetry
(config/with-config {:prefix-rewriting (if (or (core/cljs-file? path)
(core/cljc-file? path))
false
(:prefix-rewriting config/*config*))}
(let [ns-form (validate (core/read-ns-form-with-meta path))
deps-preprocessor (if (get config/*config* :prune-ns-form)
#(prune-dependencies % path)
identity)
new-ns-form (-> (ns-parser/parse-ns path)
deps-preprocessor
(rebuild-ns-form ns-form))]
(when-not (= ns-form new-ns-form)
new-ns-form))))
(defn clean-ns [{:keys [path relative-path]}]
{:pre [(seq path) (string? path)]}
;; Try first the absolute, then the relative path
(let [path (first (filter #(some-> % io/file .exists) [path relative-path]))]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

some might be better here since we're looking for the first thing that fulfills some pred

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since the predicate returns boolean rather than the truthy value I find it awkward to use some for such cases, as you end up with (some #(and (pred? %) %) coll). Would you still prefer some?

(assert (core/source-file? path))
;; Prefix notation not supported in cljs.
;; We also turn it off for cljc for reasons of symmetry
(config/with-config {:prefix-rewriting (if (or (core/cljs-file? path)
(core/cljc-file? path))
false
(:prefix-rewriting config/*config*))}
(let [ns-form (validate (core/read-ns-form-with-meta path))
deps-preprocessor (if (get config/*config* :prune-ns-form)
#(prune-dependencies % path)
identity)
new-ns-form (-> (ns-parser/parse-ns path)
deps-preprocessor
(rebuild-ns-form ns-form))]
(when-not (= ns-form new-ns-form)
new-ns-form)))))
4 changes: 2 additions & 2 deletions src/refactor_nrepl/ns/pprint.clj
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -156,6 +156,6 @@
(form-is? form :import) (pprint-import-form form)
:else (pprint form))))
forms)))
(.replaceAll "\r" "")
(str/replace "\r" "")
fmt/reformat-string
(.replaceAll (Pattern/quote "#? @") "#?@"))))
(str/replace (Pattern/quote "#? @") "#?@"))))
15 changes: 12 additions & 3 deletions test/refactor_nrepl/ns/clean_ns_test.clj
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,14 +3,16 @@
[refactor-nrepl.config :as config]
[refactor-nrepl.core :as core]
[refactor-nrepl.ns.clean-ns :refer [clean-ns]]
[refactor-nrepl.ns.pprint :refer [pprint-ns]])
[refactor-nrepl.ns.pprint :refer [pprint-ns]]
[clojure.string :as str])
(:import java.io.File))

(defn- absolute-path [^String path]
(.getAbsolutePath (File. path)))

(defn- clean-msg [path]
{:path (absolute-path path)})
{:path (absolute-path path)
:relative-path path})

(def ns1 (clean-msg "test/resources/ns1.clj"))
(def ns1-cleaned (core/read-ns-form-with-meta (absolute-path "test/resources/ns1_cleaned.clj")))
Expand Down Expand Up @@ -48,14 +50,17 @@

(def ns-using-dollar (clean-msg "test/resources/ns_using_dollar.clj"))

(def ns1-relative-path {:path "I do not exist.clj"
:relative-path "test/resources/ns1.clj"})

(deftest combines-requires
(let [requires (core/get-ns-component (clean-ns ns2) :require)
combined-requires (core/get-ns-component ns2-cleaned :require)]
(is (= combined-requires requires))))

(deftest meta-preserved
(let [cleaned (pprint-ns (clean-ns ns2-meta))]
(is (.contains cleaned "^{:author \"Trurl and Klapaucius\"
(is (str/includes? cleaned "^{:author \"Trurl and Klapaucius\"
:doc \"test ns with meta\"}"))))

(deftest rewrites-use-to-require
Expand Down Expand Up @@ -224,3 +229,7 @@
(deftest does-not-break-import-for-inner-class
(let [cleaned (pprint-ns (clean-ns ns-with-inner-classes))]
(is (re-find #":import.*Line2D\$Double" cleaned))))

(deftest fallback-to-relative-path
(is (= (pprint-ns (clean-ns ns1))
(pprint-ns (clean-ns ns1-relative-path)))))