Welcome! Please see the About page for a little more info on how this works.

0 votes
in Spec by
The every-kv doc states "takes separate key and val preds and works on associative collections". Vectors return true for associative? but do not currently work:


user=> (s/conform (s/every-kv any? any?) [])
[]
user=> (s/conform (s/every-kv any? any?) [1 2 3])
:clojure.spec/invalid
user=> (s/conform (s/every-kv integer? string?) [])
[]
user=> (s/conform (s/every-kv integer? string?) ["x"])
:clojure.spec/invalid


Another similar problem:


(s/explain-data (s/every-kv int? int?) [{:a :b}])
UnsupportedOperationException nth not supported on this type: PersistentArrayMap  clojure.lang.RT.nthFrom (RT.java:903)


*Cause:* Vectors should not work with every-kv. The combination of every-kv and every-impl assume that the collection passed to every-kv can provide a seq of map entries. In the explain case, the ::kfn created by every-kv is used to create a better path segment using the key rather than the element index. The kfn assumes that an element of the collection can call `(nth entry 0)` on the element. In the explain failure above, the map {:a :b} will throw when invoked with nth.

*Proposed:* Do the following to be clearer about the requirement that the coll elements are map entries:

* Modify the docstring to say "seqs to map entries" rather than "is a map"
* Modify the kfn to add a check that the element is an entry and if so, use it's key. If not, use the index (of the element itself). In this case, when passed an entry it will report with the actual key but when passed something that's not an entry, it will report the collection based index of the non-entry.

After the patch, the explain-data call will provide a useful error rather than the exception above:


user=> (s/explain-data (s/every-kv int? int?) [{:a :b}])
#:clojure.spec.alpha{:problems ({:path [], :pred vector?, :val {:a :b}, :via [], :in [0]})}



*Patch:* clj-2080-8.patch

8 Answers

0 votes
by

Comment made by: alexmiller

At the moment, I'm inclined to say the doc in every-kv should be tightened to say "map" instead of "associative collection" but will check with Rich.

0 votes
by

Comment made by: alexmiller

Updated to apply to master

0 votes
by

Comment made by: alexmiller

Updated patch to apply to spec.alpha.

0 votes
by
_Comment made by: stu_

Can you please update this ticket with expected behavior for the examples shown in the description, plus test showing that expected behavior.  I am still seeing vectors fail for conform.


(s/explain-data (s/every-kv integer? string?) ["x"])
=> #:clojure.spec.alpha{:problems ({:path [], :pred vector?, :val "x", :via [], :in [0]})}
0 votes
by

Comment made by: alexmiller

Vectors should be failing - every-kv requires a seq of map entries. The patch clarifies this in the docstring and fixes the exception that can (incorrectly) be thrown when producing explain-data. I updated the title, the description to include the explain-data after behavior, and added a test to the patch.

0 votes
by

Comment made by: alexmiller

Lost docstring change a ways back, re-added in -7 patch.

0 votes
by

Comment made by: alexmiller

-8 updates to apply to master, no semantic changes

0 votes
by
Reference: https://clojure.atlassian.net/browse/CLJ-2080 (reported by bbloom)
...