clean-ns now sorts ns children forms according to the ClojureStyleG…

…uide

CHANGELOG.md

@@ -2,6 +2,9 @@
 
 ## Unreleased
 
+- General
+  - `clean-ns` now sorts ns children forms according to the ClojureStyleGuide, at the moment moving require form before import form, enabled by default under flag `:clean :sort :ns` setting.
+
 - Editor
   - Improve hover output, both markdown and plain text forms.
   - Allow calling thread-first/last and thread-first-all/last-all from within the list.

docs/settings.md

@@ -137,7 +137,8 @@ Default:
 
 ```clojure
 {:ns-inner-blocks-indenation :next-line
- :sort {:require true
+ :sort {:ns true
+        :require true
         :import true
         :refer true}}
 ```
@@ -148,6 +149,10 @@ Where to place first require/import following [Clojure Style Guide](https://gith
 
 #### sort
 
+##### ns
+
+Whether to enable sort of `ns` children like require,import forms following [Clojure Style Guide](https://github.com/bbatsov/clojure-style-guide#comprehensive-ns-declaration).
+
 ##### require
 
 Whether to enable sort of `:require` form.

src/clojure_lsp/common_symbols.clj

@@ -1353,11 +1353,16 @@
    'facts        'midje.sweet})
 
 (def common-alias->info
-  {'str    'clojure.string
-   'repl   'clojure.repl
+  {'async  'clojure.core.async
+   'csv    'clojure.data.csv
+   'xml    'clojure.data.xml
    'edn    'clojure.edn
+   'io     'clojure.java.io
+   'sh     'clojure.java.shell
+   'pprint 'clojure.pprint
+   'repl   'clojure.repl
    'set    'clojure.set
+   'spec   'clojure.spec.alpha
+   'str    'clojure.string
    'walk   'clojure.walk
-   'pprint 'clojure.pprint
-   'async  'clojure.core.async
-   'io     'clojure.java.io})
+   'zip    'clojure.zip})

src/clojure_lsp/feature/diagnostics.clj

@@ -82,15 +82,14 @@
                  (contains? (symbol (-> var :ns str) (-> var :name str))))))))
 
 (defn ^:private lint-public-vars [filename analysis settings]
-  (when (not (= :off (get-in settings [:linters :unused-public-var :level])))
-    (let [start-time (System/nanoTime)
-          public-vars (->> (q/find-var-definitions analysis filename false)
-                           (filter (partial exclude-public-var? settings))
-                           (filter (comp #(= (count %) 0)
-                                         #(q/find-references-from-cursor analysis filename (:name-row %) (:name-col %) false)))
-                           (mapv (partial unused-public-var->diagnostic settings)))]
-      (log/info (format "Linting public vars took %sms" (quot (- (System/nanoTime) start-time) 1000000)))
-      public-vars)))
+  (let [start-time (System/nanoTime)
+        public-vars (->> (q/find-var-definitions analysis filename false)
+                         (filter (partial exclude-public-var? settings))
+                         (filter (comp #(= (count %) 0)
+                                       #(q/find-references-from-cursor analysis filename (:name-row %) (:name-col %) false)))
+                         (mapv (partial unused-public-var->diagnostic settings)))]
+    (log/info (format "Linting public vars took %sms" (quot (- (System/nanoTime) start-time) 1000000)))
+    public-vars))
 
 (defn ^:private find-diagnostics [uri db]
   (let [settings (get db :settings)

src/clojure_lsp/refactor/transform.clj

@@ -330,7 +330,8 @@
       (z/subedit-> ns-loc
                    (z/find-value z/next form-type)
                    (z/up)
-                   (z/replace (n/list-node forms))))))
+                   (z/replace (with-meta (n/list-node forms)
+                                         (-> removed-nodes z/node meta)))))))
 
 (defn ^:private remove-unused-refers
   [node unused-refers]
@@ -466,9 +467,34 @@
         :same-line
         :next-line)))
 
+(defn ^:private sort-ns-children
+  [ns-loc settings]
+  (if (get-in settings [:clean :sort :ns] true)
+    (let [require-loc (-> ns-loc
+                          (z/find-next-depth-first #(and (= :list (z/tag %))
+                                                         (= :require (z/sexpr (z/next %))))))
+          import-loc (-> ns-loc
+                         (z/find-next-depth-first #(and (= :list (z/tag %))
+                                                        (= :import (z/sexpr (z/next %))))))]
+      (if (and require-loc
+               import-loc
+               (< (-> import-loc z/node meta :row)
+                  (-> require-loc z/node meta :row)))
+        (z/subedit-> ns-loc
+                     (z/find-next-value z/next :import)
+                     z/up
+                     (z/replace (z/node require-loc))
+                     z/right
+                     (z/find-next-value z/next :require)
+                     z/up
+                     (z/replace (z/node import-loc)))
+        ns-loc))
+    ns-loc))
+
 (defn clean-ns
   [zloc uri]
-  (let [safe-loc (or zloc (z/of-string (get-in @db/db [:documents uri :text])))
+  (let [settings (:settings @db/db)
+        safe-loc (or zloc (z/of-string (get-in @db/db [:documents uri :text])))
         ns-loc (edit/find-namespace safe-loc)]
     (when ns-loc
       (let [ns-inner-blocks-indentation (resolve-ns-inner-blocks-identation @db/db)
@@ -478,7 +504,8 @@
             unused-imports (q/find-unused-imports (:findings @db/db) filename)
             result-loc (-> ns-loc
                            (clean-requires unused-aliases unused-refers ns-inner-blocks-indentation)
-                           (clean-imports unused-imports ns-inner-blocks-indentation))]
+                           (clean-imports unused-imports ns-inner-blocks-indentation)
+                           (sort-ns-children settings))]
         [{:range (meta (z/node result-loc))
           :loc result-loc}]))))
 

test/clojure_lsp/refactor/transform_test.clj

@@ -527,7 +527,61 @@
                      (code "(ns foo.bar"
                            " (:require"
                            "  [some :refer [foo bar baz]]))"
-                           "   foo bar baz"))))
+                           "   foo bar baz")))
+    (testing "ns children sorting"
+      (testing "import before require"
+        (test-clean-ns {}
+                       (code "(ns foo.bar"
+                             " (:import"
+                             "  [foo Bar])"
+                             " (:require"
+                             "  [clojure.string :as str]))"
+                             "str/join"
+                             "Bar")
+                       (code "(ns foo.bar"
+                             " (:require"
+                             "  [clojure.string :as str])"
+                             " (:import"
+                             "  [foo Bar]))"
+                             "str/join"
+                             "Bar")))
+      (testing "import before require with other list between"
+        (test-clean-ns {}
+                       (code "(ns foo.bar"
+                             " (:import"
+                             "  [foo Bar"
+                             "       Baz"
+                             "       Qux])"
+                             " (:refer-clojure :exclude [next])"
+                             " (:require"
+                             "  [clojure.string :as str]))"
+                             "str/join"
+                             "Bar Qux")
+                       (code "(ns foo.bar"
+                             " (:require"
+                             "  [clojure.string :as str])"
+                             " (:refer-clojure :exclude [next])"
+                             " (:import"
+                             "  [foo Bar"
+                             "       Qux]))"
+                             "str/join"
+                             "Bar Qux")))
+      (testing "don't sort when :ns sort config is disabled"
+        (test-clean-ns {:settings {:clean {:sort {:ns false}}}}
+                       (code "(ns foo.bar"
+                             " (:import"
+                             "  [foo Bar])"
+                             " (:require"
+                             "  [clojure.string :as str]))"
+                             "str/join"
+                             "Bar")
+                       (code "(ns foo.bar"
+                             " (:import"
+                             "  [foo Bar])"
+                             " (:require"
+                             "  [clojure.string :as str]))"
+                             "str/join"
+                             "Bar")))))
   (testing "single unused full package import"
     (test-clean-ns {}
                    (code "(ns foo.bar"