Clean and fix new scripting API

2018-07-13 19:05:37 +02:00
parent f2a8e5c248
commit a54675abe7
20 changed files with 111 additions and 95 deletions
@@ -9,8 +9,8 @@ interface ScriptCompiler {

    suspend fun compile(
        script: ScriptSource,
-        configurator: ScriptCompilationConfigurator? = null,
-        additionalConfiguration: ScriptCompileConfiguration? = null // overrides parameters from configurator.defaultConfiguration
+        scriptDefinition: ScriptDefinition,
+        additionalConfiguration: ScriptCompileConfiguration? = null // overrides properties from definition and configurator.defaultConfiguration
    ): ResultWithDiagnostics<CompiledScript<*>>
 }

@@ -17,7 +17,7 @@ typealias ProcessedScriptData = ChainedPropertyBag

 interface ScriptCompilationConfigurator {

-    // constructor(environment: ScriptingEnvironment) // the constructor is expected from implementations
+    // constructor(properties: ScriptDefinitionPropertiesBag) // the constructor is expected from implementations

    val defaultConfiguration: ScriptCompileConfiguration

@@ -13,7 +13,7 @@ object ScriptCompileConfigurationProperties {

    val sourceFragments by typedKey<List<ScriptSourceNamedFragment>>()

-    val baseClass = ScriptingEnvironmentProperties.baseClass
+    val baseClass = ScriptDefinitionProperties.baseClass

    val scriptBodyTarget by typedKey<ScriptBodyTarget>()

@@ -7,6 +7,7 @@

 package kotlin.script.experimental.api

+import kotlin.reflect.KClass
 import kotlin.script.experimental.util.ChainedPropertyBag
 import kotlin.script.experimental.util.typedKey

@@ -18,7 +19,7 @@ interface ScriptDefinition {

    val properties: ScriptDefinitionPropertiesBag

-    val compilationConfigurator: ScriptCompilationConfigurator
+    val compilationConfigurator: ScriptCompilationConfigurator?

    val evaluator: ScriptEvaluator<*>?
 }
@@ -28,4 +29,12 @@ object ScriptDefinitionProperties {
    val name by typedKey<String>() // Name of the script type, by default "Kotlin script"

    val fileExtension by typedKey<String>() // default: "kts"
-}
+
+    val baseClass by typedKey<KotlinType>() // script base class
+}
+
+fun ScriptDefinitionPropertiesBag.getScriptBaseClass(contextClass: KClass<*>): KClass<*> =
+    getScriptingClass(get(ScriptDefinitionProperties.baseClass), contextClass)
+
+fun ScriptDefinitionPropertiesBag.getScriptBaseClass(context: Any): KClass<*> =
+    getScriptingClass(get(ScriptDefinitionProperties.baseClass), context::class)
@@ -27,7 +27,7 @@ data class EvaluationResult(val returnValue: Any?, val environment: ScriptEvalua
 // NOTE: name inconsistency: run vs evaluate
 interface ScriptEvaluator<in ScriptBase : Any> {

-    // constructor(environment: ScriptingEnvironment) // the constructor is expected from implementations
+    // constructor(properties: ScriptDefinitionPropertiesBag) // the constructor is expected from implementations

    suspend fun eval(
        compiledScript: CompiledScript<ScriptBase>,
@@ -13,9 +13,6 @@ typealias ScriptingEnvironment = ChainedPropertyBag

 object ScriptingEnvironmentProperties {

-    // required by definitions that extract data from script base class annotations
-    val baseClass by typedKey<KotlinType>()
-
    // should contain all dependencies needed for baseClass and compilationConfigurator
    val configurationDependencies by typedKey<List<ScriptDependency>>()

@@ -40,8 +37,3 @@ fun ScriptingEnvironment.getScriptingClass(type: KotlinType, contextClass: KClas

 fun ScriptingEnvironment.getScriptingClass(type: KotlinType, context: Any): KClass<*> = getScriptingClass(type, context::class)

-fun ScriptingEnvironment.getScriptBaseClass(contextClass: KClass<*>): KClass<*> =
-    getScriptingClass(get(ScriptingEnvironmentProperties.baseClass), contextClass)
-
-fun ScriptingEnvironment.getScriptBaseClass(context: Any): KClass<*> =
-    getScriptingClass(get(ScriptingEnvironmentProperties.baseClass), context::class)
@@ -13,10 +13,10 @@ import kotlin.script.experimental.util.TypedKey
 private const val ILLEGAL_CONFIG_ANN_ARG =
    "Illegal argument to KotlinScriptDefaultCompilationConfiguration annotation: expecting List-derived object or default-constructed class of configuration parameters"

-open class AnnotationsBasedCompilationConfigurator(val environment: ScriptingEnvironment) : ScriptCompilationConfigurator {
+open class AnnotationsBasedCompilationConfigurator(val properties: ScriptDefinitionPropertiesBag) : ScriptCompilationConfigurator {

    override val defaultConfiguration by lazy(LazyThreadSafetyMode.PUBLICATION) {
-        val baseClass = environment.getScriptBaseClass(this)
+        val baseClass = properties.getScriptBaseClass(this)
        val cfg = baseClass.annotations.filterIsInstance(KotlinScriptDefaultCompilationConfiguration::class.java).flatMap { ann ->
            val params = try {
                ann.compilationConfiguration.objectInstance ?: ann.compilationConfiguration.createInstance()
@@ -29,7 +29,7 @@ open class AnnotationsBasedCompilationConfigurator(val environment: ScriptingEnv
            }
            params as List<Pair<TypedKey<*>, Any?>>
        }
-        ScriptCompileConfiguration(environment, cfg)
+        ScriptCompileConfiguration(properties, cfg)
    }
 }

@@ -13,52 +13,52 @@ import kotlin.script.experimental.api.*
 import kotlin.script.experimental.basic.AnnotationsBasedCompilationConfigurator
 import kotlin.script.experimental.basic.DummyEvaluator
 import kotlin.script.experimental.util.TypedKey
+import kotlin.script.experimental.util.chainPropertyBags

 private const val ERROR_MSG_PREFIX = "Unable to construct script definition: "

-open class ScriptDefinitionFromAnnotatedBaseClass(val environment: ScriptingEnvironment) : ScriptDefinition {
+open class ScriptDefinitionFromAnnotatedBaseClass(
+    protected val baseClassType: KotlinType,
+    val environment: ScriptingEnvironment
+) : ScriptDefinition {

    private val getScriptingClass = environment.getOrNull(ScriptingEnvironmentProperties.getScriptingClass)
-            ?: throw IllegalArgumentException("${ERROR_MSG_PREFIX}Expecting 'getScriptingClass' parameter in the scripting environment")
+        ?: throw IllegalArgumentException("${ERROR_MSG_PREFIX}Expecting 'getScriptingClass' parameter in the scripting environment")

-    private val baseClass: KClass<*> = run {
-        val baseClassType = environment.getOrNull(ScriptingEnvironmentProperties.baseClass)
-                ?: throw IllegalArgumentException("${ERROR_MSG_PREFIX}Expecting 'baseClass' parameter in the scripting environment")
+    private val baseClass: KClass<*> =
        try {
            getScriptingClass(baseClassType, this::class, environment)
        } catch (e: Throwable) {
            throw IllegalArgumentException("${ERROR_MSG_PREFIX}Unable to load base class $baseClassType", e)
        }
-    }

    private val mainAnnotation = baseClass.findAnnotation<KotlinScript>()
-            ?: throw IllegalArgumentException("${ERROR_MSG_PREFIX}Expecting KotlinScript annotation on the $baseClass")
+        ?: throw IllegalArgumentException("${ERROR_MSG_PREFIX}Expecting KotlinScript annotation on the $baseClass")

    private val explicitDefinition: ScriptDefinition? =
        baseClass.findAnnotation<KotlinScriptDefinition>()?.definition.takeIf { it != this::class }?.let { it.instantiateScriptHandler() }

    override val properties = run {
-        val baseProperties = explicitDefinition?.properties ?: environment
-        val toAdd = arrayListOf<Pair<TypedKey<*>, Any>>()
+        val baseProperties = chainPropertyBags(explicitDefinition?.properties, environment)
+        val propertiesData = arrayListOf<Pair<TypedKey<*>, Any>>(ScriptDefinitionProperties.baseClass to baseClassType)
        baseClass.findAnnotation<KotlinScriptFileExtension>()?.let {
-            toAdd += ScriptDefinitionProperties.fileExtension to it.extension
+            propertiesData += ScriptDefinitionProperties.fileExtension to it.extension
        }
        if (baseProperties.getOrNull(ScriptDefinitionProperties.name) == null) {
-            toAdd += ScriptDefinitionProperties.name to mainAnnotation.name
+            propertiesData += ScriptDefinitionProperties.name to mainAnnotation.name
        }
-        if (toAdd.isEmpty()) baseProperties
-        else ScriptingEnvironment(baseProperties, toAdd)
+        ScriptingEnvironment(baseProperties, propertiesData)
    }

    override val compilationConfigurator =
        baseClass.findAnnotation<KotlinScriptCompilationConfigurator>()?.compilationConfigurator?.instantiateScriptHandler()
-                ?: explicitDefinition?.compilationConfigurator
-                ?: AnnotationsBasedCompilationConfigurator::class.instantiateScriptHandler()
+            ?: explicitDefinition?.compilationConfigurator
+            ?: AnnotationsBasedCompilationConfigurator::class.instantiateScriptHandler()

    override val evaluator =
        baseClass.findAnnotation<KotlinScriptEvaluator>()?.evaluator?.instantiateScriptHandler()
-                ?: explicitDefinition?.evaluator
-                ?: DummyEvaluator::class.instantiateScriptHandler()
+            ?: explicitDefinition?.evaluator
+            ?: DummyEvaluator::class.instantiateScriptHandler()

    private fun <T : Any> KClass<T>.instantiateScriptHandler(): T {
        val klass: KClass<T> = try {
@@ -68,7 +68,7 @@ open class ScriptDefinitionFromAnnotatedBaseClass(val environment: ScriptingEnvi
        }
        try {
            // TODO: fix call after deciding on constructor parameters
-            return klass.objectInstance ?: klass.primaryConstructor!!.call(environment)
+            return klass.objectInstance ?: klass.primaryConstructor!!.call(properties)
        } catch (e: Throwable) {
            throw IllegalArgumentException("${ERROR_MSG_PREFIX}Unable to instantiate handler $this: $e", e)
        }
@@ -23,19 +23,20 @@ import kotlinx.coroutines.experimental.runBlocking
 import kotlin.script.experimental.api.*

 abstract class BasicScriptingHost<ScriptBase : Any>(
-    val configurator: ScriptCompilationConfigurator,
    val compiler: ScriptCompiler,
+    // TODO: does it belong here or to the definition?
    val evaluator: ScriptEvaluator<ScriptBase>
 ) {
    open fun <T> runInCoroutineContext(block: suspend CoroutineScope.() -> T): T = runBlocking { block() }

    open fun eval(
        script: ScriptSource,
+        scriptDefinition: ScriptDefinition,
        compileConfiguration: ScriptCompileConfiguration,
        environment: ScriptEvaluationEnvironment
    ): ResultWithDiagnostics<EvaluationResult> =
        runInCoroutineContext {
-            val compiled = compiler.compile(script, configurator, compileConfiguration)
+            val compiled = compiler.compile(script, scriptDefinition, compileConfiguration)
            when (compiled) {
                is ResultWithDiagnostics.Failure -> compiled
                is ResultWithDiagnostics.Success -> {
@@ -27,8 +27,8 @@ open class ChainedPropertyBag private constructor(private val parent: ChainedPro

    fun cloneWithNewParent(newParent: ChainedPropertyBag?): ChainedPropertyBag = when {
        newParent == null -> this
-        parent == null -> ChainedPropertyBag(newParent, data)
-        else -> ChainedPropertyBag(parent.cloneWithNewParent(newParent), data)
+        parent == null -> createOptimized(newParent, data)
+        else -> createOptimized(parent.cloneWithNewParent(newParent), data)
    }

    inline operator fun <reified T> get(key: TypedKey<T>): T = getRaw(key) as T
@@ -38,4 +38,16 @@ open class ChainedPropertyBag private constructor(private val parent: ChainedPro
    inline fun <reified T> getOrNull(key: TypedKey<T>): T? = getOrNullRaw(key)?.let { it as T }

    fun <T> getOrNullRaw(key: TypedKey<T>): Any? = data[key] ?: parent?.getOrNullRaw(key) ?: key.defaultValue
+
+    companion object {
+        fun createOptimized(parent: ChainedPropertyBag?, data: Map<TypedKey<*>, Any?>): ChainedPropertyBag = when {
+            parent != null && data.isEmpty() -> parent
+            else -> ChainedPropertyBag(parent, data)
+        }
+    }
 }
+
+fun chainPropertyBags(propertyBags: Iterable<ChainedPropertyBag?>): ChainedPropertyBag =
+    propertyBags.fold(ChainedPropertyBag()) { res, next -> if (next == null) res else res.cloneWithNewParent(next) }
+
+fun chainPropertyBags(vararg propertyBags: ChainedPropertyBag?): ChainedPropertyBag = chainPropertyBags(propertyBags.asIterable())
@@ -44,6 +44,7 @@ import kotlin.script.experimental.jvm.JvmScriptCompileConfigurationProperties
 import kotlin.script.experimental.jvm.JvmScriptEvaluationEnvironmentProperties
 import kotlin.script.experimental.jvm.KJVMCompilerProxy
 import kotlin.script.experimental.jvm.impl.BridgeDependenciesResolver
+import kotlin.script.experimental.util.chainPropertyBags

 class KJVMCompiledScript<out ScriptBase : Any>(
    override val configuration: ScriptCompileConfiguration,
@@ -63,7 +64,7 @@ class KJVMCompiledScript<out ScriptBase : Any>(

        val clazz = classLoader.loadClass(scriptClassFQName)
        (clazz as? ScriptBase)?.asSuccess()
-                ?: ResultWithDiagnostics.Failure("Compiled class expected to be a subclass of the <ScriptBase>, but got ${clazz.javaClass.name}".asErrorDiagnostics())
+            ?: ResultWithDiagnostics.Failure("Compiled class expected to be a subclass of the <ScriptBase>, but got ${clazz.javaClass.name}".asErrorDiagnostics())
    } catch (e: Throwable) {
        ResultWithDiagnostics.Failure(ScriptDiagnostic("Unable to instantiate class $scriptClassFQName", exception = e))
    }
@@ -73,7 +74,7 @@ class KJVMCompilerImpl : KJVMCompilerProxy {

    override fun compile(
        script: ScriptSource,
-        configurator: ScriptCompilationConfigurator?,
+        scriptDefinition: ScriptDefinition,
        additionalConfiguration: ScriptCompileConfiguration
    ): ResultWithDiagnostics<CompiledScript<*>> {
        val messageCollector = ScriptDiagnosticsMessageCollector()
@@ -84,8 +85,11 @@ class KJVMCompilerImpl : KJVMCompilerProxy {
        try {
            setIdeaIoUseFallback()

-            val scriptCompileConfiguration = configurator?.defaultConfiguration?.let { additionalConfiguration.cloneWithNewParent(it) }
-                ?: additionalConfiguration
+            val scriptCompileConfiguration = chainPropertyBags(
+                additionalConfiguration,
+                scriptDefinition.compilationConfigurator?.defaultConfiguration,
+                scriptDefinition.properties
+            )
            var environment: KotlinCoreEnvironment? = null
            var updatedScriptCompileConfiguration = scriptCompileConfiguration

@@ -104,7 +108,7 @@ class KJVMCompilerImpl : KJVMCompilerProxy {
            val kotlinCompilerConfiguration = org.jetbrains.kotlin.config.CompilerConfiguration().apply {
                add(
                    JVMConfigurationKeys.SCRIPT_DEFINITIONS,
-                    BridgeScriptDefinition(scriptCompileConfiguration, configurator, ::updateClasspath)
+                    BridgeScriptDefinition(scriptDefinition, scriptCompileConfiguration, ::updateClasspath)
                )
                put<MessageCollector>(CLIConfigurationKeys.MESSAGE_COLLECTOR_KEY, messageCollector)
                put(JVMConfigurationKeys.RETAIN_OUTPUT_IN_MEMORY, true)
@@ -157,7 +161,7 @@ class KJVMCompilerImpl : KJVMCompilerProxy {
                charset = CharsetToolkit.UTF8_CHARSET
            }
            val psiFile: KtFile = psiFileFactory.trySetupPsiForFile(virtualFile, KotlinLanguage.INSTANCE, true, false) as KtFile?
-                    ?: return failure("Unable to make PSI file from script".asErrorDiagnostics())
+                ?: return failure("Unable to make PSI file from script".asErrorDiagnostics())

            val sourceFiles = listOf(psiFile)

@@ -237,18 +241,18 @@ class ScriptDiagnosticsMessageCollector : MessageCollector {
 // A bridge to the current scripting

 internal class BridgeScriptDefinition(
-    scriptCompilerConfiguration: ScriptCompileConfiguration,
-    scriptConfigurator: ScriptCompilationConfigurator?,
+    scriptDefinition: ScriptDefinition,
+    calculatedBcriptCompilerConfiguration: ScriptCompileConfiguration,
    updateClasspath: (List<File>) -> Unit
-) : KotlinScriptDefinition(scriptCompilerConfiguration.getScriptBaseClass(BridgeScriptDefinition::class)) {
+) : KotlinScriptDefinition(calculatedBcriptCompilerConfiguration.getScriptBaseClass(BridgeScriptDefinition::class)) {
    override val acceptedAnnotations = run {
        val cl = this::class.java.classLoader
-        scriptCompilerConfiguration.getOrNull(ScriptCompileConfigurationProperties.refineConfigurationOnAnnotations)
+        calculatedBcriptCompilerConfiguration.getOrNull(ScriptCompileConfigurationProperties.refineConfigurationOnAnnotations)
            ?.map { (cl.loadClass(it.typeName) as Class<out Annotation>).kotlin }
-                ?: emptyList()
+            ?: emptyList()
    }

    override val dependencyResolver: DependenciesResolver =
-        BridgeDependenciesResolver(scriptConfigurator, scriptCompilerConfiguration, updateClasspath)
+        BridgeDependenciesResolver(scriptDefinition, calculatedBcriptCompilerConfiguration, updateClasspath)
 }

@@ -16,13 +16,12 @@ import kotlin.script.experimental.dependencies.ScriptDependencies
 import kotlin.script.experimental.dependencies.ScriptReport
 import kotlin.script.experimental.host.toScriptSource
 import kotlin.script.experimental.jvm.JvmDependency
-import kotlin.script.experimental.jvm.defaultConfiguration
 import kotlin.script.experimental.jvm.mapToLegacyScriptReportPosition
 import kotlin.script.experimental.jvm.mapToLegacyScriptReportSeverity

 class BridgeDependenciesResolver(
-    val scriptConfigurator: ScriptCompilationConfigurator?,
-    val baseScriptCompilerConfiguration: ScriptCompileConfiguration = scriptConfigurator.defaultConfiguration,
+    val scriptDefinition: ScriptDefinition,
+    val calculatedBaseScriptCompilerConfiguration: ScriptCompileConfiguration,
    val onClasspathUpdated: (List<File>) -> Unit = {}
 ) : AsyncDependenciesResolver {

@@ -38,10 +37,10 @@ class BridgeDependenciesResolver(
            val processedScriptData =
                ProcessedScriptData(ProcessedScriptDataProperties.foundAnnotations to scriptContents.annotations)

-            val refinedConfiguration = scriptConfigurator?.let {
+            val refinedConfiguration = scriptDefinition.compilationConfigurator?.let { scriptConfigurator ->
                val res = scriptConfigurator.refineConfiguration(
                    scriptContents.toScriptSource(),
-                    baseScriptCompilerConfiguration,
+                    calculatedBaseScriptCompilerConfiguration,
                    processedScriptData
                )
                when (res) {
@@ -52,12 +51,12 @@ class BridgeDependenciesResolver(
                        res.value
                    }
                }
-            } ?: baseScriptCompilerConfiguration
+            } ?: calculatedBaseScriptCompilerConfiguration

            val newClasspath = refinedConfiguration.getOrNull(ScriptCompileConfigurationProperties.dependencies)
                ?.flatMap { (it as JvmDependency).classpath } ?: emptyList()
-            if (refinedConfiguration != baseScriptCompilerConfiguration) {
-                val oldClasspath = baseScriptCompilerConfiguration.getOrNull(ScriptCompileConfigurationProperties.dependencies)
+            if (refinedConfiguration != calculatedBaseScriptCompilerConfiguration) {
+                val oldClasspath = calculatedBaseScriptCompilerConfiguration.getOrNull(ScriptCompileConfigurationProperties.dependencies)
                    ?.flatMap { (it as JvmDependency).classpath } ?: emptyList()
                if (newClasspath != oldClasspath) {
                    onClasspathUpdated(newClasspath)
@@ -16,18 +16,18 @@ open class JvmScriptCompiler(

    override suspend fun compile(
        script: ScriptSource,
-        configurator: ScriptCompilationConfigurator?,
+        scriptDefinition: ScriptDefinition,
        additionalConfiguration: ScriptCompileConfiguration?
    ): ResultWithDiagnostics<CompiledScript<*>> {
-        val baseConfiguration = additionalConfiguration?.cloneWithNewParent(configurator?.defaultConfiguration)
-                ?: configurator?.defaultConfiguration
+        val baseConfiguration = additionalConfiguration?.cloneWithNewParent(scriptDefinition.compilationConfigurator?.defaultConfiguration)
+                ?: scriptDefinition.compilationConfigurator?.defaultConfiguration
                ?: ScriptCompileConfiguration()
        val refinedConfiguration =
            if (baseConfiguration.getOrNull(ScriptCompileConfigurationProperties.refineBeforeParsing) == true) {
-                if (configurator == null) {
+                if (scriptDefinition.compilationConfigurator == null) {
                    return ResultWithDiagnostics.Failure("Non-null configurator expected".asErrorDiagnostics())
                }
-                configurator.refineConfiguration(script, baseConfiguration).let {
+                scriptDefinition.compilationConfigurator!!.refineConfiguration(script, baseConfiguration).let {
                    when (it) {
                        is ResultWithDiagnostics.Failure -> return it
                        is ResultWithDiagnostics.Success -> it.value
@@ -40,7 +40,7 @@ open class JvmScriptCompiler(

        if (cached != null) return cached.asSuccess()

-        return compilerProxy.compile(script, configurator, refinedConfiguration).also {
+        return compilerProxy.compile(script, scriptDefinition, refinedConfiguration).also {
            if (it is ResultWithDiagnostics.Success) {
                cache.store(it.value, refinedConfiguration)
            }
@@ -56,7 +56,7 @@ interface CompiledJvmScriptsCache {
 interface KJVMCompilerProxy {
    fun compile(
        script: ScriptSource,
-        configurator: ScriptCompilationConfigurator?,
+        scriptDefinition: ScriptDefinition,
        additionalConfiguration: ScriptCompileConfiguration
    ): ResultWithDiagnostics<CompiledScript<*>>
 }
@@ -7,16 +7,14 @@

 package kotlin.script.experimental.jvm

-import kotlin.script.experimental.api.ScriptCompilationConfigurator
 import kotlin.script.experimental.api.ScriptEvaluator
 import kotlin.script.experimental.host.BasicScriptingHost
 import kotlin.script.experimental.util.typedKey

 open class JvmBasicScriptingHost<ScriptBase : Any>(
-    configurationExtractor: ScriptCompilationConfigurator,
    compiler: JvmScriptCompiler,
    evaluator: ScriptEvaluator<ScriptBase>
-) : BasicScriptingHost<ScriptBase>(configurationExtractor, compiler, evaluator)
+) : BasicScriptingHost<ScriptBase>(compiler, evaluator)

 object JvmScriptEvaluationEnvironmentProperties {
    val baseClassLoader by typedKey<ClassLoader?>()