Add invalid-fn-name linter

Fixes clj-kondo#2159 paritially

CHANGELOG.md

@@ -19,6 +19,7 @@ For a list of breaking changes, check [here](#breaking-changes).
 
 ## Unreleased
 
+- [#2159](https://github.com/clj-kondo/clj-kondo/issues/2159): New linter, `:invalid-fn-name`
 - [#2207](https://github.com/clj-kondo/clj-kondo/issues/2207): New `:condition-always-true` linter, see [docs](doc/linters.md)
 - [#2013](https://github.com/clj-kondo/clj-kondo/issues/2013): Fix NPE and similar errors when linting an import with an illegal token
 

doc/linters.md

@@ -47,6 +47,7 @@ configuration. For general configurations options, go [here](config.md).
     - [Format](#format)
     - [Def + fn instead of defn](#def--fn-instead-of-defn)
     - [Inline def](#inline-def)
+    - [Invalid fn name](#invalid-fn-name)
     - [Invalid arity](#invalid-arity)
     - [Conflicting arity](#conflicting-arity)
     - [Reduce without initial value](#reduce-without-initial-value)
@@ -845,6 +846,21 @@ See [issue](https://github.com/clj-kondo/clj-kondo/issues/1920).
 
 *Example message:* `inline def`.
 
+### Invalid fn name
+
+**Keyword:** `:invalid-fn-name`.
+
+*Description:* warn when a function's name is not valid. If present, it should
+be an unquoted symbol.
+
+*Default level:* `:error`.
+
+*Example trigger:* `(fn :fn-name [x] (inc x))`.
+
+*Example message:* `First arg of fn should be a symbol, arg vector or body list`.
+
+*Config:*
+
 ### Invalid arity
 
 **Keyword:** `:invalid-arity`.

src/clj_kondo/impl/analyzer.clj

@@ -934,6 +934,21 @@
 (defn- let? [x]
   (one-of x [[clojure.core let] [cljs.core let]]))
 
+(defn- invalid-fn-name? [?name-expr]
+  (let [valid? (some-fn utils/list-node? ; fn body (usually 1 of many)
+                        utils/vector-node? ; fn args
+                        utils/symbol-token?)] ; fn name
+    (not (valid? ?name-expr))))
+
+(defn- reg-invalid-fn-name! [ctx expr filename]
+  (findings/reg-finding!
+   ctx
+   (node->line
+    filename
+    expr
+    :invalid-fn-name
+    "First arg of fn should be a symbol, arg vector or body list")))
+
 (defn- def-fn? [{:keys [callstack]}]
   (let [[_ parent extra-parent] callstack]
     (or (def? parent)
@@ -985,6 +1000,10 @@
     (when (and (not (linter-disabled? ctx :def-fn))
                (def-fn? ctx))
       (reg-def-fn! ctx expr filename))
+    (when (and (not (linter-disabled? ctx :valid-fn-name))
+               ?name-expr
+               (invalid-fn-name? ?name-expr))
+      (reg-invalid-fn-name! ctx expr filename))
     (with-meta parsed-bodies
       (when arities
         (cond-> {:arity {:fixed-arities fixed-arities

src/clj_kondo/impl/config.clj

@@ -25,6 +25,7 @@
               :private-call {:level :error}
               :inline-def {:level :warning}
               :def-fn {:level :off}
+              :invalid-fn-name {:level :error}
               :redundant-do {:level :warning}
               :redundant-let {:level :warning}
               :cond-else {:level :warning}

src/clj_kondo/impl/macroexpand.clj

@@ -12,19 +12,77 @@
         x (if m* (assoc x :meta m*) x)]
     (with-meta x m)))
 
+(defn find-children
+  "Recursively filters children by pred"
+  [pred children]
+  (mapcat #(if (pred %)
+             [(pred %)]
+             (when-let [cchildren (:children %)]
+               (find-children pred cchildren)))
+          children))
+
+(defn fn-args [children]
+  (let [args (find-children
+              #(and (= :token (tag %))
+                    (:string-value %)
+                    (when-let [[_ n] (re-matches #"%((\d?\d?)|&)" (:string-value %))]
+                      (case n
+                        "" 1
+                        "&" 0
+                        (Integer/parseInt n))))
+              children)
+        args (sort args)
+        varargs? (when-let [fst (first args)]
+                   (zero? fst))
+        args (seq (if varargs? (rest args) args))
+        max-n (last args)
+        args (when args (map (fn [i]
+                               (symbol (str "%" i)))
+                             (range 1 (inc max-n))))]
+    {:varargs? varargs?
+     :args args}))
+
+(defn expand-fn [{:keys [:children] :as expr}]
+  (let [{:keys [:row :col] :as m} (meta expr)
+        {:keys [:args :varargs?]} (fn-args children)
+        fn-body (with-meta (list-node children)
+                  (assoc m
+                         :row row
+                         :col (inc col)))
+        arg-list (vector-node
+                  (map #(with-meta (token-node %)
+                          {:clj-kondo/mark-used true
+                           :clj-kondo/skip-reg-binding true})
+                       (if varargs?
+                         (concat args '[& %&])
+                         args)))
+        has-first-arg? (= '%1 (first args))]
+    (with-meta
+      (list-node [(token-node 'fn*) arg-list
+                  fn-body])
+      (assoc m :clj-kondo.impl/fn-has-first-arg has-first-arg?))))
+
 (defn expand-> [_ctx expr]
   (let [expr expr
         children (:children expr)
         [c & cforms] (rest children)
         ret (loop [x c, forms cforms]
               (if forms
                 (let [form (first forms)
-                      threaded (if (= :list (tag form))
-                                 (with-meta-of
-                                   (list-node (list* (first (:children form))
-                                                     x
-                                                     (next (:children form))))
-                                   form)
+                      threaded (case (tag form)
+                                 :list (with-meta-of
+                                         (list-node (list* (first (:children form))
+                                                           x
+                                                           (next (:children form))))
+                                         form)
+                                 ;; Short-form fns need expanding now, to thread the name in
+                                 :fn (with-meta-of
+                                       (let [{:keys [children] :as expanded} (expand-fn form)]
+                                         (assoc expanded :children
+                                                (list* (first children)
+                                                       x
+                                                       (next children))))
+                                       form)
                                  (with-meta-of (list-node (list form x))
                                    form))]
                   (recur threaded (next forms)))
@@ -130,56 +188,6 @@
         (recur (cons node more) )
         node))))
 
-(defn find-children
-  "Recursively filters children by pred"
-  [pred children]
-  (mapcat #(if (pred %)
-             [(pred %)]
-             (when-let [cchildren (:children %)]
-               (find-children pred cchildren)))
-          children))
-
-(defn fn-args [children]
-  (let [args (find-children
-              #(and (= :token (tag %))
-                    (:string-value %)
-                    (when-let [[_ n] (re-matches #"%((\d?\d?)|&)" (:string-value %))]
-                      (case n
-                        "" 1
-                        "&" 0
-                        (Integer/parseInt n))))
-              children)
-        args (sort args)
-        varargs? (when-let [fst (first args)]
-                   (zero? fst))
-        args (seq (if varargs? (rest args) args))
-        max-n (last args)
-        args (when args (map (fn [i]
-                               (symbol (str "%" i)))
-                             (range 1 (inc max-n))))]
-    {:varargs? varargs?
-     :args args}))
-
-(defn expand-fn [{:keys [:children] :as expr}]
-  (let [{:keys [:row :col] :as m} (meta expr)
-        {:keys [:args :varargs?]} (fn-args children)
-        fn-body (with-meta (list-node children)
-                  (assoc m
-                         :row row
-                         :col (inc col)))
-        arg-list (vector-node
-                  (map #(with-meta (token-node %)
-                          {:clj-kondo/mark-used true
-                           :clj-kondo/skip-reg-binding true})
-                       (if varargs?
-                         (concat args '[& %&])
-                         args)))
-        has-first-arg? (= '%1 (first args))]
-    (with-meta
-      (list-node [(token-node 'fn*) arg-list
-                  fn-body])
-      (assoc m :clj-kondo.impl/fn-has-first-arg has-first-arg?))))
-
 (defn expand-do-template [_ctx node]
   (let [[_ argv expr & values] (:children node)
         c (count (:children argv))

src/clj_kondo/impl/utils.clj

@@ -38,6 +38,10 @@
   (and (instance? clj_kondo.impl.rewrite_clj.node.seq.SeqNode n)
        (identical? :set (tag n))))
 
+(defn vector-node? [n]
+  (and (instance? clj_kondo.impl.rewrite_clj.node.seq.SeqNode n)
+       (identical? :vector (tag n))))
+
 ;;; end export
 
 (defn print-err! [& strs]

test/clj_kondo/main_test.clj

@@ -9,7 +9,7 @@
    [clojure.edn :as edn]
    [clojure.java.io :as io]
    [clojure.string :as str]
-   [clojure.test :as t :refer [deftest is testing]]
+   [clojure.test :as t :refer [deftest is are testing]]
    [missing.test.assertions]))
 
 (deftest self-lint-test
@@ -59,6 +59,29 @@
       (is (empty? (lint! "(def x (reify Object (toString [_] \"x\")))" "--lang" (name lang) "--config" (pr-str config))))
       (is (empty? (lint! "(require '[some.ns :refer [my-reify]]) (def x (my-reify Object (toString [_] \"x\")))" "--lang" (name lang) "--config" (pr-str config)))))))
 
+(deftest invalid-fn-name-test
+  (assert-submaps
+   '({:row 1, :col 1, :level :error, :message "First arg of fn should be a symbol, arg vector or body list"})
+   (lint! "(fn \"fn-name\" [x] (inc x))"))
+  (assert-submaps
+   '({:row 1, :col 6, :level :error, :message "First arg of fn should be a symbol, arg vector or body list"})
+   (lint! "(map (fn 'symbol ([x] (inc x))) coll)"))
+  (assert-submaps
+   '({:row 1, :col 7, :level :error, :message "First arg of fn should be a symbol, arg vector or body list"})
+   (lint! "(-> 7 (fn [x] (inc x)))"))
+  (assert-submaps
+   '({:row 1, :col 7, :level :error, :message "First arg of fn should be a symbol, arg vector or body list"})
+   (lint! "(-> 7 #(inc %))"))
+  (assert-submaps
+   '({:row 1, :col 1, :level :error, :message "First arg of fn should be a symbol, arg vector or body list"})
+   (lint! "(fn* :fn-name [x] (inc x))"))
+
+  (are [lint-form] (empty? lint-form)
+    (lint! "(fn fn-name [x] (inc x))")
+    (lint! "(fn* fn-name [x] (inc x))")
+    ;; A future linter could warn here, as it's almost never intentional, especially if fn-name is bound
+    (lint! "(-> fn-name #(inc %))")))
+
 (deftest redundant-let-test
   (let [linted (lint! (io/file "corpus" "redundant_let.clj"))
         row-col-files (map #(select-keys % [:row :col :file])