diff --git a/detekt-test-utils/build.gradle.kts b/detekt-test-utils/build.gradle.kts index 51bf64e7de1..cfa4cd1d07e 100644 --- a/detekt-test-utils/build.gradle.kts +++ b/detekt-test-utils/build.gradle.kts @@ -10,5 +10,6 @@ dependencies { implementation(libs.kotlin.scriptUtil) implementation(libs.junit.api) + testImplementation(libs.assertj) runtimeOnly(libs.kotlin.scriptingCompilerEmbeddable) } diff --git a/detekt-test-utils/src/main/kotlin/io/github/detekt/test/utils/KotlinScriptEngine.kt b/detekt-test-utils/src/main/kotlin/io/github/detekt/test/utils/KotlinScriptEngine.kt index ab558b01d93..ba3d1ede4c6 100644 --- a/detekt-test-utils/src/main/kotlin/io/github/detekt/test/utils/KotlinScriptEngine.kt +++ b/detekt-test-utils/src/main/kotlin/io/github/detekt/test/utils/KotlinScriptEngine.kt @@ -1,6 +1,10 @@ package io.github.detekt.test.utils +import io.github.detekt.test.utils.KotlinScriptEnginePool.borrowEngine +import io.github.detekt.test.utils.KotlinScriptEnginePool.borrowNewEngine +import io.github.detekt.test.utils.KotlinScriptEnginePool.returnEngine import org.intellij.lang.annotations.Language +import org.jetbrains.kotlin.util.KotlinFrontEndException import javax.script.ScriptException /** @@ -10,15 +14,29 @@ object KotlinScriptEngine { /** * Compiles a given code string with the Jsr223 script engine. - * If a compilation error occurs the script engine is recovered. - * Afterwards this method throws a [KotlinScriptException]. + * Since the script engines are reused, this might cause name clashes between compilation attempts. In this case + * a new script engine is created and the compilation is attempted a second time. + * + * @throws KotlinScriptException if the given code snippet does not compile */ fun compile(@Language("kotlin") code: String) { + borrowEngine().compileWithRetryOnFrontendException(code) + } + + @Suppress("ForbiddenMethodCall") + private fun PooledScriptEngine.compileWithRetryOnFrontendException(code: String) { try { - KotlinScriptEnginePool.getEngine().compile(code) + compile(code) + } catch (_: KotlinFrontEndException) { + println( + "w: Kotlin compiler exception detected. " + + "This is most likely caused by a name clash with previously compiled snippets" + ) + borrowNewEngine().compileWithRetryOnFrontendException(code) } catch (e: ScriptException) { - KotlinScriptEnginePool.recoverEngine() throw KotlinScriptException(e) + } finally { + returnEngine(this) } } } diff --git a/detekt-test-utils/src/main/kotlin/io/github/detekt/test/utils/KotlinScriptEnginePool.kt b/detekt-test-utils/src/main/kotlin/io/github/detekt/test/utils/KotlinScriptEnginePool.kt index 739bee4acdb..e79ebf11f1f 100644 --- a/detekt-test-utils/src/main/kotlin/io/github/detekt/test/utils/KotlinScriptEnginePool.kt +++ b/detekt-test-utils/src/main/kotlin/io/github/detekt/test/utils/KotlinScriptEnginePool.kt @@ -3,35 +3,50 @@ package io.github.detekt.test.utils import org.jetbrains.kotlin.cli.common.environment.setIdeaIoUseFallback import org.jetbrains.kotlin.script.jsr223.KotlinJsr223JvmLocalScriptEngine import org.jetbrains.kotlin.script.jsr223.KotlinJsr223JvmLocalScriptEngineFactory +import java.util.Collections /** * The object to manage a pool of Kotlin script engines to distribute the load for compiling code. - * The load for compiling code is distributed over a number of engines. + * Creating script engines is expensive so they are reused when compiling snippets. The pool is able to grow + * dynamically so whenever there is no script engine available, a new one is created. Access to the pooled engines is + * thread safe. */ internal object KotlinScriptEnginePool { - private const val NUMBER_OF_ENGINES = 8 + private val availableEngines: MutableList = + Collections.synchronizedList(mutableListOf()) - private val engines: Array by lazy { - Array(NUMBER_OF_ENGINES) { createEngine() } - } - private var id = 0 - - fun getEngine(): KotlinJsr223JvmLocalScriptEngine { - id++ - if (id == NUMBER_OF_ENGINES) { - id = 0 - } - return engines[id] - } + /** + * Retrieves an engine from the pool. If none is available, a new one is created. The method is thread safe. + * + * When the caller is done using the engine, it should be returned to the pool by calling [returnEngine]. + */ + fun borrowEngine(): PooledScriptEngine = availableEngines.removeFirstOrNull() ?: createEngine() + + /** + * Creates a new engine. + * + * When the caller is done using the engine, it should be returned to the pool by calling [returnEngine]. + */ + fun borrowNewEngine(): PooledScriptEngine = createEngine() - fun recoverEngine() { - engines[id] = createEngine() + /** + * Returns a borrowed engine to the pool. This method is thread safe. + */ + fun returnEngine(engine: PooledScriptEngine) { + availableEngines.add(engine) } - private fun createEngine(): KotlinJsr223JvmLocalScriptEngine { + private fun createEngine(): PooledScriptEngine { setIdeaIoUseFallback() // To avoid error on Windows val engine = KotlinJsr223JvmLocalScriptEngineFactory().scriptEngine as? KotlinJsr223JvmLocalScriptEngine - return requireNotNull(engine) { "Kotlin script engine not supported" } + ?: error("Kotlin script engine not supported") + return PooledScriptEngine(engine) + } +} + +internal class PooledScriptEngine(private val engine: KotlinJsr223JvmLocalScriptEngine) { + fun compile(code: String) { + engine.compile(code) } } diff --git a/detekt-test-utils/src/test/kotlin/io/github/detekt/test/utils/KotlinScriptEngineTest.kt b/detekt-test-utils/src/test/kotlin/io/github/detekt/test/utils/KotlinScriptEngineTest.kt new file mode 100644 index 00000000000..265ee20bf05 --- /dev/null +++ b/detekt-test-utils/src/test/kotlin/io/github/detekt/test/utils/KotlinScriptEngineTest.kt @@ -0,0 +1,50 @@ +package io.github.detekt.test.utils + +import org.assertj.core.api.Assertions.assertThatThrownBy +import org.junit.jupiter.api.RepeatedTest +import org.junit.jupiter.api.Test + +class KotlinScriptEngineTest { + + @Test + fun `can compile a valid script`() { + val code = """ + package foo.a + + class A + """.trimIndent() + KotlinScriptEngine.compile(code) + } + + @Test + fun `fails compiling an invalid script`() { + val invalidCode = """ + package foo.b + + val unknownType: UnknownType + """.trimIndent() + assertThatThrownBy { KotlinScriptEngine.compile(invalidCode) } + .isInstanceOf(KotlinScriptException::class.java) + } + + @RepeatedTest(10) + fun `can compile the same script repeatedly`() { + val code = """ + package foo.c + + class A + """.trimIndent() + KotlinScriptEngine.compile(code) + } + + @RepeatedTest(10) + fun `fails repeatedly on invalid script`() { + val invalidCode = """ + package foo.d + + val unknownType: UnknownType + """.trimIndent() + assertThatThrownBy { KotlinScriptEngine.compile(invalidCode) } + .isInstanceOf(KotlinScriptException::class.java) + } +} diff --git a/detekt-test-utils/src/test/kotlin/io/github/detekt/test/utils/PooledScriptEngineTest.kt b/detekt-test-utils/src/test/kotlin/io/github/detekt/test/utils/PooledScriptEngineTest.kt new file mode 100644 index 00000000000..77edcba7005 --- /dev/null +++ b/detekt-test-utils/src/test/kotlin/io/github/detekt/test/utils/PooledScriptEngineTest.kt @@ -0,0 +1,56 @@ +package io.github.detekt.test.utils + +import org.assertj.core.api.Assertions.assertThatThrownBy +import org.jetbrains.kotlin.util.KotlinFrontEndException +import org.junit.jupiter.api.Test +import javax.script.ScriptException + +internal class PooledScriptEngineTest { + + @Test + fun `invalid code fails with ScriptException`() { + val invalidCode = """ + val unknownType: Foo + """.trimIndent() + + val engine = KotlinScriptEnginePool.borrowEngine() + + assertThatThrownBy { engine.compile(invalidCode) } + .isInstanceOf(ScriptException::class.java) + } + + @Test + fun `compiling the same type twice leads to a compiler error`() { + val validCode = """ + package pooled + + class A + """.trimIndent() + + val engine = KotlinScriptEnginePool.borrowEngine() + + engine.compile(validCode) + assertThatThrownBy { engine.compile(validCode) } + .isInstanceOf(KotlinFrontEndException::class.java) + } + + @Test + fun `can be reused after failing to compile an invalid script`() { + val invalidCode = """ + val unknownType: Foo + """.trimIndent() + + val validCode = """ + package pooled.c + + class A + """.trimIndent() + + val engine = KotlinScriptEnginePool.borrowEngine() + + assertThatThrownBy { engine.compile(invalidCode) } + .isInstanceOf(ScriptException::class.java) + + engine.compile(validCode) + } +}