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.

Reply via email to