metosin / malli

High-performance data-driven data specification library for Clojure/Script.
Eclipse Public License 2.0
1.5k stars 211 forks source link

Support for `:list` #431

Open helins opened 3 years ago

helins commented 3 years ago

I realized that there is currently no support for :list, unlike other collections (:vector, :set). Since this is pretty basic, I was wondering if it was somehow a deliberate choice. If not, a PR looks fairly straightforward to write.

ikitommi commented 3 years ago

I was dropped at some point of not being very useful - :sequential is a better alternative for most cases as it covers both lists and sequences and because of that, transformations don't have to coerce the mapped results back into a list.

(list? '())
(list? (conj '() 1))
(list? (reverse '()))
(list? (list (seq '())))
; => true

(list? (map identity '()))
(list? (cons 1 '()))
(list? (seq '()))
(list? (concat '() '()))
; => false

Do you find explicit list useful?

here's the commit: https://github.com/metosin/malli/commit/0acde2bc021e581aeb8ae341ec44b2d12e1200cd

helins commented 3 years ago

Granted, this has probably never caused much problems because lists are seldom used in the vast majority of projets. They are commonly used in metaprogramming but not much elsewhere. I was more surprised because of the fact that it is a core data structure rather than by unfulfilled obvious use cases.

The only reason I noticed it is, once again, due to generation. :sequential generates a vector, so do all the seqexp schemas but :cat (or when wrapped in :cat) which generates a lazy seq. So a :list or rather a :seq of ints passing both validation and generation must be written like this:

[:and
 [:cat [:* :int]]
 seq?]

 ;; Note, does not allow :ref

Or

[:and
 [:sequential
  {:gen/fmap #(or (list* %)
                            '())}
  :int]
 seq?]

Which looks a bit over-engineered for a simple list. Unless I am missing something?

ikitommi commented 3 years ago

Registering a custom -collection-schema would be the way to do that.

Sadly, the current impl of m/-collection-schema is not the best for this (one would also have to install new methods on various multimethods like malli.generator/-schema-generator etc.

But then again, we can make m/-collcection-schema more versatile: https://github.com/metosin/malli/pull/433.

helins commented 3 years ago

So it looks that there is now everything needed for having :list?

What would a good PR be like, anything besides those points? Adding:

Note on example in #433: predicate should be seq? instead of list? since seqs and lists and interchangeable in pratice. A bit suprisingly, list* produces a seq, not a list.

I would argue that it is a good idea having :list since it's a core data structure and it will make it easier using Malli for describing code-related things (macros, fns operating over code, ...).

helins commented 3 years ago

(and it is also consistent with having support for list?)