Add if-nil-return linter

src/clj_kondo/impl/config.clj

@@ -95,6 +95,7 @@
                           :exclude #{}}
               :use {:level :warning}
               :missing-else-branch {:level :warning}
+              :if-nil-return {:level :off}
               :case-duplicate-test {:level :error}
               :case-quoted-test {:level :warning}
               :case-symbol-test {:level :off}

src/clj_kondo/impl/linters.clj

@@ -113,6 +113,33 @@
                                  (node->line (:filename ctx) expr :missing-else-branch
                                              (format "Missing else branch."))))))))
 
+(defn lint-if-nil-return
+  "Lint returning nil from if-like expressions. When-like expressions are
+  preferred."
+  [ctx expr]
+  (let [config (:config ctx)
+        level (-> config :linters :if-nil-return :level)]
+    (when-not (identical? :off level)
+      (let [children (:children expr)
+            [_condition then-branch else-branch] (rest children)]
+        (cond (= [:value nil] (find else-branch :value))
+              (when-let [preferred (get '{if when
+                                          if-let when-let
+                                          if-some when-some
+                                          if-not when-not}
+                                        (:value (first children)))]
+                (findings/reg-finding! ctx
+                                       (node->line (:filename ctx) expr :if-nil-return
+                                                   (format "For nil return, prefer %s." preferred))))
+              (= [:value nil] (find then-branch :value))
+              ;; We don't check if-let and if-some here as there'd be an unused binding warning anyway
+              (when-let [preferred (get '{if when-not
+                                          if-not when}
+                                        (:value (first children)))]
+                (findings/reg-finding! ctx
+                                       (node->line (:filename ctx) expr :if-nil-return
+                                                   (format "For nil return, prefer %s." preferred)))))))))
+
 (defn lint-single-key-in [ctx called-name call]
   (when-not (utils/linter-disabled? ctx :single-key-in)
     (let [[_ _ keyvec] (:children call)]
@@ -133,7 +160,8 @@
       (lint-cond ctx (:expr call))
       ([clojure.core if-let] [clojure.core if-not] [clojure.core if-some]
        [cljs.core if-let] [cljs.core if-not] [cljs.core if-some])
-      (lint-missing-else-branch ctx (:expr call))
+      (do (lint-missing-else-branch ctx (:expr call))
+          (lint-if-nil-return ctx (:expr call)))
       ([clojure.core get-in] [clojure.core assoc-in] [clojure.core update-in]
        [cljs.core get-in] [cljs.core assoc-in] [cljs.core update-in])
       (lint-single-key-in ctx called-name (:expr call))
@@ -143,7 +171,8 @@
 
     ;; special forms which are not fns
     (when (= 'if (:name call))
-      (lint-missing-else-branch ctx (:expr call)))
+      (lint-missing-else-branch ctx (:expr call))
+      (lint-if-nil-return ctx (:expr call)))
     (when (contains? var-info/unused-values
                      (symbol (let [cns (str called-ns)]
                                (if (= "cljs.core" cns) "clojure.core" cns))

test/clj_kondo/main_test.clj

@@ -3162,6 +3162,48 @@ foo/baz
                      {:linters {:missing-else-branch {:level :warning}}
                       :config-in-ns '{foo {:linters {:missing-else-branch {:level :off}}}}}))))
 
+(deftest if-nil-return-test
+  (assert-submaps
+   [{:file "<stdin>",
+     :row 1,
+     :col 1,
+     :level :warning,
+     :message "For nil return, prefer when."}
+    {:file "<stdin>",
+     :row 1,
+     :col 17,
+     :level :warning,
+     :message "For nil return, prefer when-not."}
+    {:file "<stdin>",
+     :row 1,
+     :col 33,
+     :level :warning,
+     :message "For nil return, prefer when."}
+    {:file "<stdin>",
+     :row 1,
+     :col 53,
+     :level :warning,
+     :message "For nil return, prefer when-let."}
+    {:file "<stdin>",
+     :row 1,
+     :col 74,
+     :level :warning,
+     :message "For nil return, prefer when-some."}]
+   (lint! "(if true 1 nil) (if true nil 1) (if-not true nil 1) (if-let [x 1] x nil) (if-some [x 1] x nil)"
+          {:linters {:if-nil-return {:level :warning}}}))
+  (is (empty? (lint! "(if-let [x 7] nil :foo)"
+                     {:linters {:if-nil-return {:level :warning}, :unused-binding {:level :off}}})))
+  (is (empty? (lint! "(if true 1 nil) (if true nil 1) (if-not true nil 1) (if-let [x 1] x nil) (if-some [x 1] x nil)"
+                     {:linters {:if-nil-return {:level :off}}})))
+  (is (empty? (lint! "(ns foo {:clj-kondo/config '{:linters {:if-nil-return {:level :off}}}})
+                      (if true 1 nil) (if true nil 1) (if-not true nil 1) (if-let [x 1] x nil) (if-some [x 1] x nil)"
+                     {:linters {:if-nil-return {:level :warning}}})))
+  (is (empty? (lint! "#_:clj-kondo/ignore (if true 1 nil)"
+                     {:linters {:if-nil-return {:level :warning}}})))
+  (is (empty? (lint! "(ns foo) (if true 1 nil)"
+                     {:linters {:if-nil-return {:level :warning}}
+                      :config-in-ns '{foo {:linters {:if-nil-return {:level :off}}}}}))))
+
 (deftest single-key-in-test
   (doseq [lang ["clj" "cljs"]]
     (assert-submaps

test/clj_kondo/re_frame_test.clj

@@ -18,7 +18,8 @@
               :unused-value {:level :off}
               :unsorted-required-namespaces {:level :warning}
               :uninitialized-var {:level :off}
-              :type-mismatch {:namespaces {cljs.core {throw {:arities {1 {:args [:any]}}}}}}}
+              :type-mismatch {:namespaces {cljs.core {throw {:arities {1 {:args [:any]}}}}}}
+              :if-nil-return {:level :off}}
     :lint-as {day8.re-frame.tracing/fn-traced   clojure.core/fn
               day8.re-frame.tracing/defn-traced clojure.core/defn
               reagent.core/with-let             clojure.core/let

test/clj_kondo/recur_test.clj

@@ -31,12 +31,11 @@
     (async/alt!!
       (async/chan)
       ([x]
-       (if x
+       (when x
          (async/alt!!
            (async/chan)
            ([_]
-            (recur)))
-         nil)))))" linter-config))))
+            (recur))))))))" linter-config))))
   (testing "if-some"
     (is (empty? (lint! "(loop []
   (if true