Idea: improve error messages with compile-time information of the faulty expression

[vemv]

Hi!

First to give a bit of context, I find myself writing something like the following in every other project:

https://github.com/reducecombine/playground/blob/954104e2c673b8e9952c12c4d707dc946570a68c/src/playground/spec_utils.clj#L6-L12

i.e throw an expounded-powered runtime exception if spec/valid? fails. Probably it's a common pattern.

Now, the error messages obtained with this technique are nice, but they could be even better.

For the (check! string? (:foo (do-it 42))) input:

Current

:the/namespace: -- Spec failed --------------------

  nil

should satisfy

  string?
[...]

Desired

:the/namespace: -- Spec failed --------------------

  nil

evaluated from

  (:foo (do-it 42))

should satisfy

  string?
[...]

The "evaluated from" bit tells us what expression (e.g. which function argument) did evaluate to nil. Basically that's what we humans do when debugging these expound errors: we try to find the compile-time expression which caused the nil value.

Note that the check! function I linked to is a function, so I can't access compile-time info.

Now, I could turn it into a macro. Probably pretty easily I could achieve what I want. Now, the only pain point is concatenating my information with expound's information. Manipulating strings is dirty.

So I was thinking, why don't we provide in Expound something akin to my tiny check! helper, allowing for a clean report which includes the desired extra info.

As a nice side-effect, we'd get a "canonical" check! - not ideal having to copy/paste it, or authoring yet another "spec utils library".

WDYT? PR welcome?

Cheers - Victor

[vemv]

FWIW, here's the dirty string-manipulating version 😄

(defmacro check! [& args]
  `(do
     (doseq [[spec# x# x-quoted#] ~(mapv (fn [[a b]]
                                           [a b (list 'quote b)])
                                         (partition 2 args))]
       (or (clojure.spec.alpha/valid? spec# x#)
           (do
             (-> (expound.alpha/expound-str spec# x#)
                 (clojure.string/replace-first "should satisfy"
                                               (str "evaluated from\n\n  " (pr-str x-quoted#) "\n\nshould satisfy"))
                 println)
             (throw (ex-info "Validation failed" {:explanation (clojure.spec.alpha/explain-str spec# x#)})))))
     true))

[bhb]

@vemv Cool idea! I can definitely see how that output would the "evaluated from" output would be super useful.

Let me think a little bit more about how this would interact with some other features I have in mind.

Just for my own notes, I'm thinking about:

• Should this maybe be a expound-specific version of s/assert so it can be turned on/off with *compile-asserts*?
• Maybe it would be worth returning expound information as data so it can be more easily manipulated by callers? As you noted, manipulating the string is not ideal.
• Should I pretty-print the form?

I'm taking a break from working on this project for Feb while I work on some other stuff, but I'll let this bounce around in my mind a bit and look again with fresh eyes in March.

In the meantime, I think your string-manipulating version is a good workaround - you could also maybe exploit the fact that the value is always wrapped in some empty lines (for cases where the first value is not followed by "should satisfy" e.g.

expound/test/expound/alpha_test.cljc

Lines 122 to 126 in 1fba308

	(is (= "-- Spec failed --------------------
	:baz
	should be one of: :bar, :foo

Thanks for the idea and for using expound!

[bhb]

@vemv One other thought - it might be worth filing a Clojure JIRA ticket about capturing this information more generally when a user calls assert (or, I suppose, during macro-expansion). Up to you.

[vemv]

Hey there,

I'm taking a break from working on this project for Feb while I work on some other stuff

I kept this one in mind so I tried not to distract to with a response, but I think I overshot ;p

My suggested defmacro check! has worked pretty great meanwhile. I expanded it so it applies the same evaluated from business to both the faulty values, and the specs themselves. That saved me quite some debugging time: think partial, macroexpansions and such where specs are dynamically built.

Let me know if there's anything I could do to help this feature make it into Expound (but obviously you'll have it easier)

[vemv]

Hey there again,

what have you thought about this one?

From my side I have my check! macro hosted here: https://github.com/nedap/utils.spec/blob/ca6097b013488ed6da4463a734c3132bb0e68b8e/src/nedap/utils/spec/impl/check.cljc

Worth noting, utils.spec is a tiny isolated library, basically it contains check! and little else. i.e. it's an easy read)

I would suggest, we can copy this check! macro to https://github.com/bhb/expound and keep refining it in a PR, applying the mentioned line of improvements.

WDYT?

[bhb]

Thanks a lot for your work on this! I haven’t made progress on this. Instead, my current focus in Expound is to think about how to let libraries like ‘utils.check’ customize the output from Expound without needing to do string parsing and manipulation. My goal is to have a better story for the types of customization that advanced users or client libraries want to do.

If I did that, it would make it cleaner to build this macro and have less risk of it breaking when I change Expound.

All that said, I’m inclined to not add this macro directly to Expound (for now). I will probably eventually add a helper like this but until I have more confidence in how it should work, I’m going to lean towards keeping the Expound API as small as possible.

Thanks again!

[vemv]

Hey there,

yes, sure!

Looking forward to news, but also no real issue in keeping things as-is for now. I guess the spec 1->2 transition might make things even more delicate.