Scripting: improve reporting on cyclic dependency

#KT-48177 fixed
the issue is in fact fixed by the previous commit (report error
on duplicated import) by normalizing import path before processing,
but here we're making error reporting nicer for the case.

libraries/scripting/common/src/kotlin/script/experimental/api/errorHandling.kt

@@ -147,6 +147,17 @@ inline fun<T, R> Iterable<T>.mapSuccess(body: (T) -> ResultWithDiagnostics<R>):
         results.add(r)
     }
 
+/**
+ * maps transformation ([body]) over iterable merging diagnostics
+ * return failure with merged diagnostics after first failed transformation
+ * and success with merged diagnostics and list of not null results if all transformations succeeded
+ */
+inline fun<T, R> Iterable<T>.mapNotNullSuccess(body: (T) -> ResultWithDiagnostics<R?>): ResultWithDiagnostics<List<R>> =
+    mapSuccessImpl(body) { results, r ->
+        if (r != null)
+            results.add(r)
+    }
+
 /**
  * maps transformation ([body]) over iterable merging diagnostics and flatten the results
  * return failure with merged diagnostics after first failed transformation

libraries/tools/kotlin-main-kts-test/test/org/jetbrains/kotlin/mainKts/test/mainKtsTest.kt

@@ -127,6 +127,12 @@ class MainKtsTest {
         assertFailed("Duplicate imports:", res)
     }
 
+    @Test
+    fun testCyclicImportError() {
+        val res = evalFile(File("$TEST_DATA_ROOT/import-cycle-1.main.kts"))
+        assertFailed("Unable to handle recursive script dependencies", res)
+    }
+
     @Test
     fun testCompilerOptions() {
 

libraries/tools/kotlin-main-kts-test/testData/import-cycle-1.main.kts

@@ -0,0 +1,4 @@
+
+@file:Import("import-cycle-2.main.kts")
+
+1

libraries/tools/kotlin-main-kts-test/testData/import-cycle-2.main.kts

@@ -0,0 +1,4 @@
+
+@file:Import("import-cycle-1.main.kts")
+
+2

plugins/scripting/scripting-compiler/src/org/jetbrains/kotlin/scripting/compiler/plugin/impl/KJvmReplCompilerBase.kt

@@ -150,25 +150,25 @@ open class KJvmReplCompilerBase<AnalyzerT : ReplCodeAnalyzerBase> protected cons
                 history.push(LineId(snippetNo, 0, snippet.hashCode()), scriptDescriptor)
 
                 val dependenciesProvider = ScriptDependenciesProvider.getInstance(context.environment.project)
-                val compiledScript =
-                    makeCompiledScript(
-                        generationState,
-                        snippet,
-                        sourceFiles.first(),
-                        sourceDependencies
-                    ) { ktFile ->
-                        dependenciesProvider?.getScriptConfiguration(ktFile)?.configuration
-                            ?: context.baseScriptCompilationConfiguration
-                    }
+                makeCompiledScript(
+                    generationState,
+                    snippet,
+                    sourceFiles.first(),
+                    sourceDependencies
+                ) { ktFile ->
+                    dependenciesProvider?.getScriptConfiguration(ktFile)?.configuration
+                        ?: context.baseScriptCompilationConfiguration
+                }.onSuccess { compiledScript ->
 
-                lastCompiledSnippet = lastCompiledSnippet.add(compiledScript)
+                    lastCompiledSnippet = lastCompiledSnippet.add(compiledScript)
 
-                lastCompiledSnippet?.asSuccess(messageCollector.diagnostics)
-                    ?: failure(
-                        snippet,
-                        messageCollector,
-                        "last compiled snippet should not be null"
-                    )
+                    lastCompiledSnippet?.asSuccess(messageCollector.diagnostics)
+                        ?: failure(
+                            snippet,
+                            messageCollector,
+                            "last compiled snippet should not be null"
+                        )
+                }
             }
         }.last()
 

plugins/scripting/scripting-compiler/src/org/jetbrains/kotlin/scripting/compiler/plugin/impl/ScriptJvmCompilerImpls.kt

@@ -196,16 +196,15 @@ private fun doCompile(
     val generationState =
         generate(analysisResult, sourceFiles, context.environment.configuration)
 
-    val compiledScript =
-        makeCompiledScript(
-            generationState,
-            script,
-            sourceFiles.first(),
-            sourceDependencies,
-            getScriptConfiguration
-        )
-
-    return ResultWithDiagnostics.Success(compiledScript, messageCollector.diagnostics)
+    return makeCompiledScript(
+        generationState,
+        script,
+        sourceFiles.first(),
+        sourceDependencies,
+        getScriptConfiguration
+    ).onSuccess { compiledScript ->
+        ResultWithDiagnostics.Success(compiledScript, messageCollector.diagnostics)
+    }
 }
 
 private fun analyze(sourceFiles: Collection<KtFile>, environment: KotlinCoreEnvironment): AnalysisResult {

plugins/scripting/scripting-compiler/src/org/jetbrains/kotlin/scripting/compiler/plugin/impl/jvmCompilationUtil.kt

@@ -116,32 +116,41 @@ internal fun makeCompiledScript(
     ktFile: KtFile,
     sourceDependencies: List<ScriptsCompilationDependencies.SourceDependencies>,
     getScriptConfiguration: (KtFile) -> ScriptCompilationConfiguration
-): KJvmCompiledScript {
+): ResultWithDiagnostics<KJvmCompiledScript> {
     val scriptDependenciesStack = ArrayDeque<KtScript>()
     val ktScript = ktFile.declarations.firstIsInstanceOrNull<KtScript>()
         ?: throw IllegalStateException("Expecting script file: KtScript is not found in ${ktFile.name}")
 
-    fun makeOtherScripts(script: KtScript): List<KJvmCompiledScript> {
+    fun makeOtherScripts(script: KtScript): ResultWithDiagnostics<List<KJvmCompiledScript>> {
 
         // TODO: ensure that it is caught earlier (as well) since it would be more economical
         if (scriptDependenciesStack.contains(script))
-            throw IllegalArgumentException("Unable to handle recursive script dependencies")
+            return ResultWithDiagnostics.Failure(
+                ScriptDiagnostic(
+                    ScriptDiagnostic.unspecifiedError,
+                    "Unable to handle recursive script dependencies",
+                    sourcePath = script.containingFile.virtualFile?.path
+                )
+            )
         scriptDependenciesStack.push(script)
 
         val containingKtFile = script.containingKtFile
-        val otherScripts: List<KJvmCompiledScript> =
-            sourceDependencies.find { it.scriptFile == containingKtFile }?.sourceDependencies?.valueOrThrow()?.mapNotNull { sourceFile ->
-                sourceFile.declarations.firstIsInstanceOrNull<KtScript>()?.let {
-                    KJvmCompiledScript(
-                        containingKtFile.virtualFilePath,
-                        getScriptConfiguration(sourceFile),
-                        it.fqName.asString(),
-                        null,
-                        makeOtherScripts(it),
-                        null
-                    )
-                }
-            } ?: emptyList()
+        val otherScripts =
+            sourceDependencies.find { it.scriptFile == containingKtFile }?.sourceDependencies?.valueOrThrow()
+                ?.mapNotNullSuccess { sourceFile ->
+                    sourceFile.declarations.firstIsInstanceOrNull<KtScript>()?.let { ktScript ->
+                        makeOtherScripts(ktScript).onSuccess { otherScripts ->
+                            KJvmCompiledScript(
+                                containingKtFile.virtualFilePath,
+                                getScriptConfiguration(sourceFile),
+                                ktScript.fqName.asString(),
+                                null,
+                                otherScripts,
+                                null
+                            ).asSuccess()
+                        }
+                    } ?: null.asSuccess()
+                } ?: emptyList<KJvmCompiledScript>().asSuccess()
 
         scriptDependenciesStack.pop()
         return otherScripts
@@ -154,13 +163,15 @@ internal fun makeCompiledScript(
         else resultFieldName!! to KotlinType(resultTypeString ?: DescriptorRenderer.FQ_NAMES_IN_TYPES.renderType(resultType!!))
     }
 
-    return KJvmCompiledScript(
-        script.locationId,
-        getScriptConfiguration(ktScript.containingKtFile),
-        ktScript.fqName.asString(),
-        resultField,
-        makeOtherScripts(ktScript),
-        module
-    )
+    return makeOtherScripts(ktScript).onSuccess { otherScripts ->
+        KJvmCompiledScript(
+            script.locationId,
+            getScriptConfiguration(ktScript.containingKtFile),
+            ktScript.fqName.asString(),
+            resultField,
+            otherScripts,
+            module
+        ).asSuccess()
+    }
 }