Report argument number mismatch nicely on wrong 'call()' invocation

The default Java reflection message in such case is not very helpful: "wrong
number of arguments"

compiler/testData/codegen/boxWithStdlib/reflection/call/incorrectNumberOfArguments.kt

@@ -0,0 +1,98 @@
+import kotlin.platform.platformStatic as static
+import kotlin.reflect.jvm.isAccessible
+import kotlin.reflect.KCallable
+
+var foo: String = ""
+
+class A(private var bar: String = "") {
+    fun getBar() = A::bar
+}
+
+object O {
+    private static var baz: String = ""
+
+    static fun getBaz() = O::baz.apply { isAccessible = true }
+}
+
+fun check(callable: KCallable<*>, vararg args: Any?) {
+    val expected = callable.parameters.size()
+    val actual = args.size()
+
+    if (expected == actual) {
+        throw AssertionError("Bad test case: expected and actual number of arguments should differ (was $expected vs $actual)")
+    }
+
+    val expectedExceptionMessage = "Callable expects $expected arguments, but $actual were provided."
+
+    try {
+        callable.call(*args)
+        throw AssertionError("Fail: an IllegalArgumentException should have been thrown")
+    } catch (e: IllegalArgumentException) {
+        if (e.getMessage() != expectedExceptionMessage) {
+            // This most probably means that we don't check number of passed arguments in reflection
+            // and the default check from Java reflection yields an IllegalArgumentException, but with a not that helpful message
+            throw AssertionError("Fail: an exception with an unrecognized message was thrown: \"${e.getMessage()}\"" +
+                                 "\nExpected message was: $expectedExceptionMessage")
+        }
+    }
+}
+
+fun box(): String {
+    check(::box, null)
+    check(::box, "")
+
+    check(::A)
+    check(::A, null, "")
+
+    check(O::getBaz)
+    check(O::getBaz, null, "")
+
+
+    val f = ::foo
+    check(f, null)
+    check(f, null, null)
+    check(f, arrayOf<Any?>(null))
+    check(f, "")
+
+    check(f.getter, null)
+    check(f.getter, null, null)
+    check(f.getter, arrayOf<Any?>(null))
+    check(f.getter, "")
+
+    check(f.setter)
+    check(f.setter, null, null)
+    check(f.setter, null, "")
+
+
+    val b = A().getBar()
+
+    check(b)
+    check(b, null, null)
+    check(b, "", "")
+
+    check(b.getter)
+    check(b.getter, null, null)
+    check(b.getter, "", "")
+
+    check(b.setter)
+    check(b.setter, null)
+    check(b.setter, "")
+
+
+    val z = O.getBaz()
+
+    check(z)
+    check(z, null, null)
+    check(z, "", "")
+
+    check(z.getter)
+    check(z.getter, null, null)
+    check(z.getter, "", "")
+
+    check(z.setter)
+    check(z.setter, null)
+    check(z.setter, "")
+
+
+    return "OK"
+}

compiler/tests/org/jetbrains/kotlin/codegen/generated/BlackBoxWithStdlibCodegenTestGenerated.java

@@ -2856,6 +2856,12 @@ public class BlackBoxWithStdlibCodegenTestGenerated extends AbstractBlackBoxCode
                 doTestWithStdlib(fileName);
             }
 
+            @TestMetadata("incorrectNumberOfArguments.kt")
+            public void testIncorrectNumberOfArguments() throws Exception {
+                String fileName = JetTestUtils.navigationMetadata("compiler/testData/codegen/boxWithStdlib/reflection/call/incorrectNumberOfArguments.kt");
+                doTestWithStdlib(fileName);
+            }
+
             @TestMetadata("innerClassConstructor.kt")
             public void testInnerClassConstructor() throws Exception {
                 String fileName = JetTestUtils.navigationMetadata("compiler/testData/codegen/boxWithStdlib/reflection/call/innerClassConstructor.kt");

core/reflection.jvm/src/kotlin/reflect/jvm/internal/FunctionCaller.kt

@@ -19,28 +19,40 @@ package kotlin.reflect.jvm.internal
 import java.lang.reflect.Member
 import java.lang.reflect.Constructor as ReflectConstructor
 import java.lang.reflect.Field as ReflectField
-import java.lang.reflect.Member as ReflectMember
 import java.lang.reflect.Method as ReflectMethod
 
-internal sealed class FunctionCaller {
+internal sealed class FunctionCaller(
-    abstract val member: ReflectMember
+        private val expectedArgumentNumber: Int
+) {
+    abstract val member: Member
 
     abstract fun call(args: Array<*>): Any?
 
+    protected open fun checkArguments(args: Array<*>) {
+        if (expectedArgumentNumber != args.size()) {
+            throw IllegalArgumentException("Callable expects $expectedArgumentNumber arguments, but ${args.size()} were provided.")
+        }
+    }
+
     protected fun checkObjectInstance(obj: Any?) {
         if (obj == null || !member.declaringClass.isInstance(obj)) {
             throw IllegalArgumentException("An object member requires the object instance passed as the first argument.")
         }
     }
 
-    class Constructor(val constructor: ReflectConstructor<*>) : FunctionCaller() {
+    class Constructor(val constructor: ReflectConstructor<*>) : FunctionCaller(constructor.parameterTypes.size()) {
         override val member: Member get() = constructor
 
-        override fun call(args: Array<*>): Any? =
+        override fun call(args: Array<*>): Any? {
-                constructor.newInstance(*args)
+            checkArguments(args)
+            return constructor.newInstance(*args)
+        }
     }
 
-    abstract class Method(val method: ReflectMethod) : FunctionCaller() {
+    abstract class Method(
+            val method: ReflectMethod,
+            requiresInstance: Boolean
+    ) : FunctionCaller(method.parameterTypes.size() + (if (requiresInstance) 1 else 0)) {
         override val member: Member get() = method
         private val isVoidMethod = method.returnType == Void.TYPE
 
@@ -52,58 +64,71 @@ internal sealed class FunctionCaller {
         }
     }
 
-    class StaticMethod(method: ReflectMethod) : Method(method) {
+    class StaticMethod(method: ReflectMethod) : Method(method, requiresInstance = false) {
-        override fun call(args: Array<*>): Any? =
-                callMethod(null, args)
-    }
-
-    class InstanceMethod(method: ReflectMethod) : Method(method) {
-        override fun call(args: Array<*>): Any? =
-                callMethod(args[0], args.asList().subList(1, args.size()).toTypedArray())
-    }
-
-    class PlatformStaticInObject(method: ReflectMethod) : Method(method) {
         override fun call(args: Array<*>): Any? {
+            checkArguments(args)
+            return callMethod(null, args)
+        }
+    }
+
+    class InstanceMethod(method: ReflectMethod) : Method(method, requiresInstance = true) {
+        override fun call(args: Array<*>): Any? {
+            checkArguments(args)
+            return callMethod(args[0], args.asList().subList(1, args.size()).toTypedArray())
+        }
+    }
+
+    class PlatformStaticInObject(method: ReflectMethod) : Method(method, requiresInstance = true) {
+        override fun call(args: Array<*>): Any? {
+            checkArguments(args)
             checkObjectInstance(args.firstOrNull())
             return callMethod(null, args.asList().subList(1, args.size()).toTypedArray())
         }
     }
 
-    abstract class Field(val field: ReflectField) : FunctionCaller() {
+    abstract class FieldAccessor(val field: ReflectField, expectedArgumentNumber: Int) : FunctionCaller(expectedArgumentNumber) {
         override val member: Member get() = field
     }
 
-    class StaticFieldGetter(field: ReflectField) : Field(field) {
+    abstract class FieldGetter(
-        override fun call(args: Array<*>): Any? =
+            field: ReflectField,
-                field.get(null)
+            private val requiresInstance: Boolean
-    }
+    ) : FieldAccessor(field, if (requiresInstance) 1 else 0) {
-
-    class InstanceFieldGetter(field: ReflectField) : Field(field) {
-        override fun call(args: Array<*>): Any? =
-                field.get(args[0])
-    }
-
-    class PlatformStaticInObjectFieldGetter(field: ReflectField) : Field(field) {
         override fun call(args: Array<*>): Any? {
-            checkObjectInstance(args.firstOrNull())
+            checkArguments(args)
-            return field.get(null)
+            return field.get(if (requiresInstance) args.first() else null)
         }
     }
 
-    class StaticFieldSetter(field: ReflectField) : Field(field) {
+    abstract class FieldSetter(
-        override fun call(args: Array<*>): Any? =
+            field: ReflectField,
-                field.set(null, args[0])
+            private val requiresInstance: Boolean
-    }
+    ) : FieldAccessor(field, if (requiresInstance) 2 else 1) {
-
-    class InstanceFieldSetter(field: ReflectField) : Field(field) {
-        override fun call(args: Array<*>): Any? =
-                field.set(args[0], args[1])
-    }
-
-    class PlatformStaticInObjectFieldSetter(field: ReflectField) : Field(field) {
         override fun call(args: Array<*>): Any? {
+            checkArguments(args)
+            return field.set(if (requiresInstance) args.first() else null, args.last())
+        }
+    }
+
+    class StaticFieldGetter(field: ReflectField) : FieldGetter(field, requiresInstance = false)
+
+    class InstanceFieldGetter(field: ReflectField) : FieldGetter(field, requiresInstance = true)
+
+    class PlatformStaticInObjectFieldGetter(field: ReflectField) : FieldGetter(field, requiresInstance = true) {
+        override fun checkArguments(args: Array<*>) {
+            super.checkArguments(args)
+            checkObjectInstance(args.firstOrNull())
+        }
+    }
+
+    class StaticFieldSetter(field: ReflectField) : FieldSetter(field, requiresInstance = false)
+
+    class InstanceFieldSetter(field: ReflectField) : FieldSetter(field, requiresInstance = true)
+
+    class PlatformStaticInObjectFieldSetter(field: ReflectField) : FieldSetter(field, requiresInstance = true) {
+        override fun checkArguments(args: Array<*>) {
+            super.checkArguments(args)
             checkObjectInstance(args.firstOrNull())
-            return field.set(null, args[1])
         }
     }
 }

core/reflection.jvm/src/kotlin/reflect/jvm/internal/KPropertyImpl.kt

@@ -81,7 +81,7 @@ private fun KPropertyImpl.Accessor<*>.computeCallerForAccessor(isGetter: Boolean
     fun isPlatformStaticProperty() =
             property.descriptor.annotations.findAnnotation(PLATFORM_STATIC) != null
 
-    fun computeFieldCaller(field: Field): FunctionCaller.Field = when {
+    fun computeFieldCaller(field: Field): FunctionCaller.FieldAccessor = when {
         !Modifier.isStatic(field.modifiers) ->
             if (isGetter) FunctionCaller.InstanceFieldGetter(field)
             else FunctionCaller.InstanceFieldSetter(field)