Clojurians Log v2

Yeah I guess I could upgrade that :-)

Done

Oh nice. Good to see it wasn't problematic given the gap in time!

I didn't expect any problems

✅ tried it on cljdoc and rewrite-clj, no new findings on either

btw, if your project's default version of clojure is old, try:

clj -Sforce -Sdeps '{:deps {clj-kondo/clj-kondo {:mvn/version "RELEASE"} org.clojure/clojure {:mvn/version "1.12.5"}}}' -M -m clj-kondo.main --lint src:test

1.11 is now the min version

Would this be considered a false positive?

(let [ ...
        rebill? false
        repeat? false
        customer {}
        [tx inovio-tx]
        (mint-transaction database
                          ba
                          "decline"
                          ccstatus-params
                          amount
                          rebill? repeat?
                          ip
                          (:wsbilling/transaction-id customer))]
    [(s/explain-str :wsbilling/billing-agreement ba)
     (s/explain-str :wsbilling/transaction tx)
     (s/explain-str :wsbilling/transaction-inovio inovio-tx)
     (assoc tx
            ;; :wsbilling/saved-payment-button saved-payment-button?
            :wsbilling/date-created (java.util.Date.)
            ;; :wsbilling/device-data device-data
            :wsbilling/rebill-attempt (when rebill?
                                        (inc (:wsbilling/rebill-attempts customer))))

yes, this should already been caught, evidence behind a condition should never count. Thanks, I'll try to repro

(this is in a comment, not real code, but it is the only Expected: type warning in our entire codebase)

is it possible to make a repro of this and please make sure you're not running with the old kondo

This could be yesterday's clj-kondo. Looks like Calva didn't pick up the latest lsp nightly this time. Just a sec...

oh lol, what happens here:

(inc (:wsbilling/rebill-attempts customer))

even though it's within a condition

Confirmed with very latest Kondo. Yeah, I can see why it flags it -- but it is inside (when false ..) so I expected it to not be considered...?

not sure if we should change this, but this isn't about function tag inference, it is about the flow of map values.

(when rebill? ..) when Kondo "knows" that rebill? is false.

I guess we could respect (when false...) but wouldn't it be nice to know that the code inside the condition is going to fail no matter what?

I don't have an opinion either way. I can change the definition of customer in this comment to satisfy Kondo -- I just wasn't sure whether you thought it should flag it or not 🙂

if when false should be respected, I think we should respect unknown conditions here too, but either way, it's dead code OR the branch is going to fail

I'll think more about this, thanks for filing

I'm tempted to warn about unreachable code but perhaps false should be a sign that it's deliberate

In real code, Kondo wouldn't be able to tell what's in customer here -- it would be a parameter to the enclosing fn -- and it might have :wsbilling/rebill-attempts nil if rebill? (another parameter) is false, so it wouldn't be able to tell anything. So I think this is fine, since I'd consider it an edge case for Kondo to have any idea about the values behind those symbols.

No new findings in the 2 projects I checked

I merged a pretty big commit with a new linter :constant-condition now. I have found some new findings in a big project (all valid). Can y'all re-run once again with -Sforce + RELEASE?

I'm waiting for a new LSP nightly with that updated clj-kondo...

Looks like the LSP build fails due to new lint warnings: https://github.com/clojure-lsp/clojure-lsp/actions/runs/29781220073/job/88482671116

Including three "constant condition" warnings. /cc @ericdallo

I hope it's a valid warning

but I'm going to bed now

No new linter findings here! (Note, I did not clear .clj-kondo/.cache, no need to do that, right?)

I'm checking LSP warnings and there is a constant-condition here

(cond-> {:label (element->label element cursor-alias priority)
         :priority (generic-priority->specific-priority element priority)}
  deprecated (assoc :tags [1])
  kind (assoc :kind kind)
  detail (assoc :detail detail)
  :always (completion-item-with-unresolved-documentation
            {:name (-> element :name str)
             :uri (:uri element)
             :name-row (:name-row element)
             :name-col (:name-col element)}
            resolve-support))

yes. which line exactly though, are you sure it's about the :always` ?

I'll have a look

ah! it's in kind

wich it is always a keyword

:)

😎

all of them were true positives. After fixing them, the build succeeded:https://github.com/clojure-lsp/clojure-lsp/pull/2411

Please re-check locally with:

clj -Sforce -Sdeps '{:deps {clj-kondo/clj-kondo {:git/url "" :git/sha "bc0fccef51961c558a23c4241f8f8923c2086bfa"}}}' -M -m clj-kondo.main --lint src:test

one new constant-condition warning for me but it's true positive

Found this false positive with the latest:

(defmacro with-try [& body]
  `(try
     ~@body
     (catch Exception _# nil)))

(defn str->double [n]
  (with-try (parse-double n)))

if you lint with-try as try, does the error go away

clj-kondo collects parse-double here as evidence for n being a string, but doesn't seem to notice that it's within a try

maybe I need to let clj-kondo macroexpand it

neither worked

ok, let me try to repro

I can repro it with:

(defn str->double [n]
  (try (parse-double n)
       (catch Exception _ nil)))

(str->double nil)

@imre try again with 3aa27d563f46bd226ab5401e4bebe27edb3b7f36. thanks for the report.

@borkdude thank you, that worked (together with macroexpansion)

I think just {:clj-kondo/macroexpand-hook true} would solve that right?

that's actually what I meant by macroexpansion, I could have made that clearer

nice :)

I fixed yesterday but forgot to push, doing rn

Ah, I was about to say "Can't test the latest Kondo yet, LSP hasn't built a nightly!" 🙂 Thanks @ericdallo As soon as the new nightly is available, I'll re-lint the work codebase. Kinda hard to do from the command-line for just Kondo, given we have over 200 src and test folders at work (Polylith monorepo).

And there's the new nightly LSP... runs off to test Kondo at work

Looks like the new Kondo found another 33 new problems at work... reviewing them now, but this is definitely real:

id        (or (some-> req :params :id (parse-long)) 0)
        ;; use affiliate map in flash, if present:
        affiliate (or (-> req :params :affiliate)
                      (when id (affiliate/as-affiliate db-spec id)))

On another instance, I think there's a bug? I have this config:

:config-in-call
 {expectations.clojure.test/more-> {:ignore [:condition-always-true]}
  meander.epsilon/match {:ignore true}
  phrase.alpha/defphraser {:ignore true}}

Oh, you renamed the linter...! Just a sec...

yes, I folded it into one linter now since :condition-always-false would not make sense to me and I didn't see a migation path that made sense :)

Is :constant-condition on by default now?

yes

Thanks!

right now it finds A LOT of (if (filter ...) ..) in big codebases, I noticed

gosh maybe elisp and common lisp had that right

Heh... I had a few places where I was ignoring :condition-always-true and those popped up again (of course, due to renaming). Might want to make sure you note that is a breaking change for some folks.

After fixing my config and ignores, I have 11 new warnings. Some are redundant ignores due to the def-in-`comment` change (yay!).

Final count: 4 (new) constant-condition and they are all correct. And a couple of them are definitely bugs, maybe all four! Excellent!!

> Might want to make sure you note that is a breaking change for some folks. That's in the changelogs

Maybe it should listen to the old config when the condition is truthy in ignore hints, I guess that's the best we can do

Maybe... since it might be in library configs that get imported into your project, that you really don't have control over.

good point

FWIW, I just checked every Kondo config.edn I have locally in my OSS project tree, including all imports, and nothing mentions condition-always-true so it was only in our work config.

here's the backward compat fix https://github.com/clj-kondo/clj-kondo/pull/2906

merged

I found a new edge case

(defn build-event-map [e]
  (let [node #?(:cljs (.-target e) :clj nil)]
    (cond-> {:replicant/trigger :replicant.trigger/dom-event
             :replicant/dom-event e}
      node (assoc :replicant/node node)
      (ifn? *dispatch*) (assoc :replicant/dispatch *dispatch*))))

Also, I make exceptions for literal false since this is often a debug toggle. Maybe I should include nil which fixes this case.

For that case, it is "conditional" in the sense that it is guarded by a platform/host selector, so that feels like it should not be flagged—not because of the literal nil but because of the #? determining the value. I'm not sure whether Kondo has that info after reading the value tho'?

I think not. Code is dealing with multi-dialects, so not an issue.

well, it could be interesting to have it flagged because you made an error in clj but not in cljs. You could also write #?(:clj :always :cljs node)

but then the clj branch becomes unused which can be annoying :) but you could also write that let differently

I think there's less likelihood of something being an error if you are specifying a literal (as opposed to Kondo looking at an expression and figuring out it would always yield nil).

Multi-dialect code can already be really weird. I've had several cases where something makes sense in one dialect but is a degenerate case in another dialect—and it's hard to make a degenerate case satisfy a linter.

I'll add nil literals to the ignored constantly false conditions

Hmmm... your point is interesting about maybe having made a mistake in one dialect. I suppose you could kondo ignore inline it to state, "nope, that's cool it is what I meant".

yeah I guess that's always an option

I handled unused binding the same way.

at least that's how I thought about it 7 years ago - some people don't like it, but I still stand by it. there may be a config option for it

imo it can find important issues per dialect

or performance stuff, e.g. it's doing unnecessary work in one dialect

Ya, I changed my opinion; I appreciate the heads up from the linter. And I am happy to kondo ignore after verifying it is what I really want to do.

Kondo found suspicious-looking code, and it probably deserves a comment or a rewrite.