Splitting analyzers in different namespaces

CHANGELOG.md

@@ -2,6 +2,7 @@
 
 ## Unreleased
 
+* [#55](https://github.com/fabiodomingues/clj-depend/issues/55): Split analyzers into separate namespaces for better code organization and consequently simplify testing.
 * [#48](https://github.com/fabiodomingues/clj-depend/issues/48): Fix analyzing files outside of configured source-paths.
 
 ## 0.9.2 (2023-10-24)

project.clj

@@ -3,7 +3,8 @@
   :url "https://github.com/fabiodomingues/clj-depend"
   :license {:name "EPL-2.0 OR GPL-2.0-or-later WITH Classpath-exception-2.0"
             :url  "https://www.eclipse.org/legal/epl-2.0/"}
-  :profiles {:dev {:dependencies   [[org.clojure/test.check "1.1.1"]]
+  :profiles {:dev {:dependencies   [[org.clojure/test.check "1.1.1"]
+                                    [nubank/matcher-combinators "3.9.1"]]
                    :resource-paths ["test-resources"]}}
   :dependencies [[org.clojure/clojure "1.11.1" :scope "provided"]
                  [org.clojure/tools.namespace "1.2.0"]

src/clj_depend/analyzer.clj

@@ -1,62 +1,12 @@
-(ns clj-depend.analyzer)
-
-(defn- layer-cannot-access-dependency-layer?
-  [config layer dependency-layer]
-  (when-let [accesses-layers (get-in config [:layers layer :accesses-layers])]
-    (not-any? (partial = dependency-layer) accesses-layers)))
-
-(defn- dependency-layer-cannot-be-accessed-by-layer?
-  [config dependency-layer layer]
-  (when-let [accessed-by-layers (get-in config [:layers dependency-layer :accessed-by-layers])]
-    (not-any? (partial = layer) accessed-by-layers)))
-
-(defn- violate?
-  [config
-   {:keys [layer dependency-layer]}]
-  (and (not= layer dependency-layer)
-       (and (not (nil? layer))
-            (not (nil? dependency-layer)))
-       (or (dependency-layer-cannot-be-accessed-by-layer? config dependency-layer layer)
-           (layer-cannot-access-dependency-layer? config layer dependency-layer))))
-
-(defn- namespace-belongs-to-layer?
-  [config namespace layer]
-  (let [namespaces (get-in config [:layers layer :namespaces])
-        defined-by (get-in config [:layers layer :defined-by])]
-    (or (some #{namespace} namespaces)
-        (when defined-by (re-find (re-pattern defined-by) (str namespace))))))
-
-(defn- layer-by-namespace
-  [config namespace]
-  (some #(when (namespace-belongs-to-layer? config namespace %) %) (keys (:layers config))))
-
-(defn- layer-and-namespace [config namespace dependency-namespace]
-  (when-let [layer (layer-by-namespace config namespace)]
-    {:namespace            namespace
-     :layer                layer
-     :dependency-namespace dependency-namespace
-     :dependency-layer     (layer-by-namespace config dependency-namespace)}))
-
-(defn- layer-violations
-  [config namespace dependencies]
-  (->> dependencies
-       (map #(layer-and-namespace config namespace %))
-       (filter #(violate? config %))
-       (map (fn [{:keys [namespace dependency-namespace layer dependency-layer] :as violation}]
-              (assoc violation :message (str \" namespace \" " should not depend on " \" dependency-namespace \" " (layer " \" layer \" " on " \" dependency-layer \" ")"))))))
-
-(defn ^:private circular-dependency-violations
-  [namespace dependencies dependencies-by-namespace]
-  (->> dependencies-by-namespace
-       (filter (fn [[k _]] (contains? dependencies k)))
-       (filter (fn [[_ v]] (contains? v namespace)))
-       (map (fn [[k _]] {:namespace namespace :message (str "Circular dependency between " \" namespace \" " and " \" k \")}))))
+(ns clj-depend.analyzer
+  (:require [clj-depend.analyzers.circular-dependency :as analyzers.circular-dependency]
+            [clj-depend.analyzers.layer :as analyzers.layer]))
 
 (defn- violations
   [config dependencies-by-namespace namespace]
   (let [dependencies (get dependencies-by-namespace namespace)
-        circular-dependency-violations (circular-dependency-violations namespace dependencies dependencies-by-namespace)
-        layer-violations (layer-violations config namespace dependencies)]
+        circular-dependency-violations (analyzers.circular-dependency/analyze namespace dependencies dependencies-by-namespace)
+        layer-violations (analyzers.layer/analyze config namespace dependencies)]
     (not-empty (concat circular-dependency-violations layer-violations))))
 
 (defn analyze

src/clj_depend/analyzers/circular_dependency.clj

@@ -0,0 +1,8 @@
+(ns clj-depend.analyzers.circular-dependency)
+
+(defn analyze
+  [namespace dependencies dependencies-by-namespace]
+  (->> dependencies-by-namespace
+       (filter (fn [[k _]] (contains? dependencies k)))
+       (filter (fn [[_ v]] (contains? v namespace)))
+       (map (fn [[k _]] {:namespace namespace :dependency-namespace k :message (str "Circular dependency between " \" namespace \" " and " \" k \")}))))

src/clj_depend/analyzers/layer.clj

@@ -0,0 +1,46 @@
+(ns clj-depend.analyzers.layer)
+
+(defn ^:private layer-cannot-access-dependency-layer?
+  [config layer dependency-layer]
+  (when-let [accesses-layers (get-in config [:layers layer :accesses-layers])]
+    (not-any? (partial = dependency-layer) accesses-layers)))
+
+(defn ^:private dependency-layer-cannot-be-accessed-by-layer?
+  [config dependency-layer layer]
+  (when-let [accessed-by-layers (get-in config [:layers dependency-layer :accessed-by-layers])]
+    (not-any? (partial = layer) accessed-by-layers)))
+
+(defn ^:private violate?
+  [config
+   {:keys [layer dependency-layer]}]
+  (and (not= layer dependency-layer)
+       (and (not (nil? layer))
+            (not (nil? dependency-layer)))
+       (or (dependency-layer-cannot-be-accessed-by-layer? config dependency-layer layer)
+           (layer-cannot-access-dependency-layer? config layer dependency-layer))))
+
+(defn ^:private namespace-belongs-to-layer?
+  [config namespace layer]
+  (let [namespaces (get-in config [:layers layer :namespaces])
+        defined-by (get-in config [:layers layer :defined-by])]
+    (or (some #{namespace} namespaces)
+        (when defined-by (re-find (re-pattern defined-by) (str namespace))))))
+
+(defn ^:private layer-by-namespace
+  [config namespace]
+  (some #(when (namespace-belongs-to-layer? config namespace %) %) (keys (:layers config))))
+
+(defn ^:private layer-and-namespace [config namespace dependency-namespace]
+  (when-let [layer (layer-by-namespace config namespace)]
+    {:namespace            namespace
+     :layer                layer
+     :dependency-namespace dependency-namespace
+     :dependency-layer     (layer-by-namespace config dependency-namespace)}))
+
+(defn analyze
+  [config namespace dependencies]
+  (->> dependencies
+       (map #(layer-and-namespace config namespace %))
+       (filter #(violate? config %))
+       (map (fn [{:keys [namespace dependency-namespace layer dependency-layer] :as violation}]
+              (assoc violation :message (str \" namespace \" " should not depend on " \" dependency-namespace \" " (layer " \" layer \" " on " \" dependency-layer \" ")"))))))

test/clj_depend/analyzer_test.clj

@@ -1,93 +1,33 @@
 (ns clj-depend.analyzer-test
   (:require [clojure.test :refer :all]
-            [clj-depend.analyzer :as analyzer]))
-
-(def ns-deps-a {'foo.a.bar #{'foo.b.bar 'foo.c.bar}})
-
-(def ns-deps-b {'foo.b.bar #{'foo.b.baz}
-                'foo.b.baz #{}})
-
-(def ns-deps-c {'foo.c.bar #{}})
-
-(def ns-deps-c-with-violation {'foo.c.bar #{'foo.b.bar}})
-
-(def config-using-accessed-by-layers {:layers {:a {:defined-by         ".*\\.a\\..*"
-                                                   :accessed-by-layers #{}}
-                                               :b {:defined-by         ".*\\.b\\..*"
-                                                   :accessed-by-layers #{:a}}
-                                               :c {:defined-by         ".*\\.c\\..*"
-                                                   :accessed-by-layers #{:a :b}}}})
-
-(def config-using-accessed-by-layers-with-namespaces {:layers {:a {:namespaces         #{'foo.a.bar}
-                                                                   :accessed-by-layers #{}}
-                                                               :b {:namespaces         #{'foo.b.bar 'foo.b.baz}
-                                                                   :accessed-by-layers #{:a}}
-                                                               :c {:defined-by         ".*\\.c\\..*"
-                                                                   :accessed-by-layers #{:a :b}}}})
-
-(def config-using-access-layers {:layers {:a {:defined-by      ".*\\.a\\..*"
-                                              :accesses-layers #{:b :c}}
-                                          :b {:defined-by      ".*\\.b\\..*"
-                                              :accesses-layers #{:c}}
-                                          :c {:defined-by      ".*\\.c\\..*"
-                                              :accesses-layers #{}}}})
-
-(def ns-deps (merge ns-deps-a ns-deps-b ns-deps-c))
-
-(def ns-deps-with-violations (merge ns-deps-a ns-deps-b ns-deps-c-with-violation))
+            [clj-depend.analyzer :as analyzer]
+            [matcher-combinators.test :refer [match?]]
+            [matcher-combinators.matchers :as m]))
 
 (deftest analyze-test
-  (testing "should return zero violations when there is no layers declared"
-    (is (= []
-           (analyzer/analyze {:config                    {:layers {}}
-                              :dependencies-by-namespace ns-deps})))))
-
-(deftest analyze-test-using-accessed-by-layers
-  (testing "should return zero violations when there is no forbidden access"
-    (is (= []
-           (analyzer/analyze {:config                    config-using-accessed-by-layers
-                              :dependencies-by-namespace ns-deps}))))
-
-  (testing "should return violations when there is any forbidden access"
-    (is (= [{:namespace            'foo.c.bar
-             :dependency-namespace 'foo.b.bar
-             :layer                :c
-             :dependency-layer     :b
-             :message              "\"foo.c.bar\" should not depend on \"foo.b.bar\" (layer \":c\" on \":b\")"}]
-           (analyzer/analyze {:config                    config-using-accessed-by-layers
-                              :dependencies-by-namespace ns-deps-with-violations}))))
-
-  (testing "should return zero violations when a layer has namespaces that depend on another namespace that is not in any layer of the configuration"
-    (is (= []
-           (analyzer/analyze {:config                    (update-in config-using-accessed-by-layers [:layers] dissoc :b)
-                              :dependencies-by-namespace ns-deps})))))
-
-(deftest analyze-config-with-namespaces-test
-  (testing "should return zero violations when there is no forbidden access"
-    (is (= []
-           (analyzer/analyze {:config                    config-using-accessed-by-layers-with-namespaces
-                              :dependencies-by-namespace ns-deps}))))
-
-  (testing "should return violations when there is any forbidden access"
-    (is (= [{:namespace            'foo.c.bar
-             :dependency-namespace 'foo.b.bar
-             :layer                :c
-             :dependency-layer     :b
-             :message              "\"foo.c.bar\" should not depend on \"foo.b.bar\" (layer \":c\" on \":b\")"}]
-           (analyzer/analyze {:config                    config-using-accessed-by-layers-with-namespaces
-                              :dependencies-by-namespace ns-deps-with-violations})))))
-
-(deftest analyze-test-using-access-layers
-  (testing "should return violations when there is any forbidden access"
-    (is (= [{:namespace            'foo.c.bar
-             :dependency-namespace 'foo.b.bar
-             :layer                :c
-             :dependency-layer     :b
-             :message              "\"foo.c.bar\" should not depend on \"foo.b.bar\" (layer \":c\" on \":b\")"}]
-           (analyzer/analyze {:config                    config-using-access-layers
-                              :dependencies-by-namespace ns-deps-with-violations}))))
-
-  (testing "should return zero violations when a layer has namespaces that depend on another namespace that is not in any layer of the configuration"
-    (is (= []
-           (analyzer/analyze {:config                    (update-in config-using-access-layers [:layers] dissoc :b)
-                              :dependencies-by-namespace ns-deps-with-violations})))))
+  (testing "should not return violations when there are no circular dependencies and no layers violated"
+    (is (empty? (analyzer/analyze {:config                    {:layers {}}
+                                   :dependencies-by-namespace {'foo.a.bar  #{}
+                                                               'foo.b.bar  #{'foo.a.bar}
+                                                               'foo.any    #{'foo.a.bar}}}))))
+
+  (testing "should return violations when there are circular dependencies or layers violated"
+    (is (match? (m/in-any-order [{:namespace            'foo.a.bar
+                                  :dependency-namespace 'foo.any
+                                  :message              "Circular dependency between \"foo.a.bar\" and \"foo.any\""}
+                                 {:namespace            'foo.any
+                                  :dependency-namespace 'foo.a.bar
+                                  :message              "Circular dependency between \"foo.any\" and \"foo.a.bar\""}
+                                 {:namespace            'foo.b.bar
+                                  :layer                :b
+                                  :dependency-namespace 'foo.a.bar
+                                  :dependency-layer     :a
+                                  :message              "\"foo.b.bar\" should not depend on \"foo.a.bar\" (layer \":b\" on \":a\")"}])
+                (analyzer/analyze {:config                    {:layers {:a {:defined-by      ".*\\.a\\..*"
+                                                                            :accesses-layers #{}}
+                                                                        :b {:defined-by      ".*\\.b\\..*"
+                                                                            :accesses-layers #{}}}}
+                                   :dependencies-by-namespace {'foo.a.bar  #{'foo.any}
+                                                               'foo.b.bar  #{'foo.a.bar}
+                                                               'foo.any    #{'foo.a.bar}
+                                                               'foo.a-test #{'lib.x.y.z}}})))))

test/clj_depend/analyzers/circular_dependency_test.clj

@@ -0,0 +1,19 @@
+(ns clj-depend.analyzers.circular-dependency-test
+  (:require [clojure.test :refer :all]
+            [clj-depend.analyzers.circular-dependency :as analyzers.circular-dependency]))
+
+(deftest analyze-test
+  (testing "should return violations when there is circular dependency"
+    (is (= [{:namespace            'foo.a
+             :dependency-namespace 'foo.b
+             :message              "Circular dependency between \"foo.a\" and \"foo.b\""}]
+           (analyzers.circular-dependency/analyze 'foo.a
+                                                  #{'foo.b}
+                                                  {'foo.a #{'foo.b}
+                                                   'foo.b #{'foo.a}}))))
+
+  (testing "should not return violations when there is no circular dependency"
+    (is (empty? (analyzers.circular-dependency/analyze 'foo.a
+                                                       #{'foo.b}
+                                                       {'foo.a #{'foo.b}
+                                                        'foo.b #{}})))))

test/clj_depend/analyzers/layer_test.clj

@@ -0,0 +1,82 @@
+(ns clj-depend.analyzers.layer-test
+  (:require [clojure.test :refer :all]
+            [clj-depend.analyzers.layer :as analyzers.layer]))
+
+(deftest analyze-test
+  (testing "Given a configuration of layers using defined-by (regular expressions) and accessed-by-layer"
+    (let [config {:layers {:a {:defined-by         ".*\\.a\\..*"
+                               :accessed-by-layers #{}}
+                           :b {:defined-by         ".*\\.b\\..*"
+                               :accessed-by-layers #{:a}}}}]
+
+      (testing "when a namespace without disallowed dependencies is analyzed"
+        (let [violations (analyzers.layer/analyze config 'foo.a.bar #{'foo.b.bar})]
+          (testing "then no violations should have been returned"
+            (is (empty? violations)))))
+
+      (testing "when a namespace that depends only on a namespace that is not part of any other layer is analyzed"
+        (let [violations (analyzers.layer/analyze config 'foo.a.bar #{'foo.c.bar})]
+          (testing "then no violations should have been returned"
+            (is (empty? violations)))))
+
+      (testing "when a namespace with disallowed dependencies is analyzed"
+        (let [violations (analyzers.layer/analyze config 'foo.b.bar #{'foo.a.bar})]
+          (testing "then no violations should have been returned"
+            (is (= [{:namespace            'foo.b.bar
+                     :dependency-namespace 'foo.a.bar
+                     :layer                :b
+                     :dependency-layer     :a
+                     :message              "\"foo.b.bar\" should not depend on \"foo.a.bar\" (layer \":b\" on \":a\")"}]
+                   violations)))))))
+
+  (testing "Given a configuration of layers using namespaces and accessed-by-layer"
+    (let [config {:layers {:a {:namespaces         #{'foo.a.bar}
+                               :accessed-by-layers #{}}
+                           :b {:namespaces         #{'foo.b.bar}
+                               :accessed-by-layers #{:a}}}}]
+
+      (testing "when a namespace without disallowed dependencies is analyzed"
+        (let [violations (analyzers.layer/analyze config 'foo.a.bar #{'foo.b.bar})]
+          (testing "then no violations should have been returned"
+            (is (empty? violations)))))
+
+      (testing "when a namespace that depends only on a namespace that is not part of any other layer is analyzed"
+        (let [violations (analyzers.layer/analyze config 'foo.a.bar #{'foo.c.bar})]
+          (testing "then no violations should have been returned"
+            (is (empty? violations)))))
+
+      (testing "when a namespace with disallowed dependencies is analyzed"
+        (let [violations (analyzers.layer/analyze config 'foo.b.bar #{'foo.a.bar})]
+          (testing "then no violations should have been returned"
+            (is (= [{:namespace            'foo.b.bar
+                     :dependency-namespace 'foo.a.bar
+                     :layer                :b
+                     :dependency-layer     :a
+                     :message              "\"foo.b.bar\" should not depend on \"foo.a.bar\" (layer \":b\" on \":a\")"}]
+                   violations)))))))
+
+  (testing "Given a configuration of layers using defined-by and access-layers"
+    (let [config {:layers {:a {:defined-by      ".*\\.a\\..*"
+                               :accesses-layers #{:b}}
+                           :b {:defined-by      ".*\\.b\\..*"
+                               :accesses-layers #{}}}}]
+
+      (testing "when a namespace without disallowed dependencies is analyzed"
+        (let [violations (analyzers.layer/analyze config 'foo.a.bar #{'foo.b.bar})]
+          (testing "then no violations should have been returned"
+            (is (empty? violations)))))
+
+      (testing "when a namespace that depends only on a namespace that is not part of any other layer is analyzed"
+        (let [violations (analyzers.layer/analyze config 'foo.a.bar #{'foo.c.bar})]
+          (testing "then no violations should have been returned"
+            (is (empty? violations)))))
+
+      (testing "when a namespace with disallowed dependencies is analyzed"
+        (let [violations (analyzers.layer/analyze config 'foo.b.bar #{'foo.a.bar})]
+          (testing "then the violations should have been returned"
+            (is (= [{:namespace            'foo.b.bar
+                     :dependency-namespace 'foo.a.bar
+                     :layer                :b
+                     :dependency-layer     :a
+                     :message              "\"foo.b.bar\" should not depend on \"foo.a.bar\" (layer \":b\" on \":a\")"}]
+                   violations))))))))