Fix clj-kondo#1920: :def-fn linter (@andreyorst) (clj-kondo#1948)

Co-authored-by: Andrey Listopadov <[email protected]>

.clj-kondo/config.edn

@@ -22,7 +22,8 @@
            :missing-docstring {:level :off}
            :unsorted-required-namespaces {:level :warning}
            ;; for  nubank/matcher-combinators
-           :unresolved-symbol {:exclude [(clojure.test/is [match?])]}}
+           :unresolved-symbol {:exclude [(clojure.test/is [match?])]}
+           :def-fn {:level :warning}}
  :lint-as {me.raynes.conch/programs clojure.core/declare
            me.raynes.conch/let-programs clojure.core/let
            datalog.parser.type/deftrecord clojure.core/defrecord

CHANGELOG.md

@@ -15,6 +15,8 @@ For a list of breaking changes, check [here](#breaking-changes).
 
 ## Unreleased
 
+- [#1920](https://github.com/clj-kondo/clj-kondo/issues/1920): new linter `:def-fn`: warn when using `fn` inside `def`, or `fn` inside `let` inside `def` ([@andreyorst](https://github.com/andreyorst)).
+
 ## 2023.01.12
 
 - [#1742](https://github.com/clj-kondo/clj-kondo/issues/1742): new linter `:aliased-namespace-var-usage`: warn on var usage from namespaces that were used with `:as-alias`. See [demo](https://twitter.com/borkdude/status/1613524896625340417/photo/1).

doc/linters.md

@@ -37,6 +37,7 @@ configuration. For general configurations options, go [here](config.md).
     - [Quoted case test constant](#quoted-case-test-constant)
     - [File](#file)
     - [Format](#format)
+    - [Idiomatic closures](#idiomatic-closures)
     - [Inline def](#inline-def)
     - [Invalid arity](#invalid-arity)
     - [Conflicting arity](#conflicting-arity)
@@ -638,6 +639,32 @@ Explanation by Bozhidar Batsov:
 
 *Example message:* `Format string expects 1 arguments instead of 2.`.
 
+### Idiomatic closures
+
+*Keyword:* `:def-fn`.
+
+*Description:* tells about closures defined with the combination of
+`def` and `fn` with optional `let` in-between. In almost all cases
+`defn` can be used instead. Since `defn` always binds at the top
+level, it can be used inside a top-level `let`.
+
+*Default level:* `:off`.
+
+*Example triggers:*
+
+- `(def f (fn [] nil))`
+- `(def f (let [y 1] (fn [x] (+ x y))))`
+
+*Example messages:*
+
+- `Use defn instead of def + fn`
+
+*Config:*
+
+``` clojure
+{:linters {:def-fn {:level :info}}}
+```
+
 ### Inline def
 
 *Keyword:* `:inline-def`.

script/diff

@@ -10,7 +10,8 @@ read -r -d '' config <<'EOF' || true
    :not-a-function
     {:skip-args [clojure.pprint/defdirectives
                  cljs.pprint/defdirectives
-                 clojure.data.json/codepoint-case]}}}
+                 clojure.data.json/codepoint-case]}
+  :def-fn {:level :warning}}}
 EOF
 
 out="/tmp/clj-kondo-diff/branch.txt"

src/aaaa_this_has_to_be_first/pprint.clj

@@ -10,12 +10,12 @@
                     (zipmap (keys m)
                             (map find-var (vals m))))))
 
-(def new-table-ize
-  (fn [t m]
-    (apply hash-map
-           (mapcat
-            #(when-let [v (get t (key %))] [v (val %)])
-            m))))
+(defn new-table-ize
+  [t m]
+  (apply hash-map
+         (mapcat
+          #(when-let [v (get t (key %))] [v (val %)])
+          m)))
 
 (when-not @patched?
   (alter-var-root #'pprint/table-ize (constantly new-table-ize))
@@ -25,28 +25,28 @@
   (alter-meta! #'pprint/make-pretty-writer dissoc :private)
   (alter-meta! #'pprint/execute-format dissoc :private))
 
-(def new-write
-  (fn [object & kw-args]
-    (let [options (merge {:stream true} (apply hash-map kw-args))]
-      (with-bindings (new-table-ize pprint/write-option-table options)
-        (with-bindings
-          (if (or (not (= pprint/*print-base* 10)) pprint/*print-radix*)
-            {#'pr @#'pprint/pr-with-base} {})
-          (let [optval (if (contains? options :stream)
-                         (:stream options)
-                         true)
-                base-writer (condp = optval
-                              nil (java.io.StringWriter.)
-                              true *out*
-                              optval)]
-            (if pprint/*print-pretty*
-              (pprint/with-pretty-writer base-writer
-                (pprint/write-out object))
-              (binding [*out* base-writer]
-                (pr object)))
-            #_:clj-kondo/ignore
-            (if (nil? optval)
-              (.toString ^java.io.StringWriter base-writer))))))))
+(defn new-write
+  [object & kw-args]
+  (let [options (merge {:stream true} (apply hash-map kw-args))]
+    (with-bindings (new-table-ize pprint/write-option-table options)
+      (with-bindings
+        (if (or (not (= pprint/*print-base* 10)) pprint/*print-radix*)
+          {#'pr @#'pprint/pr-with-base} {})
+        (let [optval (if (contains? options :stream)
+                       (:stream options)
+                       true)
+              base-writer (condp = optval
+                            nil (java.io.StringWriter.)
+                            true *out*
+                            optval)]
+          (if pprint/*print-pretty*
+            (pprint/with-pretty-writer base-writer
+              (pprint/write-out object))
+            (binding [*out* base-writer]
+              (pr object)))
+          #_:clj-kondo/ignore
+          (if (nil? optval)
+            (.toString ^java.io.StringWriter base-writer)))))))
 
 (when-not @patched?
   (alter-var-root #'pprint/write (constantly new-write)))

src/clj_kondo/impl/analyzer.clj

@@ -852,6 +852,26 @@
       varargs-min-arity (assoc :varargs-min-arity varargs-min-arity)
       (seq arglist-strs) (assoc :arglist-strs arglist-strs))))
 
+(def ^:private def?
+  '#{[clojure.core def] [cljs.core def]})
+
+(def ^:private let?
+  '#{[clojure.core let] [cljs.core let]})
+
+(defn- def-fn? [{:keys [callstack]}]
+  (let [[_ parent extra-parent] callstack]
+    (or (def? parent)
+        (and (let? parent) (def? extra-parent)))))
+
+(defn- reg-def-fn! [ctx expr filename]
+  (findings/reg-finding!
+   ctx
+   (node->line
+    filename
+    expr
+    :def-fn
+    "Use defn instead of def + fn")))
+
 (defn analyze-fn [ctx expr]
   (let [ctx (assoc ctx :seen-recur? (volatile! nil))
         protocol-fn (:protocol-fn expr)
@@ -886,6 +906,9 @@
         varargs-min-arity (when arities (get-in arities [:varargs :min-arity]))
         arglist-strs (when arities (into [] (keep :arglist-str) (vals arities)))
         parsed-bodies (mapcat :parsed parsed-bodies)]
+    (when (and (not (linter-disabled? ctx :def-fn))
+               (def-fn? ctx))
+      (reg-def-fn! ctx expr filename))
     (with-meta parsed-bodies
       (when arities
         (cond-> {:arity {:fixed-arities fixed-arities