When unable to match a static method, report arity caller was looking for, avoid misleading field error

Description

Incorrectly invoking a static method with 0 parameters yields a NoSuchFieldException:

Approach: Error reporting enhanced to report desired arg count and to avoid a field exception confusion if 0 arg method.

user=> (Long/parseLong)
CompilerException java.lang.IllegalArgumentException: No matching method parseLong found taking 0 args for class java.lang.Long, compilingNO_SOURCE_PATH:1:1)
user=> (Long/parseLong "5" 10 3)
CompilerException java.lang.IllegalArgumentException: No matching method parseLong found taking 3 args for class java.lang.Long, compilingNO_SOURCE_PATH:2:1)

Patch: clj-1130-v5.diff

Screened by: Alex Miller

Environment

None

Activity

Show:
Alex Miller
December 7, 2013, 3:01 AM

This patch is intentionally trying to avoid calling StaticFieldExpr in the field code as that is where the (Long/parseLong) case (erroneously calling an n-arity static method with 0 args) will throw a field-oriented exception instead of a method-oriented exception. By adding the extra check here, this case falls through into the method case and throws later on calling StaticMethodExpr instead.

The early check is a check for methods of the specified arity. The later check is for the existence of a field of matching name. Combined, they lead to a better error message.

However, another alternative is to set maybeField in the first check based on field existence, not on invocation arity. That just improves the maybeField informaiton and the existing code then naturally throws the correct exception (and the patch is much simpler).

The similar case for calling n-arity instance methods with 0-arity has the same problem for the same reason:

Thus we can also adjust the other call that sets maybeField (which now is much less maybe).

I will attach a patch that covers these cases and update the ticket for someone to screen.

Stuart Sierra
December 8, 2013, 6:24 PM

Screened. The patch clj-1130-v3.diff works as advertised.

This patch only improves error messages for cases when the type of the
target object is known to the compiler. In reflective calls, the error
messages are still the same.

Example, after this patch, given these definitions:

The following expressions produce new error messages:

These expressions still use the old error messages:

Rich Hickey
January 3, 2014, 2:41 PM

Changing the logic to get a different error message is something that needs to be done with great care. This now seems to prefer fields over methods, changing the semantics.

Stuart Halloway
January 31, 2014, 9:12 PM

v4 patch simply enhances error messaages

Andy Fingerhut
January 31, 2014, 9:18 PM

clj-1130-v4.diff has the same patch repeated twice in the file. clj-1130-v5.diff is identical, except deleting the redundant copy.

Completed

Assignee

Unassigned

Reporter

Howard Lewis Ship

Labels

Approval

Ok

Patch

Code and Test

Fix versions

Affects versions

Priority

Major
Configure