Skip to content

Commit 395e578

Browse files
desilvainhaarman
authored andcommitted
Fixed infinite loop caused by 'copy constructors' invoked by createInstance. The fix is to skip all constructors that take in parameters with same type as the object we are trying to build.
1 parent 24108bd commit 395e578

File tree

3 files changed

+45
-2
lines changed

3 files changed

+45
-2
lines changed

mockito-kotlin/build.gradle

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -62,4 +62,4 @@ dokka {
6262
suffix = "#L"
6363
}
6464
}
65-
javadoc.dependsOn dokka
65+
javadoc.dependsOn dokka

mockito-kotlin/src/main/kotlin/com/nhaarman/mockito_kotlin/CreateInstance.kt

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -85,14 +85,26 @@ fun <T : Any> createInstance(kClass: KClass<T>): T {
8585
private fun <T : Any> KClass<T>.easiestConstructor(): KFunction<T> {
8686
return constructors
8787
.sortedBy { it.parameters.size }
88+
.withoutParametersOfType(this.defaultType)
8889
.withoutArrayParameters()
89-
.firstOrNull() ?: constructors.sortedBy { it.parameters.size }.first()
90+
.firstOrNull() ?: constructors.sortedBy { it.parameters.size }
91+
.withoutParametersOfType(this.defaultType)
92+
.first()
9093
}
9194

9295
private fun <T> List<KFunction<T>>.withoutArrayParameters() = filter {
9396
it.parameters.filter { parameter -> parameter.type.toString().toLowerCase().contains("array") }.isEmpty()
9497
}
9598

99+
/**
100+
* Filters out functions with the given type.
101+
* This is especially useful to avoid infinite loops where constructors
102+
* accepting a parameter of their own type, e.g. 'copy constructors'.
103+
*/
104+
private fun <T: Any> List<KFunction<T>>.withoutParametersOfType(type: KType) = filter {
105+
it.parameters.filter { it.type == type }.isEmpty()
106+
}
107+
96108
@Suppress("SENSELESS_COMPARISON")
97109
private fun KClass<*>.hasObjectInstance() = objectInstance != null
98110

mockito-kotlin/src/test/kotlin/CreateInstanceTest.kt

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -436,6 +436,27 @@ class CreateInstanceTest {
436436
expect(result).toNotBeNull()
437437
}
438438

439+
/**
440+
* Bug: When the copy constructor is selected, we end up with an infinite
441+
* loop. Instead, we want to select a constructor that doesn't
442+
* take in a parameter with the same type as the one we are building.
443+
*
444+
* GIVEN a class with a copy constructor (in the case of the bug, the copy
445+
* constructor has to be selected, so it must have fewer parameters
446+
* than all other constructors)
447+
* WHEN we make an instance of the given class
448+
* THEN we expect that the new, non-null instance will be created without
449+
* an exception
450+
*/
451+
@Test
452+
fun copyConstructorDoesNotCauseException() {
453+
/* When */
454+
val result = createInstance(WithCopyConstructor::class)
455+
456+
/* Then */
457+
expect(result).toNotBeNull()
458+
}
459+
439460
private class PrivateClass private constructor(val data: String)
440461

441462
class ClosedClass
@@ -475,5 +496,15 @@ class CreateInstanceTest {
475496
constructor(c: ForbiddenConstructor) : this()
476497
}
477498

499+
/**
500+
* Bug: When the copy constructor is selected, then create instance gets
501+
* into an infinite loop. We should never use the copy constructor in
502+
* createInstance.
503+
*/
504+
data class WithCopyConstructor private constructor(val x: String,
505+
val y: String) {
506+
constructor(other: WithCopyConstructor): this(other.x, other.y)
507+
}
508+
478509
enum class MyEnum { VALUE, ANOTHER_VALUE }
479510
}

0 commit comments

Comments
 (0)