I think one obvious area that specs error messages could be improved is
with some basic formatting and cosmetic changes. If spec presented errors
not as a wall of text and syntax but with some simple formatting it would
make a big difference to legibility.
As a starter for 10, why could we not render the messages at a REPL more
like this? (Note this is basically Brian's error - re-rendered):
user=> (ns such.sequences (require ,,,,))
CompilerException clojure.lang.SpecException:
Call to clojure.core/ns did not conform to fdef [:args] spec
There was unexpected extra input in: [2]
with value: (,,, (require [such.vars :as var]
[such.immigration :as immigrate])
(require midje.checking.checkers.defining
midje.checking.checkers.chatty
midje.checking.checkers.simple
midje.checking.checkers.combining
midje.checking.checkers.collection))
Input failed spec predicate: (cat :attr-map (? map?)
:clauses
:clojure.core.specs/ns-clauses)
When compiling: (such/sequences.clj:1:1)
user=>
Some things to point out:
1. Provide some extra context by subclassing IllegalArgumentException as
SpecException. This may also help separate SpecException's from other
IllegalArgumentExceptions too, and help tools do something special on a
SpecException.
2. Use of new lines to break up and separate text blocks.
3. State that it's an fdef spec, and it was the [:args] bit of that spec
that failed. By stating them together we implicitly associate [:args] with
fdef. Note I'm assuming we can also capture that fdef defined this spec.
4. It's a bit unclear what "Extra input" means... so clarify that it was
unexpected. Provide the path [2] as before.
5. State the failing value and pretty print it. Note that we also elide
the other passing parameters with a ,,,,
6. As before state the predicate that identified the failure in a humanised
manner and the location of the failing form.
I'm not suggesting these are necessarily good ideas, and I appreciate I've
not considered all of the other cases you might need to, but it seems that
something like the above would be a dramatic if entirely cosmetic
improvement. It would be a shame if clojure.core made no attempt to
humanise the display of these messages and left it entirely up to the
community.
Thoughts?
R.
On 20 August 2016 at 15:03, Alex Miller <a...@puredanger.com> wrote:
>
>
> On Saturday, August 20, 2016 at 5:17:59 AM UTC-5, Brian Marick wrote:
>>
>> Yesterday, a bug was filed against Suchwow under 1.9alpha11. It turns out
>> to have been a use of `ns …(require…` instead of `(ns …(:require`. Not in
>> Suchwow, but in Midje. Unfortunately, the Suchwow file the bug report
>> pointed at *also* had that typo - apparently I am prone to it - so adding
>> the colon to the require there didn’t make the problem go away.
>>
>> That caused me to lose my temper and make a fool of myself, which is
>> neither here nor there, except that I apologize to @puredanger.
>>
>> I have two suggestions, though:
>>
>> 1. It has long been the case that Clojure allowed `(ns (require…)` even
>> though that’s strictly incorrect. I suggest that, for backwards
>> compatibility, it be allowed going forward. That is, I think it does no
>> harm for a correct `ns` statement to allow symbols as well as keywords.
>> That wrong code in Midje has been there since Clojure 1.2.
>>
>
> We discussed this before releasing the specs and decided to start on the
> strict side. That said, this is still an alpha and there is plenty of time
> to change our minds prior to official release of 1.9 if that ends up being
> a catastrophic decision.
>
>
>>
>> 2. The following is not a good error message:
>>
>> Exception in thread "main" java.lang.IllegalArgumentException: Call to
>> clojure.core/ns did not conform to spec:
>> In: [2] val: ((require [such.vars :as var] [such.immigration :as
>> immigrate]) (require midje.checking.checkers.defining
>> midje.checking.checkers.chatty midje.checking.checkers.simple
>> midje.checking.checkers.combining midje.checking.checkers.collection))
>> fails at: [:args] predicate: (cat :attr-map (? map?) :clauses
>> :clojure.core.specs/ns-clauses), Extra input
>>
>
> You left out this next important line too since it points you to exactly
> the file and line where the error occurs:
>
> , compiling:(such/sequences.clj:1:1)
>
> spec produces very detailed error messages driven by the specs and the
> value being validated. I admit that in some cases the output from a spec
> error (particularly for complicated syntaxes where there are wide
> alternative fan-outs) is daunting. However, spec error messages are going
> to be increasingly common for all of us to see and understand and I think
> it is worth taking the time to slow down and actually read them.
>
> > Call to clojure.core/ns did not conform to spec:
> ^^^^^^^^^^^^^^ <- macro that was passed invalid values
> > In: [2]
> ^^^ <- the data path in the :args passed to the macro, here, the
> 2th element is the require clause (ns = 0, such.sequences = 1)
>
> > val: ((require [such.vars :as var] ...)
> ^^ <- the remaining part of the value that did not match (it has
> already matched or "consumed" the first two elements successfully)
>
> > fails at: [:args]
> ^^^^^^ <- the path in the ns fdef spec to the failure point
>
> > predicate: (cat :attr-map (? map?) :clauses
> :clojure.core.specs/ns-clauses),
> ^^^etc -> the remaining part of the spec it was trying
> to match
>
> > Extra input
> specs way of saying that it found something (the val above) but it
> wasn't what the spec expected next
>
>
> I'm not trying to pretend this is as easy to digest as an error message
> that would be produced by hand-written validation and error code, but it's
> also notoriously difficult to cover all possible cases (which is why the
> Clojure DSLs have so many error gaps despite having a lot of that code). We
> are looking to decrease the amount of custom error detection and reporting,
> so anything we do has to be something we can do generically.
>
> For the specific case of macroexpanded error reporting, I think there
> *are* more things we can do here (generically) that will improve
> readability. We *know* we are in the context of checking an fdef spec on a
> macro, so some of the ":args" stuff is maybe not necessary. Having the val
> and predicate for a s/cat forwarded to the point of mismatch is both great
> (as it's specific) but also confusing (because parts of both the input and
> the cat spec) are missing which removes important context. I think there
> are ways to indicate that's happening better. Earlier versions of this also
> reported the full args and we ended up removing that because in many cases
> it feels redundant (or is potentially large). Maybe there is some heuristic
> we could follow on when that would help.
>
>
>> - It would be better to say “`require` should be a keyword, not a symbol”
>> than "fails at: [:args] predicate: (cat :attr-map (? map?) :clauses
>> :clojure.core.specs/ns-clauses), Extra input”
>>
>> - Suggest Clojure.spec error messages follow the “inverted pyramid”
>> structure of news reports: https://en.wikipedia.org/wiki/Inverted_pyramid
>> That
>> would mean the text message about the error would come first, the spec
>> second, and the wrong expression last.
>>
>> - It would be better to name the namespace that has the problem.
>>
>
> As mentioned above, the next line that you omitted points to the file and
> line with the problem (I went to some trouble to ensure that was reported
> properly from the point of use).
>
>
>> - The stack trace adds nothing to the error. If anything, it makes it
>> less understandable, as the sheer amount of text is offputting.
>>
>
> I agree, but this is to some degree an orthogonal issue. Tools (including
> the base REPL) choose what to do when they receive an exception from the
> compiler. I would like to think more carefully about what the compiler is
> producing (informationally) and also how tools should be expected to handle
> these errors. Certainly in cases like this, the stack trace is unnecessary
> and that's true of many compiler errors. That is a fixable problem and I am
> interested in making improvements in this area still in 1.9.
>
>
>> My https://github.com/marick/structural-typing does (a small) part of
>> what clojure.spec does. I went to a lot of effort to get it to produce good
>> error messages, and it turned out OK. I doubt it would be applicable to
>> clojure.spec, but I’d be happy to sign over any code that could be of use.
>>
>
> Thanks, I doubt there's much we could use directly.
>
>
>> It’s unfortunate that reporting errors is so much harder than detecting
>> them.
>>
>
> Indeed. Unfortunately errors are read by people and people are hard. :)
>
> --
> You received this message because you are subscribed to the Google
> Groups "Clojure" group.
> To post to this group, send email to clojure@googlegroups.com
> Note that posts from new members are moderated - please be patient with
> your first post.
> To unsubscribe from this group, send email to
> clojure+unsubscr...@googlegroups.com
> For more options, visit this group at
> http://groups.google.com/group/clojure?hl=en
> ---
> You received this message because you are subscribed to the Google Groups
> "Clojure" group.
> To unsubscribe from this group and stop receiving emails from it, send an
> email to clojure+unsubscr...@googlegroups.com.
> For more options, visit https://groups.google.com/d/optout.
>
--
You received this message because you are subscribed to the Google
Groups "Clojure" group.
To post to this group, send email to clojure@googlegroups.com
Note that posts from new members are moderated - please be patient with your
first post.
To unsubscribe from this group, send email to
clojure+unsubscr...@googlegroups.com
For more options, visit this group at
http://groups.google.com/group/clojure?hl=en
---
You received this message because you are subscribed to the Google Groups
"Clojure" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to clojure+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.
