Improving consistency of explain-data for instrument/macroexpand-check

Question

*Description*

If you instrument a function, you may get a spec error like the following:


(defn f [x] (inc x))
(s/fdef f
  :args (s/cat :x (s/and integer? even?))
  :ret (s/and integer? odd?))

(t/instrument)

(f 3)
;; ExceptionInfo Call to #'user/f did not conform to spec:
;; In: [0] val: 3 fails at: [:args :x] predicate: even?
;; :clojure.spec.alpha/spec  #object[clojure.spec.alpha$regex_spec_impl$reify__1200
 0x19b3f9a "clojure.spec.alpha$regex_spec_impl$reify__1200@19b3f9a"]
;; :clojure.spec.alpha/value  (3)
;; :clojure.spec.alpha/args  (3)
;; :clojure.spec.alpha/failure  :instrument
;; :clojure.spec.test.alpha/caller  {:file "form-init3240393046310519022.clj", :lin
e 1, :var-scope user/eval1413}
;; clojure.core/ex-info (core.clj:4725)

(ex-data *e)
;; {:clojure.spec.alpha/problems
;;   [{:path [:args :x],
;;     :pred clojure.core/even?,
;;     :val 3,
;;     :via [],
;;     :in [0]}],
;;  :clojure.spec.alpha/spec #object[clojure.spec.alpha$regex_spec_impl$reify__1200 0x19b3f9a "clojure.spec.alpha$regex_spec_impl$reify__1200@19b3f9a"],
;;  :clojure.spec.alpha/value (3),
;;  :clojure.spec.alpha/args (3),
;;  :clojure.spec.alpha/failure :instrument,
;;  :clojure.spec.test.alpha/caller {:file "form-init3240393046310519022.clj", :line 1, :var-scope user/eval1413}}


As you can see,

- the explain-data has a regex (ie.  the spec for the args of f) in it as {{::s/spec}}
- each problem contains {{:args}} in their {{:path}}

These facts can cause a confusion to spec error reporters because the spec for the args of {{f}} ({{(s/and integer? even?)}}) has no subspec corresponding to the key {{:args}} (I believe {{:path}} should only contains keys that is a clue to indicate which subspec to be chosen from a spec).

*Possible resolutions*

To resolve this confusing situation and improve the consistency of explain-data for instrument check, I think there are two options as follows:

- *Solution 1.* removing {{:args}} from {{:path}}
- *Solution 2.* modifying explain-data for instrument check so that they have fspec (rather than {{:args}} of it) as {{::s/spec}}

Personally, I prefer *Solution 2.* since adding fspec in explain-data makes it possible to provide richer error information to {{\*explain-out\*}} implementors.

The same goes for {{macroexpand-check}}.

Answer 1

Comment made by: alexmiller

The fspec is the spec in question in here and it does have a component :args (the fspec instance supports key lookup via ILookup for :args as well). So while I would like to improve the error message and data here, I don't agree with removing :args from path. One thing that I think would be useful is for an instrumentation failure to better state the invocation (combining the function and the args into the original call). Right now those are separated and it take some mental work to knit the arg list back together.

Answer 2

Comment made by: sohta

{quote}
So while I would like to improve the error message and data here, I don't agree with removing :args from path.
{quote}

Yes, so once we decide to go with Solution 2., then I think there is no need to remove {{:args}} from {{:path}}.

{quote}
One thing that I think would be useful is for an instrumentation failure to better state the invocation (combining the function and the args into the original call).
{quote}

I totally agree that would be useful, though it sounds like it's somewhat beyond the scope of the ticket in terms of consistency improvement.

Answer 3

Comment made by: sohta

I've made a patch just to express what I meant. Any feedback will be appreciated.

Answer 4

Comment made by: bbrinck

{quote}
One thing that I think would be useful is for an instrumentation failure to better state the invocation (combining the function and the args into the original call). Right now those are separated and it take some mental work to knit the arg list back together.
{quote}

Apologies if I'm missing something, but is it even possible to get the function from the explain-data? I don't see it in explain-data, but perhaps it is recoverable somehow? In any case, I would greatly appreciate it if this type of information was available, since it would make it possible to print clearer error messages.

Answer 5

Comment made by: sohta

Hey, is this already done by CLJ-2392? :)

Answer 6

Reference: https://clojure.atlassian.net/browse/CLJ-2218 (reported by sohta)