killme2008 / killme2008/defun

Incorrect :arglists, embellishment in the README, and nonstandard `clojure.core/sigs` breaking on Babashka

Open
#24 3 comments 0 reactions 0 assignees View on GitHub
Dominant language
Clojure
Stars
502
Forks
21
PR merge metrics
No merged PRs in 30d

Description

So, the README claims that this:
```clojure
(defun say-hi
([:dennis] "Hi,good morning, dennis.")
([:catty] "Hi, catty, what time is it?")
([:green] "Hi,green, what a good day!")
([other] (str "Say hi to " other)))
```
Should expand to this:
```clojure
(defn
say-hi
{:arglists '([& args])}
[& args#]
(clojure.core.match/match
[(vec args#)]
[[:dennis]]
(do "Hi,good morning, dennis.")
[[:catty]]
(do "Hi, catty, what time is it?")
[[:green]]
(do "Hi,green, what a good day!")
[[other]]
(do (str "Say hi to " other))))
```

However, as the [`defun` macro definition](https://github.com/killme2008/defun/blob/master/src/defun/core.cljc#L156) confirms, this isn't actually how the code works, and the code in fact expand to:
```clojure
(def say-hi
(fun
([:dennis] "Hi,good morning, dennis.")
([:catty] "Hi, catty, what time is it?")
([:green] "Hi,green, what a good day!")
([other] (str "Say hi to " other))))
```
Or more fully:
```clojure
(def say-hi
(fn fn__13414
([G__13416]
(let [placeholder13415 placeholder]
(match
[G__13416]
[:dennis]
(do "Hi,good morning, dennis.")
[:catty]
(do "Hi, catty, what time is it?")
[:green]
(do "Hi,green, what a good day!")
[other]
(do (str "Say hi to " other)))))))
```

Notably, `defun` only wraps an (anonymous) `fun`, and manually runs `vary-meta` and sets `:arglists` by using a private undocumented `clojure.core/sigs` function. However, this `sigs` function, for the provided `say-hi`, gives an `:arglists` of `([:dennis] [:catty] [:green] [other])`, which seems to work well enough, but isn't the `[& args]` described in the README, but that's good, since `[& args]` is too generic and lends itself poorly to static analysis and automatic currying and so forth. The appropriate `:arglists` in this case, I believe (as this is nonstandard and undocumented), should be simply `([other])`.

So first of all and certainly, the README should be updated to reflect the state of the code, because no one should use `defun` and expect an `[& args]` `:arglists` value. But speaking more generally, it seems to me like the sigs we get could easily be post-processed to be more compliant quite simply, by removing all non-symbol values and unifying signatures per-arity looking for a non-`'_` value if one is available. I'm not sure if this is the best approach to go with (and it does add (yet more) processing at macro-time), but I submitted a PR #25 with this post-processing included, as well as updates to the tests (which don't currently check `:arglists` at all), and the README, and will keep an updated version on [wizard-enterprises/defun](https://github.com/wizard-enterprises/defun) where I also updated the project dependencies and added a `deps.edn` file.

Contributor guide

No contributing guide indexed for this repository

Research direction

Read src/defun/core.cljc around the defun macro definition at line 156, then compare its actual expansion and :arglists behavior with the README example. Review PR #25 and the updated tests mentioned in the issue; done means the documentation matches the code, :arglists behavior is covered by tests, and the Babashka breakage is addressed.

Written by the indexing model from the issue text.

Assessment

Tech stack
clojure
Domain
documentation, tooling
Issue type
Bug
Difficulty
4/5
Estimated time
3-5 days
Activity status
Stale
Clarity
Mostly clear
Newbie friendliness
30/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.