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: None
  • Component/s: None
  • Labels:
  • Patch:
    Code and Test
  • Approval:
    Prescreened

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-02.patch

Prescreened by: Alex Miller

Activity

Ascending order - Click to sort in descending order
Hide
Permalink
Marc O'Morain added a comment -

Fix typo: later => latter.

Show
Marc O'Morain added a comment - Fix typo: later => latter.
Hide
Permalink
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
Permalink
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().
Hide
Permalink
Marc O'Morain added a comment -

Attach updated patch.

Show
Marc O'Morain added a comment - Attach updated patch.
Hide
Permalink
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
Permalink
Alex Miller added a comment -

yes and yes

Show
Alex Miller added a comment - yes and yes
Hide
Permalink
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.

People

Vote (0)
Watch (1)

Dates

  • Created:
    Updated:
Atlassian JIRA (v4.4#649-r158309) | Report a problem
Powered by a free Atlassian JIRA open source license for Clojure. Try JIRA - bug tracking software for your team.