From 020a590df763d4d91888b46010ada0fa16636cf0 Mon Sep 17 00:00:00 2001 From: Ilya Chernikov Date: Thu, 27 Apr 2023 17:53:33 +0200 Subject: [PATCH] K2 Scripting: fix order of arguments processing and base class handling: Since in K2 we do not distinguish between script arguments taken from the base class and provided properties, we need this extra functionality to preserve the argument order of K1 scripts. This is a temporary measure, since we're going to deprecate base class usage at some point (KT-60449), so the relevant constructor arguments should disappear too. --- .../kotlin/fir/backend/Fir2IrVisitor.kt | 1 + .../backend/jvm/lower/ScriptLowering.kt | 4 ++++ .../experimental/impl/compilationInternals.kt | 14 +++++++++++++ .../jvmhost/test/ConfigurationDslTest.kt | 2 +- .../test/ConstructorArgumentOrderTest.kt | 2 +- .../jvm/BasicJvmScriptEvaluator.kt | 21 +++++++++++++++++-- .../kotlin/mainKts/test/mainKtsIT.kt | 4 ++-- .../kotlin/mainKts/test/mainKtsTest.kt | 2 +- .../plugin/impl/ScriptJvmCompilerImpls.kt | 2 ++ .../compiler/test/ScriptTemplateTest.kt | 2 +- 10 files changed, 46 insertions(+), 8 deletions(-) create mode 100644 libraries/scripting/common/src/kotlin/script/experimental/impl/compilationInternals.kt diff --git a/compiler/fir/fir2ir/src/org/jetbrains/kotlin/fir/backend/Fir2IrVisitor.kt b/compiler/fir/fir2ir/src/org/jetbrains/kotlin/fir/backend/Fir2IrVisitor.kt index 887601ab4fc..3d75f689da9 100644 --- a/compiler/fir/fir2ir/src/org/jetbrains/kotlin/fir/backend/Fir2IrVisitor.kt +++ b/compiler/fir/fir2ir/src/org/jetbrains/kotlin/fir/backend/Fir2IrVisitor.kt @@ -239,6 +239,7 @@ class Fir2IrVisitor( } if (isSelf) { irScript.thisReceiver = irReceiver + irScript.baseClass = irReceiver.type null } else irReceiver } diff --git a/compiler/ir/backend.jvm/lower/src/org/jetbrains/kotlin/backend/jvm/lower/ScriptLowering.kt b/compiler/ir/backend.jvm/lower/src/org/jetbrains/kotlin/backend/jvm/lower/ScriptLowering.kt index 818da759016..b710188a649 100644 --- a/compiler/ir/backend.jvm/lower/src/org/jetbrains/kotlin/backend/jvm/lower/ScriptLowering.kt +++ b/compiler/ir/backend.jvm/lower/src/org/jetbrains/kotlin/backend/jvm/lower/ScriptLowering.kt @@ -305,6 +305,10 @@ private class ScriptsToClassesLowering(val context: JvmBackendContext, val inner } else { +irDelegatingConstructorCall(baseClassCtor).also { explicitParameters.forEachIndexed { idx, valueParameter -> + // Since in K2 we're not distinguishing between base class ctor args and other script args, we need this check. + // The logic is fragile, but since we plan to deprecate baseClass support (see KT-60449), and this delegating + // call will go away, let's leave it as is for now + if (idx >= it.valueArgumentsCount) return@forEachIndexed it.putValueArgument( idx, IrGetValueImpl( diff --git a/libraries/scripting/common/src/kotlin/script/experimental/impl/compilationInternals.kt b/libraries/scripting/common/src/kotlin/script/experimental/impl/compilationInternals.kt new file mode 100644 index 00000000000..e5c63ecef00 --- /dev/null +++ b/libraries/scripting/common/src/kotlin/script/experimental/impl/compilationInternals.kt @@ -0,0 +1,14 @@ +/* + * Copyright 2010-2023 JetBrains s.r.o. and Kotlin Programming Language contributors. + * Use of this source code is governed by the Apache 2.0 license that can be found in the license/LICENSE.txt file. + */ + +package kotlin.script.experimental.impl + +import kotlin.script.experimental.api.ScriptCompilationConfigurationKeys +import kotlin.script.experimental.util.PropertiesCollection + +/** + * Set by the compiler: the language version used for script compilation + */ +val ScriptCompilationConfigurationKeys._languageVersion by PropertiesCollection.key() diff --git a/libraries/scripting/jvm-host-test/test/kotlin/script/experimental/jvmhost/test/ConfigurationDslTest.kt b/libraries/scripting/jvm-host-test/test/kotlin/script/experimental/jvmhost/test/ConfigurationDslTest.kt index 474e97f499a..25036562b61 100644 --- a/libraries/scripting/jvm-host-test/test/kotlin/script/experimental/jvmhost/test/ConfigurationDslTest.kt +++ b/libraries/scripting/jvm-host-test/test/kotlin/script/experimental/jvmhost/test/ConfigurationDslTest.kt @@ -22,7 +22,7 @@ import kotlin.script.experimental.jvmhost.createJvmEvaluationConfigurationFromTe class ConfigurationDslTest : TestCase() { @Test - fun testComposableRefinementHandlers() = expectTestToFailOnK2 { + fun testComposableRefinementHandlers() { val baseConfig = createJvmCompilationConfigurationFromTemplate { updateClasspath(classpathFromClass()) defaultImports(MyTestAnnotation1::class, MyTestAnnotation2::class) diff --git a/libraries/scripting/jvm-host-test/test/kotlin/script/experimental/jvmhost/test/ConstructorArgumentOrderTest.kt b/libraries/scripting/jvm-host-test/test/kotlin/script/experimental/jvmhost/test/ConstructorArgumentOrderTest.kt index c0ccb15fb1a..f95c02d7782 100644 --- a/libraries/scripting/jvm-host-test/test/kotlin/script/experimental/jvmhost/test/ConstructorArgumentOrderTest.kt +++ b/libraries/scripting/jvm-host-test/test/kotlin/script/experimental/jvmhost/test/ConstructorArgumentOrderTest.kt @@ -36,7 +36,7 @@ class ConstructorArgumentsOrderTest { } @Test - fun testScriptWithBoth() = expectTestToFailOnK2 { + fun testScriptWithBoth() { val res = evalString("""println(providedString + receiverString)""") { providedProperties("providedString" to "Hello") implicitReceivers(ImplicitReceiverClass(" Both!")) diff --git a/libraries/scripting/jvm/src/kotlin/script/experimental/jvm/BasicJvmScriptEvaluator.kt b/libraries/scripting/jvm/src/kotlin/script/experimental/jvm/BasicJvmScriptEvaluator.kt index 2ad4018a1d3..4aff29924ed 100644 --- a/libraries/scripting/jvm/src/kotlin/script/experimental/jvm/BasicJvmScriptEvaluator.kt +++ b/libraries/scripting/jvm/src/kotlin/script/experimental/jvm/BasicJvmScriptEvaluator.kt @@ -8,6 +8,7 @@ package kotlin.script.experimental.jvm import java.lang.reflect.InvocationTargetException import kotlin.reflect.KClass import kotlin.script.experimental.api.* +import kotlin.script.experimental.impl._languageVersion open class BasicJvmScriptEvaluator : ScriptEvaluator { @@ -71,6 +72,13 @@ open class BasicJvmScriptEvaluator : ScriptEvaluator { refinedEvalConfiguration: ScriptEvaluationConfiguration, importedScriptsEvalResults: List ): Any { + val isCompiledWithK2 = + refinedEvalConfiguration[ScriptEvaluationConfiguration.compilationConfiguration] + ?.get(ScriptCompilationConfiguration._languageVersion) + ?.let { + it.substringBefore('.').toIntOrNull()?.let { it >= 2 } + } == true + val args = ArrayList() refinedEvalConfiguration[ScriptEvaluationConfiguration.previousSnippets]?.let { @@ -81,6 +89,12 @@ open class BasicJvmScriptEvaluator : ScriptEvaluator { args.addAll(it) } + if (isCompiledWithK2) { + refinedEvalConfiguration[ScriptEvaluationConfiguration.providedProperties]?.forEach { + args.add(it.value) + } + } + importedScriptsEvalResults.forEach { args.add(it.returnValue.scriptInstance) } @@ -88,8 +102,11 @@ open class BasicJvmScriptEvaluator : ScriptEvaluator { refinedEvalConfiguration[ScriptEvaluationConfiguration.implicitReceivers]?.let { args.addAll(it) } - refinedEvalConfiguration[ScriptEvaluationConfiguration.providedProperties]?.forEach { - args.add(it.value) + + if (!isCompiledWithK2) { + refinedEvalConfiguration[ScriptEvaluationConfiguration.providedProperties]?.forEach { + args.add(it.value) + } } val ctor = java.constructors.single() diff --git a/libraries/tools/kotlin-main-kts-test/test/org/jetbrains/kotlin/mainKts/test/mainKtsIT.kt b/libraries/tools/kotlin-main-kts-test/test/org/jetbrains/kotlin/mainKts/test/mainKtsIT.kt index 9f7152b5c46..5d9c44e1183 100644 --- a/libraries/tools/kotlin-main-kts-test/test/org/jetbrains/kotlin/mainKts/test/mainKtsIT.kt +++ b/libraries/tools/kotlin-main-kts-test/test/org/jetbrains/kotlin/mainKts/test/mainKtsIT.kt @@ -66,7 +66,7 @@ class MainKtsIT { @OptIn(ExperimentalPathApi::class) @Test - fun testCache() = expectTestToFailOnK2 { + fun testCache() { val script = File("$TEST_DATA_ROOT/import-test.main.kts").absolutePath val cache = createTempDirectory("main.kts.test") @@ -93,7 +93,7 @@ class MainKtsIT { @OptIn(ExperimentalPathApi::class) @Test - fun testCacheInProcess() = expectTestToFailOnK2 { + fun testCacheInProcess() { val script = File("$TEST_DATA_ROOT/import-test.main.kts").absolutePath val cache = createTempDirectory("main.kts.test") diff --git a/libraries/tools/kotlin-main-kts-test/test/org/jetbrains/kotlin/mainKts/test/mainKtsTest.kt b/libraries/tools/kotlin-main-kts-test/test/org/jetbrains/kotlin/mainKts/test/mainKtsTest.kt index 2e849029753..2ea45bdc25b 100644 --- a/libraries/tools/kotlin-main-kts-test/test/org/jetbrains/kotlin/mainKts/test/mainKtsTest.kt +++ b/libraries/tools/kotlin-main-kts-test/test/org/jetbrains/kotlin/mainKts/test/mainKtsTest.kt @@ -126,7 +126,7 @@ class MainKtsTest { } @Test - fun testImport() = expectTestToFailOnK2 { + fun testImport() { val out = captureOut { val res = evalFile(File("$TEST_DATA_ROOT/import-test.main.kts")) diff --git a/plugins/scripting/scripting-compiler/src/org/jetbrains/kotlin/scripting/compiler/plugin/impl/ScriptJvmCompilerImpls.kt b/plugins/scripting/scripting-compiler/src/org/jetbrains/kotlin/scripting/compiler/plugin/impl/ScriptJvmCompilerImpls.kt index 31d281f1928..1c17e535433 100644 --- a/plugins/scripting/scripting-compiler/src/org/jetbrains/kotlin/scripting/compiler/plugin/impl/ScriptJvmCompilerImpls.kt +++ b/plugins/scripting/scripting-compiler/src/org/jetbrains/kotlin/scripting/compiler/plugin/impl/ScriptJvmCompilerImpls.kt @@ -45,6 +45,7 @@ import org.jetbrains.kotlin.scripting.resolve.resolvedImportScripts import org.jetbrains.kotlin.utils.topologicalSort import kotlin.script.experimental.api.* import kotlin.script.experimental.host.ScriptingHostConfiguration +import kotlin.script.experimental.impl._languageVersion import kotlin.script.experimental.jvm.JvmDependency import kotlin.script.experimental.jvm.JvmDependencyFromClassLoader import kotlin.script.experimental.jvm.compilationCache @@ -155,6 +156,7 @@ private fun compileImpl( dependenciesProvider?.getScriptConfigurationResult(ktFile, context.baseScriptCompilationConfiguration) ?.valueOrNull()?.configuration ?: context.baseScriptCompilationConfiguration refinedConfiguration.with { + _languageVersion(context.environment.configuration.languageVersionSettings.languageVersion.versionString) // Adjust definitions so all compiler dependencies are saved in the resulting compilation configuration, so evaluation // performed with the expected classpath // TODO: make this logic obsolete by injecting classpath earlier in the pipeline diff --git a/plugins/scripting/scripting-compiler/tests/org/jetbrains/kotlin/scripting/compiler/test/ScriptTemplateTest.kt b/plugins/scripting/scripting-compiler/tests/org/jetbrains/kotlin/scripting/compiler/test/ScriptTemplateTest.kt index a4586bbdcd6..a80cad14a0f 100644 --- a/plugins/scripting/scripting-compiler/tests/org/jetbrains/kotlin/scripting/compiler/test/ScriptTemplateTest.kt +++ b/plugins/scripting/scripting-compiler/tests/org/jetbrains/kotlin/scripting/compiler/test/ScriptTemplateTest.kt @@ -72,7 +72,7 @@ class ScriptTemplateTest : TestCase() { assertEqualsTrimmed(NUM_4_LINE + FIB_SCRIPT_OUTPUT_TAIL, out) } - fun testScriptWithBaseClassWithParam() = expectTestToFailOnK2 { + fun testScriptWithBaseClassWithParam() { val messageCollector = TestMessageCollector() val aClass = compileScript("fib_dsl.kts", ScriptWithBaseClass::class, null, runIsolated = false, messageCollector = messageCollector)