Clojure

Improve error message when calling a keyword with the wrong number of arguments

Details

  • Type: Enhancement Enhancement
  • Status: Open Open
  • Priority: Minor Minor
  • Resolution: Unresolved
  • Affects Version/s: Release 1.9
  • Fix Version/s: Release 1.10
  • Component/s: None
  • Labels:
  • Patch:
    Code and Test
  • Approval:
    Incomplete

Description

When you call a Keyword with the wrong number of arguments, the error message does not report how many arguments were passed:

(:kw "one" "two" "three")
=> java.lang.IllegalArgumentException: Wrong number of args passed to keyword: :kw

compare to calling an IFn, which does show the number of arguments passed:

(name "one" "two" "three")
=> clojure.lang.ArityException: Wrong number of args (3) passed to: core/name

The latter error message is more clear and makes it easier to debug.

The attached patch re-uses the ArityException class used elsewhere to generate error messages in the latter form when calling a keyword with the wrong number of arguments.

Addresses CLJ-1067

Patch: keyword-arity-exception-03.patch

Prescreened by: Alex Miller

  1. keyword-arity-exception.patch
    03/May/18 7:51 AM
    8 kB
    Marc O'Morain
  2. keyword-arity-exception-01.patch
    04/May/18 6:34 AM
    8 kB
    Marc O'Morain
  3. keyword-arity-exception-02.patch
    07/May/18 3:25 PM
    8 kB
    Marc O'Morain
  4. keyword-arity-exception-03.patch
    09/Jul/18 5:40 PM
    7 kB
    Marc O'Morain

Activity

Marc O'Morain made changes -
Field Original Value New Value
Attachment keyword-arity-exception.patch [ 18100 ]
Hide
Marc O'Morain added a comment -

Fix typo: later => latter.

Show
Marc O'Morain added a comment - Fix typo: later => latter.
Marc O'Morain made changes -
Description When you call a {{Keyword}} with the wrong number of arguments, the error message does not report how many arguments were passed:

{code}
(:kw "one" "two" "three")
=> java.lang.IllegalArgumentException: Wrong number of args passed to keyword: :kw
{code}

compare to calling an {{IFn}}, which does show the number of arguments passed:

{code}
(name "one" "two" "three")
=> clojure.lang.ArityException: Wrong number of args (3) passed to: core/name
{code}

The later error message is more clear and makes it easier to debug.

The attached patch re-uses the {{ArityException}} class used elsewhere to generate error messages in the later form when calling a keyword with the wrong number of arguments.

Addresses CLJ-1067
When you call a {{Keyword}} with the wrong number of arguments, the error message does not report how many arguments were passed:

{code}
(:kw "one" "two" "three")
=> java.lang.IllegalArgumentException: Wrong number of args passed to keyword: :kw
{code}

compare to calling an {{IFn}}, which does show the number of arguments passed:

{code}
(name "one" "two" "three")
=> clojure.lang.ArityException: Wrong number of args (3) passed to: core/name
{code}

The latter error message is more clear and makes it easier to debug.

The attached patch re-uses the {{ArityException}} class used elsewhere to generate error messages in the latter form when calling a keyword with the wrong number of arguments.

Addresses CLJ-1067

Patch: keyword-arity-exception.patch
Attachment keyword-arity-exception.patch [ 18101 ]
Alex Miller made changes -
Approval Prescreened [ 10220 ]
Description When you call a {{Keyword}} with the wrong number of arguments, the error message does not report how many arguments were passed:

{code}
(:kw "one" "two" "three")
=> java.lang.IllegalArgumentException: Wrong number of args passed to keyword: :kw
{code}

compare to calling an {{IFn}}, which does show the number of arguments passed:

{code}
(name "one" "two" "three")
=> clojure.lang.ArityException: Wrong number of args (3) passed to: core/name
{code}

The latter error message is more clear and makes it easier to debug.

The attached patch re-uses the {{ArityException}} class used elsewhere to generate error messages in the latter form when calling a keyword with the wrong number of arguments.

Addresses CLJ-1067

Patch: keyword-arity-exception.patch
When you call a {{Keyword}} with the wrong number of arguments, the error message does not report how many arguments were passed:

{code}
(:kw "one" "two" "three")
=> java.lang.IllegalArgumentException: Wrong number of args passed to keyword: :kw
{code}

compare to calling an {{IFn}}, which does show the number of arguments passed:

{code}
(name "one" "two" "three")
=> clojure.lang.ArityException: Wrong number of args (3) passed to: core/name
{code}

The latter error message is more clear and makes it easier to debug.

The attached patch re-uses the {{ArityException}} class used elsewhere to generate error messages in the latter form when calling a keyword with the wrong number of arguments.

Addresses CLJ-1067

*Patch:* keyword-arity-exception.patch

*Prescreened by:* Alex Miller
Labels errormsgs
Hide
Alexander Taggart added a comment - - edited

+100 for improving error messages.

Albeit unlikely, since `throwArity()` is a public method, removing it risks breaking external code.

Less importantly, the resulting sentence looks a bit awkward with the three colons:
"Wrong number of args (3) passed to: keyword: :my.ns/foo"

Alternatively:
"Wrong number of args (3) passed to: keyword :my.ns/foo"
"Wrong number of args (3) passed to: :my.ns/foo"

The latter parallels the result when calling a non-keyword function:
"Wrong number of args (3) passed to: my.ns/bar"

Show
Alexander Taggart added a comment - - edited +100 for improving error messages. Albeit unlikely, since `throwArity()` is a public method, removing it risks breaking external code. Less importantly, the resulting sentence looks a bit awkward with the three colons: "Wrong number of args (3) passed to: keyword: :my.ns/foo" Alternatively: "Wrong number of args (3) passed to: keyword :my.ns/foo" "Wrong number of args (3) passed to: :my.ns/foo" The latter parallels the result when calling a non-keyword function: "Wrong number of args (3) passed to: my.ns/bar"
Hide
Alex Miller added a comment -

Also, actually there is a typo artity-exceptions in the test.

And I would agree with Alexander's comment, should leave existing throwArity().

Show
Alex Miller added a comment - Also, actually there is a typo artity-exceptions in the test. And I would agree with Alexander's comment, should leave existing throwArity().
Alex Miller made changes -
Approval Prescreened [ 10220 ] Triaged [ 10120 ]
Description When you call a {{Keyword}} with the wrong number of arguments, the error message does not report how many arguments were passed:

{code}
(:kw "one" "two" "three")
=> java.lang.IllegalArgumentException: Wrong number of args passed to keyword: :kw
{code}

compare to calling an {{IFn}}, which does show the number of arguments passed:

{code}
(name "one" "two" "three")
=> clojure.lang.ArityException: Wrong number of args (3) passed to: core/name
{code}

The latter error message is more clear and makes it easier to debug.

The attached patch re-uses the {{ArityException}} class used elsewhere to generate error messages in the latter form when calling a keyword with the wrong number of arguments.

Addresses CLJ-1067

*Patch:* keyword-arity-exception.patch

*Prescreened by:* Alex Miller
When you call a {{Keyword}} with the wrong number of arguments, the error message does not report how many arguments were passed:

{code}
(:kw "one" "two" "three")
=> java.lang.IllegalArgumentException: Wrong number of args passed to keyword: :kw
{code}

compare to calling an {{IFn}}, which does show the number of arguments passed:

{code}
(name "one" "two" "three")
=> clojure.lang.ArityException: Wrong number of args (3) passed to: core/name
{code}

The latter error message is more clear and makes it easier to debug.

The attached patch re-uses the {{ArityException}} class used elsewhere to generate error messages in the latter form when calling a keyword with the wrong number of arguments.

Addresses CLJ-1067

*Patch:* keyword-arity-exception.patch

*Prescreened by:*
Hide
Marc O'Morain added a comment -

Attach updated patch.

Show
Marc O'Morain added a comment - Attach updated patch.
Marc O'Morain made changes -
Attachment keyword-arity-exception-01.patch [ 18103 ]
Marc O'Morain made changes -
Description When you call a {{Keyword}} with the wrong number of arguments, the error message does not report how many arguments were passed:

{code}
(:kw "one" "two" "three")
=> java.lang.IllegalArgumentException: Wrong number of args passed to keyword: :kw
{code}

compare to calling an {{IFn}}, which does show the number of arguments passed:

{code}
(name "one" "two" "three")
=> clojure.lang.ArityException: Wrong number of args (3) passed to: core/name
{code}

The latter error message is more clear and makes it easier to debug.

The attached patch re-uses the {{ArityException}} class used elsewhere to generate error messages in the latter form when calling a keyword with the wrong number of arguments.

Addresses CLJ-1067

*Patch:* keyword-arity-exception.patch

*Prescreened by:*
When you call a {{Keyword}} with the wrong number of arguments, the error message does not report how many arguments were passed:

{code}
(:kw "one" "two" "three")
=> java.lang.IllegalArgumentException: Wrong number of args passed to keyword: :kw
{code}

compare to calling an {{IFn}}, which does show the number of arguments passed:

{code}
(name "one" "two" "three")
=> clojure.lang.ArityException: Wrong number of args (3) passed to: core/name
{code}

The latter error message is more clear and makes it easier to debug.

The attached patch re-uses the {{ArityException}} class used elsewhere to generate error messages in the latter form when calling a keyword with the wrong number of arguments.

Addresses CLJ-1067

*Patch:* keyword-arity-exception-01.patch

*Prescreened by:*
Hide
Marc O'Morain added a comment -

I've attached an updated patch that fixes the type in the deftest declaration and formats the exception message in the way Alexander suggested.

Alex - should I add a Java comment to explain why the 0-arity version of Keyword.throwArtity() exists? Something like:

/**
 * @deprecated CLJ-2350 This function is no longer called, but has not been removed to maintain the public interface.
 */

Reflecting on the public interface, perhaps we could make the new function, throwArity(int n), package-private. This would avoid it becoming part of the public API of clojure.lang.

Show
Marc O'Morain added a comment - I've attached an updated patch that fixes the type in the deftest declaration and formats the exception message in the way Alexander suggested. Alex - should I add a Java comment to explain why the 0-arity version of Keyword.throwArtity() exists? Something like:
/**
 * @deprecated CLJ-2350 This function is no longer called, but has not been removed to maintain the public interface.
 */
Reflecting on the public interface, perhaps we could make the new function, throwArity(int n), package-private. This would avoid it becoming part of the public API of clojure.lang.
Hide
Alex Miller added a comment -

yes and yes

Show
Alex Miller added a comment - yes and yes
Marc O'Morain made changes -
Attachment keyword-arity-exception-02.patch [ 18107 ]
Hide
Marc O'Morain added a comment -

Added patch making throwArity(int) package private, and adding deprecation docstring.

Show
Marc O'Morain added a comment - Added patch making throwArity(int) package private, and adding deprecation docstring.
Marc O'Morain made changes -
Description When you call a {{Keyword}} with the wrong number of arguments, the error message does not report how many arguments were passed:

{code}
(:kw "one" "two" "three")
=> java.lang.IllegalArgumentException: Wrong number of args passed to keyword: :kw
{code}

compare to calling an {{IFn}}, which does show the number of arguments passed:

{code}
(name "one" "two" "three")
=> clojure.lang.ArityException: Wrong number of args (3) passed to: core/name
{code}

The latter error message is more clear and makes it easier to debug.

The attached patch re-uses the {{ArityException}} class used elsewhere to generate error messages in the latter form when calling a keyword with the wrong number of arguments.

Addresses CLJ-1067

*Patch:* keyword-arity-exception-01.patch

*Prescreened by:*
When you call a {{Keyword}} with the wrong number of arguments, the error message does not report how many arguments were passed:

{code}
(:kw "one" "two" "three")
=> java.lang.IllegalArgumentException: Wrong number of args passed to keyword: :kw
{code}

compare to calling an {{IFn}}, which does show the number of arguments passed:

{code}
(name "one" "two" "three")
=> clojure.lang.ArityException: Wrong number of args (3) passed to: core/name
{code}

The latter error message is more clear and makes it easier to debug.

The attached patch re-uses the {{ArityException}} class used elsewhere to generate error messages in the latter form when calling a keyword with the wrong number of arguments.

Addresses CLJ-1067

*Patch:* keyword-arity-exception-02.patch

*Prescreened by:*
Alex Miller made changes -
Approval Triaged [ 10120 ] Prescreened [ 10220 ]
Description When you call a {{Keyword}} with the wrong number of arguments, the error message does not report how many arguments were passed:

{code}
(:kw "one" "two" "three")
=> java.lang.IllegalArgumentException: Wrong number of args passed to keyword: :kw
{code}

compare to calling an {{IFn}}, which does show the number of arguments passed:

{code}
(name "one" "two" "three")
=> clojure.lang.ArityException: Wrong number of args (3) passed to: core/name
{code}

The latter error message is more clear and makes it easier to debug.

The attached patch re-uses the {{ArityException}} class used elsewhere to generate error messages in the latter form when calling a keyword with the wrong number of arguments.

Addresses CLJ-1067

*Patch:* keyword-arity-exception-02.patch

*Prescreened by:*
When you call a {{Keyword}} with the wrong number of arguments, the error message does not report how many arguments were passed:

{code}
(:kw "one" "two" "three")
=> java.lang.IllegalArgumentException: Wrong number of args passed to keyword: :kw
{code}

compare to calling an {{IFn}}, which does show the number of arguments passed:

{code}
(name "one" "two" "three")
=> clojure.lang.ArityException: Wrong number of args (3) passed to: core/name
{code}

The latter error message is more clear and makes it easier to debug.

The attached patch re-uses the {{ArityException}} class used elsewhere to generate error messages in the latter form when calling a keyword with the wrong number of arguments.

Addresses CLJ-1067

*Patch:* keyword-arity-exception-02.patch

*Prescreened by:* Alex Miller
Rich Hickey made changes -
Fix Version/s Release 1.10 [ 11451 ]
Approval Prescreened [ 10220 ] Vetted [ 10003 ]
Hide
Alex Miller added a comment -

The final invoke takes a variadic as the final arg so it's actually 21+, not 21. Could count the args array. The patch has also drifted from master and needs to be rebased.

Show
Alex Miller added a comment - The final invoke takes a variadic as the final arg so it's actually 21+, not 21. Could count the args array. The patch has also drifted from master and needs to be rebased.
Alex Miller made changes -
Approval Vetted [ 10003 ] Incomplete [ 10006 ]
Hide
Marc O'Morain added a comment -

Added patch that improves the message for the 21+ arg case.

Show
Marc O'Morain added a comment - Added patch that improves the message for the 21+ arg case.
Marc O'Morain made changes -
Attachment keyword-arity-exception-03.patch [ 18291 ]
Marc O'Morain made changes -
Description When you call a {{Keyword}} with the wrong number of arguments, the error message does not report how many arguments were passed:

{code}
(:kw "one" "two" "three")
=> java.lang.IllegalArgumentException: Wrong number of args passed to keyword: :kw
{code}

compare to calling an {{IFn}}, which does show the number of arguments passed:

{code}
(name "one" "two" "three")
=> clojure.lang.ArityException: Wrong number of args (3) passed to: core/name
{code}

The latter error message is more clear and makes it easier to debug.

The attached patch re-uses the {{ArityException}} class used elsewhere to generate error messages in the latter form when calling a keyword with the wrong number of arguments.

Addresses CLJ-1067

*Patch:* keyword-arity-exception-02.patch

*Prescreened by:* Alex Miller
When you call a {{Keyword}} with the wrong number of arguments, the error message does not report how many arguments were passed:

{code}
(:kw "one" "two" "three")
=> java.lang.IllegalArgumentException: Wrong number of args passed to keyword: :kw
{code}

compare to calling an {{IFn}}, which does show the number of arguments passed:

{code}
(name "one" "two" "three")
=> clojure.lang.ArityException: Wrong number of args (3) passed to: core/name
{code}

The latter error message is more clear and makes it easier to debug.

The attached patch re-uses the {{ArityException}} class used elsewhere to generate error messages in the latter form when calling a keyword with the wrong number of arguments.

Addresses CLJ-1067

*Patch:* keyword-arity-exception-03.patch

*Prescreened by:* Alex Miller
Hide
Marc O'Morain added a comment - - edited
Show
Marc O'Morain added a comment - - edited There are two more places in the code that have the same bug in the 21+ arg case: https://github.com/clojure/clojure/blob/71511b7800e18c83377a322f43585a853b303698/src/jvm/clojure/lang/RestFn.java#L4078 https://github.com/clojure/clojure/blob/71511b7800e18c83377a322f43585a853b303698/src/jvm/clojure/lang/AFn.java#L140 Would you like an additional patch to cover those cases?
Hide
Alex Miller added a comment -

Sure, separate ticket though.

Show
Alex Miller added a comment - Sure, separate ticket though.
Hide
Marc O'Morain added a comment -

Hi Alex, any update on this ticket? I've updated the patch to the latest master as requested, so it should be good to merge now.

Show
Marc O'Morain added a comment - Hi Alex, any update on this ticket? I've updated the patch to the latest master as requested, so it should be good to merge now.
Hide
Alex Miller added a comment -

I’ll get to it next time I cycle through.

Show
Alex Miller added a comment - I’ll get to it next time I cycle through.

People

Vote (1)
Watch (1)

Dates

  • Created:
    Updated: