Skip to content

Commit

Permalink
Handling generic data classes when determining identifier stability
Browse files Browse the repository at this point in the history
By inspecting the stability of the actual values, we can see beyond the
generics of the data class.

Fixes #3947
  • Loading branch information
Kantis committed May 9, 2024
1 parent 3ab9358 commit ea65df0
Show file tree
Hide file tree
Showing 4 changed files with 66 additions and 11 deletions.
2 changes: 2 additions & 0 deletions kotest-common/api/kotest-common.api
Original file line number Diff line number Diff line change
Expand Up @@ -210,7 +210,9 @@ public final class io/kotest/mpp/ReplayKt {

public final class io/kotest/mpp/StableKt {
public static final fun isStable (Lkotlin/reflect/KClass;)Z
public static final fun isStable (Lkotlin/reflect/KClass;Ljava/lang/Object;)Z
public static final fun isStable (Lkotlin/reflect/KType;)Z
public static synthetic fun isStable$default (Lkotlin/reflect/KClass;Ljava/lang/Object;ILjava/lang/Object;)Z
}

public abstract interface class io/kotest/mpp/StackTraces {
Expand Down
41 changes: 33 additions & 8 deletions kotest-common/src/commonMain/kotlin/io/kotest/mpp/stable.kt
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,9 @@ import kotlin.reflect.KType
/**
* Returns true if the given type is a class and considered "stable".
*/
@Deprecated("Internal class, will not be exposed in the future. Deprecated since 5.9")
fun isStable(type: KType) = when (val classifier = type.classifier) {
is KClass<*> -> isStable(classifier)
is KClass<*> -> isStable(classifier, null)
else -> false
}

Expand All @@ -20,11 +21,24 @@ fun isStable(type: KType) = when (val classifier = type.classifier) {
* - a data class that only contains stable classes as members
* - a stable type on the executing platform
*/
fun isStable(kclass: KClass<*>): Boolean {
@Deprecated("Internal class, will not be exposed in the future. Deprecated since 5.9")
fun isStable(kclass: KClass<*>) = isStable(kclass, null)

/**
* Returns true if the given class is a "stable" class, ie one that has a consistent toString().
*
* A stable class is either:
* - a Kotlin or Java primitive type or a String
* - an enum class
* - a data class that only contains stable classes as members
* - a stable type on the executing platform
*/
@Deprecated("Internal class, will not be exposed in the future. Deprecated since 5.9")
fun isStable(kclass: KClass<*>, t: Any? = null): Boolean {
return when {
allPlatformStableTypes.contains(kclass) -> true
reflection.isEnumClass(kclass) -> true
reflection.isDataClass(kclass) && hasStableMembers(kclass) -> true
reflection.isDataClass(kclass) && hasStableMembers(kclass, t) -> true
isPlatformStable(kclass) -> true
else -> {
println("Warning, type $kclass used in data testing does not have a stable toString()")
Expand All @@ -33,14 +47,27 @@ fun isStable(kclass: KClass<*>): Boolean {
}
}

@Deprecated("Internal class, will not be exposed in the future. Deprecated since 5.9")
expect fun isPlatformStable(kclass: KClass<*>): Boolean

/**
* Returns true if all members of this class are "stable".
*/
private fun hasStableMembers(kclass: KClass<*>) = reflection.primaryConstructorMembers(kclass).let { members ->
members.isNotEmpty() && members.all { isStable(it.type) }
}
private fun hasStableMembers(kclass: KClass<*>, t: Any? = null) =
reflection.primaryConstructorMembers(kclass).let { members ->
members.isNotEmpty() && members.all { getter ->
val typeIsStable = isStable(getter.type)
if (t == null) {
typeIsStable
} else {
val valueIsStable = getter.call(t)?.let { memberValue ->
isStable(memberValue::class, memberValue)
} ?: false

typeIsStable || valueIsStable
}
}
}

private val allPlatformStableTypes = setOf(
String::class,
Expand All @@ -51,7 +78,5 @@ private val allPlatformStableTypes = setOf(
Byte::class,
Short::class,
Boolean::class,
Pair::class,
Triple::class,
Char::class,
)
30 changes: 28 additions & 2 deletions kotest-common/src/jvmTest/kotlin/io/kotest/mpp/StableTest.kt
Original file line number Diff line number Diff line change
@@ -1,9 +1,10 @@
package io.kotest.mpp

import io.kotest.core.spec.style.StringSpec
import io.kotest.core.spec.style.FreeSpec
import io.kotest.data.Row3
import io.kotest.matchers.shouldBe

class StableTest : StringSpec({
class StableTest : FreeSpec({
"class should be correctly identified as stable" {
data class StableClass(
val string: String,
Expand All @@ -29,4 +30,29 @@ class StableTest : StringSpec({
isStable(Unstable::class) shouldBe false
isStable(UnstableDataClass::class) shouldBe false
}

"Given a data class with generics" - {
"When all members are stable, then the class should be considered stable" {
val x: Row3<String, Int, Double> = Row3("x", 1, 2.0)
isStable(x::class, x) shouldBe true
}

"When isStable does not have access to actual values, it cannot determine the stability of the class" {
val x: Row3<String, Int, Double> = Row3("x", 1, 2.0)
isStable(x::class, t = null) shouldBe false
}

"When an unstable type is included, the entire data class is considered unstable, even when including values" {
val y: Row3<String, Int, Array<Int>> = Row3("x", 1, arrayOf(1, 2))
isStable(y::class, y) shouldBe false
}

"When layering generic data classes, it should still work" {
val stable = Pair(Pair("a", "b"), Pair("c", "d"))
isStable(stable::class, stable) shouldBe true

val unstable = Pair(Pair("a", "b"), Pair("c", arrayOf(1, 2)))
isStable(unstable::class, unstable) shouldBe false
}
}
})
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
package io.kotest.datatest

import io.kotest.common.KotestInternal
import io.kotest.mpp.bestName
import io.kotest.mpp.isStable

Expand Down Expand Up @@ -34,6 +35,7 @@ import io.kotest.mpp.isStable
*
* Therefore, to avoid this, data-testing requires data test elements to be stable.
*/
@Deprecated("Internal class, will not be exposed in the future. Deprecated since 5.9")
object StableIdentifiers {

/**
Expand All @@ -43,7 +45,7 @@ object StableIdentifiers {
* Note: If the user has overridden `toString()` and the returned value is not stable, tests may not appear.
*/
fun stableIdentifier(t: Any): String {
return if (isStable(t::class)) {
return if (isStable(t::class, t)) {
t.toString()
} else {
t::class.bestName()
Expand Down

0 comments on commit ea65df0

Please sign in to comment.