Fork me on GitHub
#clj-kondo
<
2023-06-01
>
lread23:06:33

Small observations on :arglists overridden by metadata in analysis data ๐Ÿงต

lread23:06:11

Using clj-kondo v2023.05.26, given a little bb script alana.clj

(ns alana
  (:require [babashka.process :as p]
            [clojure.edn :as edn]
            [clojure.pprint :as pprint]))

(let [code (first *command-line-args*)
      config {:config-paths ^:replace []
              :analysis {:var-definitions {:meta true}
                         :arglists true}
              :output {:format :edn}}]
  (-> (p/shell {:in code
                :continue true
                :out :string}
               "clj-kondo"
               "--config" (binding [*print-meta* true] (pr-str config))
               "--lint" "-")
      :out
      edn/read-string
      :analysis :var-definitions
      pprint/pprint))
If I analyze a simple defn, I get back the :arglist-strs I would expect:
โฏ bb alana.clj '(defn foo [x])'                     
var analysis for code: (defn foo [x])
[{:fixed-arities #{1},
  :end-row 1,
  :meta nil,
  :name-end-col 10,
  :name-end-row 1,
  :name-row 1,
  :ns user,
  :name foo,
  :defined-by clojure.core/defn,
  :filename "<stdin>",
  :col 1,
  :name-col 7,
  :defined-by->lint-as clojure.core/defn,
  :end-col 15,
  :arglist-strs ["[x]"],
  :row 1}]
(The encoding of arglists to strings seems like a good design choice to me, btw; no worries about edn encoding/decoding.) But if I try to analyze some code where I override :arglists with metadata like so:
โฏ bb alana.clj $'(defn foo {:arglists \'([y])} [x])'
var analysis for code: (defn foo {:arglists '([y])} [x])
[{:fixed-arities #{1},
  :end-row 1,
  :meta {:arglists '([y])},
  :name-end-col 10,
  :name-end-row 1,
  :name-row 1,
  :ns user,
  :name foo,
  :defined-by clojure.core/defn,
  :filename "<stdin>",
  :col 1,
  :name-col 7,
  :defined-by->lint-as clojure.core/defn,
  :end-col 34,
  :arglist-strs ["[x]"],
  :row 1}]
My observations are that: 1. I do see the overridden :arglists under :meta 2. The :arglists under meta does not have the same string encoding as :arglist-strs (nor do we have an :arglist-strs under meta) 3. The overridden :arglists are not applied to :arglist-strs This is not a complaint. Just a thing I noticed while exploring static API analysis for cljdoc. If this is expected and intended behaviour, then I shall carry on with that understanding. If it is unintended behaviour, lemme know, I can raise at least raise an issue.

lread12:06:21

I see, a lotta words without a question.... Apologies @U04V15CAJ, so the question is, I suppose, should arglists overridden by metadata be applied to :arglist-strs?

borkdude12:06:11

I think arglists-strs is how the arglist is written and metadata contains additional arglist if supplied as meta?

borkdude12:06:00

with :meta [:arglists] you'll get that, I think

borkdude13:06:48

$ clj-kondo --lint /tmp/arg.clj --config "{:analysis {:var-definitions {:meta [:arglists]}} :output {:format :edn}}" | jet -t ':analysis :var-definitions'
[{:col 1,
  :defined-by clojure.core/defn,
  :defined-by->lint-as clojure.core/defn,
  :end-col 5,
  :end-row 4,
  :filename "/tmp/arg.clj",
  :fixed-arities #{1},
  :meta {:arglists (quote [y])},
  :name foo,
  :name-col 7,
  :name-end-col 10,
  :name-end-row 1,
  :name-row 1,
  :ns user,
  :row 1}]

lread13:06:22

Thanks @U04V15CAJ, so the behaviour is as intended. That works for me. Just wondered if I had found something that was unintended.

borkdude13:06:46

yeah, intended, so you can access both values if necessary

lread13:06:37

Thanks, that makes sense. Do you remember why you decided to go with :arglist-strs instead of :arglists ? I'm guessing that the string encoding helped to avoid forms that might not be edn-readable?

borkdude13:06:17

yeah don't exactly remember, I think this was something that clojure-lsp needed

lread13:06:18

Like for odd stuff: (defn foo [{:keys [:bar :baz] :or {bar #"regex"}}]) :arglist-strs preserves all just fine: ["[{:keys [:bar :baz] :or {bar #\"regex\"}}]"] But if we have same as meta: (defn foo {:arglists '([{:keys [:bar :baz] :or {bar #"regex"}}])} [x]) :meta :arglists is a bit off: ([{:or {bar (re-pattern "regex")}, :keys [:bar :baz]}]) I suppose this is pretty edge-casey.

borkdude13:06:25

right, I think arglists-strs is for showing and maybe it makes sense to override it from meta

borkdude13:06:43

we could change that I think

borkdude13:06:51

What do you think @UKFSJSM38?

lread14:06:32

:arglists treatment different from :doc . For :doc we always see :doc overridden by metadata in root var map. But for :doc , unlike :arglists , an original value is never interesting.

ericdallo14:06:58

Yes, it's only for showing in clojure-lsp if one hover or ask the docs of a function that has the arglists meta, so it would work as well for lsp

borkdude14:06:13

ok, let's do it then

borkdude14:06:22

this would also simplify quickdoc

ericdallo14:06:34

Just let me know when you do as it will require minor changes in LSP

lread14:06:17

So is original (un-overridden value) of :arglists ever interesting tho? (It isn't for my use case).

ericdallo14:06:16

for LSP it is, as it checks for the meta arglists, if not present, use the root arglists-strs

borkdude14:06:26

it is only for clj-kondo to deduce the arities, but I don't think clj-kondo makes use of that API

ericdallo14:06:38

Oh is there other arglists besides the arglists-strs in the root?

borkdude14:06:59

@UKFSJSM38 I think what you are using arglist-strs for is just for showing info, you're never interested in the "original", e.g. if a user writes:

(defn foo {:arglists '([x y z])} [& xs])
you are only ever interested in [x y z] , not in [& xs]

ericdallo14:06:35

I think so, let me double check

lread14:06:42

@U04V15CAJ, I assume arities are deduced from ([x y z]) and not [& xs] , amiright?

ericdallo14:06:38

Confirmed @U04V15CAJ, you are correct

borkdude14:06:55

@UE21H2HHD no, :arglists metadata can't be trusted for arities

๐Ÿ‘ 2
ericdallo14:06:11

It:s not only hover info, but completion, call hierarchy, signature help and other features, but all to present info only

borkdude14:06:28

it often uses informal notation like '([xs* xs?)] which is garbage for deducing arity info

lread14:06:56

yeah, that makes sense, tx

lread14:06:50

arglists overrides are only to describe usage for docs then, is that right?

๐Ÿ‘ 2
lread14:06:29

So I can write up an issue @U04V15CAJ, but in a nutshell :arglist-strs should be overriden by any :arglists metadata. Zat the idea?

๐Ÿ‘ 2
borkdude14:06:15

thanks for bringing this up, I might have just worked around the issue in the past

lread14:06:58

Very nice discussion, always a pleasure to work with @U04V15CAJ and @UKFSJSM38!

clj-kondo 4
clojure-lsp 4
clojure-spin 4
serioga14:06:43

> no, :arglists metadata can't be trusted for arities @U04V15CAJ can :arities metadata be trusted for arities of def's?

(def ^{:arities #{1 2}} f ...)

borkdude14:06:26

I've never seen that

serioga14:06:45

yes, there is only :inline-arities metadata in use but I wish to be able to say kondo about arities for functions defined via def so we could utilize :arities similar to :inline-arities

borkdude14:06:48

why not write a defn with the right arities then?

serioga14:06:28

why to write defn in case of

(def f another-ns/f)

borkdude14:06:47

@UE21H2HHD These are :arglists, not :arities - you weren't reacting to arities I guess?

lread14:06:00

ooopsie, misread. ignore.

serioga14:06:43

also there is defmulti which I can document only via :arglists

serioga14:06:42

so :arglists metadata works fine for documenting defs and defmultia (and other macros which generates global functions) but unfortunately kondo don't check arities in these case what is sad ๐Ÿ™‚

borkdude14:06:53

@U0HJNJWJH it would cause false positives which is even sadder

borkdude14:06:13

I guess clj-kondo could do a best effort attempt: if arglists only has regular symbols and not * or ? in the name, try to deduce arity info from it. But why the hassle, just define a wrapper function with a proper docstring + arglist using defn : this is what I do in my own libs, tools.build also does it, etc

serioga14:06:04

> it would cause false positives which is even sadder that's why I offer some specific way to say correct data to kondo like :arities

borkdude14:06:40

that works for code you own, but not for code you don't own, like clojure.core, etc

serioga14:06:09

> define a wrapper function this sounds suboptimal for me, to generate new java class and add level of indirection just for documentation purpose ๐Ÿ™‚ I'll better live without arity check from kondo side

borkdude14:06:16

if you avoid writing a function because you'll save a Java class, you're optimizing for the wrong thing in a functional language like Clojure if you ask me ;)

serioga15:06:03

for defmulti the wrapper can be not intended...

serioga15:06:15

(phew, agreed in something) ๐Ÿ™‚

๐Ÿ˜† 2
borkdude15:06:39

I've been pondering a different solution for the above problem. If you write:

(def foo impl/foo)
clj-kondo could just inherit the arity information from impl/foo

2
serioga15:06:33

let's narrow to defmulti case do you see a way to define multi-fn and teach kondo about it's arity?

serioga15:06:38

well, maybe kondo can inherit the arity information from dispatch function as well?

borkdude15:06:22

it is this issue: https://github.com/clj-kondo/clj-kondo/issues/412 feel free to comment + upvote

borkdude15:06:02

I mean, not about defmulti, just the (def foo iml/foo), you posted other stuff while I was searching this issue

serioga15:06:54

upvoted as 3rd one ๐Ÿ™‚

๐Ÿ™Œ 2
serioga15:06:05

other of defmulti there are other ways to define functions using various macros so I don't mind to have clj-kondo specific way to provide arity information for linting any global vars (dynamic, declared in advance...), like #_:clj-kondo/arity ... etc.

serioga15:06:07

I tend to have universal way, not just smart inheritance or similar But inheritance is good as well ๐Ÿ™‚

borkdude15:06:11

what is smart inheritance?

borkdude15:06:44

I've also thought about a reader conditional for clj-kondo so you can specify what code clj-kondo should analyze instead of other code, or so. I'll keep thinking about it, thanks for your input

2
lread16:06:36

@U04V15CAJ, @UKFSJSM38 issue raised: https://github.com/clj-kondo/clj-kondo/issues/2096, I look forward to your review and feedback.

borkdude16:06:16

ah yes, I forgot that the :arglists is something that is evaluated and there often quoted. so presenting it as is maybe doesn't make sense :-S

borkdude17:06:27

I think this might be the reason that things are the way they are since it's not guaranteed that the expression is parse-able or representable as something useful

borkdude17:06:37

since it has to be evaluated first