Is keys destructuring for documentation reasons good style?

Is keys destructuring for documentation reasons good style? The reason I’m asking is that clj-kondo, a linter for Clojure, reports unused bindings. E.g.:

(let [x 1, y 2] y)

results in a warning about x being unused.

Sometimes people do this:

(defn public-foo [{:keys [:foo :bar] :as x}]
  (private-baz x))

only to get a better docstring (or generated documentation) for public-foo. But they will get a warning about foo and bar being unused. An example from the wild: link.

Destructuring is not free in terms of performance, so using spec or :arglists may be a better alternative to get these documentation benefits. E.g.:

user=>
(defn public-foo
  {:arglists '([{:keys [:foo :bar] :as x}])}
  [x]
  ;; (private-baz x)
  )
#'user/public-foo
user=> (doc public-foo)
-------------------------
user/public-foo
([{:keys [:foo :bar], :as x}])
nil

I’d like some consensus on this before I make any configuration support in clj-kondo to suppress warnings for unused bindings introduced by keys destructuring in function arguments.

5 Likes

My style would normally be:

(defn public-foo [{:keys [:foo :bar] :as x}]
  (->> (private-baz foo bar)
    (assoc x :res)))

But here you are actually using the destructured values, so that’s not only for documentation purposes.

Ya, so I meant I wouldn’t destructure things I don’t use locally. And my style of code wouldn’t pass maps around in my impl code. So I’d still end up using the destructured values.

1 Like

What you said makes good sense.

On the other hand, using destructuring for this purpose is easier to write and, more importantly, maintain (ie easy to forget to update argslist when I eg add a second param) and the performance hit is most likely neglible / insignificant unless in a perf critical code. So it perhaps makes sense to make it possible to allow this, when explicitly asked for?

Clojure’s coding guidelines have some information on this:

  • Idiomatic code uses destructuring a lot. However, you should only destructure in the arg list if you want to communicate the substructure as part of the caller contract. Otherwise, destructure in a first-line let.

However, the newer version of spec seems additive to this in terms of documentation, reducing the need for communicating the structure via arg list destructuring.

That quote doesn’t explicitly mention if destructuring for communication only (while not needing the individual values) is good style. It only tells you, if you are going to destructure, where to do it.

Suppressing unused keys destructuring bindings in function arguments is now supported in clj-kondo: https://github.com/borkdude/clj-kondo/blob/master/doc/config.md#exclude-unused-bindings-from-being-reported

2 Likes

This topic was automatically closed 182 days after the last reply. New replies are no longer allowed.