Skip to content

Commit

Permalink
Consider a namespace as part of a layer only when the namespace is in…
Browse files Browse the repository at this point in the history
… the source-paths. (#58)
  • Loading branch information
fabiodomingues authored Apr 19, 2024
1 parent 8d643dc commit fdd0cc2
Show file tree
Hide file tree
Showing 14 changed files with 145 additions and 106 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
# CHANGELOG

## Unreleased
* [#56](https://github.com/fabiodomingues/clj-depend/issues/56): Added the `:only-ns-in-source-paths` attribute for when it is necessary to consider only namespaces in source paths as part of a layer.

## 0.10.0 (2024-04-08)
* [#51](https://github.com/fabiodomingues/clj-depend/issues/51): Namespace rule analyzer.
Expand Down
6 changes: 4 additions & 2 deletions docs/config.md
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ Default: `{}`.
A map where each key is a layer and the value is a map, where:
- The layer is defined by a regex using the `:defined-by` key or a set of namespaces using the `:namespaces` key.
- The accesses allowed by it declared using the `:accesses-layers` key, or the accesses that are allowed to the layer using the `:accessed-by-layers` key. Since both keys accept a set of layers.
- `:only-ns-in-source-paths` optional, only considers namespaces in source paths as part of a layer. Available values: `true`, `false` with default value of `false`.

Config example:
```clojure
Expand All @@ -45,8 +46,9 @@ Config example:
:accesses-layers #{:logic :model}}
:logic {:defined-by ".*\\.logic\\..*"
:accesses-layers #{:model}}
:model {:defined-by ".*\\.model\\..*"
:accesses-layers #{}}}
:model {:defined-by ".*\\.model\\..*"
:accesses-layers #{}
:only-ns-in-source-paths true}}
,,,}
```

Expand Down
4 changes: 2 additions & 2 deletions lein-clj-depend/src/leiningen/clj_depend.clj
Original file line number Diff line number Diff line change
Expand Up @@ -3,12 +3,12 @@
(:require [clj-depend.main :as clj-depend.main]
[leiningen.core.main :as leiningen.core]))

(defn- project->args
(defn ^:private project->args
[{:keys [root]} args]
(concat (or args [])
["--project-root" root]))

(defn- run!
(defn ^:private run!
[project args]
(let [result (apply clj-depend.main/run! (project->args project args))]
(when-let [message (:message result)]
Expand Down
16 changes: 9 additions & 7 deletions src/clj_depend/analyzer.clj
Original file line number Diff line number Diff line change
Expand Up @@ -3,15 +3,17 @@
[clj-depend.analyzers.layer :as analyzers.layer]
[clj-depend.analyzers.rule :as analyzers.rule]))

(defn- violations
(defn ^:private violations
[config dependencies-by-namespace namespace]
(let [dependencies (get dependencies-by-namespace namespace)
circular-dependency-violations (analyzers.circular-dependency/analyze namespace dependencies dependencies-by-namespace)
layer-violations (analyzers.layer/analyze config namespace dependencies)
rule-violations (analyzers.rule/analyze config namespace dependencies)]
(let [circular-dependency-violations (analyzers.circular-dependency/analyze namespace dependencies-by-namespace)
layer-violations (analyzers.layer/analyze config namespace dependencies-by-namespace)
rule-violations (analyzers.rule/analyze config namespace dependencies-by-namespace)]
(not-empty (concat circular-dependency-violations layer-violations rule-violations))))

(defn analyze
"Analyze namespaces dependencies."
[{:keys [config dependencies-by-namespace]}]
(flatten (keep #(violations config dependencies-by-namespace %) (keys dependencies-by-namespace))))
[{:keys [config namespaces-to-be-analyzed namespaces-and-dependencies]}]
(let [dependencies-by-namespace (reduce-kv (fn [m k v] (assoc m k (:dependencies (first v))))
{}
(group-by :namespace namespaces-and-dependencies))]
(flatten (keep #(violations config dependencies-by-namespace %) namespaces-to-be-analyzed))))
11 changes: 6 additions & 5 deletions src/clj_depend/analyzers/circular_dependency.clj
Original file line number Diff line number Diff line change
@@ -1,8 +1,9 @@
(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 \")}))))
[namespace dependencies-by-namespace]
(let [current-namespace-dependencies (get dependencies-by-namespace namespace)]
(->> dependencies-by-namespace
(filter (fn [[k _]] (contains? current-namespace-dependencies k)))
(filter (fn [[_ v]] (contains? v namespace)))
(map (fn [[k _]] {:namespace namespace :dependency-namespace k :message (str "Circular dependency between " \" namespace \" " and " \" k \")})))))
41 changes: 28 additions & 13 deletions src/clj_depend/analyzers/layer.clj
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
(ns clj-depend.analyzers.layer)
(ns clj-depend.analyzers.layer
(:require [clojure.set :as set]))

(defn ^:private layer-cannot-access-dependency-layer?
[config layer dependency-layer]
Expand All @@ -19,28 +20,42 @@
(or (dependency-layer-cannot-be-accessed-by-layer? config dependency-layer layer)
(layer-cannot-access-dependency-layer? config layer dependency-layer))))

(defn ^:private namespace-in-source-paths?
[namespace dependencies-by-namespace]
(contains? (set (keys dependencies-by-namespace)) namespace))

(defn ^:private namespace-belongs-to-layer?
[config namespace layer]
[config namespace layer dependencies-by-namespace]
(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))))))
defined-by (get-in config [:layers layer :defined-by])
only-ns-in-source-paths (get-in config [:layers layer :only-ns-in-source-paths])]
(and (or (not only-ns-in-source-paths)
(and only-ns-in-source-paths (namespace-in-source-paths? namespace dependencies-by-namespace)))
(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))))
[config namespace dependencies-by-namespace]
(some #(when (namespace-belongs-to-layer? config namespace % dependencies-by-namespace) %) (keys (:layers config))))

(defn ^:private layer-and-namespace [config namespace dependency-namespace]
(when-let [layer (layer-by-namespace config namespace)]
(defn ^:private layer-and-namespace [config namespace dependency-namespace dependencies-by-namespace]
(when-let [layer (layer-by-namespace config namespace dependencies-by-namespace)]
{:namespace namespace
:layer layer
:dependency-namespace dependency-namespace
:dependency-layer (layer-by-namespace config dependency-namespace)}))
:dependency-layer (layer-by-namespace config dependency-namespace dependencies-by-namespace)}))

(defn ^:private namespace-dependencies
[{:keys [only-ns-in-source-paths]} namespace dependencies-by-namespace]
(let [namespace-dependencies (get dependencies-by-namespace namespace)]
(if only-ns-in-source-paths
(set/intersection (set namespace-dependencies) (set (keys dependencies-by-namespace)))
namespace-dependencies)))

(defn analyze
[config namespace dependencies]
(->> dependencies
(map #(layer-and-namespace config namespace %))
[config namespace dependencies-by-namespace]
(->> (get dependencies-by-namespace namespace)
(map #(layer-and-namespace config namespace % dependencies-by-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 \" ")"))))))
9 changes: 5 additions & 4 deletions src/clj_depend/analyzers/rule.clj
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,8 @@
(defn analyze
[{:keys [rules]}
namespace
dependencies]
(->> (filter #(rule-applies-to-namespace? % namespace) rules)
(keep #(violations-by-rule % namespace dependencies))
flatten))
dependencies-by-namespace]
(let [current-namespace-dependencies (get dependencies-by-namespace namespace)]
(->> (filter #(rule-applies-to-namespace? % namespace) rules)
(keep #(violations-by-rule % namespace current-namespace-dependencies))
flatten)))
74 changes: 46 additions & 28 deletions src/clj_depend/internal_api.clj
Original file line number Diff line number Diff line change
@@ -1,52 +1,70 @@
(ns clj-depend.internal-api
(:require [clj-depend.analyzer :as analyzer]
[clj-depend.config :as config]
[clj-depend.parser :as parser]
[clj-depend.snapshot :as snapshot]
[clojure.java.io :as io]
[clojure.string :as string]))
[clojure.string :as string]
[clojure.tools.namespace.file :as file]
[clojure.tools.namespace.find :as namespace.find]
[clojure.tools.namespace.parse :as namespace.parse]))

(defn- ->project-root
(defn ^:private ->project-root
[{:keys [project-root]} context]
(assoc context :project-root project-root))

(defn- ->config
(defn ^:private ->config
[{:keys [config]}
{:keys [project-root] :as context}]
(assoc context :config (config/resolve-config! project-root config)))

(defn ^:private file-within-some-source-paths?
[file source-paths]
(some #(.startsWith (.toPath file) (.toPath %)) source-paths))
(defn ^:private source-paths-or-project-root->files
[{:keys [project-root] {:keys [source-paths]} :config}]
(if (not-empty source-paths)
(map #(io/file project-root %) source-paths)
#{project-root}))

(defn ^:private files-within-source-paths
[files source-paths]
(filter #(file-within-some-source-paths? % source-paths) files))
(defn ^:private analyze?
[{:keys [file namespace]} files-to-be-analyzed namespaces-to-be-analyzed]
(boolean (cond
(and (not-empty files-to-be-analyzed) (not-empty namespaces-to-be-analyzed))
(and (some #(.startsWith (.toPath file) (.toPath %)) files-to-be-analyzed)
(contains? namespaces-to-be-analyzed namespace))

(defn- ->files
[{:keys [files]}
{:keys [project-root] {:keys [source-paths]} :config :as context}]
(let [source-paths (map #(io/file project-root %) source-paths)]
(cond
(not-empty files) (assoc context :files (files-within-source-paths files source-paths))
(not-empty source-paths) (assoc context :files source-paths)
:else (assoc context :files #{project-root}))))
(not-empty files-to-be-analyzed)
(some #(.startsWith (.toPath file) (.toPath %)) files-to-be-analyzed)

(defn- ->namespaces
[{:keys [namespaces]}
{:keys [files] :as context}]
(let [ns-deps (parser/parse-clojure-files! files namespaces)]
(assoc context :dependencies-by-namespace (reduce-kv (fn [m k v] (assoc m k (:dependencies (first v))))
{}
(group-by :name ns-deps)))))
(not-empty namespaces-to-be-analyzed)
(contains? namespaces-to-be-analyzed namespace)

(defn- build-context
:else
true)))

(defn ^:private ->namespaces-and-dependencies
[_options
context]
(let [files (source-paths-or-project-root->files context)
clojure-files (mapcat #(namespace.find/find-sources-in-dir %) files)]
(assoc context :namespaces-and-dependencies (keep (fn [file]
(when-let [ns-decl (file/read-file-ns-decl file)]
{:namespace (namespace.parse/name-from-ns-decl ns-decl)
:dependencies (namespace.parse/deps-from-ns-decl ns-decl)
:file file})) clojure-files))))

(defn ^:private ->namespaces-to-be-analyzed
[{files-to-be-analyzed :files
namespaces-to-be-analyzed :namespaces}
{:keys [namespaces-and-dependencies] :as context}]
(assoc context :namespaces-to-be-analyzed (->> namespaces-and-dependencies
(filter #(analyze? % files-to-be-analyzed namespaces-to-be-analyzed))
(map :namespace))))

(defn ^:private build-context
[options]
(->> {}
(->project-root options)
(->config options)
(->files options)
(->namespaces options)))
(->namespaces-and-dependencies options)
(->namespaces-to-be-analyzed options)))

(defn configured?
[project-root]
Expand Down
2 changes: 1 addition & 1 deletion src/clj_depend/main.clj
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@
:id :snapshot?
:default false]])

(defn- exit!
(defn ^:private exit!
[exit-code message]
(when message (println message))
(System/exit (or exit-code 2)))
Expand Down
13 changes: 0 additions & 13 deletions src/clj_depend/parser.clj

This file was deleted.

32 changes: 17 additions & 15 deletions test/clj_depend/analyzer_test.clj
Original file line number Diff line number Diff line change
Expand Up @@ -6,12 +6,13 @@

(deftest analyze-test
(testing "should not return violations when there are no circular dependencies and no layers/rules violated"
(is (empty? (analyzer/analyze {:config {:layers {}
:rules []}
:dependencies-by-namespace {'foo.a.bar #{}
'foo.b.bar #{'foo.a.bar}
'foo.any #{'foo.a.bar}
'foo.a-test #{'lib.x.y.z}}}))))
(is (empty? (analyzer/analyze {:config {:layers {}
:rules []}
:namespaces-and-dependencies [{:namespace 'foo.a.bar :dependencies #{}}
{:namespace 'foo.b.bar :dependencies #{'foo.a.bar}}
{:namespace 'foo.any :dependencies #{'foo.a.bar}}
{:namespace 'foo.a-test :dependencies #{'lib.x.y.z}}]
:namespaces-to-be-analyzed #{'foo.a.bar 'foo.b.bar 'foo.any 'foo.a-test}}))))

(testing "should return violations when there are circular dependencies or layers/rules violated"
(is (match? (m/in-any-order [{:namespace 'foo.a.bar
Expand All @@ -28,12 +29,13 @@
{:namespace 'foo.a-test
:dependency-namespace 'lib.x.y.z
:message "\"foo.a-test\" should not depend on \"lib.x.y.z\""}])
(analyzer/analyze {:config {:layers {:a {:defined-by ".*\\.a\\..*"
:accesses-layers #{}}
:b {:defined-by ".*\\.b\\..*"
:accesses-layers #{}}}
:rules [{:namespaces #{'foo.a-test} :should-not-depend-on #{'lib.x.y.z}}]}
: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}}})))))
(analyzer/analyze {:config {:layers {:a {:defined-by ".*\\.a\\..*"
:accesses-layers #{}}
:b {:defined-by ".*\\.b\\..*"
:accesses-layers #{}}}
:rules [{:namespaces #{'foo.a-test} :should-not-depend-on #{'lib.x.y.z}}]}
:namespaces-and-dependencies [{:namespace 'foo.a.bar :dependencies #{'foo.any}}
{:namespace 'foo.b.bar :dependencies #{'foo.a.bar}}
{:namespace 'foo.any :dependencies #{'foo.a.bar}}
{:namespace 'foo.a-test :dependencies #{'lib.x.y.z}}]
:namespaces-to-be-analyzed #{'foo.a.bar 'foo.b.bar 'foo.any 'foo.a-test}})))))
2 changes: 0 additions & 2 deletions test/clj_depend/analyzers/circular_dependency_test.clj
Original file line number Diff line number Diff line change
Expand Up @@ -8,12 +8,10 @@
: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 #{}})))))
Loading

0 comments on commit fdd0cc2

Please sign in to comment.