Repository: clojure-lsp/clojure-lsp
Pull request #249 (2021-01-10 20:53:32)

Support clean unused imports
- Remove unused java imports

![clojure-clean-import](https://user-images.githubusercontent.com/7820865/104135115-ad0fcf80-536c-11eb-8f45-e9d148cb4966.gif)



Patch:
diff --git a/src/clojure_lsp/crawler.clj b/src/clojure_lsp/crawler.clj
index 116e61eb..1f1301e4 100644
--- a/src/clojure_lsp/crawler.clj
+++ b/src/clojure_lsp/crawler.clj
@@ -52,6 +52,17 @@
          set
          (set/difference (set (map :sym declared-refers))))))
 
+(defn find-unused-imports [usages]
+  (let [excludes (-> (get-in @db/db [:settings :linters :unused-import :exclude] #{}) set)
+        declared-imports (->> usages
+                              (filter (comp #(contains? % :import) :tags))
+                              (remove (comp excludes :sym)))]
+    (->> usages
+         (filter (comp #(contains? % :java) :tags))
+         (map :sym)
+         set
+         (set/difference (set (map :sym declared-imports))))))
+
 (defn crawl-jars [jars dependency-scheme]
   (let [jar-dependency-scheme? (= "jar" dependency-scheme)
         xf (comp
diff --git a/src/clojure_lsp/refactor/transform.clj b/src/clojure_lsp/refactor/transform.clj
index 4eca8e46..f5beb31b 100644
--- a/src/clojure_lsp/refactor/transform.clj
+++ b/src/clojure_lsp/refactor/transform.clj
@@ -1,5 +1,6 @@
 (ns clojure-lsp.refactor.transform
   (:require
+   [clojure-lsp.clojure-core :as cc]
    [clojure-lsp.crawler :as crawler]
    [clojure-lsp.db :as db]
    [clojure-lsp.feature.references :as f.references]
@@ -11,9 +12,7 @@
    [rewrite-clj.custom-zipper.core :as cz]
    [rewrite-clj.node :as n]
    [rewrite-clj.zip :as z]
-   [rewrite-clj.zip.subedit :as zsub]
-   [clojure.tools.logging :as log]
-   [clojure-lsp.clojure-core :as cc]))
+   [rewrite-clj.zip.subedit :as zsub]))
 
 (defn result [zip-edits]
   (mapv (fn [zip-edit]
@@ -256,6 +255,32 @@
                        (z/insert-child 'let)
                        (edit/join-let))}]))))))
 
+(defn ^:private process-clean-ns
+  [ns-loc removed-nodes col keep-at-start? form-type]
+  (let [sep (n/whitespace-node (apply str (repeat col " ")))
+        single-space (n/whitespace-node " ")
+        imports (->> removed-nodes
+                        z/node
+                        n/children
+                        (remove n/printable-only?)
+                        (sort-by (comp str n/sexpr))
+                        (map-indexed (fn [idx node]
+                                       (if (and keep-at-start?
+                                                (= idx 0))
+                                         [single-space node]
+                                         [(n/newlines 1) sep node])))
+                        (apply concat)
+                        (cons (n/keyword-node form-type)))]
+    (if (empty? (z/child-sexprs removed-nodes))
+                       (z/subedit-> ns-loc
+                                    (z/find-value z/next form-type)
+                                    (z/up)
+                                    z/remove)
+                       (z/subedit-> ns-loc
+                                    (z/find-value z/next form-type)
+                                    (z/up)
+                                    (z/replace (n/list-node imports))))))
+
 (defn ^:private remove-unused-refers
   [node unused-refers]
   (let [node-refers (-> node z/down (z/find-next-value ':refer) z/right z/sexpr set)
@@ -304,53 +329,99 @@
                                  set))
         single-unused?  (when (and single-require? (z/vector? first-node))
                           (or (contains? unused-aliases first-node-ns)
-                              (set/subset? first-node-refers unused-refers)))]
+                              (and (seq first-node-refers)
+                                   (seq unused-refers)
+                                   (set/subset? first-node-refers unused-refers))))]
     (if single-unused?
       (z/remove first-node)
       (edit/map-children nodes #(remove-unused-require % unused-aliases unused-refers)))))
 
+(defn ^:private clean-requires
+  [ns-loc usages keep-at-start?]
+  (if-let [require-loc (z/find-value (zsub/subzip ns-loc) z/next :require)]
+    (let [col (if require-loc
+                (if keep-at-start?
+                  (-> require-loc z/node meta :end-col)
+                  (-> require-loc z/right z/node meta :col dec))
+                4)
+          unused-aliases (crawler/find-unused-aliases usages)
+          unused-refers (crawler/find-unused-refers usages)
+          removed-nodes (->> require-loc
+                             z/remove
+                             (remove-unused-requires unused-aliases unused-refers))]
+      (process-clean-ns ns-loc removed-nodes col keep-at-start? :require))
+    ns-loc))
+
+(defn ^:private package-import?
+  [node]
+  (or (z/vector? node)
+      (z/list? node)))
+
+(defn ^:private remove-unused-package-import
+  [node base-package col unused-imports]
+  (cond
+    (string/includes? (z/string node) ".")
+    node
+
+    (contains? unused-imports
+               (symbol (str base-package "." (z/string node))))
+    (z/remove node)
+
+    (z/rightmost? node)
+    node
+
+    :else
+    (if (contains? unused-imports
+                   (symbol (str base-package "." (z/string (z/right node)))))
+      node
+      (z/edit-> node
+                (z/insert-right (n/spaces (+ col (count base-package))))
+                (z/insert-right (n/newlines 1))))))
+
+(defn ^:private remove-unused-import
+  [parent-node col unused-imports]
+  (cond
+    (package-import? parent-node)
+    (let [base-package (-> parent-node z/down z/leftmost z/string)
+          removed (edit/map-children parent-node
+                                     #(remove-unused-package-import % base-package col unused-imports))]
+      (if (= 1 (count (z/child-sexprs removed)))
+        (z/remove removed)
+        removed))
+
+    (contains? unused-imports (z/sexpr parent-node))
+    (z/remove parent-node)
+
+    :else
+    parent-node))
+
+(defn ^:private clean-imports
+  [ns-loc usages keep-at-start?]
+  (if-let [import-loc (z/find-value (zsub/subzip ns-loc) z/next :import)]
+    (let [col (if import-loc
+                (if keep-at-start?
+                  (-> import-loc z/node meta :end-col)
+                  (-> import-loc z/right z/node meta :col dec))
+                4)
+          unused-imports (crawler/find-unused-imports usages)
+          removed-nodes (-> import-loc
+                             z/remove
+                             (edit/map-children #(remove-unused-import % col unused-imports)))]
+      (process-clean-ns ns-loc removed-nodes col keep-at-start? :import))
+    ns-loc))
+
 (defn clean-ns
   [zloc uri]
   (let [safe-loc (or zloc (z/of-string (get-in @db/db [:documents uri :text])))
-        ns-loc (edit/find-namespace safe-loc)
-        require-loc (z/find-value (zsub/subzip ns-loc) z/next :require)
-        keep-require-at-start? (get-in @db/db [:settings :keep-require-at-start?])
-        col (if require-loc
-              (if keep-require-at-start?
-                (-> require-loc z/node meta :end-col)
-                (-> require-loc z/right z/node meta :col dec))
-              4)
-        sep (n/whitespace-node (apply str (repeat col " ")))
-        single-space (n/whitespace-node " ")
-        usages (f.references/safe-find-references uri (slurp uri) false false)
-        unused-aliases (crawler/find-unused-aliases usages)
-        unused-refers (crawler/find-unused-refers usages)
-        removed-nodes (->> require-loc
-                           z/remove
-                           (remove-unused-requires unused-aliases unused-refers))
-        requires (->> removed-nodes
-                      z/node
-                      n/children
-                      (remove n/printable-only?)
-                      (sort-by (comp str n/sexpr))
-                      (map-indexed (fn [idx node]
-                                     (if (and keep-require-at-start?
-                                              (= idx 0))
-                                       [single-space node]
-                                       [(n/newlines 1) sep node])))
-                      (apply concat)
-                      (cons (n/keyword-node :require)))
-        result-loc (if (empty? (z/child-sexprs removed-nodes))
-                     (z/subedit-> ns-loc
-                                  (z/find-value z/next :require)
-                                  (z/up)
-                                  z/remove)
-                     (z/subedit-> ns-loc
-                                  (z/find-value z/next :require)
-                                  (z/up)
-                                  (z/replace (n/list-node requires))))]
-    [{:range (meta (z/node result-loc))
-      :loc result-loc}]))
+        ns-loc (edit/find-namespace safe-loc)]
+    (when ns-loc
+      (let [keep-at-start? (get-in @db/db [:settings :keep-require-at-start?])
+            usages (f.references/safe-find-references uri (slurp uri) false false)
+            result-loc (-> ns-loc
+                           (clean-requires usages keep-at-start?)
+                           (clean-imports usages keep-at-start?))]
+        [{:range (meta (z/node result-loc))
+          :loc result-loc}]))))
 
 (defn ^:private add-form-to-namespace [zloc form-to-add form-type form-to-check-exists]
   (let [ns-loc (edit/find-namespace zloc)


Test patch:
diff --git a/test/clojure_lsp/crawler_test.clj b/test/clojure_lsp/crawler_test.clj
index e698e8a3..43c0160c 100644
--- a/test/clojure_lsp/crawler_test.clj
+++ b/test/clojure_lsp/crawler_test.clj
@@ -1,54 +1,93 @@
 (ns clojure-lsp.crawler-test
   (:require
-   [clojure-lsp.crawler :as crawler]
-   [clojure-lsp.feature.references :as f.references]
-   [clojure.test :refer :all]))
+    [clojure-lsp.crawler :as crawler]
+    [clojure-lsp.db :as db]
+    [clojure-lsp.feature.references :as f.references]
+    [clojure.string :as string]
+    [clojure.test :refer :all]))
 
-(def unused-alias-code
-  "(ns foo.bar
-     (:require [foo.baz :as baz]
-               [foo.bah :as bah]))
-   (def zas (bah/some))")
+(defn code [& strings] (string/join "\n" strings))
 
 (deftest find-unused-aliases-test
   (testing "When there is unused aliases"
-    (let [usages (f.references/safe-find-references "file://a.clj" unused-alias-code false false)]
+    (let [code (code "(ns foo.bar"
+                     "  (:require [foo.baz :as baz]"
+                     "  [foo.bah :as bah]))"
+                     "(def zas (bah/some))")
+          usages (f.references/safe-find-references "file://a.clj" code false false)]
       (is (= #{'foo.baz}
              (crawler/find-unused-aliases usages))))))
 
-(def unused-refer-code
-  "(ns foo.bar
-     (:require [foo.baz :refer [other]]
-               [foo.bah :as bah]))
-   (def zas (bah/some))")
-
-(def unused-refer-with-used-refers-on-other-require-code
-  "(ns foo.bar
-     (:require [foo.baz :refer [other]]
-               [foo.bao :refer [another]]
-               [foo.bah :as bah]))
-   (def zas
-     (bah/some)
-     (another))")
-
-(def unused-refer-with-used-refers-on-same-require-code
-  "(ns foo.bar
-     (:require [foo.baz :refer [other another some-other]]
-               [foo.bah :as bah]))
-   (def zas
-     (bah/some)
-     (another))")
-
 (deftest find-unused-refers-test
   (testing "When there is a unused refer"
-    (let [usages (f.references/safe-find-references  "file://a.clj" unused-refer-code false false)]
+    (let [code (code "(ns foo.bar"
+                     "  (:require [foo.baz :refer [other]]"
+                     "            [foo.bah :as bah]))"
+                     "(def zas (bah/some))")
+          usages (f.references/safe-find-references  "file://a.clj" code false false)]
       (is (= #{'foo.baz/other}
              (crawler/find-unused-refers usages)))))
   (testing "When there is a unused refer but used refers on other require"
-    (let [usages (f.references/safe-find-references  "file://a.clj" unused-refer-with-used-refers-on-other-require-code false false)]
+    (let [code (code "(ns foo.bar"
+                     "  (:require [foo.baz :refer [other]]"
+                     "            [foo.bao :refer [another]]"
+                     "            [foo.bah :as bah]))"
+                     "(def zas"
+                     "(bah/some)"
+                     "(another))")
+          usages (f.references/safe-find-references  "file://a.clj" code false false)]
       (is (= #{'foo.baz/other}
              (crawler/find-unused-refers usages)))))
   (testing "When there is a unused refer but used refers on same require"
-    (let [usages (f.references/safe-find-references  "file://a.clj" unused-refer-with-used-refers-on-same-require-code false false)]
+    (let [code (code "(ns foo.bar"
+                     "  (:require [foo.baz :refer [other another some-other]]"
+                     "            [foo.bah :as bah]))"
+                     "(def zas"
+                     "(bah/some)"
+                     "(another))")
+          usages (f.references/safe-find-references  "file://a.clj" code false false)]
       (is (= #{'foo.baz/other 'foo.baz/some-other}
              (crawler/find-unused-refers usages))))))
+
+(deftest find-unused-imports-test
+  (testing "When there is a used full import"
+    (reset! db/db {})
+    (let [code (code "(ns foo.bar"
+                     "  (:import java.util.Date))"
+                     "Date.")
+          usages (f.references/safe-find-references  "file://a.clj" code false false)]
+      (is (= #{}
+             (crawler/find-unused-imports usages)))))
+  (testing "When there is a unused full import"
+    (reset! db/db {})
+    (let [code (code "(ns foo.bar"
+                     "  (:import java.util.Date))")
+          usages (f.references/safe-find-references  "file://a.clj" code false false)]
+      (is (= #{'java.util.Date}
+             (crawler/find-unused-imports usages)))))
+  (testing "When there is a unused full import on exclude settings"
+    (reset! db/db {:settings {:linters {:unused-import {:exclude '[java.util.Date]}}}})
+    (let [code (code "(ns foo.bar"
+                     "  (:import java.util.Date))"
+                     "Date.")
+          usages (f.references/safe-find-references  "file://a.clj" code false false)]
+      (is (= #{}
+             (crawler/find-unused-imports usages)))))
+  (testing "When there is a unused and used full import"
+    (reset! db/db {})
+    (let [code (code "(ns foo.bar"
+                     "  (:import java.util.Date java.util.Calendar))"
+                     "Date.")
+          usages (f.references/safe-find-references  "file://a.clj" code false false)]
+      (is (= #{'java.util.Calendar}
+             (crawler/find-unused-imports usages)))))
+  (testing "When there is multiple unused imports"
+    (reset! db/db {})
+    (let [code (code "(ns foo.bar"
+                     "  (:import java.time.LocalDate java.time.LocalDateTime"
+                     "           [java.util Date"
+                     "                      Calendar]))"
+                     "LocalDate.")
+          usages (f.references/safe-find-references  "file://a.clj" code false false)]
+      (is (= '#{java.util.Date java.util.Calendar java.time.LocalDateTime}
+             (crawler/find-unused-imports usages))))))
diff --git a/test/clojure_lsp/refactor/transform_test.clj b/test/clojure_lsp/refactor/transform_test.clj
index a815ed49..135b13cd 100644
--- a/test/clojure_lsp/refactor/transform_test.clj
+++ b/test/clojure_lsp/refactor/transform_test.clj
@@ -295,6 +295,18 @@
                    (code "(ns foo.bar)"
                          "(defn func []"
                          "  (b/some))")))
+  (testing "with single used require on ns"
+    (test-clean-ns {}
+                   (code "(ns foo.bar"
+                         " (:require"
+                         "   [foo  :as f] ))"
+                         "(defn func []"
+                         "  (f/some))")
+                   (code "(ns foo.bar"
+                         " (:require"
+                         "   [foo  :as f]))"
+                         "(defn func []"
+                         "  (f/some))")))
   (testing "with multiple unused requires on ns"
     (test-clean-ns {}
                    (code "(ns foo.bar"
@@ -400,7 +412,80 @@
                            " (:require"
                            "   [bar :refer [some] ]"
                            "   [baz :as b]))")
-                     (code "(ns foo.bar)"))))
+                     (code "(ns foo.bar)")))
+  (testing "single unused full package import"
+    (test-clean-ns {}
+                   (code "(ns foo.bar"
+                         " (:import"
+                         "  java.util.Date))")
+                   (code "(ns foo.bar)")))
+  (testing "single unused package import"
+    (test-clean-ns {}
+                   (code "(ns foo.bar"
+                         " (:import"
+                         "  [java.util Date]))")
+                   (code "(ns foo.bar)")))
+  (testing "unused full package imports"
+    (test-clean-ns {}
+                   (code "(ns foo.bar"
+                         " (:import "
+                         "  java.util.Date java.util.Calendar java.util.List))"
+                         "Calendar.")
+                   (code "(ns foo.bar"
+                         " (:import"
+                         "  java.util.Calendar))"
+                         "Calendar.")))
+  (testing "unused package imports"
+    (test-clean-ns {}
+                   (code "(ns foo.bar"
+                         " (:import "
+                         "  [java.util Date Calendar List Map]))"
+                         "Calendar."
+                         "Map.")
+                   (code "(ns foo.bar"
+                         " (:import"
+                         "  [java.util Calendar Map]))"
+                         "Calendar."
+                         "Map.")))
+  (testing "unused package imports with keep-at-start?"
+    (test-clean-ns {:settings {:keep-require-at-start? true}}
+                   (code "(ns foo.bar"
+                         " (:import [java.util Date Calendar List Map]))"
+                         "Calendar."
+                         "Map.")
+                   (code "(ns foo.bar"
+                         " (:import [java.util Calendar Map]))"
+                         "Calendar."
+                         "Map.")))
+  (testing "unused package imports with single import"
+    (test-clean-ns {}
+                   (code "(ns foo.bar"
+                         " (:import"
+                         "  [java.util Date List]"
+                         "  java.util.Calendar))"
+                         "Calendar."
+                         "List.")
+                   (code "(ns foo.bar"
+                         " (:import"
+                         "  [java.util List]"
+                         "  java.util.Calendar))"
+                         "Calendar."
+                         "List.")))
+  (testing "unused package imports spacing"
+    (test-clean-ns {}
+                   (code "(ns foo.bar"
+                         " (:import"
+                         "  [java.util Date"
+                         "             Calendar"
+                         "             List]))"
+                         "Date."
+                         "List.")
+                   (code "(ns foo.bar"
+                         " (:import"
+                         "  [java.util Date"
+                         "             List]))"
+                         "Date."
+                         "List."))))
 
 (deftest add-missing-libspec
   (testing "aliases"