Skip to content

Commit 9298bc2

Browse files
dotemacsvemv
authored andcommitted
Upgrade tools.namespace
It was eight releases behind compared to the latest release. With the upgrade of `org.clojure/tools.namespace`, some of the tests failed. This was because ClojureScript files weren't touched/considered until org.clojure/tools.namespace 1.2.0 version, when this bug was fixed and ClojureScript files started to be considered. Closes #394
1 parent c1ff3fe commit 9298bc2

File tree

6 files changed

+71
-35
lines changed

6 files changed

+71
-35
lines changed

CHANGELOG.md

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,8 +2,11 @@
22

33
## Unreleased
44

5-
* Upgrade various dependencies: [#393](https://github.com/clojure-emacs/refactor-nrepl/pull/393), [#395](https://github.com/clojure-emacs/refactor-nrepl/pull/395).
5+
* Upgrade various dependencies: [#393](https://github.com/clojure-emacs/refactor-nrepl/pull/393), [#394](https://github.com/clojure-emacs/refactor-nrepl/pull/394) & [#395](https://github.com/clojure-emacs/refactor-nrepl/pull/395).
66
* Does not impact users, since we use [mranderson](https://github.com/benedekfazekas/mranderson).
7+
* The `rename-file-or-dir` is now also able to rename .cljs files that depended on .clj files via :require-macros [#394](https://github.com/clojure-emacs/refactor-nrepl/pull/394).
8+
* `ns-tracker` no longer ignores cljs files with string requires [#394](https://github.com/clojure-emacs/refactor-nrepl/pull/394).
9+
* note that `ns-tracker` is normally used for clj files only.
710

811
## 3.7.1
912

project.clj

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@
1111
^:inline-dep [http-kit "2.5.0"]
1212
^:inline-dep [org.clojure/data.json "2.4.0"]
1313
^:inline-dep [org.clojure/tools.analyzer.jvm "1.2.3"]
14-
^:inline-dep [org.clojure/tools.namespace "1.1.0" :exclusions [org.clojure/tools.reader]]
14+
^:inline-dep [org.clojure/tools.namespace "1.4.4" :exclusions [org.clojure/tools.reader]]
1515
^:inline-dep [org.clojure/tools.reader "1.3.6"]
1616
^:inline-dep [cider/orchard "0.12.0"]
1717
^:inline-dep [cljfmt "0.9.2" :exclusions [rewrite-clj rewrite-cljs]]

src/refactor_nrepl/ns/tracker.clj

Lines changed: 1 addition & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -7,22 +7,10 @@
77
[clojure.tools.namespace.repl :as tools.namespace.repl]
88
[clojure.tools.namespace.track :as tracker]
99
[refactor-nrepl.core :as core]
10-
[refactor-nrepl.ns.ns-parser :as ns-parser]
1110
[refactor-nrepl.util :as util])
1211
(:import
1312
(java.io File)))
1413

15-
;; Exclude cljs files that use npm string requires until they fix this bug:
16-
;; https://clojure.atlassian.net/projects/TNS/issues/TNS-51
17-
(defn- safe-for-clojure-tools-namespace? [f]
18-
(->> (io/file f)
19-
(.getAbsolutePath)
20-
ns-parser/parse-ns
21-
:cljs
22-
:require
23-
(map :ns)
24-
(not-any? string?)))
25-
2614
;; Adapted from CIDER: https://git.io/JOYUT
2715
(defn- user-refresh-dirs
2816
"Directories to watch and reload, as configured by the user.
@@ -38,8 +26,7 @@
3826
;; corner case - use the mranderson-ized refresh-dirs (for supporting this project's test suite):
3927
tools.namespace.repl/refresh-dirs))
4028

41-
(def default-file-filter-predicate (every-pred core/source-file?
42-
safe-for-clojure-tools-namespace?))
29+
(def default-file-filter-predicate core/source-file?)
4330

4431
(defn build-tracker
4532
"Build a tracker for the project.

test/global_test_setup.clj

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,8 +3,8 @@
33

44
(def set-refresh-dirs
55
(try
6-
(require '[refactor-nrepl.inlined-deps.toolsnamespace.v1v1v0.clojure.tools.namespace.repl])
7-
@(resolve 'refactor-nrepl.inlined-deps.toolsnamespace.v1v1v0.clojure.tools.namespace.repl/set-refresh-dirs)
6+
(require '[refactor-nrepl.inlined-deps.toolsnamespace.v1v4v4.clojure.tools.namespace.repl])
7+
@(resolve 'refactor-nrepl.inlined-deps.toolsnamespace.v1v4v4.clojure.tools.namespace.repl/set-refresh-dirs)
88
(catch Exception _
99
(require '[clojure.tools.namespace.repl])
1010
@(resolve 'clojure.tools.namespace.repl/set-refresh-dirs))))

test/refactor_nrepl/rename_file_or_dir_test.clj

Lines changed: 61 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,7 @@
11
(ns refactor-nrepl.rename-file-or-dir-test
22
(:require
3+
[clojure.data :as data]
4+
[clojure.java.io :as io]
35
[clojure.string :as string]
46
[clojure.test :refer [deftest is testing]]
57
[refactor-nrepl.core :refer [get-ns-component ns-form-from-string]]
@@ -27,12 +29,24 @@
2729
refactor-nrepl.rename-file-or-dir/update-ns! (fn [_path _old-ns])
2830
refactor-nrepl.rename-file-or-dir/update-dependents! (fn [_dependents])
2931
refactor-nrepl.rename-file-or-dir/file-or-symlink-exists? (constantly true)]
30-
(let [res (sut/rename-file-or-dir from-file-path to-file-path ignore-errors?)]
31-
(is (or (list? res)
32-
(instance? clojure.lang.Cons res))
33-
(pr-str [pr-str res, (class res)]))
34-
(is (= 4 (count res))
35-
(pr-str res)))));; currently not tracking :require-macros!!
32+
(let [files->absolute-path (fn [files]
33+
(->> files
34+
(map #(-> % io/file .getAbsolutePath))
35+
(into #{})))
36+
clj ["testproject/src/com/move/dependent_ns1.clj"
37+
"testproject/src/com/move/subdir/dependent_ns_3.clj"
38+
"testproject/src/com/move/dependent_ns2.clj"]
39+
cljs ["testproject/src/com/move/dependent_ns1_cljs.cljs"
40+
"testproject/src/com/move/subdir/dependent_ns_3_cljs.cljs"]
41+
common-referencing-files (files->absolute-path (into clj cljs))
42+
files-referencing-old-ns (conj common-referencing-files from-file-path)
43+
res (sut/rename-file-or-dir from-file-path to-file-path ignore-errors?)
44+
[old-file-name new-file-name files-in-both]
45+
(data/diff files-referencing-old-ns (files->absolute-path res))]
46+
(is (= old-file-name #{from-file-path}) "That the old filename is not present")
47+
(is (= new-file-name #{to-file-path}) "That the new filename is present")
48+
(is (= files-in-both common-referencing-files)
49+
"That the files referencing the old & the new namespace are the same"))))
3650

3751
(deftest replaces-ns-references-in-dependents
3852
(let [dependents (atom [])]
@@ -133,14 +147,23 @@
133147
(is (not (.contains content old-package-prefix-dir)))))))
134148

135149
(deftest calls-rename-file!-on-the-right-files-when-moving-dirs
136-
(let [files (atom [])]
150+
(let [files (atom [])
151+
original-files ["testproject/src/com/move/dependent_ns1.clj"
152+
"testproject/src/com/move/dependent_ns1_cljs.cljs"
153+
"testproject/src/com/move/dependent_ns2.clj"
154+
"testproject/src/com/move/dependent_ns2_cljs.cljs"
155+
"testproject/src/com/move/non_clj_file"
156+
"testproject/src/com/move/ns_to_be_moved.clj"
157+
"testproject/src/com/move/ns_to_be_moved_cljs.cljs"
158+
"testproject/src/com/move/subdir/dependent_ns_3.clj"
159+
"testproject/src/com/move/subdir/dependent_ns_3_cljs.cljs"]]
137160
(with-redefs [refactor-nrepl.rename-file-or-dir/rename-file!
138161
(fn [old new]
139162
(swap! files conj [old new]))
140163
refactor-nrepl.rename-file-or-dir/update-ns! (fn [_path _old-ns])
141164
refactor-nrepl.rename-file-or-dir/update-dependents! (fn [_deps])]
142165
(sut/rename-file-or-dir from-dir-path to-dir-path ignore-errors?)
143-
(is (= (count @files) 9))
166+
(is (= (count @files) (count original-files)))
144167
(doseq [[^String old ^String new] @files]
145168
(is (.contains old "/move/"))
146169
(is (.contains new "/moved/"))))))
@@ -166,9 +189,22 @@
166189
refactor-nrepl.rename-file-or-dir/update-ns! (fn [_path _old-ns])
167190
refactor-nrepl.rename-file-or-dir/update-dependents! (fn [_dependents])
168191
refactor-nrepl.rename-file-or-dir/file-or-symlink-exists? (constantly true)]
169-
(let [res (sut/rename-file-or-dir from-file-path-cljs to-file-path-cljs ignore-errors?)]
170-
(is (or (list? res) (instance? clojure.lang.Cons res)))
171-
(is (= 4 (count res))))))
192+
(let [[_ _ file-with-string-requires :as files]
193+
(->> [to-file-path-cljs
194+
"testproject/src/com/move/dependent_ns1_cljs.cljs"
195+
"testproject/src/com/move/dependent_ns2_cljs.cljs"
196+
"testproject/src/com/move/subdir/dependent_ns_3_cljs.cljs"]
197+
(mapv (fn [^String s]
198+
(-> s File. .getAbsolutePath))))
199+
res (sut/rename-file-or-dir from-file-path-cljs to-file-path-cljs ignore-errors?)]
200+
(is (seq? res))
201+
(is (= (count files) (count res)))
202+
(assert (-> file-with-string-requires slurp read-string last last (= '["string-require-some-javascript-library"
203+
:as
204+
foo])))
205+
(testing "a .cljs file with string requires in it was not excluded from the rename,
206+
and the string requires remain there as-is"
207+
(is (some #{file-with-string-requires} res))))))
172208

173209
(deftest replaces-ns-references-in-dependent-for-cljs
174210
(let [dependents (atom [])]
@@ -179,12 +215,19 @@
179215
(doseq [[_f content] deps]
180216
(swap! dependents conj content)))]
181217
(sut/rename-file-or-dir from-file-path-cljs to-file-path-cljs ignore-errors?)
218+
(assert (seq @dependents))
182219
(doseq [content @dependents
183220
:let [ns-form (ns-form-from-string content)
184221
require-form (get-ns-component ns-form :require)
185222
libspec (second require-form)
186-
required-ns (if (sequential? libspec) (first libspec) libspec)]]
187-
(is (= 'com.move.moved-ns-cljs required-ns))))))
223+
required-ns (if (sequential? libspec)
224+
(first libspec)
225+
libspec)]]
226+
(if (string? required-ns)
227+
(testing "a .cljs file with string requires in it was not excluded from the rename,
228+
and the string requires remain there as-is"
229+
(is (= "string-require-some-javascript-library" required-ns)))
230+
(is (= 'com.move.moved-ns-cljs required-ns)))))))
188231

189232
(deftest replaces-fully-qualified-vars-in-dependents-for-cljs
190233
(let [dependents (atom [])]
@@ -216,13 +259,13 @@
216259
(let [dependents (atom [])]
217260
(with-redefs [refactor-nrepl.rename-file-or-dir/rename-file! (fn [_old _new])
218261
refactor-nrepl.rename-file-or-dir/update-ns! (fn [_path _old-ns])
219-
220262
refactor-nrepl.rename-file-or-dir/update-dependents!
221263
(fn [deps]
222264
(doseq [[^String path content] deps]
223265
(when (.endsWith path ".cljs")
224266
(swap! dependents conj content))))]
225267
(sut/rename-file-or-dir from-dir-path to-dir-path ignore-errors?)
268+
(assert (seq @dependents))
226269
(doseq [content @dependents
227270
:let [ns-form (ns-form-from-string content)
228271
require-form (get-ns-component ns-form :require)
@@ -232,9 +275,11 @@
232275
required-macro-ns (-> require-macros-form second first)]]
233276
;; This is a little gross, but the changes are done in two
234277
;; passes, so each file has one of them.
235-
(is (or (= 'com.moved.ns-to-be-moved-cljs required-ns)
278+
(is (or (= required-ns 'com.moved.ns-to-be-moved-cljs)
279+
(= required-ns "string-require-some-javascript-library")
236280
(when required-macro-ns
237-
(= 'com.moved.ns-to-be-moved required-macro-ns))))))))
281+
(= 'com.moved.ns-to-be-moved required-macro-ns)))
282+
(pr-str [(second ns-form) required-ns required-macro-ns]))))))
238283

239284
(deftest returns-list-of-affected-files-when-moving-dirs-for-cljs
240285
(with-redefs [refactor-nrepl.rename-file-or-dir/rename-file! (fn [_old _new])

testproject/src/com/move/dependent_ns2_cljs.cljs

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
(ns com.move.dependent-ns2-cljs
2-
(:require [com.move.ns-to-be-moved-cljs :refer [fn-to-be-moved]]))
2+
(:require [com.move.ns-to-be-moved-cljs :refer [fn-to-be-moved]]
3+
["string-require-some-javascript-library" :as foo]))
34

45
(defn- use-some-private-stuff []
56
(#'com.move.ns-to-be-moved/private-fn-to-be-moved

0 commit comments

Comments
 (0)