bhb / expound

Human-optimized error messages for clojure.spec
Eclipse Public License 1.0
924 stars 24 forks source link

Wrapped `s/keys` does not properly display unqualified keyword specs #215

Closed kelvinqian00 closed 3 years ago

kelvinqian00 commented 3 years ago

If you use s/keys with :req-un and wrap it as part of a larger spec, expound is unable to determine the spec for the unqualified spec identifier. For example, this example from the tests works fine at first:

(s/def :keys-spec/name string?)
(s/def :keys-spec/age int?)
(s/def :keys-spec/user  (s/keys :req [:keys-spec/name] :req-un [:keys-spec/age]))

(expound.alpha/expound :keys-spec/user {})

in which you get

-- Spec failed --------------------

  {}

should contain keys: :age, :keys-spec/name

| key             | spec    |
|=================+=========|
| :age            | int?    |
|-----------------+---------|
| :keys-spec/name | string? |

But when you wrap the user spec in an s/and or an s/or, you get the following:

-- Spec failed --------------------

  {}

should contain keys: :age, :keys-spec/name

| key             | spec                                              |
|=================+===================================================|
| :age            | <can't find spec for unqualified spec identifier> |
|-----------------+---------------------------------------------------|
| :keys-spec/name | string?                                           |
bhb commented 3 years ago

@kelvinqian00 Thanks for the bug report and repro! I'll take a look.

bhb commented 3 years ago

@kelvinqian00 I'm having a hard time reproing this. I tried the following specs and I can't seem to get the results you showed above.

(s/and #(< 0 (count %)) :keys-spec/user)
(s/or :nil nil?
      :user :keys-spec/user)

I'm likely misunderstanding some detail of your repro. Can you should the spec you used to get the result above? Thanks!

kelvinqian00 commented 3 years ago

Ah, sorry for the lack of clarity. When I meant "when you wrap the user spec in an s/and or an s/or" I meant wrap the predicates that make up :keys-spec/user in another spec, not the keyword. For example:

(s/def :keys-spec/user (s/and (s/keys :req [:keys-spec/name]
                                      :req-un [:keys-spec/age])
                              (constantly true)))

will give the desired result.

I actually had to move the s/ands in my project up one level so that they wrap the keywords, similar to what you did, and that was my workaround for this issue.

vemv commented 3 years ago

Hi @kelvinqian00! Likely it would be useful to have a bug repro following your new insight, WDYT?

Cheers - V

bhb commented 3 years ago

@kelvinqian00 Thanks for reporting this! This is fixed in 78be8006104a5f93f243e4fd3e8382e40e3c714e