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

String conforming to number? fails for integers #92

Closed
benalbrecht opened this issue Dec 14, 2017 · 2 comments · Fixed by #93
Closed

String conforming to number? fails for integers #92

benalbrecht opened this issue Dec 14, 2017 · 2 comments · Fixed by #93

Comments

@benalbrecht
Copy link
Contributor

(spec-tools.spec/number? 1)
=> true
(spec-tools.core/conform spec-tools.spec/number? 1 spec-tools.core/string-conforming)
=> :clojure.spec.alpha/invalid

i think, the issue here is, that number? is classified as a double and string->double only checks whether the input is a double? before trying Double/parseDouble.
Since Double/parseDouble expects a string, an exception is thrown and ::s/invalid is returned.

@ikitommi
Copy link
Member

Here's the code: https://github.com/metosin/spec-tools/blob/master/src/spec_tools/conform.cljc#L26-L35

It think it should just test for string? and tranform, otherwise reuturn the original value and let the spec itself return the ::s/invalid. I think it should work. Would you have time for a pull-request on this?

@benalbrecht
Copy link
Contributor Author

benalbrecht commented Dec 14, 2017

i implemented your suggestion in #93, but as a result, this test fails:

FAIL in (string->double) (conform_test.cljc:18)
expected: (= st/+invalid+ (conform/string->double _ true))
  actual: (not (= :clojure.spec.alpha/invalid true))

another option would be to return the original value for number? instead of double? and keep everything else as is.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants