Fix error during snippet compilation (#4536)

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)
 }

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)
         }
     }
 }

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<PooledScriptEngine> =
+        Collections.synchronizedList(mutableListOf())
 
-    private val engines: Array<KotlinJsr223JvmLocalScriptEngine> 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)
     }
 }

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)
+    }
+}

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)
+    }
+}