clj-kondo 2024-04-03 | Slack Archive

I just wrote this piece of Java-interop:

(defn map->Headers [headers]
  (let [headers-obj ^Headers (Headers.)]
    (for [[k v] headers]
      (->> (cond-> v
             (not (coll? v)) vector)
           (map str)
           (.add headers-obj (name k))))
    headers-obj))

After some struggling I realized that the for here of course does nothing (mutability hurts my brain...). I had to replace it with a doseq. This pattern - a for outside of a let that has forms after it will probably not do what the user intended. An idea for a linting rule?

💯 1

magnars08:04:54

The amount of times I've been bit by a lazy for evaluating just fine in my repl, but not running at all in code, is too damn high!

😂 2

borkdude08:04:42

something like this? feel free to upvote it: https://github.com/clj-kondo/clj-kondo/issues/1757

borkdude08:04:54

Actually the middle thing here is reported as an unused value:

$ clj -M:clj-kondo/dev --lint - <<< '(defn foo [] (map inc [1 2 3]) (map inc [1 2 3]))'
<stdin>:1:14: warning: Unused value

borkdude08:04:10

but given that your function returns a lazy value you can't reliably say that it will never be executed

➕ 1

cjohansen08:04:44

The linked idea is also good, but not my exact case. My code simply does not do anything, as it has a lazy form in "the middle position".

cjohansen08:04:52

The code returns a Java object, not a lazy sequence

borkdude08:04:10

ok, I think the case is similar though, the for thing should be reported as an unused value

cjohansen08:04:57

If it was, then that would be good. Do you mean to say this should already be happening?

borkdude08:04:36

it probably should, but isn't working yet as this is a more advanced case, since it's inside of of a let and not on the top level of a function body

cjohansen08:04:38

borkdude08:04:40

issue welcome

cjohansen08:04:50

Aha, I see. Sure, will open an issue.

cjohansen08:04:33

https://github.com/clj-kondo/clj-kondo/issues/2309

borkdude08:04:08

thanks!

Noah Bogart19:04:51

I cracked up reading the issue https://github.com/clj-kondo/clj-kondo/issues/2310 where @alexmiller wrote: "Describe alternatives you've considered: Considering changes in Clojure core." Maybe the only person capable of writing such a line.

Alex Miller (Clojure team)20:04:06

I filed one PR for this that I saw in the wild https://github.com/funcool/promesa/pull/152 Think I also found a case in Datomic :)

😍 1

Alex Miller (Clojure team)20:04:03

it may be less widespread than I initially thought, several other regression tests I had were traced to other issues

👍 1

Noah Bogart20:04:34

This is a good change, imo. Catching these things can be hard, so I'm glad y'all are gonna start enforcing it

Alex Miller (Clojure team)20:04:37

still on the fence about that in core, hate to cause new errors where none existed before. I'm (currently) using the type hint to determine the user expected overload type in an interop call so we can apply function conversion, but I need to resolve the class to do that check. I was surprised to find these cases where the invalid class was not previously failing.

2024-04-03

Channels