mockito / mockito-kotlin

Using Mockito with Kotlin
MIT License
3.11k stars 202 forks source link

Mockito Kotlin is breaking with new type inference #302

Open vRallev opened 6 years ago

vRallev commented 6 years ago

I'm referring to https://youtu.be/MyljSWm0Y_k?t=1282 I turned it on this way

    tasks.withType(org.jetbrains.kotlin.gradle.tasks.KotlinCompile).all {
        kotlinOptions {
            // progressive helps detecting unstable Kotlin code earlier, see https://blog.jetbrains.com/kotlin/2018/06/kotlin-1-2-50-is-out/
            freeCompilerArgs = ["-Xjsr305=strict", "-progressive", "-XXLanguage:+NewInference", "-XXLanguage:+InlineClasses"]
            allWarningsAsErrors = true
        }
    }

The error is the following

e: /Users/rwondratschek/dev/projects/evernote/evernote/app/src/test/kotlin/com/evernote/EvernoteAppSetupRule.kt: (68, 64): Overload resolution ambiguity:
public inline fun <reified T : Any> mock(extraInterfaces: Array<KClass<out Any>>? = ..., name: String? = ..., spiedInstance: Any? = ..., defaultAnswer: Answer<Any>? = ..., serializable: Boolean = ..., serializableMode: SerializableMode? = ..., verboseLogging: Boolean = ..., invocationListeners: Array<InvocationListener>? = ..., stubOnly: Boolean = ..., @Incubating useConstructor: UseConstructor? = ..., @Incubating outerInstance: Any? = ...): TypeVariable(T) defined in com.nhaarman.mockitokotlin2
public inline fun <reified T : Any> mock(extraInterfaces: Array<KClass<out Any>>? = ..., name: String? = ..., spiedInstance: Any? = ..., defaultAnswer: Answer<Any>? = ..., serializable: Boolean = ..., serializableMode: SerializableMode? = ..., verboseLogging: Boolean = ..., invocationListeners: Array<InvocationListener>? = ..., stubOnly: Boolean = ..., @Incubating useConstructor: UseConstructor? = ..., @Incubating outerInstance: Any? = ..., stubbing: KStubbing<TypeVariable(T)>.(TypeVariable(T)) -> Unit): TypeVariable(T) defined in com.nhaarman.mockitokotlin2
@Deprecated public inline fun <reified T : Any> mock(a: Answer<Any>): TypeVariable(T) defined in com.nhaarman.mockitokotlin2

The issue should be easy to reproduce, basically every call like this fails

        val account = mock<AppAccount> {
            on { databaseHelper } doThrow IllegalStateException()
        }
reitzig commented 6 years ago

I have a similar issue.

verify(someBean).genericMethod(org.mockito.ArgumentMatchers.any()) 
// --> compiles
verify(someBean).genericMethod(com.nhaarman.mockitokotlin2.any())
 // --> Type inference failed: Not enough information to infer parameter T 

Comparing the type signatures:

public static <T> T any()  { ... }
inline fun <reified T : Any> any(): T { ... }

Not that much of a difference. It seems that inline is not ideal here?

(This is with Kotlin 1.2.71.)

wiyarmir commented 6 years ago

I've got a less cosmetic solution which is to specify the parameter name, so the overload resolution works.

val account = mock<AppAccount>(stubbing = {
// on {} etc...
})
nhaarman commented 6 years ago

This is something that breaks compilation of the library itself as well when -XXLanguage:+NewInference is enabled. Since there isn't any (at least I haven't found any) official documentation or discussion available on this "new type inference" system, it's hard to tell how the Kotlin team will handle issues like these.

One specific case in the library itself is argThat(T.() -> Boolean) and argThat(ArgumentMatcher), which results in the following compilation error:

e: /home/nhaarman/dev/nhaarman/mockito-kotlin/mockito-kotlin/src/main/kotlin/com/nhaarman/mockitokotlin2/Matchers.kt: (93, 12): Overload resolution ambiguity: 
public inline fun <reified T : Any> argThat(noinline predicate: TypeVariable(T).() -> Boolean): TypeVariable(T) defined in com.nhaarman.mockitokotlin2
public inline fun <reified T : Any> argThat(matcher: ArgumentMatcher<TypeVariable(T)>): TypeVariable(T) defined in com.nhaarman.mockitokotlin2

The simplest solution is to remove the custom functions that accept a lambda and let the new type inference system handle this. However, this is a breaking change and will affect users that don't have the new type inference system enabled. Since the -XXLanguage:+NewInference is an internal compiler argument, I'm favoring for not addressing this issue in 2.x.
I know it's a pain for people that do have this flag enabled, but stuff like this is expected for experimental features.

3.x might be able to support his, but for support for the new type inference to have any standing at least some official documentation and stance from the Kotlin team needs to be available, and the feature needs to be stable.

bohsen commented 5 years ago

@vRallev @reitzig @wiyarmir Is this still an issue with kotlin 1.3.40-eap-32 where the new type inference is enabled by default?

reitzig commented 5 years ago

I don't have access anymore to the codebase I observed this in. Now (Kotlin 1.3.40) I have trouble reproducing.

import com.nhaarman.mockitokotlin2.any
import com.nhaarman.mockitokotlin2.doReturn
import com.nhaarman.mockitokotlin2.mock
import com.nhaarman.mockitokotlin2.verify
import org.junit.Test

interface Mockable {
    fun <T> mockMe(m: T) : String
}

class Foo(val m: Mockable) {
    fun bar() : String = "${m.mockMe("me")}, Amadeus!"
}

internal class SomeTest {
    @Test
    fun testBar() {
        val mock = mock<Mockable> {
            on { mockMe(any<String>()) } doReturn "Mock me"
        }

        val foo = Foo(mock)
        println(foo.bar())
        verify(mock).mockMe(org.mockito.ArgumentMatchers.any())
        verify(mock).mockMe(com.nhaarman.mockitokotlin2.any())
    }
}

Both variants are red.

If the type parameter lives on interface, not method level, both are green:

interface Mockable<T> {
    fun mockMe(m: T) : String
}

class Foo(val m: Mockable<String>) {
    fun bar() : String = "${m.mockMe("me")}, Amadeus!"
}

internal class SomeTest {
    @Test
    fun testBar() {
        val mock = mock<Mockable<String>> {
            on { mockMe(any()) } doReturn "Mock me"
        }

        val foo = Foo(mock)
        println(foo.bar())
        verify(mock).mockMe(org.mockito.ArgumentMatchers.any())
        verify(mock).mockMe(com.nhaarman.mockitokotlin2.any())
    }
}