From 0c6d5dbcf57bdcfdc44a9eef8c0c81516ac71add Mon Sep 17 00:00:00 2001 From: vemv Date: Mon, 27 Jan 2020 22:23:47 +0100 Subject: [PATCH 1/3] Implement `linters.one-resource-per-ns` Fixes nedap/formatting-stack#78 --- project.clj | 3 +- src/formatting_stack/branch_formatter.clj | 6 +- src/formatting_stack/defaults.clj | 8 ++- .../linters/one_resource_per_ns.clj | 60 +++++++++++++++++++ src/formatting_stack/project_formatter.clj | 6 +- test-resources-extra/orpn.clj | 1 + test-resources-extra/orpn.cljc | 1 + test-resources-extra/orpn.cljs | 1 + test-resources/orpn.clj | 1 + test-resources/orpn.cljc | 1 + test-resources/orpn.cljs | 1 + test/formatting_stack/test_helpers.clj | 10 ++++ .../linters/one_resource_per_ns.clj | 43 +++++++++++++ .../linters/one_resource_per_ns.clj | 22 +++++++ 14 files changed, 159 insertions(+), 5 deletions(-) create mode 100644 src/formatting_stack/linters/one_resource_per_ns.clj create mode 100644 test-resources-extra/orpn.clj create mode 100644 test-resources-extra/orpn.cljc create mode 100644 test-resources-extra/orpn.cljs create mode 100644 test-resources/orpn.clj create mode 100644 test-resources/orpn.cljc create mode 100644 test-resources/orpn.cljs create mode 100644 test/formatting_stack/test_helpers.clj create mode 100644 test/integration/formatting_stack/linters/one_resource_per_ns.clj create mode 100644 test/unit/formatting_stack/linters/one_resource_per_ns.clj diff --git a/project.clj b/project.clj index 0f8f3cb8..b571a304 100644 --- a/project.clj +++ b/project.clj @@ -77,7 +77,8 @@ :dependencies [[com.nedap.staffing-solutions/utils.test "1.6.2"]] :jvm-opts ["-Dclojure.core.async.go-checking=true" "-Duser.language=en-US"] - :resource-paths ["test-resources"]} + :resource-paths ["test-resources-extra" + "test-resources"]} :cider-nrepl {:plugins [[cider/cider-nrepl "0.21.1"]]} diff --git a/src/formatting_stack/branch_formatter.clj b/src/formatting_stack/branch_formatter.clj index 62857d75..876d1091 100644 --- a/src/formatting_stack/branch_formatter.clj +++ b/src/formatting_stack/branch_formatter.clj @@ -14,6 +14,7 @@ [formatting-stack.linters.kondo :as linters.kondo] [formatting-stack.linters.loc-per-ns :as linters.loc-per-ns] [formatting-stack.linters.ns-aliases :as linters.ns-aliases] + [formatting-stack.linters.one-resource-per-ns :as linters.one-resource-per-ns] [formatting-stack.processors.cider :as processors.cider] [formatting-stack.strategies :as strategies] [medley.core :refer [mapply]])) @@ -63,7 +64,10 @@ (assoc :strategies (conj default-strategies strategies/exclude-edn strategies/exclude-clj - strategies/exclude-cljc)))]) + strategies/exclude-cljc)))] + (-> (linters.one-resource-per-ns/new {}) + (assoc :strategies (conj default-strategies + strategies/files-with-a-namespace)))) (def default-processors [(processors.cider/new {:third-party-indent-specs third-party-indent-specs})]) diff --git a/src/formatting_stack/defaults.clj b/src/formatting_stack/defaults.clj index 74ea9f7b..0874f5d3 100644 --- a/src/formatting_stack/defaults.clj +++ b/src/formatting_stack/defaults.clj @@ -11,6 +11,7 @@ [formatting-stack.linters.kondo :as linters.kondo] [formatting-stack.linters.loc-per-ns :as linters.loc-per-ns] [formatting-stack.linters.ns-aliases :as linters.ns-aliases] + [formatting-stack.linters.one-resource-per-ns :as linters.one-resource-per-ns] [formatting-stack.processors.cider :as processors.cider] [formatting-stack.strategies :as strategies])) @@ -63,8 +64,11 @@ (assoc :strategies (conj extended-strategies strategies/exclude-edn strategies/exclude-clj - strategies/exclude-cljc)))]) + strategies/exclude-cljc))) + (-> (linters.one-resource-per-ns/new {}) + (assoc :strategies (conj extended-strategies + strategies/files-with-a-namespace)))]) (defn default-processors [third-party-indent-specs] [(processors.cider/new {:third-party-indent-specs third-party-indent-specs - :strategies extended-strategies})]) + :strategies extended-strategies})]) diff --git a/src/formatting_stack/linters/one_resource_per_ns.clj b/src/formatting_stack/linters/one_resource_per_ns.clj new file mode 100644 index 00000000..b16de65c --- /dev/null +++ b/src/formatting_stack/linters/one_resource_per_ns.clj @@ -0,0 +1,60 @@ +(ns formatting-stack.linters.one-resource-per-ns + (:require + [clojure.spec.alpha :as spec] + [clojure.string :as string] + [clojure.tools.namespace.file :as file] + [formatting-stack.protocols.linter :as linter] + [formatting-stack.util :refer [process-in-parallel!]] + [nedap.speced.def :as speced] + [nedap.utils.modular.api :refer [implement]] + [nedap.utils.spec.predicates :refer [present-string?]])) + +(spec/def ::ns-decl (spec/and sequential? + (fn [x] + (->> x first #{'ns `ns})))) + +(spec/def ::resource-path (spec/and present-string? + (complement #{\. \! \? \-}) + (fn [x] + (re-find #"\.clj([cs])?$" x)))) + +(speced/defn ^::resource-path ns-decl->resource-path [^::ns-decl ns-decl, extension] + (-> ns-decl + second + str + munge + (string/replace "." "/") + (str extension))) + +(speced/defn resource-path->filenames [^::resource-path resource-path] + (->> (-> (Thread/currentThread) + (.getContextClassLoader) + (.getResources resource-path)) + (enumeration-seq) + (distinct) ;; just in case + (mapv str))) + +(speced/defn analyze [^present-string? filename] + (for [extension [".clj" ".cljs" ".cljc"] + :let [decl (-> filename file/read-file-ns-decl) + resource-path (ns-decl->resource-path decl extension) + filenames (resource-path->filenames resource-path)] + :when (-> filenames count (> 1))] + {:extension extension + :decl decl + :filenames filenames})) + +(defn lint! [this filenames] + (->> filenames + (process-in-parallel! (fn [filename] + (->> filename + analyze + (run! (speced/fn [{:keys [^some? decl, ^some? filenames]}] + (println "Warning: the namespace" + (str "`" (-> decl second) "`") + "is defined over more than one file.\nFound:" + (->> filenames (interpose ", ") (apply str)))))))))) + +(speced/defn new [^map? opts] + (implement opts + linter/--lint! lint!)) diff --git a/src/formatting_stack/project_formatter.clj b/src/formatting_stack/project_formatter.clj index 7972e1c0..4c82de10 100644 --- a/src/formatting_stack/project_formatter.clj +++ b/src/formatting_stack/project_formatter.clj @@ -14,6 +14,7 @@ [formatting-stack.linters.kondo :as linters.kondo] [formatting-stack.linters.loc-per-ns :as linters.loc-per-ns] [formatting-stack.linters.ns-aliases :as linters.ns-aliases] + [formatting-stack.linters.one-resource-per-ns :as linters.one-resource-per-ns] [formatting-stack.processors.cider :as processors.cider] [formatting-stack.strategies :as strategies])) @@ -64,7 +65,10 @@ (assoc :strategies (conj default-strategies strategies/exclude-edn strategies/exclude-clj - strategies/exclude-cljc)))]) + strategies/exclude-cljc))) + (-> (linters.one-resource-per-ns/new {}) + (assoc :strategies (conj default-strategies + strategies/files-with-a-namespace)))]) (def default-processors [(processors.cider/new {:third-party-indent-specs third-party-indent-specs})]) diff --git a/test-resources-extra/orpn.clj b/test-resources-extra/orpn.clj new file mode 100644 index 00000000..041e8590 --- /dev/null +++ b/test-resources-extra/orpn.clj @@ -0,0 +1 @@ +(ns orpn) diff --git a/test-resources-extra/orpn.cljc b/test-resources-extra/orpn.cljc new file mode 100644 index 00000000..041e8590 --- /dev/null +++ b/test-resources-extra/orpn.cljc @@ -0,0 +1 @@ +(ns orpn) diff --git a/test-resources-extra/orpn.cljs b/test-resources-extra/orpn.cljs new file mode 100644 index 00000000..041e8590 --- /dev/null +++ b/test-resources-extra/orpn.cljs @@ -0,0 +1 @@ +(ns orpn) diff --git a/test-resources/orpn.clj b/test-resources/orpn.clj new file mode 100644 index 00000000..041e8590 --- /dev/null +++ b/test-resources/orpn.clj @@ -0,0 +1 @@ +(ns orpn) diff --git a/test-resources/orpn.cljc b/test-resources/orpn.cljc new file mode 100644 index 00000000..041e8590 --- /dev/null +++ b/test-resources/orpn.cljc @@ -0,0 +1 @@ +(ns orpn) diff --git a/test-resources/orpn.cljs b/test-resources/orpn.cljs new file mode 100644 index 00000000..041e8590 --- /dev/null +++ b/test-resources/orpn.cljs @@ -0,0 +1 @@ +(ns orpn) diff --git a/test/formatting_stack/test_helpers.clj b/test/formatting_stack/test_helpers.clj new file mode 100644 index 00000000..fbb7a3ca --- /dev/null +++ b/test/formatting_stack/test_helpers.clj @@ -0,0 +1,10 @@ +(ns formatting-stack.test-helpers + (:require + [nedap.speced.def :as speced]) + (:import + (java.io File))) + +(speced/defn complete-filename [^string? filename] + (str "file:" (-> filename + File. + .getAbsolutePath))) diff --git a/test/integration/formatting_stack/linters/one_resource_per_ns.clj b/test/integration/formatting_stack/linters/one_resource_per_ns.clj new file mode 100644 index 00000000..1c78197a --- /dev/null +++ b/test/integration/formatting_stack/linters/one_resource_per_ns.clj @@ -0,0 +1,43 @@ +(ns integration.formatting-stack.linters.one-resource-per-ns + (:require + [clojure.test :refer :all] + [formatting-stack.linters.one-resource-per-ns :as sut] + [formatting-stack.test-helpers :as test-helpers] + [formatting-stack.util :refer [rcomp]])) + +(defn first! [coll] + {:pre [(->> coll count #{1})]} + (->> coll first)) + +(deftest analyze + + (testing "This namespace is unambiguous" + (is (= (sut/analyze "test/integration/formatting_stack/linters/one_resource_per_ns.clj") + ()))) + + (testing "Sample files are ambiguous (on a per-extension basis)" + (are [input extension expected] (testing input + (is (= (->> expected + (mapv test-helpers/complete-filename) + (set)) + (->> input + sut/analyze + (filter (rcomp :extension #{extension})) + (first!) + :filenames + set))) + true) + "test-resources/orpn.clj" ".clj" #{"test-resources-extra/orpn.clj" + "test-resources/orpn.clj"} + "test-resources-extra/orpn.clj" ".clj" #{"test-resources-extra/orpn.clj" + "test-resources/orpn.clj"} + + "test-resources/orpn.cljs" ".cljs" #{"test-resources-extra/orpn.cljs" + "test-resources/orpn.cljs"} + "test-resources-extra/orpn.cljs" ".cljs" #{"test-resources-extra/orpn.cljs" + "test-resources/orpn.cljs"} + + "test-resources/orpn.cljc" ".cljc" #{"test-resources-extra/orpn.cljc" + "test-resources/orpn.cljc"} + "test-resources-extra/orpn.cljc" ".cljc" #{"test-resources-extra/orpn.cljc" + "test-resources/orpn.cljc"}))) diff --git a/test/unit/formatting_stack/linters/one_resource_per_ns.clj b/test/unit/formatting_stack/linters/one_resource_per_ns.clj new file mode 100644 index 00000000..cacbd635 --- /dev/null +++ b/test/unit/formatting_stack/linters/one_resource_per_ns.clj @@ -0,0 +1,22 @@ +(ns unit.formatting-stack.linters.one-resource-per-ns + (:require + [clojure.test :refer :all] + [formatting-stack.linters.one-resource-per-ns :as sut] + [formatting-stack.test-helpers :as test-helpers])) + +(deftest ns-decl->resource-path + (are [input expected] (testing input + (is (= expected + (sut/ns-decl->resource-path input ".clj"))) + true) + '(ns unit) "unit.clj" + '(ns unit.formatting-stack.linters.one-resource-per-ns) "unit/formatting_stack/linters/one_resource_per_ns.clj" + '(ns foo!?) "foo_BANG__QMARK_.clj")) + +(deftest resource-path->filenames + (are [input] (testing input + (is (= (-> "test/" (str input) test-helpers/complete-filename vector) + (sut/resource-path->filenames input))) + true) + "unit/formatting_stack/linters/one_resource_per_ns.clj" + "unit/formatting_stack/linters/ns_aliases.clj")) From d65c108e47885e92fa211f5cdfafa6195516d230 Mon Sep 17 00:00:00 2001 From: vemv Date: Tue, 28 Jan 2020 07:45:24 +0100 Subject: [PATCH 2/3] v2.0.0-alpha1 --- README.md | 2 +- project.clj | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/README.md b/README.md index 4fe1f8b1..c2ed1fc5 100644 --- a/README.md +++ b/README.md @@ -73,7 +73,7 @@ The general intent is to make formatting: #### Coordinates ```clojure -[formatting-stack "1.0.1"] +[formatting-stack "2.0.0-alpha1"] ``` **Also** you have to add the latest [cider-nrepl](https://clojars.org/cider/cider-nrepl). diff --git a/project.clj b/project.clj index b571a304..97246638 100644 --- a/project.clj +++ b/project.clj @@ -1,5 +1,5 @@ ;; Please don't bump the library version by hand - use ci.release-workflow instead. -(defproject formatting-stack "1.0.1" +(defproject formatting-stack "2.0.0-alpha1" ;; Please keep the dependencies sorted a-z. :dependencies [[clj-kondo "2019.05.19-alpha"] [cljfmt "0.6.5" :exclusions [rewrite-clj]] From 6f1f783e6a883f8c3f1742705088ee24bb2df957 Mon Sep 17 00:00:00 2001 From: vemv Date: Tue, 28 Jan 2020 09:17:29 +0100 Subject: [PATCH 3/3] PR feedback --- README.md | 1 + .../linters/one_resource_per_ns.clj | 17 +++++++++-------- test-resources-extra/orpn.clj | 3 ++- test-resources-extra/orpn.cljc | 3 ++- test-resources-extra/orpn.cljs | 3 ++- test-resources/orpn.clj | 3 ++- test-resources/orpn.cljc | 3 ++- test-resources/orpn.cljs | 3 ++- test/formatting_stack/test_helpers.clj | 2 +- .../linters/one_resource_per_ns.clj | 6 +++--- .../linters/one_resource_per_ns.clj | 2 +- 11 files changed, 27 insertions(+), 19 deletions(-) diff --git a/README.md b/README.md index c2ed1fc5..a7a47f8e 100644 --- a/README.md +++ b/README.md @@ -31,6 +31,7 @@ And it also bundles a few tiny linters of its own: * [loc-per-ns](https://github.com/nedap/formatting-stack/blob/debdab8129dae7779d390216490625a3264c9d2c/src/formatting_stack/linters/loc_per_ns.clj) warns if a given NS surpasses a targeted LOC count. * [ns-aliases](https://github.com/nedap/formatting-stack/blob/debdab8129dae7779d390216490625a3264c9d2c/src/formatting_stack/linters/ns_aliases.clj) warns if [Sierra's](https://stuartsierra.com/2015/05/10/clojure-namespace-aliases) aliasing guide is disregarded. + * [one-resource-per-ns](https://github.com/nedap/formatting-stack/blob/master/src/formatting_stack/linters/one_resource_per_ns.clj) warns if a Clojure namespace is defined in more than one file. It is fully extensible: you can configure the bundled formatters, remove them, and/or add your own. diff --git a/src/formatting_stack/linters/one_resource_per_ns.clj b/src/formatting_stack/linters/one_resource_per_ns.clj index b16de65c..fe528e72 100644 --- a/src/formatting_stack/linters/one_resource_per_ns.clj +++ b/src/formatting_stack/linters/one_resource_per_ns.clj @@ -1,24 +1,25 @@ (ns formatting-stack.linters.one-resource-per-ns + "This linter ensures that there's exactly once classpath resource per namespace and extension. + + Note that generally it's fine to define identically-named Clojure/Script namespaces with _different_ extensions, + so that is allowed." (:require [clojure.spec.alpha :as spec] [clojure.string :as string] [clojure.tools.namespace.file :as file] [formatting-stack.protocols.linter :as linter] [formatting-stack.util :refer [process-in-parallel!]] + [formatting-stack.util.ns :as util.ns] [nedap.speced.def :as speced] [nedap.utils.modular.api :refer [implement]] [nedap.utils.spec.predicates :refer [present-string?]])) -(spec/def ::ns-decl (spec/and sequential? - (fn [x] - (->> x first #{'ns `ns})))) - (spec/def ::resource-path (spec/and present-string? (complement #{\. \! \? \-}) (fn [x] (re-find #"\.clj([cs])?$" x)))) -(speced/defn ^::resource-path ns-decl->resource-path [^::ns-decl ns-decl, extension] +(speced/defn ^::resource-path ns-decl->resource-path [^::util.ns/ns-form ns-decl, extension] (-> ns-decl second str @@ -41,7 +42,7 @@ filenames (resource-path->filenames resource-path)] :when (-> filenames count (> 1))] {:extension extension - :decl decl + :ns-name (-> decl second) :filenames filenames})) (defn lint! [this filenames] @@ -49,9 +50,9 @@ (process-in-parallel! (fn [filename] (->> filename analyze - (run! (speced/fn [{:keys [^some? decl, ^some? filenames]}] + (run! (speced/fn [{:keys [^symbol? ns-name, ^coll? filenames]}] (println "Warning: the namespace" - (str "`" (-> decl second) "`") + (str "`" ns-name "`") "is defined over more than one file.\nFound:" (->> filenames (interpose ", ") (apply str)))))))))) diff --git a/test-resources-extra/orpn.clj b/test-resources-extra/orpn.clj index 041e8590..7a3e43ba 100644 --- a/test-resources-extra/orpn.clj +++ b/test-resources-extra/orpn.clj @@ -1 +1,2 @@ -(ns orpn) +(ns orpn + "This namespace aides testing `formatting-stack.linters.one-resource-per-ns`") diff --git a/test-resources-extra/orpn.cljc b/test-resources-extra/orpn.cljc index 041e8590..7a3e43ba 100644 --- a/test-resources-extra/orpn.cljc +++ b/test-resources-extra/orpn.cljc @@ -1 +1,2 @@ -(ns orpn) +(ns orpn + "This namespace aides testing `formatting-stack.linters.one-resource-per-ns`") diff --git a/test-resources-extra/orpn.cljs b/test-resources-extra/orpn.cljs index 041e8590..7a3e43ba 100644 --- a/test-resources-extra/orpn.cljs +++ b/test-resources-extra/orpn.cljs @@ -1 +1,2 @@ -(ns orpn) +(ns orpn + "This namespace aides testing `formatting-stack.linters.one-resource-per-ns`") diff --git a/test-resources/orpn.clj b/test-resources/orpn.clj index 041e8590..7a3e43ba 100644 --- a/test-resources/orpn.clj +++ b/test-resources/orpn.clj @@ -1 +1,2 @@ -(ns orpn) +(ns orpn + "This namespace aides testing `formatting-stack.linters.one-resource-per-ns`") diff --git a/test-resources/orpn.cljc b/test-resources/orpn.cljc index 041e8590..7a3e43ba 100644 --- a/test-resources/orpn.cljc +++ b/test-resources/orpn.cljc @@ -1 +1,2 @@ -(ns orpn) +(ns orpn + "This namespace aides testing `formatting-stack.linters.one-resource-per-ns`") diff --git a/test-resources/orpn.cljs b/test-resources/orpn.cljs index 041e8590..7a3e43ba 100644 --- a/test-resources/orpn.cljs +++ b/test-resources/orpn.cljs @@ -1 +1,2 @@ -(ns orpn) +(ns orpn + "This namespace aides testing `formatting-stack.linters.one-resource-per-ns`") diff --git a/test/formatting_stack/test_helpers.clj b/test/formatting_stack/test_helpers.clj index fbb7a3ca..8d58ca52 100644 --- a/test/formatting_stack/test_helpers.clj +++ b/test/formatting_stack/test_helpers.clj @@ -4,7 +4,7 @@ (:import (java.io File))) -(speced/defn complete-filename [^string? filename] +(speced/defn filename-as-resource [^string? filename] (str "file:" (-> filename File. .getAbsolutePath))) diff --git a/test/integration/formatting_stack/linters/one_resource_per_ns.clj b/test/integration/formatting_stack/linters/one_resource_per_ns.clj index 1c78197a..41e37241 100644 --- a/test/integration/formatting_stack/linters/one_resource_per_ns.clj +++ b/test/integration/formatting_stack/linters/one_resource_per_ns.clj @@ -17,9 +17,9 @@ (testing "Sample files are ambiguous (on a per-extension basis)" (are [input extension expected] (testing input - (is (= (->> expected - (mapv test-helpers/complete-filename) - (set)) + (is (= (into #{} + (map test-helpers/filename-as-resource) + expected) (->> input sut/analyze (filter (rcomp :extension #{extension})) diff --git a/test/unit/formatting_stack/linters/one_resource_per_ns.clj b/test/unit/formatting_stack/linters/one_resource_per_ns.clj index cacbd635..e9ae3191 100644 --- a/test/unit/formatting_stack/linters/one_resource_per_ns.clj +++ b/test/unit/formatting_stack/linters/one_resource_per_ns.clj @@ -15,7 +15,7 @@ (deftest resource-path->filenames (are [input] (testing input - (is (= (-> "test/" (str input) test-helpers/complete-filename vector) + (is (= (-> "test/" (str input) test-helpers/filename-as-resource vector) (sut/resource-path->filenames input))) true) "unit/formatting_stack/linters/one_resource_per_ns.clj"