Fix clj-kondo#2207: condition-always-true linter (clj-kondo#2208)

tomdl89 · Oct 27, 2023 · 6ed9cf6 · 6ed9cf6
1 parent e0c5839
commit 6ed9cf6
Show file tree

Hide file tree

Showing 7 changed files with 134 additions and 97 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -19,6 +19,7 @@ For a list of breaking changes, check [here](#breaking-changes).
 
 ## Unreleased
 
+- [#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
 
 ## 2023.10.20

diff --git a/doc/linters.md b/doc/linters.md
@@ -7,100 +7,100 @@ configuration. For general configurations options, go [here](config.md).
 **Table of Contents**
 
 - [Linters](#linters)
-  - [Aliased namespace symbol](#aliased-namespace-symbol)
-  - [Aliased namespace var usage](#aliased-namespace-var-usage)
-  - [Case](#case)
-  - [Case duplicate test](#case-duplicate-test)
-    - [Case quoted test](#case-quoted-test)
-    - [Case symbol test constant](#case-symbol-test-constant)
-  - [Clj-kondo config](#clj-kondo-config)
-  - [Cond-else](#cond-else)
-  - [Conflicting-alias](#conflicting-alias)
-  - [Consistent-alias](#consistent-alias)
-  - [Datalog syntax](#datalog-syntax)
-  - [Deprecated var](#deprecated-var)
-  - [Deprecated namespace](#deprecated-namespace)
-  - [Deps.edn](#depsedn)
-  - [Bb.edn](#bbedn)
-    - [Bb.edn dependency on undefined task](#bbedn-dependency-on-undefined-task)
-    - [Bb.edn cyclic task dependency](#bbedn-cyclic-task-dependency)
-    - [Bb.edn Unexpected key](#bbedn-unexpected-key)
-    - [Bb.edn task docstring missing](#bbedn-task-docstring-missing)
-  - [Discouraged var](#discouraged-var)
-  - [Discouraged namespace](#discouraged-namespace)
-  - [Discouraged tag](#discouraged-tag)
-  - [Docstring blank](#docstring-blank)
-  - [Docstring no summary](#docstring-no-summary)
-  - [Docstring leading trailing whitespace](#docstring-leading-trailing-whitespace)
-  - [Duplicate map key](#duplicate-map-key)
-  - [Duplicate require](#duplicate-require)
-  - [Duplicate set key](#duplicate-set-key)
-  - [Duplicate field name](#duplicate-field-name)
-  - [Dynamic vars](#dynamic-vars)
-    - [Dynamic var not earmuffed](#dynamic-var-not-earmuffed)
-    - [Earmuffed var not dynamic](#earmuffed-var-not-dynamic)
-  - [Quoted case test constant](#quoted-case-test-constant)
-  - [Equals false](#equals-false)
-  - [Equals true](#equals-true)
-  - [File](#file)
-  - [Format](#format)
-  - [Def + fn instead of defn](#def--fn-instead-of-defn)
-  - [Inline def](#inline-def)
-  - [Invalid arity](#invalid-arity)
-  - [Conflicting arity](#conflicting-arity)
-  - [Reduce without initial value](#reduce-without-initial-value)
-  - [Loop without recur](#loop-without-recur)
-  - [Line length](#line-length)
-  - [Keyword in binding vector](#keyword-in-binding-vector)
-  - [Main without gen-class](#main-without-gen-class)
-  - [Minus one](#minus-one)
-  - [Misplaced docstring](#misplaced-docstring)
-  - [Missing body in when](#missing-body-in-when)
-  - [Missing clause in try](#missing-clause-in-try)
-  - [Missing docstring](#missing-docstring)
-  - [Missing else branch](#missing-else-branch)
-  - [Missing map value](#missing-map-value)
-  - [Missing test assertion](#missing-test-assertion)
-  - [Namespace name mismatch](#namespace-name-mismatch)
-  - [Non-arg vec return type hint](#non-arg-vec-return-type-hint)
-  - [Not empty?](#not-empty)
-  - [Plus one](#plus-one)
-  - [Private call](#private-call)
-  - [Protocol method varargs](#protocol-method-varargs)
-  - [Redefined var](#redefined-var)
-  - [Var same name except case](#var-same-name-except-case)
-  - [Redundant do](#redundant-do)
-  - [Redundant fn wrapper](#redundant-fn-wrapper)
-  - [Redundant call](#redundant-call)
-  - [Redundant let](#redundant-let)
-  - [Refer](#refer)
-  - [Refer all](#refer-all)
-  - [Single key in](#single-key-in)
-  - [Single logical operand](#single-logical-operand)
-  - [Single operand comparison](#single-operand-comparison)
-  - [Shadowed var](#shadowed-var)
-  - [Syntax](#syntax)
-  - [Type mismatch](#type-mismatch)
-  - [Unbound destructuring default](#unbound-destructuring-default)
-  - [Uninitialized var](#uninitialized-var)
-  - [Unused alias](#unused-alias)
-  - [Unused binding](#unused-binding)
-  - [Unused value](#unused-value)
-  - [Used underscored bindings](#used-underscored-bindings)
-  - [Unknown :require option](#unknown-require-option)
-  - [Unreachable code](#unreachable-code)
-  - [Unused import](#unused-import)
-  - [Unresolved namespace](#unresolved-namespace)
-  - [Unresolved symbol](#unresolved-symbol)
-    - [:exclude-patterns](#exclude-patterns)
-  - [Unresolved var](#unresolved-var)
-  - [Unsorted imports](#unsorted-imports)
-  - [Unsorted required namespaces](#unsorted-required-namespaces)
-  - [Unused namespace](#unused-namespace)
-  - [Unused private var](#unused-private-var)
-  - [Unused referred var](#unused-referred-var)
-  - [Use](#use)
-  - [Warn on reflection](#warn-on-reflection)
+    - [Aliased namespace symbol](#aliased-namespace-symbol)
+    - [Aliased namespace var usage](#aliased-namespace-var-usage)
+    - [Case](#case)
+    - [Case duplicate test](#case-duplicate-test)
+        - [Case quoted test](#case-quoted-test)
+        - [Case symbol test constant](#case-symbol-test-constant)
+    - [Clj-kondo config](#clj-kondo-config)
+    - [Cond-else](#cond-else)
+    - [Conflicting-alias](#conflicting-alias)
+    - [Consistent-alias](#consistent-alias)
+    - [Datalog syntax](#datalog-syntax)
+    - [Deprecated var](#deprecated-var)
+    - [Deprecated namespace](#deprecated-namespace)
+    - [Deps.edn](#depsedn)
+    - [Bb.edn](#bbedn)
+        - [Bb.edn dependency on undefined task](#bbedn-dependency-on-undefined-task)
+        - [Bb.edn cyclic task dependency](#bbedn-cyclic-task-dependency)
+        - [Bb.edn Unexpected key](#bbedn-unexpected-key)
+        - [Bb.edn task docstring missing](#bbedn-task-docstring-missing)
+    - [Discouraged var](#discouraged-var)
+    - [Discouraged namespace](#discouraged-namespace)
+    - [Discouraged tag](#discouraged-tag)
+    - [Docstring blank](#docstring-blank)
+    - [Docstring no summary](#docstring-no-summary)
+    - [Docstring leading trailing whitespace](#docstring-leading-trailing-whitespace)
+    - [Duplicate map key](#duplicate-map-key)
+    - [Duplicate require](#duplicate-require)
+    - [Duplicate set key](#duplicate-set-key)
+    - [Duplicate field name](#duplicate-field-name)
+    - [Dynamic vars](#dynamic-vars)
+        - [Dynamic var not earmuffed](#dynamic-var-not-earmuffed)
+        - [Earmuffed var not dynamic](#earmuffed-var-not-dynamic)
+    - [Quoted case test constant](#quoted-case-test-constant)
+    - [Equals false](#equals-false)
+    - [Equals true](#equals-true)
+    - [File](#file)
+    - [Format](#format)
+    - [Def + fn instead of defn](#def--fn-instead-of-defn)
+    - [Inline def](#inline-def)
+    - [Invalid arity](#invalid-arity)
+    - [Conflicting arity](#conflicting-arity)
+    - [Reduce without initial value](#reduce-without-initial-value)
+    - [Loop without recur](#loop-without-recur)
+    - [Line length](#line-length)
+    - [Keyword in binding vector](#keyword-in-binding-vector)
+    - [Main without gen-class](#main-without-gen-class)
+    - [Minus one](#minus-one)
+    - [Misplaced docstring](#misplaced-docstring)
+    - [Missing body in when](#missing-body-in-when)
+    - [Missing clause in try](#missing-clause-in-try)
+    - [Missing docstring](#missing-docstring)
+    - [Missing else branch](#missing-else-branch)
+    - [Missing map value](#missing-map-value)
+    - [Missing test assertion](#missing-test-assertion)
+    - [Namespace name mismatch](#namespace-name-mismatch)
+    - [Non-arg vec return type hint](#non-arg-vec-return-type-hint)
+    - [Not empty?](#not-empty)
+    - [Plus one](#plus-one)
+    - [Private call](#private-call)
+    - [Protocol method varargs](#protocol-method-varargs)
+    - [Redefined var](#redefined-var)
+    - [Var same name except case](#var-same-name-except-case)
+    - [Redundant do](#redundant-do)
+    - [Redundant fn wrapper](#redundant-fn-wrapper)
+    - [Redundant call](#redundant-call)
+    - [Redundant let](#redundant-let)
+    - [Refer](#refer)
+    - [Refer all](#refer-all)
+    - [Single key in](#single-key-in)
+    - [Single logical operand](#single-logical-operand)
+    - [Single operand comparison](#single-operand-comparison)
+    - [Shadowed var](#shadowed-var)
+    - [Syntax](#syntax)
+    - [Type mismatch](#type-mismatch)
+    - [Unbound destructuring default](#unbound-destructuring-default)
+    - [Uninitialized var](#uninitialized-var)
+    - [Unused alias](#unused-alias)
+    - [Unused binding](#unused-binding)
+    - [Unused value](#unused-value)
+    - [Used underscored bindings](#used-underscored-bindings)
+    - [Unknown :require option](#unknown-require-option)
+    - [Unreachable code](#unreachable-code)
+    - [Unused import](#unused-import)
+    - [Unresolved namespace](#unresolved-namespace)
+    - [Unresolved symbol](#unresolved-symbol)
+        - [:exclude-patterns](#exclude-patterns)
+    - [Unresolved var](#unresolved-var)
+    - [Unsorted imports](#unsorted-imports)
+    - [Unsorted required namespaces](#unsorted-required-namespaces)
+    - [Unused namespace](#unused-namespace)
+    - [Unused private var](#unused-private-var)
+    - [Unused referred var](#unused-referred-var)
+    - [Use](#use)
+    - [Warn on reflection](#warn-on-reflection)
 
 <!-- markdown-toc end -->
 
@@ -234,6 +234,19 @@ enabling this linter, you can prepend the `case` expression with
 
 *Example message:* `use :else as the catch-all test expression in cond`.
 
+### Condition always true
+
+*Keyword:* `:condition-always-true`.
+
+*Description:* warn on a condition that evaluates to an always truthy constant,
+like when passing a function instead of calling it
+
+*Default level:* `:warning`.
+
+*Example trigger:* `(if odd? :odd :even)`.
+
+*Example message:* `Condition always true`.
+
 ### Conflicting-alias
 
 *Keyword:* `:conflicting-alias`.

diff --git a/src/clj_kondo/impl/analyzer.clj b/src/clj_kondo/impl/analyzer.clj
@@ -1673,7 +1673,10 @@
        (node->line (:filename ctx) expr
                    linter
                    msg)))
-    (analyze-children ctx args false)))
+    (let [[condition & args] args]
+      (when condition
+        (analyze-expression** ctx (assoc condition :condition true)))
+      (analyze-children ctx args false))))
 
 (defn analyze-constructor
   "Analyzes (new Foo ...) constructor call."

diff --git a/src/clj_kondo/impl/analyzer/usages.clj b/src/clj_kondo/impl/analyzer/usages.clj
@@ -245,7 +245,8 @@
                                       :interop? interop?
                                       ;; save some memory
                                       :expr (when-not dependencies expr)
-                                      :resolved-core? resolved-core?}]
+                                      :resolved-core? resolved-core?
+                                      :condition (:condition expr)}]
                            (namespace/reg-var-usage! ctx ns-name
                                                      usage)
                            (utils/reg-call ctx usage (:id expr))

diff --git a/src/clj_kondo/impl/config.clj b/src/clj_kondo/impl/config.clj
@@ -155,7 +155,8 @@
               :minus-one {:level :off}
               :protocol-method-varargs {:level :error}
               :unused-alias {:level :off}
-              :self-requiring-namespace {:level :off}}
+              :self-requiring-namespace {:level :off}
+              :condition-always-true {:level :warning}}
     ;; :hooks {:macroexpand ... :analyze-call ...}
     :lint-as {cats.core/->= clojure.core/->
               cats.core/->>= clojure.core/->>

diff --git a/src/clj_kondo/impl/linters.clj b/src/clj_kondo/impl/linters.clj
@@ -395,6 +395,10 @@
                   (and call?
                        (not (utils/linter-disabled? call :single-logical-operand))
                        (lint-single-logical-operand call))]]
+      (when (and (not call?)
+                 (identical? :fn (:type called-fn)))
+        (when (:condition call)
+          (findings/reg-finding! ctx (assoc call :message "Condition always true" :type :condition-always-true))))
       (when arity-error?
         (findings/reg-finding!
          ctx

diff --git a/test/clj_kondo/condition_always_true_test.clj b/test/clj_kondo/condition_always_true_test.clj
@@ -0,0 +1,14 @@
+(ns clj-kondo.condition-always-true-test
+  (:require
+   [clj-kondo.test-utils :refer [lint! assert-submaps2]]
+   [clojure.test :as t :refer [deftest is testing]]))
+
+(deftest condition-always-true-test
+  (assert-submaps2
+   '({:file "<stdin>", :row 1, :col 19, :level :warning, :message "Condition always true"})
+   (lint! "(defn foo [x] (if inc x 2))"
+          '{:linters {:condition-always-true {:level :warning}}}))
+  (is (empty?
+       (lint! "(defn foo [x] (if x inc 2))
+               (defn bar [x] (if x 2 inc))"
+              '{:linters {:condition-always-true {:level :warning}}}))))