Skip to content

Commit

Permalink
Don't prune cljsjs namespaces in clean-ns
Browse files Browse the repository at this point in the history
These namespaces represent externs.  For now the exact symbols are
unknowable, so we can't determine if the ns is in use.  As a temporary
fix we just ignore these libspecs.  While I consider this a temp hack
it's unlikely to ever get improved on, unless someone else builds tooling
we can delegate to.

This fixes clojure-emacs/clj-refactor.el#275
  • Loading branch information
expez committed Jan 14, 2016
1 parent 4f9ee4c commit 603dbb4
Show file tree
Hide file tree
Showing 4 changed files with 23 additions and 11 deletions.
10 changes: 5 additions & 5 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -13,17 +13,17 @@
* Make `find-symbol` able to handle macros.

### Bugs fixed

[clojure-emacs/clj-refactor.el#223](https://github.com/clojure-emacs/clj-refactor.el/issues/223) Fix clean-ns removing metadata when file starts with a comment.
[#103](https://github.com/clojure-emacs/refactor-nrepl/issues/108) `find-symbol` failes in projects with `cljc` files.
[#106](https://github.com/clojure-emacs/refactor-nrepl/issues/106) Failure to build ASTs when namespaced keywords are present in the project.
* [clojure-emacs/clj-refactor.el#223](https://github.com/clojure-emacs/clj-refactor.el/issues/223) Fix clean-ns removing metadata when file starts with a comment.
* [#103](https://github.com/clojure-emacs/refactor-nrepl/issues/108) `find-symbol` failes in projects with `cljc` files.
* [#106](https://github.com/clojure-emacs/refactor-nrepl/issues/106) Failure to build ASTs when namespaced keywords are present in the project.

### Changes

* [clojure-emacs/clj-refactor.el#275](https://github.com/clojure-emacs/clj-refactor.el/issues/275) clean-ns will no longer prune cljsjs requires.
* New option for `clean-ns`, `prune-ns-form`, to avoid pruning the ns-form.
* Get rid of the client namespace

* [#118](https://github.com/clojure-emacs/refactor-nrepl/issues/118)Improve the `find-symbol` reply. It's now a map instead of a
* [#118](https://github.com/clojure-emacs/refactor-nrepl/issues/118) Improve the `find-symbol` reply. It's now a map instead of a
vector.
* Remove `find-debug-fns`. None of us ever used this and there's some
overlap with `find-usages`.
Expand Down
20 changes: 15 additions & 5 deletions src/refactor_nrepl/ns/prune_dependencies.clj
Original file line number Diff line number Diff line change
Expand Up @@ -99,9 +99,8 @@
(= (:refer libspec) :all))
(:refer libspec)))

(defn- remove-unused-requires [symbols-in-file current-ns libspecs]
(map (partial remove-unused-syms-and-specs symbols-in-file current-ns)
libspecs))
(defn- remove-unused-requires [symbols-in-file current-ns libspec]
(remove-unused-syms-and-specs symbols-in-file current-ns libspec))

(defn- remove-unused-renamed-symbols
[symbols-in-file {:keys [rename] :as libspec}]
Expand All @@ -111,11 +110,22 @@
:when (symbols-in-file (str alias))]
[sym alias]))))

(defn- libspec-should-never-be-pruned? [libspec]
;; The symbols of cljsjs namespaces are externs, and unknowable for now,
;; so we just ignore these libspecs
(.startsWith (str (:ns libspec)) "cljsjs"))

(defn- prune-libspec [symbols-in-file current-ns libspec]
(if (libspec-should-never-be-pruned? libspec)
libspec
(some->> libspec
(remove-unused-renamed-symbols symbols-in-file)
(remove-unused-requires symbols-in-file current-ns))))

(defn- prune-libspecs
[libspecs symbols-in-file current-ns]
(->> libspecs
(map (partial remove-unused-renamed-symbols symbols-in-file))
(remove-unused-requires symbols-in-file current-ns)
(map (partial prune-libspec symbols-in-file current-ns))
(filter (complement nil?))))

(defn- prune-imports
Expand Down
3 changes: 2 additions & 1 deletion test/resources/cljsns.cljs
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
(ns resources.cljsns
(:require [cljs.test :refer-macros [is deftest]]
[cljs.test :refer-macros [is]]
[cljsjs.js-yaml] ; this one should not be pruned as it contains externs
[clojure.string :refer [split-lines join]]
[cljs.pprint :as pprint]
[clojure.set :as set])
Expand All @@ -17,7 +18,7 @@

(deftest tt
(testing "whatever"
(is (= 1 1))))
(is (= 1 1))))

(defn foo []
`(join "foo bar"))
Expand Down
1 change: 1 addition & 0 deletions test/resources/cljsns_cleaned.cljs
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
(ns resources.cljsns
(:require [cljs.pprint :as pprint]
[cljs.test :refer-macros [deftest is]]
cljsjs.js-yaml
[clojure.set :as set]
[clojure.string :refer [join split-lines]])
(:require-macros cljs.analyzer.api
Expand Down

0 comments on commit 603dbb4

Please sign in to comment.