Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

s/and fails in some cases #165

Closed
mping opened this issue Feb 18, 2019 · 4 comments
Closed

s/and fails in some cases #165

mping opened this issue Feb 18, 2019 · 4 comments
Labels

Comments

@mping
Copy link

mping commented Feb 18, 2019

Using s/and with a single spec, the behaviour should be the same as without s/and (at least thats the principle of least surprise).

(:require [clj-time.coerce :as coerce])
(:import  [org.joda.time LocalDate])

(s/def ::custom-date (st/spec {:spec        #(instance? LocalDate %)
                                                    :decode/json #(coerce/to-local-date %2)}))
(st/coerce ::custom-date "2019-01-01" st/json-transformer) ;;#object[org.joda.time.LocalDate 0x7b3e48 "2019-01-01"]
(st/coerce (s/and ::custom-date) "2019-01-01" st/json-transformer) ;;"2019-01-01"

@ikitommi ikitommi added the bug label Feb 18, 2019
@ikitommi
Copy link
Member

That's not good. It seems that the form-walker just tries to walk over :spec-tools.parse/items, which only uses :type for the transformation, not the encode & decode keys. Same applies for all specs that the parses sees as one of: :or, :and, :vector or :set.

Should be easy to fix thou.

@ikitommi
Copy link
Member

Checked all the wrapped specs, by adding the original spec reference to parse results, all of these work:

(s/def ::pos? (st/spec {:spec (partial pos?), :decode/string transform/string->long}))

(testing "referenced specs, #165"
  (is (= 1 (st/coerce (s/and ::pos?) "1" st/string-transformer)))
  (is (= 1 (st/coerce (s/or :default ::pos?) "1" st/string-transformer)))
  (is (= [1 2 3 :4] (st/coerce (s/coll-of ::pos?) ["1" "2" "3" :4] st/string-transformer)))
  (is (= {1 :2, :3 4} (st/coerce (s/map-of ::pos? ::pos?) {"1" :2, :3 "4"} st/string-transformer)))
  (is (= [1 :2 3 "4"] (st/coerce (s/tuple ::pos? keyword? ::pos?) ["1" "2" "3" "4"] st/string-transformer)))
  (is (= 1 (st/coerce (s/nilable ::pos?) "1" st/string-transformer))))

looks ok?

@mping
Copy link
Author

mping commented Feb 19, 2019

@ikitommi looks really good!

ikitommi added a commit that referenced this issue Feb 19, 2019
* new parsed types :tuple and :nilable
ikitommi added a commit that referenced this issue Feb 19, 2019
* new parsed types :tuple and :nilable
@ikitommi
Copy link
Member

ikitommi commented May 1, 2019

FIxed in 0.9.0-alpha1.

@ikitommi ikitommi closed this as completed May 1, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants