Clojure

GC Issue 99: Incorrect error with if-let

Details

  • Type: Enhancement Enhancement
  • Status: Closed Closed
  • Priority: Minor Minor
  • Resolution: Completed
  • Affects Version/s: None
  • Fix Version/s: Release 1.5
  • Component/s: None
  • Labels:
    None
  • Patch:
    Code
  • Approval:
    Ok

Description

Reported by rosejn, Mar 19, 2009

Example:
user=> (if-let [foo (+ 1 2)] foo + 1 foo)
java.lang.IllegalArgumentException: if-let requires a vector for its
binding (NO_SOURCE_FILE:2)

The error reports that if-let requires a vector for its binding, but the
problem here has nothing to do with the binding being a vector.  A paren
was forgotten before the '+', so the number of arguments is incorrect for
the if form.

Activity

Hide
Assembla Importer added a comment -
Show
Assembla Importer added a comment - Converted from http://www.assembla.com/spaces/clojure/tickets/103
Hide
Assembla Importer added a comment -

richhickey said: Updating tickets (#8, #19, #30, #31, #126, #17, #42, #47, #50, #61, #64, #69, #71, #77, #79, #84, #87, #89, #96, #99, #103, #107, #112, #113, #114, #115, #118, #119, #121, #122, #124)

Show
Assembla Importer added a comment - richhickey said: Updating tickets (#8, #19, #30, #31, #126, #17, #42, #47, #50, #61, #64, #69, #71, #77, #79, #84, #87, #89, #96, #99, #103, #107, #112, #113, #114, #115, #118, #119, #121, #122, #124)
Hide
John Szakmeister added a comment -

I have a patch that gives a better error message. It will now say: "IllegalArgumentException if-let requires 1 or 2 forms to be passed". I'm not sure if that's a better error message or not, but I'm happy to tweak it.

Show
John Szakmeister added a comment - I have a patch that gives a better error message. It will now say: "IllegalArgumentException if-let requires 1 or 2 forms to be passed". I'm not sure if that's a better error message or not, but I'm happy to tweak it.
Hide
John Szakmeister added a comment -

First iteration of the patch.

Show
John Szakmeister added a comment - First iteration of the patch.
Hide
Andy Fingerhut added a comment -

clj-103-incorrect-if-let-error-patch2.txt is same as John's except updated to apply cleanly to latest master as of Feb 23, 2012. No compiler errors or warnings, no test failures. No new tests for this one – I could write one, but the only way to do so is to check the contents of the error message in the exception thrown, which seems a bit too specific. The author John Szakmeister is on the contributor list.

Show
Andy Fingerhut added a comment - clj-103-incorrect-if-let-error-patch2.txt is same as John's except updated to apply cleanly to latest master as of Feb 23, 2012. No compiler errors or warnings, no test failures. No new tests for this one – I could write one, but the only way to do so is to check the contents of the error message in the exception thrown, which seems a bit too specific. The author John Szakmeister is on the contributor list.
Hide
Brenton Ashworth added a comment -

The provided patch from John and updated by Andy

clj-103-incorrect-if-let-error-patch2.txt

applies cleanly and doesn't cause any problems. I don't like the wording of the error message: "requires 1 or 2 forms to be passed" so I added another patch

clj-103-improved-if-let-error-message-patch.txt

which is the same change but with the wording "requires 1 or 2 forms after binding vector" which I think is a little more clear.

With this change, a user would now get the following error messages:

> (if-let foo)
ArityException 
Wrong number of args (1) passed to: core$if-let 
clojure.lang.Compiler.macroexpand1 (Compiler.java:6371)

> (if-let foo (+ 1 3))
IllegalArgumentException 
clojure.core/if-let requires a vector for its binding in clojure.core: 
clojure.core/if-let (NO_SOURCE_FILE:124)

> (if-let [foo (+ 1 3)])
ArityException
Wrong number of args (1) passed to: core$if-let 
clojure.lang.Compiler.macroexpand1 (Compiler.java:6371)

> (if-let [foo (+ 1 3)] foo 7 5)
IllegalArgumentException
if-let requires 1 or 2 forms after binding vector in clojure.core:142
clojure.core/if-let (NO_SOURCE_FILE:124)

> (if-let [foo (+ 1 3) bar 7] foo 7)
IllegalArgumentException
if-let requires exactly 2 forms in binding vector in clojure.core:143
clojure.core/if-let (NO_SOURCE_FILE:124)
Show
Brenton Ashworth added a comment - The provided patch from John and updated by Andy clj-103-incorrect-if-let-error-patch2.txt applies cleanly and doesn't cause any problems. I don't like the wording of the error message: "requires 1 or 2 forms to be passed" so I added another patch clj-103-improved-if-let-error-message-patch.txt which is the same change but with the wording "requires 1 or 2 forms after binding vector" which I think is a little more clear. With this change, a user would now get the following error messages:
> (if-let foo)
ArityException 
Wrong number of args (1) passed to: core$if-let 
clojure.lang.Compiler.macroexpand1 (Compiler.java:6371)

> (if-let foo (+ 1 3))
IllegalArgumentException 
clojure.core/if-let requires a vector for its binding in clojure.core: 
clojure.core/if-let (NO_SOURCE_FILE:124)

> (if-let [foo (+ 1 3)])
ArityException
Wrong number of args (1) passed to: core$if-let 
clojure.lang.Compiler.macroexpand1 (Compiler.java:6371)

> (if-let [foo (+ 1 3)] foo 7 5)
IllegalArgumentException
if-let requires 1 or 2 forms after binding vector in clojure.core:142
clojure.core/if-let (NO_SOURCE_FILE:124)

> (if-let [foo (+ 1 3) bar 7] foo 7)
IllegalArgumentException
if-let requires exactly 2 forms in binding vector in clojure.core:143
clojure.core/if-let (NO_SOURCE_FILE:124)
Hide
John Szakmeister added a comment -

I like the improved wording. I struggled with that myself. Thanks Brenton!

Show
John Szakmeister added a comment - I like the improved wording. I struggled with that myself. Thanks Brenton!
Hide
Andy Fingerhut added a comment -

Removed obsolete patch from Feb 23 2012. Preference should be given to patch clj-103-improved-if-let-error-message-patch.txt dated Apr 20 2012 instead.

Show
Andy Fingerhut added a comment - Removed obsolete patch from Feb 23 2012. Preference should be given to patch clj-103-improved-if-let-error-message-patch.txt dated Apr 20 2012 instead.

People

Vote (0)
Watch (1)

Dates

  • Created:
    Updated:
    Resolved: