From 86511ead6baf52a20872271fd621faee91393217 Mon Sep 17 00:00:00 2001 From: Tom Dalziel Date: Mon, 26 Feb 2024 00:41:57 +0100 Subject: [PATCH] Expand fn-literals in lists --- CHANGELOG.md | 1 + src/clj_kondo/impl/analyzer.clj | 69 ++++++++++++++++++---------- src/clj_kondo/impl/linters.clj | 2 +- src/clj_kondo/impl/utils.clj | 3 +- test/clj_kondo/main_test.clj | 6 ++- test/clj_kondo/unused_value_test.clj | 9 ++++ 6 files changed, 63 insertions(+), 27 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 17e0872d2a..3ebdfe4961 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -24,6 +24,7 @@ For a list of breaking changes, check [here](#breaking-changes). - [#2416](https://github.com/clj-kondo/clj-kondo/issues/2416): detect empty `require` and `:require` forms ([@NoahTheDuke](https://github.com/NoahTheDuke)) - [#1786](https://github.com/clj-kondo/clj-kondo/issues/1786): Support `gen-interface` (by suppressing unresolved symbols) - [#2420](https://github.com/clj-kondo/clj-kondo/issues/2420): Detect uneven number of clauses in cond-> and cond->> +- [#1923](https://github.com/clj-kondo/clj-kondo/issues/1923): Lint invalid fn name for threaded fn literals ## 2024.09.27 diff --git a/src/clj_kondo/impl/analyzer.clj b/src/clj_kondo/impl/analyzer.clj index f87ddf221d..911b4a491f 100644 --- a/src/clj_kondo/impl/analyzer.clj +++ b/src/clj_kondo/impl/analyzer.clj @@ -2876,7 +2876,7 @@ (and (not generated?) core? (not (:clj-kondo.impl/generated (meta parent-call))) - (one-of core-sym [do fn defn defn- + (one-of core-sym [do fn fn* defn defn- let when-let loop binding with-open doseq try when when-not when-first when-some future]))] @@ -2886,6 +2886,39 @@ :message "Unused value" :filename (:filename ctx)))))))) +(defn update-literal-fn-ctx [ctx expr] + (cond-> (assoc ctx :arg-types nil :in-fn-literal true) + (:clj-kondo.impl/fn-has-first-arg (meta expr)) + (update :bindings assoc '% {}))) + +(defn analyze-literal-fn! [lang ctx expr] + (lint-unused-value ctx expr) + (when (and (:in-fn-literal ctx) + (not (:clj-kondo.impl/generated expr))) + (findings/reg-finding! ctx (assoc (meta expr) + :filename (:filename ctx) + :level :error + :type :syntax + :message "Nested #()s are not allowed"))) + (when (identical? :edn lang) + (findings/reg-finding! ctx (assoc (meta expr) + :filename (:filename ctx) + :level :error + :type :syntax + :message "#()s are not allowed in EDN")))) + +(defn expand-fns + "For every child of expr (assumed to be a call) expand function + literals (i.e. #(...) forms) to (fn* ...) forms and mark these as expanded." + [expr] + (update expr :children + (fn [children] + (for [node children] + (cond-> node + (identical? :fn (tag node)) + (-> macroexpand/expand-fn + (vary-meta assoc :clj-kondo.impl/expanded? true))))))) + #_(requiring-resolve 'clojure.set/union) (defn analyze-expression** @@ -2896,7 +2929,7 @@ (:quoted ctx)) (meta/lift-meta-content2 (dissoc ctx :arg-types) expr) expr) - t (tag expr) + t (if (:clj-kondo.impl/expanded? (meta expr)) :exp-fn (tag expr)) {:keys [row col]} (meta expr) arg-count (count (rest children))] (utils/handle-ignore ctx expr) @@ -2963,27 +2996,15 @@ (analyze-children (update ctx :callstack #(cons [nil t] %)) children)) - :fn (do - (lint-unused-value ctx expr) - (when (and (:in-fn-literal ctx) - (not (:clj-kondo.impl/generated expr))) - (findings/reg-finding! ctx (assoc (meta expr) - :filename (:filename ctx) - :level :error - :type :syntax - :message "Nested #()s are not allowed"))) - (when (identical? :edn lang) - (findings/reg-finding! ctx (assoc (meta expr) - :filename (:filename ctx) - :level :error - :type :syntax - :message "#()s are not allowed in EDN"))) - (let [expanded-node (macroexpand/expand-fn expr) - m (meta expanded-node) - has-first-arg? (:clj-kondo.impl/fn-has-first-arg m)] - (recur (cond-> (assoc ctx :arg-types nil :in-fn-literal true) - has-first-arg? (update :bindings assoc '% {})) - expanded-node))) + + :exp-fn (do (analyze-literal-fn! lang ctx expr) + (recur (update-literal-fn-ctx ctx expr) + (vary-meta expr assoc :clj-kondo.impl/expanded? false))) + :fn (do (analyze-literal-fn! lang ctx expr) + (let [expanded-node (macroexpand/expand-fn expr)] + (recur (update-literal-fn-ctx ctx expanded-node) + expanded-node))) + :token (let [edn? (= :edn lang)] (if (or edn? @@ -3077,7 +3098,7 @@ :full-fn-name full-fn-name :row row :col col - :expr expr}) + :expr (expand-fns expr)}) _ (dorun ret) ;; realize all returned expressions ;; to not be bitten by laziness maybe-call (some #(when (= id (:id %)) %) ret)] diff --git a/src/clj_kondo/impl/linters.clj b/src/clj_kondo/impl/linters.clj index a8e0d01fa6..5523c44839 100644 --- a/src/clj_kondo/impl/linters.clj +++ b/src/clj_kondo/impl/linters.clj @@ -508,7 +508,7 @@ ;; doseq always return nil (utils/one-of core-sym [doseq]) (< idx (dec (:len call)))) - (utils/one-of core-sym [do fn defn defn- + (utils/one-of core-sym [do fn fn* defn defn- let when-let loop binding with-open doseq try when when-not when-first when-some future])) diff --git a/src/clj_kondo/impl/utils.clj b/src/clj_kondo/impl/utils.clj index 61af65bb64..1a79c45d7f 100644 --- a/src/clj_kondo/impl/utils.clj +++ b/src/clj_kondo/impl/utils.clj @@ -19,7 +19,8 @@ ;;; export rewrite-clj functions (defn tag [expr] - (node/tag expr)) + (when (satisfies? node/Node expr) + (node/tag expr))) (def map-node seq/map-node) (def vector-node seq/vector-node) diff --git a/test/clj_kondo/main_test.clj b/test/clj_kondo/main_test.clj index 904928ecf1..efe143d28d 100644 --- a/test/clj_kondo/main_test.clj +++ b/test/clj_kondo/main_test.clj @@ -3507,7 +3507,11 @@ foo/"))) '({:file "", :row 1, :col 7, :level :error, :message "Function name must be simple symbol but got: :foo"} {:file "", :row 1, :col 20, :level :error, :message "Function name must be simple symbol but got: :foo"} {:file "", :row 1, :col 33, :level :error, :message "Function name must be simple symbol but got: \"foo\""}) - (lint! "(defn :foo []) (fn :foo []) (fn \"foo\" [])"))) + (lint! "(defn :foo []) (fn :foo []) (fn \"foo\" [])")) + (assert-submaps + '({:file "", :row 1, :col 5, :level :error, :message "Function name must be simple symbol but got: :foo"} + {:file "", :row 1, :col 24, :level :error, :message "Function name must be simple symbol but got: \"foo\""}) + (lint! "(-> :foo #(inc %)) (-> \"foo\" #(inc %))"))) (deftest lint-stdin-exclude-files-test (is (empty? diff --git a/test/clj_kondo/unused_value_test.clj b/test/clj_kondo/unused_value_test.clj index 79dd83f70f..bcb28d82eb 100644 --- a/test/clj_kondo/unused_value_test.clj +++ b/test/clj_kondo/unused_value_test.clj @@ -182,3 +182,12 @@ bar)" (is (empty? (lint! "(fn [] '({:a 1} {:b 2} {:c 3}))" {:linters {:unused-value {:level :warning}}})))) + +(deftest thread-last-test + (assert-submaps + '({:file "", :row 1, :col 12, :level :warning, :message "Unused value"}) + (lint! "(->> :foo #(name %))" + {:linters {:unused-value {:level :warning} + :redundant-fn-wrapper {:level :off}}})) + (is (empty? (lint! "(->> :foo (#(name %)))" {:linters {:unused-value {:level :warning} + :redundant-fn-wrapper {:level :off}}}))))