bugfix to make the coercion work properly with s/or

[wandersoncferreira]

Fixes #178 . The solution is a bit more convoluted than I expected because of the special case for clojure.core/any? forms that would introduce a bug if I stopped the reduce operation when I first get a valid data from a or spec.

The example provided in the issue is now working as expected:

(st/coerce ::vehicle chevy st/strip-extra-keys-transformer)
;; => {:doors 4}

[coveralls]

Coverage increased (+0.08%) to 96.487% when pulling ce88495 on wandersoncferreira:bugfix/or-spec-coercion into b02175b on metosin:master.

[ikitommi]

Thanks! This needs a test to verify that it works (and won't regress later). I didn't understand the any? guard. Could you clarify why it is needed?

[wandersoncferreira]

I did a bit more of investigation about this any workaround. What happened was the following:

Consider these specs:

(s/def ::car (st/spec (s/keys :req-un [::doors])))
(s/def ::bike (st/spec (s/keys :req-un [::wheels])))
(s/def ::tires (s/coll-of (s/and int?) :into #{}))
(s/def ::vehicle (s/or :car ::car
                       :bike ::bike))
(s/def ::new-vehicle (s/map-of
                      keyword?
                      (s/or :vehicle ::vehicle
                            :tires (s/coll-of (s/and int?) :into #{}))))

The issue happened in one of the specs in the test suite that used the (s/coll-of (s/and int?) :into {}) construct. It seems that if you define the coll-of as a "unnamed spec" the current implementation will leave the :into {} operation as a separate iteration in the recursive path performed during the coercion.

Therefore you should be expecting this:

(st/coerce ::new-vehicle {:rodas [1 "1" 3]} st/strip-extra-keys-transformer)
;; => {:rodas #{1 "1" 3}}

But you will get

(st/coerce ::new-vehicle {:rodas [1 "1" 3]} st/strip-extra-keys-transformer)
;; => {:rodas [1 "1" 3]}

This happens because the spec created by (st/create-spec...) will match the vector because the into {} is not part of that spec in the current iteration, if I reduced the answer now, it will prevent to get into the second iteration where the into {} operation will happen. I also noticed the created spec produced a property :form equal to clojure.core/any therefore I skipped to the old behavior as soon as I encounter this situation.

I'll push some tests.

[wandersoncferreira]

The breaking test was

spec-tools/test/cljc/spec_tools/core_test.cljc

Line 427 in 6d1ffd4

(testing "composed"

[wandersoncferreira]

@ikitommi I will study this better today and tomorrow. I started to investigate the issue #212 and it seems related somehow to the problem I described earlier. Need more time to look in more depth.

[wandersoncferreira]

@ikitommi Now this is correct, I improved the mechanism to perform the validity check from coerced values and all went more smoothly now. There is no special case to handle. Solve both issues.

[ikitommi]

Awesome, thanks!

[ikitommi]

Simplified a bit more: f0bd074. Tests pass, does that look correct?

[wandersoncferreira]

Sure! Great, seems like specize has an implementation to fn interfaces, didn't know that.