Fix handling of primitive value class arguments in eq matcher and argumentCaptor.

Author: m-koops

mockito-kotlin/src/main/kotlin/org/mockito/kotlin/ArgumentCaptor.kt

@@ -25,43 +25,45 @@
 
 package org.mockito.kotlin
 
-import org.mockito.kotlin.internal.createInstance
 import org.mockito.ArgumentCaptor
+import org.mockito.kotlin.internal.createInstance
 import java.lang.reflect.Array
 import kotlin.reflect.KClass
+import kotlin.reflect.KType
+import kotlin.reflect.typeOf
 
 /**
  * Creates a [KArgumentCaptor] for given type.
  */
-inline fun <reified T : Any> argumentCaptor(): KArgumentCaptor<T> {
-    return KArgumentCaptor(T::class)
+inline fun <reified T : Any?> argumentCaptor(): KArgumentCaptor<T> {
+    return KArgumentCaptor(typeOf<T>())
 }
 
 /**
  * Creates 2 [KArgumentCaptor]s for given types.
  */
 inline fun <reified A : Any, reified B : Any> argumentCaptor(
-    a: KClass<A> = A::class,
-    b: KClass<B> = B::class
+    @Suppress("unused") a: KClass<A> = A::class,
+    @Suppress("unused") b: KClass<B> = B::class
 ): Pair<KArgumentCaptor<A>, KArgumentCaptor<B>> {
     return Pair(
-        KArgumentCaptor(a),
-        KArgumentCaptor(b)
+        KArgumentCaptor(typeOf<A>()),
+        KArgumentCaptor(typeOf<B>())
     )
 }
 
 /**
  * Creates 3 [KArgumentCaptor]s for given types.
  */
 inline fun <reified A : Any, reified B : Any, reified C : Any> argumentCaptor(
-    a: KClass<A> = A::class,
-    b: KClass<B> = B::class,
-    c: KClass<C> = C::class
+    @Suppress("unused") a: KClass<A> = A::class,
+    @Suppress("unused") b: KClass<B> = B::class,
+    @Suppress("unused") c: KClass<C> = C::class
 ): Triple<KArgumentCaptor<A>, KArgumentCaptor<B>, KArgumentCaptor<C>> {
     return Triple(
-        KArgumentCaptor(a),
-        KArgumentCaptor(b),
-        KArgumentCaptor(c)
+        KArgumentCaptor(typeOf<A>()),
+        KArgumentCaptor(typeOf<B>()),
+        KArgumentCaptor(typeOf<C>())
     )
 }
 
@@ -97,35 +99,35 @@ class ArgumentCaptorHolder5<out A, out B, out C, out D, out E>(
  * Creates 4 [KArgumentCaptor]s for given types.
  */
 inline fun <reified A : Any, reified B : Any, reified C : Any, reified D : Any> argumentCaptor(
-    a: KClass<A> = A::class,
-    b: KClass<B> = B::class,
-    c: KClass<C> = C::class,
-    d: KClass<D> = D::class
+    @Suppress("unused") a: KClass<A> = A::class,
+    @Suppress("unused") b: KClass<B> = B::class,
+    @Suppress("unused") c: KClass<C> = C::class,
+    @Suppress("unused") d: KClass<D> = D::class
 ): ArgumentCaptorHolder4<KArgumentCaptor<A>, KArgumentCaptor<B>, KArgumentCaptor<C>, KArgumentCaptor<D>> {
     return ArgumentCaptorHolder4(
-        KArgumentCaptor(a),
-        KArgumentCaptor(b),
-        KArgumentCaptor(c),
-        KArgumentCaptor(d)
+        KArgumentCaptor(typeOf<A>()),
+        KArgumentCaptor(typeOf<B>()),
+        KArgumentCaptor(typeOf<C>()),
+        KArgumentCaptor(typeOf<D>())
     )
 }
 
 /**
  * Creates 4 [KArgumentCaptor]s for given types.
  */
 inline fun <reified A : Any, reified B : Any, reified C : Any, reified D : Any, reified E : Any> argumentCaptor(
-    a: KClass<A> = A::class,
-    b: KClass<B> = B::class,
-    c: KClass<C> = C::class,
-    d: KClass<D> = D::class,
-    e: KClass<E> = E::class
+    @Suppress("unused") a: KClass<A> = A::class,
+    @Suppress("unused") b: KClass<B> = B::class,
+    @Suppress("unused") c: KClass<C> = C::class,
+    @Suppress("unused") d: KClass<D> = D::class,
+    @Suppress("unused") e: KClass<E> = E::class
 ): ArgumentCaptorHolder5<KArgumentCaptor<A>, KArgumentCaptor<B>, KArgumentCaptor<C>, KArgumentCaptor<D>, KArgumentCaptor<E>> {
     return ArgumentCaptorHolder5(
-        KArgumentCaptor(a),
-        KArgumentCaptor(b),
-        KArgumentCaptor(c),
-        KArgumentCaptor(d),
-        KArgumentCaptor(e)
+        KArgumentCaptor(typeOf<A>()),
+        KArgumentCaptor(typeOf<B>()),
+        KArgumentCaptor(typeOf<C>()),
+        KArgumentCaptor(typeOf<D>()),
+        KArgumentCaptor(typeOf<E>())
     )
 }
 
@@ -140,7 +142,7 @@ inline fun <reified T : Any> argumentCaptor(f: KArgumentCaptor<T>.() -> Unit): K
  * Creates a [KArgumentCaptor] for given nullable type.
  */
 inline fun <reified T : Any> nullableArgumentCaptor(): KArgumentCaptor<T?> {
-    return KArgumentCaptor(T::class)
+    return KArgumentCaptor(typeOf<T>())
 }
 
 /**
@@ -157,17 +159,17 @@ inline fun <reified T : Any> capture(captor: ArgumentCaptor<T>): T {
     return captor.capture() ?: createInstance()
 }
 
-class KArgumentCaptor<out T : Any?> (
-    private val tClass: KClass<*>
-) {
+class KArgumentCaptor<out T : Any?>(private val kType: KType) {
+    private val clazz = kType.classifier as KClass<*>
+
     private val captor: ArgumentCaptor<Any?> =
-        if (tClass.isValue) {
+        if (clazz.isValue && !kType.isMarkedNullable) {
             val boxImpl =
-                tClass.java.declaredMethods
+                clazz.java.declaredMethods
                     .single { it.name == "box-impl" && it.parameterCount == 1 }
             boxImpl.parameters[0].type // is the boxed type of the value type
         } else {
-            tClass.java
+            clazz.java
         }.let {
             ArgumentCaptor.forClass(it)
         }
@@ -219,27 +221,29 @@ class KArgumentCaptor<out T : Any?> (
         // In Java, `captor.capture` returns null and so the method is called with `[null]`
         // In Kotlin, we have to create `[null]` explicitly.
         // This code-path is applied for non-vararg array arguments as well, but it seems to work fine.
-        return captor.capture() as T ?: if (tClass.java.isArray) {
+        return toKotlinType(captor.capture()) ?: if (clazz.java.isArray) {
             singleElementArray()
         } else {
-            createInstance(tClass)
+            createInstance(clazz)
         } as T
     }
 
-    private fun singleElementArray(): Any? = Array.newInstance(tClass.java.componentType, 1)
+    private fun singleElementArray(): Any? = Array.newInstance(clazz.java.componentType, 1)
 
     @Suppress("UNCHECKED_CAST")
-    private fun toKotlinType(rawCapturedValue: Any?) : T {
-        return if(tClass.isValue) {
-            rawCapturedValue
-                ?.let {
+    private fun toKotlinType(rawCapturedValue: Any?): T {
+        if (rawCapturedValue == null) return null as T
+
+        if (clazz.isValue && rawCapturedValue::class != clazz) {
+            return rawCapturedValue
+                .let {
                     val boxImpl =
-                        tClass.java.declaredMethods.single { it.name == "box-impl" && it.parameterCount == 1 }
+                        clazz.java.declaredMethods.single { it.name == "box-impl" && it.parameterCount == 1 }
                     boxImpl.invoke(null, it)
                 } as T
-        } else {
-            rawCapturedValue as T
         }
+
+        return rawCapturedValue as T
     }
 }
 

mockito-kotlin/src/main/kotlin/org/mockito/kotlin/Matchers.kt

@@ -29,6 +29,7 @@ import org.mockito.ArgumentMatcher
 import org.mockito.ArgumentMatchers
 import org.mockito.kotlin.internal.createInstance
 import kotlin.reflect.KClass
+import kotlin.reflect.typeOf
 
 /** Object argument that is equal to the given value. */
 inline fun <reified T : Any?> eq(value: T): T {
@@ -91,11 +92,17 @@ inline fun <reified T > anyValueClass(): T {
     return boxImpl.invoke(null, ArgumentMatchers.any(boxedType)) as T
 }
 
-inline fun <reified T > eqValueClass(value: T): T {
+inline fun <reified T> eqValueClass(value: T): T {
     require(T::class.isValue) {
         "${T::class.qualifiedName} is not a value class."
     }
 
+    if (typeOf<T>().isMarkedNullable) {
+        // if the value is both value class and nullable, then Kotlin passes the value class boxed
+        // towards Mockito java code.
+        return ArgumentMatchers.eq(value)
+    }
+
     val unboxImpl =
         T::class.java.declaredMethods
             .single { it.name == "unbox-impl" && it.parameterCount == 0 }

tests/src/test/kotlin/test/ArgumentCaptorTest.kt

@@ -2,9 +2,16 @@ package test
 
 import com.nhaarman.expect.expect
 import com.nhaarman.expect.expectErrorWithMessage
-import org.junit.Ignore
 import org.junit.Test
-import org.mockito.kotlin.*
+import org.mockito.kotlin.KArgumentCaptor
+import org.mockito.kotlin.any
+import org.mockito.kotlin.argumentCaptor
+import org.mockito.kotlin.doNothing
+import org.mockito.kotlin.mock
+import org.mockito.kotlin.nullableArgumentCaptor
+import org.mockito.kotlin.times
+import org.mockito.kotlin.verify
+import org.mockito.kotlin.whenever
 import java.util.*
 
 class ArgumentCaptorTest : TestBase() {
@@ -385,7 +392,6 @@ class ArgumentCaptorTest : TestBase() {
     }
 
     @Test
-    @Ignore("See issue #555")
     fun argumentCaptor_primitive_value_class() {
         /* Given */
         val m: SynchronousFunctions = mock()
@@ -401,7 +407,6 @@ class ArgumentCaptorTest : TestBase() {
     }
 
     @Test
-    @Ignore("See issue #555")
     fun argumentCaptor_nullable_primitive_value_class() {
         /* Given */
         val m: SynchronousFunctions = mock()
@@ -411,7 +416,7 @@ class ArgumentCaptorTest : TestBase() {
         m.nullablePrimitiveValueClass(valueClass)
 
         /* Then */
-        val captor = argumentCaptor<PrimitiveValueClass>()
+        val captor = argumentCaptor<PrimitiveValueClass?>()
         verify(m).nullablePrimitiveValueClass(captor.capture())
         expect(captor.firstValue).toBe(valueClass)
     }

tests/src/test/kotlin/test/Classes.kt

@@ -98,11 +98,13 @@ interface SynchronousFunctions {
     fun valueClass(v: ValueClass)
     fun nullableValueClass(v: ValueClass?)
     fun nestedValueClass(v: NestedValueClass)
+    fun primitiveValueClass(v: PrimitiveValueClass)
+    fun nullablePrimitiveValueClass(v: PrimitiveValueClass?)
     fun valueClassResult(): ValueClass
     fun nullableValueClassResult(): ValueClass?
     fun nestedValueClassResult(): NestedValueClass
-    fun primitiveValueClass(v: PrimitiveValueClass)
-    fun nullablePrimitiveValueClass(v: PrimitiveValueClass?)
+    fun primitiveValueClassResult(): PrimitiveValueClass
+    fun nullablePrimitiveValueClassResult(): PrimitiveValueClass?
 }
 
 interface SuspendFunctions {
@@ -118,6 +120,8 @@ interface SuspendFunctions {
     suspend fun valueClassResult(): ValueClass
     suspend fun nullableValueClassResult(): ValueClass?
     suspend fun nestedValueClassResult(): NestedValueClass
+    suspend fun primitiveValueClassResult(): PrimitiveValueClass
+    suspend fun nullablePrimitiveValueClassResult(): PrimitiveValueClass?
     suspend fun builderMethod(): SuspendFunctions
 }
 

tests/src/test/kotlin/test/CoroutinesOngoingStubbingTest.kt

@@ -337,4 +337,40 @@ class CoroutinesOngoingStubbingTest {
         expect(result).toBe(nestedValueClass)
         expect(result.value).toBe(nestedValueClass.value)
     }
+
+    @Test
+    fun `should stub suspendable function call with primitive value class result`() = runTest {
+        /* Given */
+        val primitiveValueClass = PrimitiveValueClass(42)
+        val mock = mock<SuspendFunctions> {
+            on(mock.primitiveValueClassResult()) doSuspendableAnswer {
+                delay(1)
+                primitiveValueClass
+            }
+        }
+
+        /* When */
+        val result: PrimitiveValueClass = mock.primitiveValueClassResult()
+
+        /* Then */
+        expect(result).toBe(primitiveValueClass)
+    }
+
+    @Test
+    fun `should stub suspendable function call with nullable primitive value class result`() = runTest {
+        /* Given */
+        val primitiveValueClass = PrimitiveValueClass(42)
+        val mock = mock<SuspendFunctions> {
+            on (mock.nullablePrimitiveValueClassResult()) doSuspendableAnswer {
+                delay(1)
+                primitiveValueClass
+            }
+        }
+
+        /* When */
+        val result: PrimitiveValueClass? = mock.nullablePrimitiveValueClassResult()
+
+        /* Then */
+        expect(result).toBe(primitiveValueClass)
+    }
 }

tests/src/test/kotlin/test/MatchersTest.kt

@@ -3,7 +3,6 @@ package test
 import com.nhaarman.expect.expect
 import com.nhaarman.expect.expectErrorWithMessage
 import kotlinx.coroutines.test.runTest
-import org.junit.Ignore
 import org.junit.Test
 import org.junit.experimental.runners.Enclosed
 import org.junit.runner.RunWith
@@ -629,12 +628,20 @@ class MatchersTest : TestBase() {
         }
 
         @Test
-        @Ignore("See issue #555")
+        fun eqPrimitiveValueClass() {
+            val primitiveValueClass = PrimitiveValueClass(123)
+            mock<SynchronousFunctions>().apply {
+                primitiveValueClass(primitiveValueClass)
+                verify(this).primitiveValueClass(eq(primitiveValueClass))
+            }
+        }
+
+        @Test
         fun eqNullablePrimitiveValueClass() {
-            val valueClass = PrimitiveValueClass(123)
+            val primitiveValueClass = PrimitiveValueClass(123) as PrimitiveValueClass?
             mock<SynchronousFunctions>().apply {
-                nullablePrimitiveValueClass(valueClass)
-                verify(this).nullablePrimitiveValueClass(eq(valueClass))
+                nullablePrimitiveValueClass(primitiveValueClass)
+                verify(this).nullablePrimitiveValueClass(eq(primitiveValueClass))
             }
         }
 

tests/src/test/kotlin/test/OngoingStubbingTest.kt

@@ -317,6 +317,36 @@ class OngoingStubbingTest : TestBase() {
         expect(result.value).toBe(nestedValueClass.value)
     }
 
+    @Test
+    fun `should stub function call with primitive value class result`() {
+        /* Given */
+        val primitiveValueClass = PrimitiveValueClass(42)
+        val mock = mock<SynchronousFunctions> {
+            on { primitiveValueClassResult() } doReturn primitiveValueClass
+        }
+
+        /* When */
+        val result: PrimitiveValueClass = mock.primitiveValueClassResult()
+
+        /* Then */
+        expect(result).toBe(primitiveValueClass)
+    }
+
+    @Test
+    fun `should stub function call with nullable primitive value class result`() {
+        /* Given */
+        val primitiveValueClass = PrimitiveValueClass(42)
+        val mock = mock<SynchronousFunctions> {
+            on { nullablePrimitiveValueClassResult() } doReturn primitiveValueClass
+        }
+
+        /* When */
+        val result: PrimitiveValueClass? = mock.nullablePrimitiveValueClassResult()
+
+        /* Then */
+        expect(result).toBe(primitiveValueClass)
+    }
+
     @Test
     fun `should stub consecutive function calls with value class results`() {
         /* Given */