Seems to be a regression

Issue welcome. Could be nice to bisect to which clj-kondo version this changed and to see what caused it

@UE21H2HHD

borkdude@MBP2019 /tmp $ clojure -Sdeps '{:deps {clj-kondo/clj-kondo {:mvn/version "2020.03.20"}}}' -M -m clj-kondo.main --lint - <<< "(defn foo [x] (if-let [x 1] x x x))"
<stdin>:1:15: error: if-let body requires one or two forms
linting took 171ms, errors: 1, warnings: 0
borkdude@MBP2019 /tmp $ clojure -Sdeps '{:deps {clj-kondo/clj-kondo {:mvn/version "2020.04.05"}}}' -M -m clj-kondo.main --lint - <<< "(defn foo [x] (if-let [x 1] x x x))"
linting took 71ms, errors: 0, warnings: 0

https://github.com/clj-kondo/clj-kondo/compare/v2020.03.20...v2020.04.05

perhaps related: https://github.com/clj-kondo/clj-kondo/commit/935d70c7a718f7dc6c449e540f38faf87fd4bd10

Raised the issue: https://github.com/clj-kondo/clj-kondo/issues/1426

Thanks

My pleasure

I guess it doesn't report it because of this: https://github.com/clojure/clojure/blob/b8132f92f3c3862aa6cdd8a72e4e74802a63f673/src/clj/clojure/core.clj#L1853

but now spec does check for it

so perhaps core could also change it

but I don't think they won't ;)

Huh. Ok interesting.

So it looks like they allowed for more args to support better assert errors? Or maybe some backward compat since it is called oldform?

I think so, but the clojure.spec doen't allow it

I posted a message about it in #clojure-dev

if-some has the same problem

I think we must just must provide some overrides for this

let me try to do this

yep, special casing works

Fixed

There is some low hanging fruit to move the override thing to a different place which prevents us from doing this at runtime each time

but not high priority, might be fun

The :doc issue is still coming right

oh lol, you just posted it

@UE21H2HHD did you know you can write jet --query ':foo :bar :baz' ? :)

Oh you just optimized my jet workflow!

!!!

I shall also raise one more issue. Not sure if you’ll want to it fixed. For, what seem to me obscure, signatures of defn and defmacro, there can be an optional 2nd attr-map, which clj-kondo currently ignores.

(defn name doc-string? attr-map? ([params*] prepost-map? body) + attr-map?)
(defmacro name doc-string? attr-map? ([params*] body) + attr-map?)

you mean in the tail end right?

Yeah

I don't think anyone uses that

can support later

not high priority now

I think I won’t bother with an issue then.

I just learned it existed yesterday!

actually SCI does support it, let me look up the issue of why I supported it, it was because of some lib

dang, no context: https://github.com/borkdude/sci/issues/567

Could grasp find usages in the wild?

I would guess it would be very rare, but really don’t know.

yes, grasp would find this

we can do a grasp

actually we can use the clojure core spec for defn here in grasp I think

Ok, maybe I’ll raise the issue, and your grasp results can be a good enough reason to close it. Documentation of why we don’t do it, in case it crops up again. Or grasp might tell us we should do it.

yeah good idea

I'm cooking up a grasp

I'm grasping like this:

(ns trailing-meta)

(require
 '[clojure.core.specs.alpha :as specs]
 '[clojure.spec.alpha :as s]
 '[grasp.api :as g])

(s/def ::defn (s/cat :defn #{'defn} :args ::specs/defn-args))
(s/def ::defn-trailing-attr-map (s/and ::defn (fn [conformed]
                                                (->> conformed
                                                     :args :fn-tail second :attr-map))))

(def matches (g/grasp "/Users/borkdude/.m2" ::defn-trailing-attr-map))

;; (prn (s/conform ::defn-trailing-attr-map (first matches)))
(run! (fn [match]
        (print (meta meta) " " match)
        (println)) matches)

but so far no match

Cool!

ah I found something

but I made a typo in my program so I didn't see where that was ;)

running again

Oh. You might curse me. Because it might kind of work already.

clojure -M:clj-kondo/dev --config '{:config-paths ^:replace [] :output {:format :edn :analysis {:var-definitions {:meta true}}}}' \
  --lint - <<< '(defn x ([]) {:doc "attr-map2"})' \
 | jet --query ':analysis :var-definitions first' --pretty
{:fixed-arities #{0},
 :end-row 1,
 :meta {:doc "attr-map2"},
 :name-end-col 8,
 :name-end-row 1,
 :name-row 1,
 :ns user,
 :name x,
 :defined-by clojure.core/defn,
 :filename "<stdin>",
 :col 1,
 :name-col 7,
 :end-col 33,
 :doc "attr-map2",
 :row 1}

ah, it's omniconf

I think that was also the reason I supported it in SCI

Ok there is something amiss certainly with :doc derivation from this source but we already have an issue for that. And I think the new :meta analysis might not merge the 2nd attr-map properly if at all. Soo… I do have a small bit of work here.

> {:line 272, :column 1, :url "jar:file:/Users/borkdude/.m2/repository/com/grammarly/omniconf/0.4.3/omniconf-0.4.3.jar!/omniconf/core.clj"}

That grasp tool is very cool!

{:line 272, :column 1, :url "jar:file:/Users/borkdude/.m2/repository/com/grammarly/omniconf/0.4.3/omniconf-0.4.3.jar!/omniconf/core.clj"}
{:line 312, :column 1, :url "jar:file:/Users/borkdude/.m2/repository/com/grammarly/omniconf/0.4.3/omniconf-0.4.3.jar!/omniconf/core.clj"}
{:line 372, :column 1, :url "jar:file:/Users/borkdude/.m2/repository/com/grammarly/omniconf/0.4.3/omniconf-0.4.3.jar!/omniconf/core.clj"}
{:line 390, :column 1, :url "jar:file:/Users/borkdude/.m2/repository/com/grammarly/omniconf/0.4.3/omniconf-0.4.3.jar!/omniconf/core.clj"}

That's all I found in my .m2

I'll post this info in the SCI issue

and in the clj-kondo issue if you make one

Yeah, I think I have to do a bit more digging to see what is not working!

Yeah… :meta analysis is missing the 2nd attr-map. I can raise for that.

And at same time fix meta merge order (which might be fixed by :doc derivation fix).

thanks

Issue raised: https://github.com/clj-kondo/clj-kondo/issues/1429

Oh, I guess we’ll have to address this one too:

❯ clj-kondo --config '{:config-paths ^:replace []}' --lint - <<< '(defn x ([]) ([x] x) {:added "attr-map2"})'
<stdin>:1:22: error: Function arguments should be wrapped in vector.
linting took 8ms, errors: 1, warnings: 0