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

Attachments

7

Activity

Show:

Andy FingerhutJanuary 31, 2014 at 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.

Stuart HallowayJanuary 31, 2014 at 9:12 PM

v4 patch simply enhances error messaages

Rich HickeyJanuary 3, 2014 at 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 SierraDecember 8, 2013 at 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:

Alex MillerDecember 7, 2013 at 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.

Completed

Details

Assignee

Reporter

Approval

Ok

Patch

Code and Test

Priority

Affects versions

Fix versions

Created December 17, 2012 at 11:57 PM
Updated June 27, 2018 at 3:10 PM
Resolved June 27, 2018 at 3:10 PM