diff --git a/CHANGELOG.md b/CHANGELOG.md index 3c3731c5..f0d19094 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,6 +3,7 @@ ## Unreleased * [#173](https://github.com/clojure-emacs/refactor-nrepl/issues/173): `rename-file-or-dir`: rename more kinds of constructs in dependent namespaces: namespace-qualified maps, fully-qualified functions, metadata. +* [#194](https://github.com/clojure-emacs/refactor-nrepl/issues/194): Don't prune `require` forms if they are needed for a given `import` to work. ## 3.3.1 diff --git a/src/refactor_nrepl/ns/prune_dependencies.clj b/src/refactor_nrepl/ns/prune_dependencies.clj index 90af6f9e..b487373f 100644 --- a/src/refactor_nrepl/ns/prune_dependencies.clj +++ b/src/refactor_nrepl/ns/prune_dependencies.clj @@ -1,6 +1,8 @@ (ns refactor-nrepl.ns.prune-dependencies (:require [cider.nrepl.middleware.info :as info] + [clojure.set :as set] + [clojure.string :as string] [refactor-nrepl.core :as core] [refactor-nrepl.find.symbols-in-file :as symbols-in-file] [refactor-nrepl.ns.libspec-allowlist :as libspec-allowlist] @@ -116,18 +118,77 @@ (-> pattern re-pattern (re-find ns-name))) (libspec-allowlist/libspec-allowlist))))) -(defn- prune-libspec [symbols-in-file current-ns libspec] - (if (libspec-should-never-be-pruned? libspec) +(defn imports->namespaces + "Given a collection of `:import` clauses, returns the set of namespaces denoted by them, as symbols. + + Some of those namespace symbols may not refer to actual namespaces. + e.g. a `java.io.File` import would return `java.io`, which isn't a Clojure namespace." + [imports] + (into #{} + (map (fn [import] + (-> (if (sequential? import) + (first import) + (->> (-> import str (string/split #"\.")) + (butlast) + (string/join "."))) + str + (string/replace "_" "-") + symbol))) + imports)) + +(defn libspec->namespaces + "Given a libspec, returns the namespaces denoted by it (typically one, but possibly multiple, + if prefix notation was used), as symbols." + [libspec] + (cond + (symbol? libspec) + [libspec] + + ;; Check if it doesn't denote prefix notation: + (and (sequential? libspec) + (or (-> libspec count #{1}) + (some keyword? libspec))) + [(first libspec)] + + :else + (let [suffixes (->> libspec + rest + (map (fn [suffix] + (cond-> suffix + (sequential? suffix) first))))] + (map (fn [prefix suffix] + (symbol (str prefix "." suffix))) + (repeat (first libspec)) + suffixes)))) + +(defn imports-contain-libspec? + "Do `import-namespaces` contain at least one namespace that is denoted by `libspec`? + + This is useful for keeping requires that emit classes (i.e. those defining deftypes/defrecords), + which are imported via `:import`." + [imports-namespaces libspec] + {:pre [(set? imports-namespaces)]} + (let [require-namespaces (set (libspec->namespaces libspec))] + (some? (seq (set/intersection imports-namespaces require-namespaces))))) + +(defn- prune-libspec [symbols-in-file current-ns imports-namespaces libspec] + (cond + (libspec-should-never-be-pruned? libspec) + libspec + + (imports-contain-libspec? imports-namespaces (:ns libspec)) libspec + + :else (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 prune-libspec symbols-in-file current-ns)) - (filter (complement nil?)))) + [libspecs symbols-in-file current-ns imports] + (let [imports-namespaces (imports->namespaces imports)] + (keep (partial prune-libspec symbols-in-file current-ns imports-namespaces) + libspecs))) (defn- prune-imports [imports symbols-in-file] @@ -141,15 +202,17 @@ symbols-in-file (->> (symbols-in-file/symbols-in-file path parsed-ns dialect) (map str) - set)] - {dialect (merge {:require - (prune-libspecs required-libspecs symbols-in-file current-ns) - :import (prune-imports (some-> parsed-ns dialect :import) - symbols-in-file)} + set) + ;; `imports` are calculated before `requires`, because + ;; the former's needs affect whether the latter can be pruned: + imports (prune-imports (some-> parsed-ns dialect :import) + symbols-in-file) + requires (prune-libspecs required-libspecs symbols-in-file current-ns imports)] + {dialect (merge {:require requires + :import imports} (when (= dialect :cljs) {:require-macros - (prune-libspecs required-macro-libspecs symbols-in-file - current-ns)}))})) + (prune-libspecs required-macro-libspecs symbols-in-file current-ns #{})}))})) (defn- prune-cljc-dependencies [parsed-ns path] (merge diff --git a/test-resources/defines_deftype.clj b/test-resources/defines_deftype.clj new file mode 100644 index 00000000..6e9d097f --- /dev/null +++ b/test-resources/defines_deftype.clj @@ -0,0 +1,3 @@ +(ns defines-deftype) + +(defrecord SomeDefType []) diff --git a/test-resources/ns1.clj b/test-resources/ns1.clj index be0fe049..37201af4 100644 --- a/test-resources/ns1.clj +++ b/test-resources/ns1.clj @@ -14,11 +14,13 @@ (clojure data edn) [clojure.pprint :refer [get-pretty-writer formatter cl-format]] clojure.test.junit - [clojure.xml]) + [clojure.xml] + [defines-deftype]) (:use clojure.test clojure.test [clojure.string :rename {replace foo reverse bar} :reload-all true :reload true]) - (:import java.util.Random + (:import [defines_deftype SomeDefType] + java.util.Random java.io.PushbackReader java.io.PushbackReader java.io.FilenameFilter @@ -30,6 +32,8 @@ SomeClass$InnerClass$InnerInnerClassThree] (java.util Date Calendar))) +(SomeDefType.) + (defmacro tt [writer] (Random.) `(get-pretty-writer ~writer)) diff --git a/test-resources/ns1_cleaned.clj b/test-resources/ns1_cleaned.clj index 64fed56f..ae52cd69 100644 --- a/test-resources/ns1_cleaned.clj +++ b/test-resources/ns1_cleaned.clj @@ -15,8 +15,10 @@ [test :refer :all] [walk :refer [postwalk prewalk]] xml] - clojure.test.junit) + clojure.test.junit + [defines-deftype]) (:import + [defines_deftype SomeDefType] [java.io Closeable FilenameFilter PushbackReader] [java.util Calendar Date Random] [refactor.nrepl diff --git a/test-resources/ns1_cleaned_and_pprinted b/test-resources/ns1_cleaned_and_pprinted index d597267b..1101c4b5 100644 --- a/test-resources/ns1_cleaned_and_pprinted +++ b/test-resources/ns1_cleaned_and_pprinted @@ -17,8 +17,10 @@ [clojure.test :refer :all] [clojure.test.junit] [clojure.walk :refer [postwalk prewalk]] - [clojure.xml]) + [clojure.xml] + [defines-deftype]) (:import + (defines_deftype SomeDefType) (java.io Closeable FilenameFilter PushbackReader) (java.util Calendar Date Random) (refactor.nrepl SomeClass$InnerClass$InnerInnerClassOne SomeClass$InnerClass$InnerInnerClassTwo))) diff --git a/test-resources/ns1_cleaned_and_pprinted_prefix_notation b/test-resources/ns1_cleaned_and_pprinted_prefix_notation index 95b52a81..ee6c66f7 100644 --- a/test-resources/ns1_cleaned_and_pprinted_prefix_notation +++ b/test-resources/ns1_cleaned_and_pprinted_prefix_notation @@ -15,8 +15,10 @@ [string :refer :all :reload-all true] [test :refer :all] [walk :refer [postwalk prewalk]]] - [clojure.test.junit]) + [clojure.test.junit] + [defines-deftype]) (:import + (defines_deftype SomeDefType) (java.io Closeable FilenameFilter PushbackReader) (java.util Calendar Date Random) (refactor.nrepl SomeClass$InnerClass$InnerInnerClassOne SomeClass$InnerClass$InnerInnerClassTwo))) diff --git a/test/refactor_nrepl/ns/prune_dependencies_test.clj b/test/refactor_nrepl/ns/prune_dependencies_test.clj new file mode 100644 index 00000000..209866ef --- /dev/null +++ b/test/refactor_nrepl/ns/prune_dependencies_test.clj @@ -0,0 +1,62 @@ +(ns refactor-nrepl.ns.prune-dependencies-test + (:require + [clojure.test :refer [are deftest]] + [refactor-nrepl.ns.prune-dependencies :as sut])) + +(deftest imports->namespaces + (are [input expected] (= expected + (sut/imports->namespaces input)) + ['java.io.File] #{'java.io} + ['my_ns.File] #{'my-ns} + ['[java.io File]] #{'java.io} + ['[java.io File FileReader]] #{'java.io} + ['(java.io File)] #{'java.io} + ['java.io.File + 'my_ns.File + '[java.io File] + '(java.io File)] #{'java.io 'my-ns})) + +(deftest libspec->namespaces + (are [input expected] (= expected + (sut/libspec->namespaces input)) + 'foo.bar + '[foo.bar], + + '[foo.bar] + '[foo.bar], + + '[foo.bar :as bar] + '[foo.bar], + + '[clojure data edn + [instant :as inst :reload true] + [pprint :refer [cl-format formatter get-pretty-writer]] + [string :refer :all :reload-all true] + [test :refer :all] + [walk :refer [postwalk prewalk]] + xml] + '[clojure.data clojure.edn clojure.instant clojure.pprint clojure.string clojure.test clojure.walk clojure.xml])) + +(deftest imports-contain-libspec? + (are [imports libspec expected] (= expected + (sut/imports-contain-libspec? (sut/imports->namespaces imports) + libspec)) + #_imports #_libspec #_expected + '[] 'foo.bar false + '[foo.bar.SomeType] 'foo.bar true + '[foo_bar.SomeType] 'foo-bar true + '[[foo.bar SomeType]] 'foo.bar true + '[[foo_bar SomeType]] 'foo-bar true + '[foo.bar.SomeType] 'foo.baz false + '[] '[foo.bar] false + '[foo.bar.SomeType] '[foo.bar] true + '[foo.bar.SomeType] '[foo.bar :as f] true + '[[foo.bar SomeType]] '[foo.bar] true + '[[foo_bar SomeType]] '[foo-bar :as f] true + '[foo_bar.SomeType] '[foo-bar :as f] true + '[[foo_bar SomeType]] '[foo-bar] true + '[[foo_bar SomeType]] '[foo-bar :as f] true + '[foo.bar.SomeType] '[foo.baz] false + '[clojure.data.Data] '[clojure data] true + '[clojure.data.Data] '[clojure [data :as d]] true + '[clojure.data.Data] '[clojure foo] false))