Skip to content

Commit

Permalink
Lint bb.edn and deps.edn :paths for data type (clj-kondo#1358)
Browse files Browse the repository at this point in the history
* Lint bb.edn and deps.edn :paths for data type

We recognize that deps.edn and bb.edn differ slightly in what elem types
they support for :paths.
- deps.edn - strings or keywords
- bb.edn - strings onlys

When reporting on data type errors, I've favoured generic English words
over specific class names. For example instead of:

 Expected vector, found: clojure.lang.PersistentHashMap

We report:

 Expected vector, found: map

Closes clj-kondo#1353

* Respond to reviewer feedback on clj-kondo#1353

Switch from condp to case.
  • Loading branch information
lread authored Aug 19, 2021
1 parent 8cee4e8 commit b6f76ea
Show file tree
Hide file tree
Showing 4 changed files with 109 additions and 4 deletions.
2 changes: 1 addition & 1 deletion doc/linters.md
Original file line number Diff line number Diff line change
Expand Up @@ -199,7 +199,7 @@ A regex is also permitted, e.g. to exclude all test namespaces:

*Keyword:* `:deps.edn`

*Description:* warn on common errors in a `deps.edn`.
*Description:* warn on common errors in `deps.edn` and `bb.edn` files.

*Default level:* `:warning`

Expand Down
8 changes: 5 additions & 3 deletions src/clj_kondo/impl/analyzer.clj
Original file line number Diff line number Diff line change
Expand Up @@ -2177,9 +2177,11 @@
(analyze-expressions ctx (:children parsed))
;; analyze-expressions should go first in order to process ignores
(when (identical? :edn lang)
(let [fn (.getName (io/file filename))]
(when (= fn "deps.edn")
(deps-edn/lint-deps-edn ctx (first (:children parsed)))))))))
(let [fname (.getName (io/file filename))]
(case fname
"deps.edn" (deps-edn/lint-deps-edn ctx (first (:children parsed)))
"bb.edn" (deps-edn/lint-bb-edn ctx (first (:children parsed)))
nil))))))
(catch Exception e
(if dev?
(throw e)
Expand Down
67 changes: 67 additions & 0 deletions src/clj_kondo/impl/linters/deps_edn.clj
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
(ns clj-kondo.impl.linters.deps-edn
"Linter for deps.edn and bb.edn file contents."
(:require [clj-kondo.impl.findings :as findings]
[clj-kondo.impl.utils :refer [sexpr node->line]]
[clojure.string :as str]))
Expand Down Expand Up @@ -27,6 +28,62 @@
vnodes)
(take-nth 2 (rest (:children map-node)))))

(defn name-for-type [form]
(cond
(nil? form) "nil"
(symbol? form) "symbol"
(int? form) "int"
(float? form) "float"
(keyword? form) "keyword"
(char? form) "char"
(string? form) "string"
(map? form) "map"
(vector? form) "vector"
(list? form) "list"
(set? form) "set"
:else (.getName (class form))))

(defn lint-paths-container [ctx node ]
(let [form (sexpr node)]
(when-not (vector? form)
(findings/reg-finding!
ctx
(node->line (:filename ctx)
node
:deps.edn
(str "Expected vector, found: " (name-for-type form))))
true)))

(defn lint-bb-edn-paths-elems [ctx node]
(doseq [node (:children node)]
(let [form (sexpr node)]
(when-not (string? form)
(findings/reg-finding!
ctx
(node->line (:filename ctx)
node
:deps.edn
(str "Expected string, found: " (name-for-type form))))))))

(defn lint-deps-edn-paths-elems [ctx node]
(doseq [node (:children node)]
(let [form (sexpr node)]
(when-not (or (keyword? form) (string? form))
(findings/reg-finding!
ctx
(node->line (:filename ctx)
node
:deps.edn
(str "Expected string or keyword, found: " (name-for-type form))))))))

(defn lint-bb-edn-paths [ctx node]
(when (and node (not (lint-paths-container ctx node)))
(lint-bb-edn-paths-elems ctx node)))

(defn lint-deps-edn-paths [ctx node]
(when (and node (not (lint-paths-container ctx node)))
(lint-deps-edn-paths-elems ctx node)))

(defn lint-qualified-lib [ctx node]
(let [form (sexpr node)]
(when-not (or (qualified-symbol? form)
Expand Down Expand Up @@ -167,9 +224,19 @@
(zipmap (-> raw-node key-nodes)
(-> raw-node val-nodes)))

(defn lint-bb-edn [ctx expr]
(try
(let [bb-edn (sexpr-keys expr)]
(lint-bb-edn-paths ctx (:paths bb-edn)))
;; Due to ubiquitous use of sexpr, we're catching coercion errors here and let them slide.
(catch Exception e
(binding [*out* *err*]
(println "ERROR: " (.getMessage e))))))

(defn lint-deps-edn [ctx expr]
(try
(let [deps-edn (sexpr-keys expr)
_ (lint-deps-edn-paths ctx (:paths deps-edn))
deps (:deps deps-edn)
_ (lint-deps ctx (deps-map deps))
aliases (:aliases deps-edn)
Expand Down
36 changes: 36 additions & 0 deletions test/clj_kondo/deps_edn_test.clj
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,42 @@
(:require [clj-kondo.test-utils :refer [lint! assert-submaps]]
[clojure.test :refer [deftest testing is]]))

(deftest paths-test
(testing "no report on valid value types"
(let [bb-edn "{:paths [\"src\" \"test\"]}"]
(is (empty? (lint! bb-edn
"--filename" "bb.edn"))))
(let [deps-edn "{:paths [\"src\" \"test\" :alias1]}"]
(is (empty? (lint! deps-edn
"--filename" "deps.edn")))))
(testing "report on value not a vector"
(let [edn "{:paths scripts}"]
(doseq [fname ["bb.edn" "deps.edn"]]
(assert-submaps
(list {:file fname :row 1 :col 9 :level :warning :message "Expected vector, found: symbol"})
(lint! edn
"--filename" fname)))))
(testing "when container type wrong, report only on container type and not container elems"
(let [edn "{:paths {bad 32}}"]
(doseq [fname ["bb.edn" "deps.edn"]]
(assert-submaps
(list {:file fname :row 1 :col 9 :level :warning :message "Expected vector, found: map"})
(lint! edn
"--filename" fname)))) )
(testing "report on each unexpected vector elem type"
(let [deps-edn "{:paths [scripts 42 \"ok\" :alias]}"]
(assert-submaps
'({:file "deps.edn" :row 1 :col 10 :level :warning :message "Expected string or keyword, found: symbol"}
{:file "deps.edn" :row 1 :col 18 :level :warning :message "Expected string or keyword, found: int"})
(lint! deps-edn
"--filename" "deps.edn"))
(assert-submaps
'({:file "bb.edn" :row 1 :col 10 :level :warning :message "Expected string, found: symbol"}
{:file "bb.edn" :row 1 :col 18 :level :warning :message "Expected string, found: int"}
{:file "bb.edn" :row 1 :col 26 :level :warning :message "Expected string, found: keyword"})
(lint! deps-edn
"--filename" "bb.edn")))))

(deftest qualified-lib-test
(let [deps-edn '{:deps {clj-kondo {:mvn/version "2020.10.10"}}
:aliases {:foo {:extra-deps {clj-kondo {:mvn/version "2020.10.10"
Expand Down

0 comments on commit b6f76ea

Please sign in to comment.