Improved Error Messages - Part XXXVIII

[Mark Addleman]

Searching through this forum, I found many posts relating to improving
Clojure's error messages. I stumbled across one where Rich makes a
plea for concrete suggestions to specific cases. I'm very sympathetic
to that, so I'd like to start a thread of specific coding errors that
I've run across where I've felt the error messages could be improved.

(*) Improperly specifying a map, e.g. "{:a 1 :b}" yields an array out
of bounds exception.

[Timothy Pratley]

Here are some of my common mistakes:

user=> (def data (ref {}))
#'user/data
user=> (dosync alter data assoc :a "a")
"a"
user=> (dosync (alter data assoc :a "a"))
{:a "a"}
;; leaving out the parenthesis on a dosync can be silent and fatal,
like a ninja


(if-let [a 1]
(println a)
a
(inc c))
java.lang.IllegalArgumentException: if-let requires a vector for its
binding (NO_SOURCE_FILE:0)
;; rather cryptic errors from if-let when not quite right, like an
illusionist using misdirection to awe the audience


(javax.swing.UIManager/setLookAndFeel
javax.swing.UIManager/getSystemLookAndFeelClassName)
java.lang.Exception: Unable to find static field:
getSystemLookAndFeelClassName in class javax.swing.UIManager
(NO_SOURCE_FILE:7)
;; this one drives me nuts, I forget to call the method
;; if you don't read carefully it looks like maybe I just typoed the
long method name so I spend about 10minutes checking javadocs, cutting
and pasting, importing etc
;; The error while correct seems a bit odd seeing Class/method looks a
method (though I understand why it isn't).

(java.lang.Math/abs 2/3)
java.lang.IllegalArgumentException: No matching method found: abs
(NO_SOURCE_FILE:0)
;; abs wont work on ratios, but the error message doesn't give the
signature being sought.
;; abs for java numbers exists, abs for ratios doesn't.
;; It is not always obvious that 2/3 is a ratio if it is a passed in
variable.


Regards,
Tim.

[James Reeves]

On Jun 25, 5:38 am, Mark Addleman <mark_addle...@bigfoot.com> wrote:
> Searching through this forum, I found many posts relating to improving
> Clojure's error messages.  I stumbled across one where Rich makes a
> plea for concrete suggestions to specific cases.

Here's one that regularly confuses me:

(prn [10)

=> java.lang.Exception: Unmatched delimiter: )

Ideally the exception should correctly report the type of unmatched
bracket. In this case:

=> java.lang.Exception: Unmatched delimiter: ]

- James

[James Reeves]

On Jun 25, 10:02 am, James Reeves <weavejes...@googlemail.com> wrote:
> Here's one that regularly confuses me:
>
>   (prn [10)
>
>   => java.lang.Exception: Unmatched delimiter: )
>
> Ideally the exception should correctly report the type of unmatched
> bracket. In this case:
>
>   => java.lang.Exception: Unmatched delimiter: ]

I've just realised I've made a mistake. The error message is not
complaining that there is a parenthesis missing; it is complaining
that an ending parenthesis has no matching start parenthesis.

So this is an technically accurate error message, but perhaps not the
most intuitive, as a user is more likely to miss off an ending bracket
than a starting one. A better error message might be:

=> java.lang.Exception: Missing delimiter: ]

- James

[Mark Addleman]

java.lang.UnsupportedOperationException: nth not supported on this
type: Symbol (db.clj:101)

The line number isn't very useful because it is always the first line
of the definition. To help me locate the error, it would be helpful
to list the specific symbol such as "nth not supported on ':symbol' of
type Symbol "

[Mark Addleman]

On Jun 25, 4:15 am, Mark Addleman <mark_addle...@bigfoot.com> wrote:
> java.lang.UnsupportedOperationException: nth not supported on this
> type: Symbol (db.clj:101)

It turns out this error was due to not specifying my macro's arg list
within brackets:

(defmacro with-table table rows & instructions ...)

of course, it should be

(defmacro with-table [table rows & instructions] ...)

[Rich Hickey]

But it doesn't know when it sees the ) that ] is missing, it might not be:

(prn [10)]

i.e. the ) has nothing to do with the [, there might not even be a
pending aggregate:

user=> )
java.lang.Exception: Unmatched delimiter: )

Rich

[Chouser]

On Thu, Jun 25, 2009 at 2:20 AM, Timothy
Pratley<timothy...@gmail.com> wrote:
>
> Here are some of my common mistakes:
>
> user=> (def data (ref {}))
> #'user/data
> user=> (dosync alter data assoc :a "a")
> "a"
> user=> (dosync (alter data assoc :a "a"))
> {:a "a"}
> ;; leaving out the parenthesis on a dosync can be silent and fatal,
> like a ninja

Interesting. Can there ever be a purpose in having
something other than a list before the tail position of
a 'do'?

Hm, I suppose you could make calls that have side-effects
within a map or vector literal, although that would be weird
since the map or vector itself would be thrown away:

(do
[(println "why??")]
5)

So even if the compiler ought to let that pass, is there
ever a purpose in having a bare symbol before the tail
position of a 'do'?

(do
samurai
10)

It might be nice to get a "useless bare symbol" warning in
cases like this.

I'd be happy to try to write a patch for this if one is
welcome.

> (if-let [a 1]
>  (println a)
>  a
>  (inc c))
> java.lang.IllegalArgumentException: if-let requires a vector for its
> binding (NO_SOURCE_FILE:0)
> ;; rather cryptic errors from if-let when not quite right, like an
> illusionist using misdirection to awe the audience

This message is the result of an earlier attempt at specific
and helpful error messages. It is based on the assumption
that you're trying to use an old form of if-let:

(if-let [a b] my-pair

a
(inc c))
java.lang.IllegalArgumentException: if-let requires a vector for its
binding (NO_SOURCE_FILE:0)

Would you recommend a more general and therefore more
accurate message? Something like "your if-let syntax is
incorrect"?

> (javax.swing.UIManager/setLookAndFeel
>  javax.swing.UIManager/getSystemLookAndFeelClassName)
> java.lang.Exception: Unable to find static field:
> getSystemLookAndFeelClassName in class javax.swing.UIManager
> (NO_SOURCE_FILE:7)
> ;; this one drives me nuts, I forget to call the method
> ;; if you don't read carefully it looks like maybe I just typoed the
> long method name so I spend about 10minutes checking javadocs, cutting
> and pasting, importing etc
> ;; The error while correct seems a bit odd seeing Class/method looks a
> method (though I understand why it isn't).

So in this case the message is accurate but too general?
These must be hard to get right! I suppose the compiler
could look for a no-arg method by that name and suggest that
specific cause in the message.

> (java.lang.Math/abs 2/3)
> java.lang.IllegalArgumentException: No matching method found: abs
> (NO_SOURCE_FILE:0)
> ;; abs wont work on ratios, but the error message doesn't give the
> signature being sought.
> ;; abs for java numbers exists, abs for ratios doesn't.
> ;; It is not always obvious that 2/3 is a ratio if it is a passed in
> variable.

I agree that it'd be helpful to list the arg types passed in:
"No 'abs' method found for [clojure.lang.Ratio]"

I wouldn't expect this to be too hard to patch either.

--Chouser

[Mark Addleman]

On Jun 25, 4:26 am, Rich Hickey <richhic...@gmail.com> wrote:

In your example, couldn't the parser know that when it sees the ),
something has gone wrong? Maybe it's due to my IDE's lack of
sophistication, but I find my paren/bracket/brace problems are almost
always around closing them properly (hey, that's punny!) so it would
help me if the error message indicated that it saw ) but expected ].

As an aside, it would be helpful if the parser error messages could
include the column number as well as the line number. Even without
further IDE support, it would be helpful in determining where the
nesting has gone wrong. I think it would also be helpful if compiler
error messages included column numbers as well, but I can see how that
might be difficult. I know this violates my earlier statement about
being specific, but maybe a custom exception for Compiler errors that
included the specific statement list with the error. It seems like
IDEs could do something interesting with that information such as
opening a REPL populated with the statement so I could do some
exploratory debugging or offer assistance by consulting some database
of common mistakes...

[Chouser]

It seems like this could be caught at read time, just by
having the Map classes complain about non-even-length init arrays.

--Chouser

[James Reeves]

On Jun 25, 12:26 pm, Rich Hickey <richhic...@gmail.com> wrote:
> But it doesn't know when it sees the ) that ] is missing, it might not be:
>
> (prn [10)]
>
> i.e. the ) has nothing to do with the [, there might not even be a
> pending aggregate:
>
> user=> )
> java.lang.Exception: Unmatched delimiter: )

Good point. Then what about:

user=> (prn [10)]
java.lang.Exception: Unmatched delimiter ')', expecting ']'

user=> )
java.lang.Exception: Unmatched delimeter ')', expecting end

Where the expected value is the closing bracket of last opened
bracket, or "end", if there was no previous bracket.

- James

[Rich Hickey]

It does know when it sees the ')', and reports it then. But what
exactly has gone wrong (beyond the ')' has no partner) is subjective.
Another example:

(let [x map inc nums)] ...)

Clearly the message: "java.lang.Exception: Missing delimiter: ]" would
not be helpful here.

This seems to me an edit-time bug best fixed using an editor with
proper matching support.

Rich

[David Nolen]

FWIW, this is the kind of problem that paredit mode in Emacs eliminates almost entirely.

[James Reeves]

On Jun 25, 4:14 pm, Rich Hickey <richhic...@gmail.com> wrote:
> (let [x map inc nums)] ...)
>
> Clearly the message: "java.lang.Exception: Missing delimiter: ]" would
> not be helpful here.

But perhaps the message:

java.lang.Exception: Unmatched delimiter ')', expecting ']'

Would be quite useful? It would tell the user that the unmatched
delimiter was inside the [] block.

Some languages go a step further, and highlight syntax errors directly
with some ASCII art:

Unmatched delimiter:

(let [x map inc nums)]

^

Though that seems quite tricky to do within the limits of Java's
exception handling. You couldn't really guarentee your output would be
to a terminal with a fixed-width font.

- James

[Ethan Herdrick]

I think it's important that Clojure be good at exploring Java
libraries. Helpful and correct error messages are important for this.
I often go looking for a function at the REPL, like so:

user=> substring
java.lang.Exception: Unable to resolve symbol: substring in this
context (NO_SOURCE_FILE:0)
user=> substr
java.lang.Exception: Unable to resolve symbol: substr in this context
(NO_SOURCE_FILE:0)
user=> subs
#<core$subs__5256 clojure.core$subs__5256@37699720>

Aha! It's 'subs'. This doesn't work with Java methods though.

user=> (.substringXXXX (String. "foo"))
java.lang.IllegalArgumentException: No matching field found:
substringXXXX for class java.lang.String (NO_SOURCE_FILE:0)

user=> (.substring (String. "foo"))
java.lang.IllegalArgumentException: No matching field found: substring
for class java.lang.String (NO_SOURCE_FILE:0)

You've got to supply the right arguments else you get no hint that you
have found a method:

user=> (.substring (String. "foo") 1)
"oo"


Same thing with static methods:

user=> (String/valueOf)
java.lang.NoSuchFieldException: valueOf (NO_SOURCE_FILE:89)

Not true. And there's no hint that I'm any closer to a real method
than if I'd tried gibberish:

user=> (String/valueOfXXX)
java.lang.NoSuchFieldException: valueOfXXX (NO_SOURCE_FILE:88)

Here's the correct usage:
user=> (String/valueOf 9)
"9"

[Four of Seventeen]

On Jun 25, 12:18 pm, James Reeves <weavejes...@googlemail.com> wrote:
> Some languages go a step further, and highlight syntax errors directly
> with some ASCII art:
>
>   Unmatched delimiter:
>     (let [x map inc nums)]
>                         ^

The javac compiler, for one.

> Though that seems quite tricky to do within the limits of Java's
> exception handling. You couldn't really guarentee your output would be
> to a terminal with a fixed-width font.

The important case is that it work at the REPL, which does use a fixed-
width font. And I'm pretty sure exception detail messages can contain
newlines.

[James Reeves]

On Jun 25, 7:01 pm, Four of Seventeen <fsevent...@gmail.com> wrote:
> On Jun 25, 12:18 pm, James Reeves <weavejes...@googlemail.com> wrote:
>
> > Some languages go a step further, and highlight syntax errors directly
> > with some ASCII art:
>
> >   Unmatched delimiter:
> >     (let [x map inc nums)]
> >                         ^
>
> The javac compiler, for one.

Huh. So it does. Okay, maybe it's not an impractical idea after all :)

- James

[Glen Stampoultzis]

2009/6/26 James Reeves <weave...@googlemail.com>

I think it is worth looking at what JRuby does when it spits out an error.  They don't show a Java stacktrace at all but rather one that looks a lot closer to what native Ruby shows.  I'd suggest Java stack traces are a poor fit for reporting clojure errors since they don't match that closely to the underlying language structure.

[Timothy Pratley]

Thanks for considering those Chouser,

> It might be nice to get a "useless bare symbol" warning in
> cases like this.

That sounds useful to me!

Another special case similar to 'bare symbol' which would be nice to
have reported is when a lazy sequence is created and thrown away.
Usually this means it was intended for side-effects which will never
occur. Unfortunately I have no idea how to detect such a thing seeing
it would look like a list. I suppose an explicit check could be made
for map which is the common one.
(map println [1 2 3 4])
is usually an error. The tricky thing is that from the REPL its not
such an error in that it gets forced, so might need to be treated
differently (I'd argue even then it should be an error - but don't
have much to base that on).

> Would you recommend a more general and therefore more
> accurate message?  Something like "your if-let syntax is
> incorrect"?

My expectation was that (if-let binding a b c) would complain about
the extraneous c specifically, and only about the binding if it was
not a vector. But I see now as you described there is an & oldform to
catch the old style. For this kind of situation the signature can
greatly clarify:
"Too many args passed to if-let: (if-let ([bindings then] [bindings
then else]))"
;; assuming & oldform gets fully deprecated at some point
Leaving aside if-let with its oldform perhaps this would be useful in
other scenarios:
user=> (mod 1 2 3)
java.lang.IllegalArgumentException: Wrong number of args passed to:
core$mod (NO_SOURCE_FILE:0)
"Too many args passed to mod: (mod ([num div]))"

> I suppose the compiler
> could look for a no-arg method by that name and suggest that
> specific cause in the message.

This is a special case where the compiler could hint to me, "oh that's
a method call, not a field". The symbol is resolvable and has a
meaning, the fact that it is a method and can't be passed like so is
the real problem from my (stupid user) perspective. From the compiler
perspective not found makes perfect sense because it is specifically
looking for a field. If I see "that's a method not a field", or "can't
pass method" I will realise, oh I forgot to call it! But when I see
"not found" I start looking in all the wrong places.

> I agree that it'd be helpful to list the arg types passed in:
> "No 'abs' method found for [clojure.lang.Ratio]"

Yes, that would make it nice and obvious.
Candidate method hints would also be nice (again, showing the
signatures but this time java signatures which involve types). Ethan
Herdrick gave some good examples where showing candidate signatures
really helps. In this case there are:
static double abs(double a)
static float abs(float a)
static int abs(int a)
static long abs(long a)

Just another one that can be a bit hard to decipher:
user=> (1 2 3)
java.lang.ClassCastException: java.lang.Integer cannot be cast to
clojure.lang.IFn (NO_SOURCE_FILE:0)
;; obviously very correct, but might be easier to understand as
"(1 2 3) is a function call, but 1 is not a function"
However (1 2 3) might be a really long string so this might not be
practical.
Maybe "<symbol> is not a function." and optionally suggest "Perhaps
you meant to specify a list? '()"
This does seem to turn up a bit hence I think it warrants special
handling.



Regards,
Tim.

[Four of Seventeen]

Got another one.

(if-not (zero? diskqueue-count (if value
(trait-dir trait)
(trait-undecided-dir trait)))

#<CompilerException java.lang.IllegalArgumentException: Wrong number
of args passed to: core$fn (foo.clj:1625)>

This should be saying wrong number of args passed to core$zero? rather
than core$fn. I don't know why it isn't.

(For those who missed it, the error is a missing pair of parentheses:

(if-not (zero? (diskqueue-count (if value
(trait-dir trait)
(trait-undecided-dir trait))))

).

[Rich Hickey]

http://www.assembla.com/spaces/clojure/tickets/136-Enhancement--give-meaningful-names-to-inline-expanders

Thanks for the report,

Rich

[Mark Addleman]

Here's another one:
java.lang.IllegalArgumentException: Wrong number of args passed to: db
$table

It would be more helpful if the message included the number of actual
versus expected arguments. For example:
java.lang.IllegalAgumentException: Wrong number of args passed to :db
$table. Expected 4, actual 3.

For multiple arity signatures, something like "Expected 1 or 4 or 5,
actual 3."