Handle not in :condition-always-true / new linter - :condition-always-false

[skynet-gh]

[ Leave this text in if you want to promote it. ]

To upvote this issue, give it a thumbs up. See this list for the most upvoted issues.

• I have read the Clojure etiquette and will respect it when communicating on this platform.

Is your feature request related to a problem? Please describe.

The :condition-always-true linter is very helpful, but it "breaks" when inverting the check with not for example.

Describe the solution you'd like

It'd be great if :condition-always-true could work in cases like (when (not true) ...) as well, or maybe a separate linter :condition-always-false.

Describe alternatives you've considered

Sometimes the boolean logic is just setup in a way where you get (not ...) and so the linter fails to catch errors in there.

Additional context

repro.clj:

(ns repro)

(defn x? [& _]
  "z")

(when (not x?)
  (println "y"))

$ clj-kondo --lint repro.clj
linting took 5ms, errors: 0, warnings: 0

[borkdude]

Hmm yes, perhaps we should rename :condition-always-true to :condition-... what actually? :condition-always-constant? but constant already implies always :) If you can recommend a better name, please do.

[borkdude]

Btw, one thing that prevented me to handle the false case in a Slack discussion was the following:

(def debug false)

(if debug ...)

It would be annoying if clj-kondo complained about that particular usage.

[borkdude]

Btw, this does warn:

(when-not x?
  (println "y"))

[skynet-gh]

@borkdude maybe :condition-is-constant? Or maybe :boolean-test-is-constant? (since the Clojure docs for if and when call it test)

For your example of a debug flag, maybe it'd be possible to add config to the linter with exception names? Like :exclude [debug]

[borkdude]

Ah wait, this already doesn't warn:

(def DEBUG true)

(if DEBUG 1 2)

but (if #'DEBUG 1 2) does warn. Let me check how that was implemented. We can do the same for :condition-always-false

[borkdude]

OK, the current way :condition-always-true is pretty limited, it will only warn in this these cases:

(if #'inc .. ..) ;; var

and

(if inc 1 2)

so only on passing a fn and a var, which are commonly reported mistakes. I guess (not inc) was a variation on that common theme?