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

Better reporting for evaluated-from when eval #4

Closed
thumbnail opened this issue Aug 13, 2019 · 6 comments
Closed

Better reporting for evaluated-from when eval #4

thumbnail opened this issue Aug 13, 2019 · 6 comments

Comments

@thumbnail
Copy link
Member

Problem statement

If a spec is dynamically created using eval the evaluated-from part becomes unwieldy.

example;

(let [options #{:a :b :c}]
     (check! (eval `(s/map-of ~options any?)) {:d :d}))
-- Spec failed --------------------

  {:d ...}
   ^^

should be one of: :a, :b, :c

evaluated from

  (eval (clojure.core/seq (clojure.core/concat (clojure.core/list (quote clojure.spec.alpha/map-of)) (clojure.core/list options) (clojure.core/list (quote clojure.core/any?)))))

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

This is sometimes necessary if the actual spec is unknown. Using the var yields an unhelpful spec failure:

(let [options #{:a :b :c}]
     (check! (s/map-of options any?) {:d :d}))
-- Spec failed --------------------

  {:d ...}
   ^^

should satisfy

  options

evaluated from

  (s/map-of options any?)

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

Proposal

quick testing suggests removing eval if it's the first form might fix the printing;

(let [options #{:a}]
  (str `(eval `(s/map-of ~options any?))))
=>
"(clojure.core/eval (clojure.core/seq (clojure.core/concat (clojure.core/list (quote clojure.spec.alpha/map-of)) (clojure.core/list dev/options) (clojure.core/list (quote clojure.core/any?)))))"
(let [options #{:a}]
  (str `(s/map-of ~options any?)))
=> "(clojure.spec.alpha/map-of #{:a} clojure.core/any?)"

Alternatives and comparison

  • fix this in expound, as the same example without map-of does seem to work
  • ignore this, expect devs to be hinted by (eval ... at the start
@vemv
Copy link
Contributor

vemv commented Aug 14, 2019

Note that for your example:

(let [options #{:a :b :c}]
  (check! (eval `(s/map-of ~options any?)) {:d :d}))

...one also could have written:

(let [options #{:a :b :c}]
  (check! options (keys {:d :d})))

...which results in a usable report:

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

  (:d)

should be one of: :a, :b, :c

evaluated from

  options

-------------------------

So, the eval-specing pattern is probably quite rare and unproven atm. If not proven otherwise, we may consider the ugly reports a desirable feature - it signals that a spec was defined weirdly.

@thumbnail
Copy link
Member Author

The example you provide isn't correct because this spec would fails for any map. as a list of symbols is never included in the set of symbols;

e.g.

(let [options #{:a :b :c}]
  (check! options (keys {:a :a})))
-- Spec failed --------------------

  (:a)

should be one of: :a, :b, :c

evaluated from

  options

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

I agree we should aim for rewriting the spec for example opposed to fixing this u-s if possible, just not sure if it's possible.

@vemv
Copy link
Contributor

vemv commented Aug 15, 2019

Of course the example I gave would always fail; the point was about avoiding spec/map-of:

nedap.speced.def.impl.analysis> (defn example [{:as options}]
                                  {:pre [(check! (partial some #{:a :b :c}) (keys options))]}
                                  options)
#'nedap.speced.def.impl.analysis/example
nedap.speced.def.impl.analysis> (example {:aa 1})
-- Spec failed --------------------

  (:aa)

evaluated from

  (keys options)

should satisfy

  <anonymous function>

evaluated from

  (partial some #{:c :b :a})

You can also use this other syntax:

(speced/defn foo [^{::speced/spec (comp (partial some #{:a :b :c}) keys)} options]
  options)

It having an equivalent reporting.

There's nedap/speced.def#31 for making things more concise.

@thumbnail
Copy link
Member Author

the point was about avoiding spec/map-of:

I see no other way of using a var instead of a set directly with proper reporting. In your other example you're using the set directly, which is impossible in my situation where the set is dynamic.

maybe I'm just not seeing it, but while your evaluated from in your last example is descriptive enough to be helpful, I'm unsure how to accomplish that without manually specifying the set (opposed to using a var)

@vemv
Copy link
Contributor

vemv commented Aug 15, 2019

Ok, got it.

How about moving the eval one level up?

unit.nedap.speced.def.defn> (let [options #{:a :b :c}]
                              (eval `(check! (spec/map-of ~options any?) {:d :d})))
-- Spec failed --------------------

  {:d ...}
   ^^

should be one of: :a, :b, :c

evaluated from

  (clojure.spec.alpha/map-of #{:c :b :a} clojure.core/any?)

@thumbnail
Copy link
Member Author

That worked, seems proper enough not to warrant changes to the reporting for now.

Thanks !

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

2 participants