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

Strange handling of complement / not #50

Closed
vemv opened this issue Nov 14, 2017 · 9 comments
Closed

Strange handling of complement / not #50

vemv opened this issue Nov 14, 2017 · 9 comments

Comments

@vemv
Copy link
Contributor

vemv commented Nov 14, 2017

Hi again!

By evaluating (expound/expound (complement string/blank?) "") one will get:

nil
-- Spec failed --------------------

  ""

should satisfy

  /



-------------------------
Detected 1 error

The / is quite confusing.

If I try with (expound/expound (fn [x] (not (string/blank? x))) "") I'll get the same output.

Tried out with clojurescript.

Hope this one can be improved!

Cheers - Victor

@bhb
Copy link
Owner

bhb commented Nov 15, 2017

@vemv Thanks for reporting this! I had noticed this as well last night, and you're right, this is really confusing! I'll work on a fix.

@bhb
Copy link
Owner

bhb commented Nov 15, 2017

@vemv Unfortunately, it turns out that we can only do a little better in this case. The slash is really confusing, but the best I can do at this point is to print out "" since spec doesn't capture the original code. Here's what it looks like in my branch (compared to the default spec output for reference):

user=> (require '[clojure.spec.alpha :as s]                                                                                                                               
  #_=>          '[clojure.string :as string]
  #_=>          '[expound.alpha :as expound])
nil
user=>
user=> (s/explain (fn [x] (not (string/blank? x))) "")
val: "" fails predicate: :clojure.spec.alpha/unknown
nil
user=> (expound/expound (fn [x] (not (string/blank? x))) "")
-- Spec failed --------------------

  ""

should satisfy

  <anonymous function>



-------------------------
Detected 1 error
nil

@bhb
Copy link
Owner

bhb commented Nov 15, 2017

I can likely do better once https://dev.clojure.org/jira/browse/CLJ-2068 is fixed!

@bhb
Copy link
Owner

bhb commented Nov 15, 2017

Ah, nevermind:

Functions - demunge the function name and use the qualified function name symbol as the form. Add a special check for anonymous functions and revert to ::unknown for those (not much we can do with an eval'ed anonymous function).

@bhb
Copy link
Owner

bhb commented Nov 15, 2017

OK, I've just released a new version of "0.3.4-SNAPSHOT" which contains these changes (you'll need to delete your old copy in ~/.m2/repository/expound/expound and reinstall)

@athos
Copy link

athos commented Nov 15, 2017

BTW I don't think this really matters in practice, though.

clojure.spec is not good at handling bare predicates in terms of error reporting while you can work around it by wrapping them with s/spec:

=> (ex/expound (s/spec (complement str/blank?)) "")
-- Spec failed --------------------

  ""

should satisfy

  (complement clojure.string/blank?)



-------------------------
Detected 1 error
nil
=>

It also works to just s/def a name for the predicate:

=> (s/def ::non-blank (complement str/blank?))
:user/non-blank
=> (ex/expound ::non-blank "")
-- Spec failed --------------------

  ""

should satisfy

  (complement clojure.string/blank?)

-- Relevant specs -------

:user/non-blank:
  (clojure.core/complement clojure.string/blank?)

-------------------------
Detected 1 error
nil
=> 

@bhb bhb closed this as completed in #52 Nov 15, 2017
@vemv
Copy link
Contributor Author

vemv commented Nov 15, 2017

Hey there!

Thank you, much better now 🍻

Perhaps if expound/expound was a macro, it could take the quoted predicate form as a "fallback print value" in case the predicate was unprintable?

I realise turning defns into macros is bit of a dubious move (and a breaking change). Maybe expound* could be offered?

@bhb
Copy link
Owner

bhb commented Nov 15, 2017

@vemv Excellent, I'm glad it works.

You're correct, if expound was a macro, I could do more. However, this might quickly get complicated. For instance, if the call to expound was (expound/expound (s/coll-of (complement string/blank?)) [""]), expound would need to locate the predicate within the spec and match it with the failing predicate.

I suspect it could be done, but since wrapping the predicate in s/spec (or defining a new named spec) seems like a pretty good workaround, I'm going to leave this for now. I may revisit it in the future if this becomes a common pain point.

Thanks for testing the fix!

@bhb
Copy link
Owner

bhb commented Nov 20, 2017

I've just released 0.3.4, which includes this bug fix.

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

No branches or pull requests

3 participants