`eq()` matcher throws NPE when used with nullable value class and null value

[KonaEspresso94]

Description

After upgrading from 6.1.0 to 6.2.0, eq() matcher throws NullPointerException when:

• The parameter type is a nullable value class (e.g., MyValueClass?)
• The actual value is null

This is a regression introduced in #545.

Reproduction

@JvmInline
value class MyValueClass(val value: Long)

interface Foo {
    fun bar(x: MyValueClass?)
}

@Test
fun `eq with nullable value class and null value`() {
    val foo = mock<Foo>()
    foo.bar(null)
    
    verify(foo).bar(eq(null))  // Throws NPE in 6.2.0+
}

Error

java.lang.NullPointerException: Cannot invoke "Object.getClass()" because "value" is null
    at org.mockito.kotlin.MatchersKt.eqValueClass(Matchers.kt:91)
    at org.mockito.kotlin.MatchersKt.eq(Matchers.kt:40)

Affected Versions

Version	Status
6.1.0	✅ Works
6.2.0	❌ Broken
6.2.1	❌ Broken

Root Cause

In Matchers.kt, the eq() function checks T::class.isValue before checking if value is null:

inline fun <reified T : Any?> eq(value: T): T {
    if (T::class.isValue) return eqValueClass(value)  // Enters this branch even when value is null
    // ...
}

When T = MyValueClass? and value = null:

1. T::class.isValue returns true (compile-time type is a value class)
2. eqValueClass(null) is called
3. require(value::class.isValue) throws NPE because null::class is invalid

[nuhkoca]

@TimvdLippe this is still reproducible in 6.2.2. Seems #579 didn't fix it properly

[TimvdLippe]

@m-koops FYI

[m-koops]

@nuhkoca can you elleborate on your claim that the isseu still exists in 6.2.2? Are you sure you're testing with 6.2.2?

The tests in

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

Lines 676 to 692 in 4d1b211

	@Test
	fun eqNullableValueClass_nullValue() {
	val valueClass = null as ValueClass?
	mock<SynchronousFunctions>().apply {
	nullableValueClass(valueClass)
	verify(this).nullableValueClass(eq(valueClass))
	}
	}
	@Test
	fun eqNullableLongValueClass_nullValue() {
	val longValueClass = null as LongValueClass?
	mock<SynchronousFunctions>().apply {
	nullableLongValueClass(longValueClass)
	verify(this).nullableLongValueClass(eq(longValueClass))
	}
	}

were directly inspired on the reproduction scenario shared by @KonaEspresso94.
These new tests and the main line change to fix the NPE are incorporated in release 6.2.2 through PR #579.

To be sure, I've just locally copied the reproduction scenario of @KonaEspresso94 as-is into MatchersTest.kt on main branch and the test passes (as expected).

[nuhkoca]

Hey @m-koops, I can share couple of screenshots. Sorry codebase is too large I couldn't create a quick repro but let me know if you need more help.

[m-koops]

@nuhkoca Can you share some more details about this method call that you are trying to verify?
What is the signature of the errorhandler.handle() function? What is the type of the 3 arguments?

Still, a little code fragment reproducing the issue would rely help to fix it.

[nuhkoca]

@m-koops There are many examples but I am sharing two of them with you. Btw when I replaced them to isNull, it then works

Still, a little code fragment reproducing the issue would rely help to fix it.

If you can't get the issue from screenshots, I'll try to create a repro if I can

[TimvdLippe]

If you can, please submit a PR with the failing test, so that we can debug that. Thanks!

[nuhkoca]

Hey @m-koops @TimvdLippe it turned out to be Anvil issue. Anvil modifies how the Kotlin compiler processes code. It appears to make type inference stricter, causing eq(null) to resolve to Nothing? instead of inferring the type from the method signature context. Any suggestion you have here?

[jselbo]

@nuhkoca I encountered the same issue when compiling code with the legacy K1 frontend, fixed by using K2. Based on the Anvil readme:

NOTE: Anvil is deprecated and will only be supported for K1 going forward, and development will end when K2 becomes a requirement

I'm not sure what the right solution for your case is.

[nuhkoca]

Yeah was surprised a bit 😂 Please feel free to close the issue, sorry for confusion.

[TimvdLippe]

Thanks for digging into it! Given that isNull() is a valid replacement (and arguably semantically more correct), let's close this issue. If others run into this issue, they can find this one and learn about the alternatives.