From 75f22a57b97fa64bca33248d36fae0edd0b44124 Mon Sep 17 00:00:00 2001
From: Davide Taviani <helios89@gmail.com>
Date: Tue, 28 Jan 2020 15:46:04 +0100
Subject: [PATCH] [#708] Unsorted namespaces linter

---
 .clj-kondo/config.edn                         |  3 +-
 doc/config.md                                 |  1 +
 doc/dev.md                                    |  5 +++
 src/clj_kondo/impl/analyzer.clj               | 27 ++++++-------
 src/clj_kondo/impl/analyzer/namespace.clj     | 13 ++++---
 src/clj_kondo/impl/analyzer/spec.clj          |  8 ++--
 src/clj_kondo/impl/analyzer/test.clj          |  4 +-
 src/clj_kondo/impl/analyzer/usages.clj        | 11 +++---
 src/clj_kondo/impl/config.clj                 |  1 +
 src/clj_kondo/impl/linters.clj                | 12 +++---
 src/clj_kondo/impl/linters/misc.clj           | 24 ------------
 src/clj_kondo/impl/macroexpand.clj            |  5 +--
 src/clj_kondo/impl/metadata.clj               |  6 +--
 src/clj_kondo/impl/namespace.clj              | 38 ++++++++++++++++++-
 src/clj_kondo/impl/parser.clj                 |  4 +-
 src/clj_kondo/impl/utils.clj                  | 11 ++++++
 test/clj_kondo/impl/extract_var_info_test.clj |  4 +-
 test/clj_kondo/main_test.clj                  | 21 ++++++++++
 test/clj_kondo/test_test.clj                  |  6 +--
 test/clj_kondo/test_utils.clj                 |  3 +-
 test/clj_kondo/unresolved_symbol_test.clj     |  8 ++--
 21 files changed, 134 insertions(+), 81 deletions(-)
 delete mode 100644 src/clj_kondo/impl/linters/misc.clj

diff --git a/.clj-kondo/config.edn b/.clj-kondo/config.edn
index 3c513f99ab..3450e4ed6b 100644
--- a/.clj-kondo/config.edn
+++ b/.clj-kondo/config.edn
@@ -17,7 +17,8 @@
                                                 :ret :map}}}}
              clj-kondo.impl.config #include "../src/clj_kondo/impl/config.types.edn"
              clj-kondo.impl.findings #include "../src/clj_kondo/impl/findings.types.edn"}}
-           :missing-docstring {:level :off}}
+           :missing-docstring {:level :off}
+           :unsorted-namespaces {:level :warning}}
  :lint-as {me.raynes.conch/programs clojure.core/declare
            me.raynes.conch/let-programs clojure.core/let}
  :output {:exclude-files ["src/clj_kondo/impl/rewrite_clj_patch.clj"]}}
diff --git a/doc/config.md b/doc/config.md
index 1d6928e011..7d212178d0 100644
--- a/doc/config.md
+++ b/doc/config.md
@@ -37,6 +37,7 @@ linting took 10ms, errors: 0, warnings: 0
 Some linters are not enabled by default. Right now these linters are:
 
 - `:missing-docstring`: warn when public var doesn't have a docstring.
+- `:unsorted-namespaces`: warn when namespaces in `:require` are not sorted.
 
 You can enable these linters by setting the `:level`:
 
diff --git a/doc/dev.md b/doc/dev.md
index 8873b32a88..efdefae733 100644
--- a/doc/dev.md
+++ b/doc/dev.md
@@ -22,6 +22,11 @@
    3. Configuration spread across multiple files is harder to debug.
 
  6) Clj-kondo should be unobtrusive. Users of clj-kondo should not have to change their code only to make the linter happy. Team members who do not wish to use clj-kondo should not be confronted with clj-kondo-related annotations in their code.
+ 
+## Adding a new linter
+If you wish to add a new linter, do not forget to add the appropriate keyword in `clj-kondo.impl.config/default-config`, the map that defines the default configuration.
+
+This is necessary because only the linters with a keyword in the default config appear in the report.  
 
 ## PR
 
diff --git a/src/clj_kondo/impl/analyzer.clj b/src/clj_kondo/impl/analyzer.clj
index e5c9cd3e33..3baa6c53f3 100644
--- a/src/clj_kondo/impl/analyzer.clj
+++ b/src/clj_kondo/impl/analyzer.clj
@@ -2,7 +2,10 @@
   {:no-doc true}
   (:refer-clojure :exclude [ns-name])
   (:require
+   [clj-kondo.impl.analyzer.common :refer [common]]
+   [clj-kondo.impl.analyzer.compojure :as compojure]
    [clj-kondo.impl.analyzer.core-async :as core-async]
+   [clj-kondo.impl.analyzer.datalog :as datalog]
    [clj-kondo.impl.analyzer.namespace :as namespace-analyzer
     :refer [analyze-ns-decl]]
    [clj-kondo.impl.analyzer.potemkin :as potemkin]
@@ -23,10 +26,7 @@
    [clj-kondo.impl.utils :as utils :refer
     [symbol-call node->line parse-string tag select-lang deep-merge one-of
      linter-disabled? tag sexpr string-from-token assoc-some ctx-with-bindings]]
-   [clojure.string :as str]
-   [clj-kondo.impl.analyzer.datalog :as datalog]
-   [clj-kondo.impl.analyzer.common :refer [common]]
-   [clj-kondo.impl.analyzer.compojure :as compojure]))
+   [clojure.string :as str]))
 
 (set! *warn-on-reflection* true)
 
@@ -930,15 +930,16 @@
   (let [ns-name (-> ctx :ns :name)
         children (:children expr)
         require-node (first children)
-        children (next children)]
-    (doseq [child children]
-      (if (= :quote (tag child))
-        (when-let [libspec-expr (first (:children child))]
-          (let [analyzed
-                (namespace-analyzer/analyze-require-clauses ctx ns-name [[require-node [libspec-expr]]])]
-            (namespace/reg-required-namespaces! ctx ns-name analyzed)))
-        (analyze-children ctx children)))
-    (analyze-children ctx children)))
+        children (next children)
+        [quoted-children non-quoted-children]
+        (utils/filter-remove #(= :quote (tag %))
+                             children)
+        libspecs (map #(first (:children %)) quoted-children)]
+    (let [analyzed
+          (namespace-analyzer/analyze-require-clauses ctx ns-name [[require-node libspecs]])]
+      (namespace/reg-required-namespaces! ctx ns-name analyzed))
+    ;; also analyze children that weren't quoted
+    (analyze-children ctx non-quoted-children)))
 
 (defn analyze-import-libspec [ctx ns-name expr]
   (let [libspec-expr (if (= :quote (tag expr))
diff --git a/src/clj_kondo/impl/analyzer/namespace.clj b/src/clj_kondo/impl/analyzer/namespace.clj
index e46775223c..5662586e79 100644
--- a/src/clj_kondo/impl/analyzer/namespace.clj
+++ b/src/clj_kondo/impl/analyzer/namespace.clj
@@ -5,7 +5,6 @@
    [clj-kondo.impl.analysis :as analysis]
    [clj-kondo.impl.analyzer.common :as common]
    [clj-kondo.impl.findings :as findings]
-   [clj-kondo.impl.linters.misc :refer [lint-duplicate-requires!]]
    [clj-kondo.impl.metadata :as meta]
    [clj-kondo.impl.namespace :as namespace]
    [clj-kondo.impl.utils :refer [node->line one-of tag sexpr vector-node
@@ -214,11 +213,13 @@
                                        :referred #{}})
                                acc))
                            {}
-                           analyzed)]
-    (lint-duplicate-requires! ctx (map (juxt :require-kw :ns) analyzed))
-    {:required (map (fn [req]
-                      (vary-meta (:ns req)
-                                 #(assoc % :alias (:as req)))) analyzed)
+                           analyzed)
+        required-namespaces (map (fn [req]
+                                   (vary-meta (:ns req)
+                                              #(assoc % :alias (:as req)))) analyzed)]
+    (namespace/lint-unsorted-namespaces! ctx required-namespaces)
+    (namespace/lint-duplicate-requires! ctx (map (juxt :require-kw :ns) analyzed))
+    {:required required-namespaces
      :qualify-ns (reduce (fn [acc sc]
                            (cond-> (assoc acc (:ns sc) (:ns sc))
                              (:as sc)
diff --git a/src/clj_kondo/impl/analyzer/spec.clj b/src/clj_kondo/impl/analyzer/spec.clj
index 420ea8822c..59893fa290 100644
--- a/src/clj_kondo/impl/analyzer/spec.clj
+++ b/src/clj_kondo/impl/analyzer/spec.clj
@@ -1,10 +1,10 @@
 (ns clj-kondo.impl.analyzer.spec
   {:no-doc true}
   (:require
-   [clj-kondo.impl.utils :as utils]
-   [clj-kondo.impl.findings :as findings]
-   [clj-kondo.impl.namespace :as namespace]
-   [clj-kondo.impl.linters.keys :as keys]))
+     [clj-kondo.impl.findings :as findings]
+     [clj-kondo.impl.linters.keys :as keys]
+     [clj-kondo.impl.namespace :as namespace]
+     [clj-kondo.impl.utils :as utils]))
 
 (defn analyze-fdef [{:keys [:analyze-children :ns] :as ctx} expr]
   (let [[sym-expr & body] (next (:children expr))
diff --git a/src/clj_kondo/impl/analyzer/test.clj b/src/clj_kondo/impl/analyzer/test.clj
index 073a3eb311..83f07045b1 100644
--- a/src/clj_kondo/impl/analyzer/test.clj
+++ b/src/clj_kondo/impl/analyzer/test.clj
@@ -1,8 +1,8 @@
 (ns clj-kondo.impl.analyzer.test
   {:no-doc true}
   (:require
-   [clj-kondo.impl.utils :as utils]
-   [clj-kondo.impl.analyzer.common :as common]))
+    [clj-kondo.impl.analyzer.common :as common]
+    [clj-kondo.impl.utils :as utils]))
 
 (defn analyze-deftest [ctx deftest-ns expr]
   (common/analyze-defn
diff --git a/src/clj_kondo/impl/analyzer/usages.clj b/src/clj_kondo/impl/analyzer/usages.clj
index 01343563ca..356061ff7b 100644
--- a/src/clj_kondo/impl/analyzer/usages.clj
+++ b/src/clj_kondo/impl/analyzer/usages.clj
@@ -2,12 +2,11 @@
   {:no-doc true}
   (:refer-clojure :exclude [ns-name])
   (:require
-   [clj-kondo.impl.namespace :as namespace]
-   [clj-kondo.impl.analyzer.common :as common]
-   [clj-kondo.impl.utils :as utils :refer
-    [tag one-of symbol-from-token tag kw->sym]]
-   [clj-kondo.impl.metadata :as meta]
-   [clojure.string :as str])
+    [clj-kondo.impl.analyzer.common :as common]
+    [clj-kondo.impl.metadata :as meta]
+    [clj-kondo.impl.namespace :as namespace]
+    [clj-kondo.impl.utils :as utils :refer [tag one-of symbol-from-token tag kw->sym]]
+    [clojure.string :as str])
   (:import [clj_kondo.impl.rewrite_clj.node.seq NamespacedMapNode]))
 
 (set! *warn-on-reflection* true)
diff --git a/src/clj_kondo/impl/config.clj b/src/clj_kondo/impl/config.clj
index 66ecad1818..a1445874ec 100644
--- a/src/clj_kondo/impl/config.clj
+++ b/src/clj_kondo/impl/config.clj
@@ -33,6 +33,7 @@
               :unbound-destructuring-default {:level :warning}
               :unused-binding {:level :warning
                                :exclude-destructured-keys-in-fn-args false}
+              :unsorted-namespaces {:level :off}
               :unused-namespace {:level :warning
                                  ;; don't warn about these namespaces:
                                  :exclude [#_clj-kondo.impl.var-info-gen]
diff --git a/src/clj_kondo/impl/linters.clj b/src/clj_kondo/impl/linters.clj
index 7b846dd676..e111b4125c 100644
--- a/src/clj_kondo/impl/linters.clj
+++ b/src/clj_kondo/impl/linters.clj
@@ -1,15 +1,15 @@
 (ns clj-kondo.impl.linters
   {:no-doc true}
   (:require
-   [clj-kondo.impl.utils :as utils :refer [node->line constant? sexpr]]
-   [clj-kondo.impl.var-info :as var-info]
-   [clj-kondo.impl.config :as config]
    [clj-kondo.impl.analysis :as analysis]
+   [clj-kondo.impl.config :as config]
    [clj-kondo.impl.findings :as findings]
-   [clojure.set :as set]
    [clj-kondo.impl.namespace :as namespace]
-   [clojure.string :as str]
-   [clj-kondo.impl.types :as types]))
+   [clj-kondo.impl.types :as types]
+   [clj-kondo.impl.utils :as utils :refer [node->line constant? sexpr]]
+   [clj-kondo.impl.var-info :as var-info]
+   [clojure.set :as set]
+   [clojure.string :as str]))
 
 (set! *warn-on-reflection* true)
 
diff --git a/src/clj_kondo/impl/linters/misc.clj b/src/clj_kondo/impl/linters/misc.clj
deleted file mode 100644
index d2ca0242bb..0000000000
--- a/src/clj_kondo/impl/linters/misc.clj
+++ /dev/null
@@ -1,24 +0,0 @@
-(ns clj-kondo.impl.linters.misc
-  {:no-doc true}
-  (:require
-   [clj-kondo.impl.findings :as findings]
-   [clj-kondo.impl.utils :refer [node->line]]))
-
-(defn lint-duplicate-requires!
-  ([ctx namespaces] (lint-duplicate-requires! ctx #{} namespaces))
-  ([ctx init namespaces]
-   (reduce (fn [required ns]
-             (if (contains? required ns)
-               (let [ns (if (symbol? ns) ns (second ns))]
-                 (findings/reg-finding!
-                  (:findings ctx)
-                  (node->line (:filename ctx)
-                              ns
-                              :warning
-                              :duplicate-require
-                              (str "duplicate require of " ns)))
-                 required)
-               (conj required ns)))
-           (set init)
-           namespaces)
-   nil))
diff --git a/src/clj_kondo/impl/macroexpand.clj b/src/clj_kondo/impl/macroexpand.clj
index b4010d26b6..f28527bdde 100644
--- a/src/clj_kondo/impl/macroexpand.clj
+++ b/src/clj_kondo/impl/macroexpand.clj
@@ -1,9 +1,8 @@
 (ns clj-kondo.impl.macroexpand
   {:no-doc true}
   (:require
-   [clj-kondo.impl.utils :refer [parse-string tag vector-node list-node
-                                 token-node]]
-   [clj-kondo.impl.profiler :as profiler]))
+    [clj-kondo.impl.profiler :as profiler]
+    [clj-kondo.impl.utils :refer [parse-string tag vector-node list-node token-node]]))
 
 (defn expand-> [_ctx expr]
   (profiler/profile
diff --git a/src/clj_kondo/impl/metadata.clj b/src/clj_kondo/impl/metadata.clj
index 6fe61ef415..05d97aa0df 100644
--- a/src/clj_kondo/impl/metadata.clj
+++ b/src/clj_kondo/impl/metadata.clj
@@ -1,9 +1,9 @@
 (ns clj-kondo.impl.metadata
   {:no-doc true}
   (:require
-   [clj-kondo.impl.linters.keys :as key-linter]
-   [clj-kondo.impl.utils :as utils]
-   [clj-kondo.impl.analyzer.common :as common]))
+    [clj-kondo.impl.analyzer.common :as common]
+    [clj-kondo.impl.linters.keys :as key-linter]
+    [clj-kondo.impl.utils :as utils]))
 
 (defn meta-node->map [ctx node]
   (let [s (utils/sexpr node)]
diff --git a/src/clj_kondo/impl/namespace.clj b/src/clj_kondo/impl/namespace.clj
index 2f05424246..421c816ce8 100644
--- a/src/clj_kondo/impl/namespace.clj
+++ b/src/clj_kondo/impl/namespace.clj
@@ -5,7 +5,6 @@
    [clj-kondo.impl.analysis :as analysis]
    [clj-kondo.impl.config :as config]
    [clj-kondo.impl.findings :as findings]
-   [clj-kondo.impl.linters.misc :refer [lint-duplicate-requires!]]
    [clj-kondo.impl.utils :refer [node->line deep-merge linter-disabled? one-of]]
    [clj-kondo.impl.var-info :as var-info]
    [clojure.string :as str])
@@ -13,6 +12,42 @@
 
 (set! *warn-on-reflection* true)
 
+(defn lint-duplicate-requires!
+  ([ctx namespaces] (lint-duplicate-requires! ctx #{} namespaces))
+  ([ctx init namespaces]
+   (reduce (fn [required ns]
+             (if (contains? required ns)
+               (let [ns (if (symbol? ns) ns (second ns))]
+                 (findings/reg-finding!
+                   (:findings ctx)
+                   (node->line (:filename ctx)
+                               ns
+                               :warning
+                               :duplicate-require
+                               (str "duplicate require of " ns)))
+                 required)
+               (conj required ns)))
+           (set init)
+           namespaces)
+   nil))
+
+(defn lint-unsorted-namespaces! [{:keys [config filename findings] :as _ctx} namespaces]
+  (when-not (= :off (-> config :linters :unsorted-namespaces :level))
+    (loop [last-processed-ns (first namespaces)
+           ns-list (next namespaces)]
+      (when ns-list
+        (let [ns (first ns-list)]
+          (if-not (neg? (compare last-processed-ns ns))
+            (findings/reg-finding!
+              findings
+              (node->line filename
+                          ns
+                          :warning
+                          :unsorted-namespaces
+                          (str "Unsorted namespace: " ns)))
+            (recur ns
+                   (next ns-list))))))))
+
 (defn reg-namespace!
   "Registers namespace. Deep-merges with already registered namespaces
   with the same name. Returns updated namespace."
@@ -148,6 +183,7 @@
   [{:keys [:base-lang :lang :namespaces] :as ctx} ns-sym analyzed-require-clauses]
   (swap! namespaces update-in [base-lang lang ns-sym]
          (fn [ns]
+           (lint-unsorted-namespaces! ctx (:required analyzed-require-clauses))
            (lint-duplicate-requires! ctx (:required ns) (:required analyzed-require-clauses))
            (merge-with into ns analyzed-require-clauses)))
   nil)
diff --git a/src/clj_kondo/impl/parser.clj b/src/clj_kondo/impl/parser.clj
index 2111407330..5d2915e7a1 100644
--- a/src/clj_kondo/impl/parser.clj
+++ b/src/clj_kondo/impl/parser.clj
@@ -1,8 +1,8 @@
 (ns clj-kondo.impl.parser
   {:no-doc true}
   (:require
-   [clj-kondo.impl.utils :as utils :refer [parse-string-all]]
-   [clj-kondo.impl.profiler :refer [profile]]))
+    [clj-kondo.impl.profiler :refer [profile]]
+    [clj-kondo.impl.utils :as utils :refer [parse-string-all]]))
 
 (defn parse-string [s]
   (let [parsed (profile :parse-string-all (parse-string-all s))]
diff --git a/src/clj_kondo/impl/utils.clj b/src/clj_kondo/impl/utils.clj
index e096c97d4f..f4b5ba9345 100644
--- a/src/clj_kondo/impl/utils.clj
+++ b/src/clj_kondo/impl/utils.clj
@@ -225,6 +225,17 @@
             (transient {})
             ks))))
 
+(defn filter-remove [p xs]
+  (loop [xs xs
+         filtered (transient [])
+         removed (transient [])]
+    (if xs
+      (let [x (first xs)] (if (p x)
+                            (recur (next xs)
+                                   (conj! filtered x) removed)
+                            (recur (next xs) filtered (conj! removed x))))
+      [(persistent! filtered) (persistent! removed)])))
+
 ;;;; Scratch
 
 (comment
diff --git a/test/clj_kondo/impl/extract_var_info_test.clj b/test/clj_kondo/impl/extract_var_info_test.clj
index 0512cb46bb..96c6a99159 100644
--- a/test/clj_kondo/impl/extract_var_info_test.clj
+++ b/test/clj_kondo/impl/extract_var_info_test.clj
@@ -1,7 +1,7 @@
 (ns clj-kondo.impl.extract-var-info-test
   (:require
-   [clojure.test :as t :refer [deftest is]]
-   [clj-kondo.impl.extract-var-info :as extract-var-info]))
+    [clj-kondo.impl.extract-var-info :as extract-var-info]
+    [clojure.test :refer [deftest is]]))
 
 (deftest eastwood-var-info-test
   (let [var-info (extract-var-info/eastwood-var-info)]
diff --git a/test/clj_kondo/main_test.clj b/test/clj_kondo/main_test.clj
index 7f0200c534..9b2a9a46ba 100644
--- a/test/clj_kondo/main_test.clj
+++ b/test/clj_kondo/main_test.clj
@@ -2428,6 +2428,27 @@
   (is (empty? (lint! "(ns foo (:require [clojure.string])) clojure.string/join"
                      {:linters {:consistent-alias {:aliases '{clojure.string str}}}}))))
 
+(deftest unsorted-namespaces-test
+  (assert-submaps
+    [{:file "<stdin>"
+      :row 1
+      :col 31
+      :level :warning
+      :message "Unsorted namespace: abar.core"}]
+    (lint! "(ns foo (:require [bar.core] [abar.core]))" {:linters {:unsorted-namespaces {:level :warning}}}))
+  (assert-submaps
+    [{:file "<stdin>"
+      :row 1
+      :col 21
+      :level :warning
+      :message "Unsorted namespace: abar.core"}]
+    (lint! "(require 'bar.core 'abar.core)" {:linters {:unsorted-namespaces {:level :warning}}}))
+  (is (empty? (lint! "(ns foo (:require [bar.core] [abar.core]))" {:linters {:unsorted-namespaces {:level :off}}})))
+  (is (empty? (lint! "(ns foo (:require [abar.core] [bar.core]))" {:linters {:unsorted-namespaces {:level :warning}}})))
+  (is (empty? (lint! "(ns foo (:require [abar.core] [bar.core]) (:import [java.lib JavaClass] [ajava.lib AnotherClass]))"
+                     {:linters {:unsorted-namespaces {:level :warning}
+                                :unused-import {:level :off}}}))))
+
 (deftest set!-test
   (assert-submaps '[{:col 13 :message #"arg"}]
                   (lint! "(declare x) (set! (.-foo x) 1 2 3)"))
diff --git a/test/clj_kondo/test_test.clj b/test/clj_kondo/test_test.clj
index 17a910c130..d3bae5c428 100644
--- a/test/clj_kondo/test_test.clj
+++ b/test/clj_kondo/test_test.clj
@@ -1,8 +1,8 @@
 (ns clj-kondo.test-test
   (:require
-   [clj-kondo.test-utils :refer [lint! assert-submaps]]
-   [clojure.test :refer [deftest is]]
-   [clojure.java.io :as io]))
+    [clj-kondo.test-utils :refer [lint! assert-submaps]]
+    [clojure.java.io :as io]
+    [clojure.test :refer [deftest is]]))
 
 (deftest missing-test-assertion-test
   (is (empty? (lint! "(ns foo (:require [clojure.test :as t])) (t/deftest (t/is (odd? 1)))")))
diff --git a/test/clj_kondo/test_utils.clj b/test/clj_kondo/test_utils.clj
index b134b94cfb..299ece805e 100644
--- a/test/clj_kondo/test_utils.clj
+++ b/test/clj_kondo/test_utils.clj
@@ -66,7 +66,8 @@
   '{:linters {:unused-binding {:level :off}
               :unresolved-symbol {:level :off}
               :refer-all {:level :off}
-              :type-mismatch {:level :off}}})
+              :type-mismatch {:level :off}
+              :unsorted-namespaces {:level :off}}})
 
 (defn lint-jvm!
   ([input]
diff --git a/test/clj_kondo/unresolved_symbol_test.clj b/test/clj_kondo/unresolved_symbol_test.clj
index 6446180a97..60faa0d49d 100644
--- a/test/clj_kondo/unresolved_symbol_test.clj
+++ b/test/clj_kondo/unresolved_symbol_test.clj
@@ -1,9 +1,9 @@
 (ns clj-kondo.unresolved-symbol-test
   (:require
-   [clj-kondo.test-utils :refer [lint! assert-submaps]]
-   [clojure.java.io :as io]
-   [clojure.test :as t :refer [deftest is testing]]
-   [clojure.edn :as edn]))
+    [clj-kondo.test-utils :refer [lint! assert-submaps]]
+    [clojure.edn :as edn]
+    [clojure.java.io :as io]
+    [clojure.test :refer [deftest is testing]]))
 
 (deftest unresolved-symbol-test
   (assert-submaps