Improve script and REPL result handling:

- implement error result - refactor other result classes - implement handling in the script evaluation extension - also restores previous script error reporting functionality - add possibility to customize result fileds in script and REPL - refactor result calculation in the backend: cleanup, rename (since it is not only about REPL now)
2019-07-04 13:33:15 +02:00
parent dc4370ff08
commit 9ae0ff03fa
23 changed files with 421 additions and 127 deletions
@@ -7,7 +7,9 @@

 package kotlin.script.experimental.api

+import com.sun.xml.internal.messaging.saaj.util.ByteOutputStream
 import java.io.File
+import java.io.PrintStream

 /**
 * The single script diagnostic report
@@ -28,11 +30,28 @@ data class ScriptDiagnostic(
     */
    enum class Severity { FATAL, ERROR, WARNING, INFO, DEBUG }

-    override fun toString(): String = buildString {
-        append(severity.name)
-        append(' ')
+    override fun toString(): String = render()
+
+    /**
+     * Render diagnostics message as a string in a form:
+     * "[SEVERITY ]message[ (file:line:column)][: exception message[\n exception stacktrace]]"
+     * @param withSeverity add severity prefix, true by default
+     * @param withLocation add error location in the compiled script, if present, true by default
+     * @param withException add exception message, if present, true by default
+     * @param withStackTrace add exception stacktrace, if exception is present and [withException] is true, false by default
+     */
+    fun render(
+        withSeverity: Boolean = true,
+        withLocation: Boolean = true,
+        withException: Boolean = true,
+        withStackTrace: Boolean = false
+    ): String = buildString {
+        if (withSeverity) {
+            append(severity.name)
+            append(' ')
+        }
        append(message)
-        if (sourcePath != null || location != null) {
+        if (withLocation && (sourcePath != null || location != null)) {
            append(" (")
            sourcePath?.let { append(it.substringAfterLast(File.separatorChar)) }
            location?.let {
@@ -43,9 +62,18 @@ data class ScriptDiagnostic(
            }
            append(')')
        }
-        if (exception != null) {
+        if (withException && exception != null) {
            append(": ")
            append(exception)
+            if (withStackTrace) {
+                ByteOutputStream().use { os ->
+                    val ps = PrintStream(os)
+                    exception.printStackTrace(ps)
+                    ps.flush()
+                    append("\n")
+                    append(os.toString())
+                }
+            }
        }
    }
 }
@@ -6,6 +6,7 @@
 package kotlin.script.experimental.api

 import java.io.Serializable
+import kotlin.script.experimental.util.PropertiesCollection

 const val REPL_SNIPPET_FIRST_NO = 1
 const val REPL_SNIPPET_FIRST_GEN = 1
@@ -29,3 +30,21 @@ data class ReplSnippetIdImpl(override val no: Int, override val generation: Int,
        private val serialVersionUID: Long = 1L
    }
 }
+
+interface ReplScriptCompilationConfigurationKeys
+
+open class ReplScriptCompilationConfigurationBuilder : PropertiesCollection.Builder(),
+    ReplScriptCompilationConfigurationKeys {
+    companion object : ReplScriptCompilationConfigurationKeys
+}
+
+val ScriptCompilationConfigurationKeys.repl
+    get() = ReplScriptCompilationConfigurationBuilder()
+
+
+/**
+ * The prefix of the name of the generated script class field to assign the snipped results to, empty means disabled
+ * see also ScriptCompilationConfigurationKeys.resultField
+ */
+val ReplScriptCompilationConfigurationKeys.resultFieldPrefix by PropertiesCollection.key<String>("res")
+
@@ -99,6 +99,12 @@ val ScriptCompilationConfigurationKeys.defaultImports by PropertiesCollection.ke
 */
 val ScriptCompilationConfigurationKeys.importScripts by PropertiesCollection.key<List<SourceCode>>()

+/**
+ * The name of the generated script class field to assign the script results to, empty means disabled
+ * see also ReplScriptCompilationConfigurationKeys.resultFieldPrefix
+ */
+val ScriptCompilationConfigurationKeys.resultField by PropertiesCollection.key<String>("\$\$result")
+
 /**
 * The list of script dependencies - platform specific
 */
@@ -121,17 +121,40 @@ data class RefineEvaluationConfigurationData(
 /**
 * The script evaluation result value
 */
-sealed class ResultValue {
-    class Value(val name: String, val value: Any?, val type: String, val scriptInstance: Any) : ResultValue() {
+sealed class ResultValue(val scriptInstance: Any? = null) {
+
+    /**
+     * The result value representing a script return value - the value of the last expression in the script
+     * @param name assigned name of the result field - used e.g. in REPL
+     * @param value actual result value
+     * @param type name of the result type
+     * @param scriptInstance instance of the script class
+     */
+    class Value(val name: String, val value: Any?, val type: String, scriptInstance: Any) : ResultValue(scriptInstance) {
        override fun toString(): String = "$name: $type = $value"
    }

-    class UnitValue(val scriptInstance: Any) : ResultValue() {
+    /**
+     * The result value representing unit result, e.g. when the script ends with a statement
+     * @param scriptInstance instance of the script class
+     */
+    class Unit(scriptInstance: Any) : ResultValue(scriptInstance) {
        override fun toString(): String = "Unit"
    }

-    // TODO: obsolete it, use differently named value in the saving evaluators
-    object Unit : ResultValue()
+    /**
+     * The result value representing an exception from script itself
+     * @param error the actual exception thrown on script evaluation
+     * @param wrappingException the wrapping exception e.g. InvocationTargetException, sometimes useful for calculating the relevant stacktrace
+     */
+    class Error(val error: Throwable, val wrappingException: Throwable? = null) : ResultValue() {
+        override fun toString(): String = error.toString()
+    }
+
+    /**
+     * The result value used in non-evaluating "evaluators"
+     */
+    object NotEvaluated : ResultValue()
 }

 /**
@@ -27,7 +27,7 @@ class ReplTest : TestCase() {
    @Test
    fun testCompileAndEval() {
        val out = captureOut {
-            chechEvaluateInReplNoErrors(
+            chechEvaluateInRepl(
                simpleScriptompilationConfiguration,
                simpleScriptEvaluationConfiguration,
                sequenceOf(
@@ -41,6 +41,32 @@ class ReplTest : TestCase() {
        Assert.assertEquals("x = 3", out)
    }

+    @Test
+    fun testEvalWithResult() {
+        chechEvaluateInRepl(
+            simpleScriptompilationConfiguration,
+            simpleScriptEvaluationConfiguration,
+            sequenceOf(
+                "val x = 5",
+                "x + 6",
+                "res1 * 2"
+            ),
+            sequenceOf(null, 11, 22)
+        )
+    }
+
+    @Test
+    fun testEvalWithError() {
+        chechEvaluateInRepl(
+            simpleScriptompilationConfiguration,
+            simpleScriptEvaluationConfiguration,
+            sequenceOf(
+                "throw RuntimeException(\"abc\")"
+            ),
+            sequenceOf(RuntimeException("abc"))
+        )
+    }
+
    fun evaluateInRepl(
        compilationConfiguration: ScriptCompilationConfiguration,
        evaluationConfiguration: ScriptEvaluationConfiguration,
@@ -62,15 +88,12 @@ class ReplTest : TestCase() {
                    }
                }
                .onSuccess {
-                    val snippetInstance = when (val retVal = it.returnValue) {
-                        is ResultValue.Value -> retVal.scriptInstance
-                        is ResultValue.UnitValue -> retVal.scriptInstance
-                        else -> throw IllegalStateException("Expecting value with script instance, got $it")
-                    }
-                    currentEvalConfig = ScriptEvaluationConfiguration(currentEvalConfig) {
-                        previousSnippets.append(snippetInstance)
-                        jvm {
-                            baseClassLoader(snippetInstance::class.java.classLoader)
+                    it.returnValue.scriptInstance?.let { snippetInstance ->
+                        currentEvalConfig = ScriptEvaluationConfiguration(currentEvalConfig) {
+                            previousSnippets.append(snippetInstance)
+                            jvm {
+                                baseClassLoader(snippetInstance::class.java.classLoader)
+                            }
                        }
                    }
                    it.asSuccess()
@@ -78,7 +101,7 @@ class ReplTest : TestCase() {
        }
    }

-    fun chechEvaluateInReplNoErrors(
+    fun chechEvaluateInRepl(
        compilationConfiguration: ScriptCompilationConfiguration,
        evaluationConfiguration: ScriptEvaluationConfiguration,
        snippets: Sequence<String>,
@@ -90,11 +113,19 @@ class ReplTest : TestCase() {
                is ResultWithDiagnostics.Failure -> Assert.fail("#$index: Expected result, got $res")
                is ResultWithDiagnostics.Success -> {
                    val expectedVal = expectedIter.next()
-                    val resVal = res.value.returnValue
-                    if (resVal is ResultValue.Value && resVal.type.isNotBlank()) // TODO: the latter check is temporary while the result is used to return the instance too
-                        Assert.assertEquals("#$index: Expected $expectedVal, got $resVal", expectedVal, resVal.value)
-                    else
-                        Assert.assertTrue("#$index: Expected $expectedVal, got Unit", expectedVal == null)
+                    when (val resVal = res.value.returnValue) {
+                        is ResultValue.Value -> Assert.assertEquals(
+                            "#$index: Expected $expectedVal, got $resVal",
+                            expectedVal,
+                            resVal.value
+                        )
+                        is ResultValue.Unit -> Assert.assertTrue("#$index: Expected $expectedVal, got Unit", expectedVal == null)
+                        is ResultValue.Error -> Assert.assertTrue(
+                            "#$index: Expected $expectedVal, got Error: ${resVal.error}",
+                            expectedVal is Throwable && expectedVal.message == resVal.error.message
+                        )
+                        else -> Assert.assertTrue("#$index: Expected $expectedVal, got unknown result $resVal", expectedVal == null)
+                    }
                }
            }
        }
@@ -15,6 +15,7 @@ import org.jetbrains.org.objectweb.asm.Opcodes
 import org.junit.Assert
 import org.junit.Test
 import java.io.*
+import java.lang.RuntimeException
 import java.net.URLClassLoader
 import java.nio.file.Files
 import java.security.MessageDigest
@@ -51,6 +52,41 @@ class ScriptingHostTest : TestCase() {
        Assert.assertEquals(greeting, output2)
    }

+    @Test
+    fun testValueResult() {
+        val resVal = evalScriptWithResult("42") as ResultValue.Value
+        Assert.assertEquals(42, resVal.value)
+        Assert.assertEquals("\$\$result", resVal.name)
+        Assert.assertEquals("kotlin.Int", resVal.type)
+        val resField = resVal.scriptInstance!!::class.java.getDeclaredField("\$\$result")
+        Assert.assertEquals(42, resField.get(resVal.scriptInstance!!))
+    }
+
+    @Test
+    fun testUnitResult() {
+        val resVal = evalScriptWithResult("val x = 42")
+        Assert.assertTrue(resVal is ResultValue.Unit)
+    }
+
+    @Test
+    fun testErrorResult() {
+        val resVal = evalScriptWithResult("throw RuntimeException(\"abc\")")
+        Assert.assertTrue(resVal is ResultValue.Error)
+        val resValError = (resVal as ResultValue.Error).error
+        Assert.assertTrue(resValError is RuntimeException)
+        Assert.assertEquals("abc", resValError.message)
+    }
+
+    @Test
+    fun testCustomResultField() {
+        val resVal = evalScriptWithResult("42") {
+            resultField("outcome")
+        } as ResultValue.Value
+        Assert.assertEquals("outcome", resVal.name)
+        val resField = resVal.scriptInstance!!::class.java.getDeclaredField("outcome")
+        Assert.assertEquals(42, resField.get(resVal.scriptInstance!!))
+    }
+
    @Test
    fun testSaveToClasses() {
        val greeting = "Hello from script classes!"
@@ -366,7 +402,8 @@ class ScriptingHostTest : TestCase() {
        val evaluator = BasicJvmScriptEvaluator()
        val host = BasicJvmScriptingHost(compiler = compiler, evaluator = evaluator)

-        val scriptCompilationConfiguration = createJvmCompilationConfigurationFromTemplate<SimpleScriptTemplate>(body = configurationBuilder)
+        val scriptCompilationConfiguration =
+            createJvmCompilationConfigurationFromTemplate<SimpleScriptTemplate>(body = configurationBuilder)

        Assert.assertEquals(0, cache.storedScripts)
        var compiledScript: CompiledScript<*>? = null
@@ -453,6 +490,13 @@ fun <T> ResultWithDiagnostics<T>.throwOnFailure(): ResultWithDiagnostics<T> = ap
 private fun evalScript(script: String, host: BasicScriptingHost = BasicJvmScriptingHost()): ResultWithDiagnostics<*> =
    evalScriptWithConfiguration(script, host)

+private fun evalScriptWithResult(
+    script: String,
+    host: BasicScriptingHost = BasicJvmScriptingHost(),
+    body: ScriptCompilationConfiguration.Builder.() -> Unit = {}
+): ResultValue =
+    evalScriptWithConfiguration(script, host, body).throwOnFailure().valueOrNull()!!.returnValue
+
 private fun evalScriptWithConfiguration(
    script: String,
    host: BasicScriptingHost = BasicJvmScriptingHost(),
@@ -41,7 +41,7 @@ open class BasicJvmScriptClassFilesGenerator(val outputDir: File) : ScriptEvalua
                    writeBytes(bytes)
                }
            }
-            return ResultWithDiagnostics.Success(EvaluationResult(ResultValue.Unit, scriptEvaluationConfiguration))
+            return ResultWithDiagnostics.Success(EvaluationResult(ResultValue.NotEvaluated, scriptEvaluationConfiguration))
        } catch (e: Throwable) {
            return ResultWithDiagnostics.Failure(
                e.asDiagnostics("Cannot generate script classes: ${e.message}", path = compiledScript.sourceLocationId)
@@ -96,7 +96,7 @@ open class BasicJvmScriptJarGenerator(val outputJar: File) : ScriptEvaluator {
            if (compiledScript !is KJvmCompiledScript<*>)
                return failure("Cannot generate jar: unsupported compiled script type $compiledScript")
            compiledScript.saveToJar(outputJar)
-            return ResultWithDiagnostics.Success(EvaluationResult(ResultValue.Unit, scriptEvaluationConfiguration))
+            return ResultWithDiagnostics.Success(EvaluationResult(ResultValue.NotEvaluated, scriptEvaluationConfiguration))
        } catch (e: Throwable) {
            return ResultWithDiagnostics.Failure(
                e.asDiagnostics("Cannot generate script jar: ${e.message}", path = compiledScript.sourceLocationId)
@@ -56,20 +56,24 @@ class JvmReplEvaluator(
        val res = runBlocking { scriptEvaluator(compiledScript, currentConfiguration) }

        when (res) {
-            is ResultWithDiagnostics.Success -> when (val retVal = res.value.returnValue) {
-                is ResultValue.Value -> {
-                    history.replaceOrPush(compileResult.lineId, retVal.scriptInstance)
-                    // TODO: the latter check is temporary while the result is used to return the instance too
-                    if (retVal.type.isNotBlank())
+            is ResultWithDiagnostics.Success -> {
+                when (val retVal = res.value.returnValue) {
+                    is ResultValue.Error -> {
+                        ReplEvalResult.Error.Runtime(
+                            retVal.error.message ?: "unknown error",
+                            (retVal.error as? Exception) ?: (retVal.wrappingException as? Exception)
+                        )
+                    }
+                    is ResultValue.Value -> {
+                        history.replaceOrPush(compileResult.lineId, retVal.scriptInstance!!)
                        ReplEvalResult.ValueResult(retVal.name, retVal.value, retVal.type)
-                    else
+                    }
+                    is ResultValue.Unit -> {
+                        history.replaceOrPush(compileResult.lineId, retVal.scriptInstance!!)
                        ReplEvalResult.UnitResult()
+                    }
+                    else -> throw IllegalStateException("Unexpected snippet result value $retVal")
                }
-                is ResultValue.UnitValue -> {
-                    history.replaceOrPush(compileResult.lineId, retVal.scriptInstance)
-                    ReplEvalResult.UnitResult()
-                }
-                else -> throw IllegalStateException("Expecting value with script instance, got $retVal")
            }
            else ->
                ReplEvalResult.Error.Runtime(
@@ -5,6 +5,7 @@

 package kotlin.script.experimental.jvm

+import java.lang.reflect.InvocationTargetException
 import kotlin.reflect.KClass
 import kotlin.script.experimental.api.*
 import kotlin.script.experimental.jvm.impl.getConfigurationWithClassloader
@@ -43,13 +44,18 @@ open class BasicJvmScriptEvaluator : ScriptEvaluator {
                            ?.valueOrNull()
                            ?: configuration

-                    val instance =
-                        scriptClass.evalWithConfigAndOtherScriptsResults(refinedEvalConfiguration, importedScriptsEvalResults)
+                    val resultValue = try {
+                        val instance =
+                            scriptClass.evalWithConfigAndOtherScriptsResults(refinedEvalConfiguration, importedScriptsEvalResults)

-                    val resultValue = compiledScript.resultField?.let { (resultFieldName, resultType) ->
-                        val resultField = scriptClass.java.getDeclaredField(resultFieldName).apply { isAccessible = true }
-                        ResultValue.Value(resultFieldName, resultField.get(instance), resultType.typeName, instance)
-                    } ?: ResultValue.Value("", instance, "", instance)
+                        compiledScript.resultField?.let { (resultFieldName, resultType) ->
+                            val resultField = scriptClass.java.getDeclaredField(resultFieldName).apply { isAccessible = true }
+                            ResultValue.Value(resultFieldName, resultField.get(instance), resultType.typeName, instance)
+                        } ?: ResultValue.Unit(instance)
+
+                    } catch (e: InvocationTargetException) {
+                        ResultValue.Error(e.targetException ?: e, e)
+                    }

                    EvaluationResult(resultValue, refinedEvalConfiguration).let {
                        sharedScripts?.put(scriptClass, it)
@@ -59,10 +65,7 @@ open class BasicJvmScriptEvaluator : ScriptEvaluator {
        }
    } catch (e: Throwable) {
        ResultWithDiagnostics.Failure(
-            e.asDiagnostics(
-                "Error evaluating script",
-                path = compiledScript.sourceLocationId
-            )
+            e.asDiagnostics(path = compiledScript.sourceLocationId)
        )
    }

@@ -89,13 +92,12 @@ open class BasicJvmScriptEvaluator : ScriptEvaluator {
        }

        importedScriptsEvalResults.forEach {
-            args.add((it.returnValue as ResultValue.Value).scriptInstance)
+            args.add(it.returnValue.scriptInstance)
        }

        val ctor = java.constructors.single()
-        val instance = ctor.newInstance(*args.toArray())

-        return instance
+        return ctor.newInstance(*args.toArray())
    }
 }