From 236428c358b66e9e8ade1e95208249bbc7ad31a7 Mon Sep 17 00:00:00 2001 From: nikolay-egorov Date: Fri, 2 Jul 2021 14:40:00 +0300 Subject: [PATCH 1/8] Command :vars added, Notebook.kt stores vars state --- .../jetbrains/kotlinx/jupyter/api/Notebook.kt | 7 +- .../compiler/util/serializedCompiledScript.kt | 1 + .../kotlinx/jupyter/common/ReplCommand.kt | 3 +- .../org/jetbrains/kotlinx/jupyter/apiImpl.kt | 25 ++++--- .../org/jetbrains/kotlinx/jupyter/commands.kt | 3 + .../org/jetbrains/kotlinx/jupyter/repl.kt | 68 ++++++------------- .../kotlinx/jupyter/repl/InternalEvaluator.kt | 1 + .../repl/impl/InternalEvaluatorImpl.kt | 60 +++++++++++++++- .../kotlinx/jupyter/test/repl/ReplTests.kt | 47 +++++++++++++ .../jupyter/test/repl/TrackedCellExecutor.kt | 2 + .../kotlinx/jupyter/test/testUtil.kt | 1 + 11 files changed, 157 insertions(+), 61 deletions(-) diff --git a/jupyter-lib/api/src/main/kotlin/org/jetbrains/kotlinx/jupyter/api/Notebook.kt b/jupyter-lib/api/src/main/kotlin/org/jetbrains/kotlinx/jupyter/api/Notebook.kt index 577a506be..371f01de3 100644 --- a/jupyter-lib/api/src/main/kotlin/org/jetbrains/kotlinx/jupyter/api/Notebook.kt +++ b/jupyter-lib/api/src/main/kotlin/org/jetbrains/kotlinx/jupyter/api/Notebook.kt @@ -1,7 +1,5 @@ package org.jetbrains.kotlinx.jupyter.api -import kotlin.jvm.Throws - /** * [Notebook] is a main entry point for Kotlin Jupyter API */ @@ -11,6 +9,11 @@ interface Notebook { */ val cellsList: Collection + /** + * Current state of visible variables + */ + val variablesMap: MutableMap + /** * Mapping allowing to get cell by execution number */ diff --git a/jupyter-lib/shared-compiler/src/main/kotlin/org/jetbrains/kotlinx/jupyter/compiler/util/serializedCompiledScript.kt b/jupyter-lib/shared-compiler/src/main/kotlin/org/jetbrains/kotlinx/jupyter/compiler/util/serializedCompiledScript.kt index 388d8dbb8..90a1bb51a 100644 --- a/jupyter-lib/shared-compiler/src/main/kotlin/org/jetbrains/kotlinx/jupyter/compiler/util/serializedCompiledScript.kt +++ b/jupyter-lib/shared-compiler/src/main/kotlin/org/jetbrains/kotlinx/jupyter/compiler/util/serializedCompiledScript.kt @@ -25,6 +25,7 @@ class EvaluatedSnippetMetadata( val newClasspath: Classpath = emptyList(), val compiledData: SerializedCompiledScriptsData = SerializedCompiledScriptsData.EMPTY, val newImports: List = emptyList(), + val variablesMap: MutableMap = mutableMapOf() ) { companion object { val EMPTY = EvaluatedSnippetMetadata() diff --git a/kotlin-jupyter-plugin/common-dependencies/src/main/kotlin/org/jetbrains/kotlinx/jupyter/common/ReplCommand.kt b/kotlin-jupyter-plugin/common-dependencies/src/main/kotlin/org/jetbrains/kotlinx/jupyter/common/ReplCommand.kt index 2cc342711..79dc74276 100644 --- a/kotlin-jupyter-plugin/common-dependencies/src/main/kotlin/org/jetbrains/kotlinx/jupyter/common/ReplCommand.kt +++ b/kotlin-jupyter-plugin/common-dependencies/src/main/kotlin/org/jetbrains/kotlinx/jupyter/common/ReplCommand.kt @@ -2,7 +2,8 @@ package org.jetbrains.kotlinx.jupyter.common enum class ReplCommand(val desc: String) { HELP("display help"), - CLASSPATH("show current classpath"); + CLASSPATH("show current classpath"), + GETVARS("get visible variables values"); val nameForUser = getNameForUser(name) diff --git a/src/main/kotlin/org/jetbrains/kotlinx/jupyter/apiImpl.kt b/src/main/kotlin/org/jetbrains/kotlinx/jupyter/apiImpl.kt index 852bc39bc..bed940e97 100644 --- a/src/main/kotlin/org/jetbrains/kotlinx/jupyter/apiImpl.kt +++ b/src/main/kotlin/org/jetbrains/kotlinx/jupyter/apiImpl.kt @@ -1,15 +1,7 @@ package org.jetbrains.kotlinx.jupyter import jupyter.kotlin.JavaRuntime -import org.jetbrains.kotlinx.jupyter.api.CodeCell -import org.jetbrains.kotlinx.jupyter.api.DisplayContainer -import org.jetbrains.kotlinx.jupyter.api.DisplayResult -import org.jetbrains.kotlinx.jupyter.api.DisplayResultWithCell -import org.jetbrains.kotlinx.jupyter.api.JREInfoProvider -import org.jetbrains.kotlinx.jupyter.api.KotlinKernelVersion -import org.jetbrains.kotlinx.jupyter.api.Notebook -import org.jetbrains.kotlinx.jupyter.api.RenderersProcessor -import java.lang.IllegalStateException +import org.jetbrains.kotlinx.jupyter.api.* class DisplayResultWrapper private constructor( val display: DisplayResult, @@ -104,6 +96,8 @@ class NotebookImpl( override val cellsList: Collection get() = cells.values + override val variablesMap = mutableMapOf() + override fun getCell(id: Int): CodeCellImpl { return cells[id] ?: throw ArrayIndexOutOfBoundsException( "There is no cell with number '$id'" @@ -132,6 +126,19 @@ class NotebookImpl( override val jreInfo: JREInfoProvider get() = JavaRuntime + fun updateVarsState(varsMap: Map) { + variablesMap += varsMap + } + + fun varsAsString() : String { + if (variablesMap.isEmpty()) return "" + val stringBuilder = StringBuilder("Visible vars: \n") + variablesMap.forEach { (t, u) -> + stringBuilder.append("\t$t : $u\n") + } + return stringBuilder.append('\n').toString() + } + fun addCell( internalId: Int, preprocessedCode: String, diff --git a/src/main/kotlin/org/jetbrains/kotlinx/jupyter/commands.kt b/src/main/kotlin/org/jetbrains/kotlinx/jupyter/commands.kt index 9a21ec805..2d2ca162d 100644 --- a/src/main/kotlin/org/jetbrains/kotlinx/jupyter/commands.kt +++ b/src/main/kotlin/org/jetbrains/kotlinx/jupyter/commands.kt @@ -56,6 +56,9 @@ fun runCommand(code: String, repl: ReplForJupyter): Response { val cp = repl.currentClasspath OkResponseWithMessage(textResult("Current classpath (${cp.count()} paths):\n${cp.joinToString("\n")}")) } + ReplCommand.GETVARS -> { + OkResponseWithMessage(textResult(repl.notebook.varsAsString())) + } ReplCommand.HELP -> { val commands = ReplCommand.values().asIterable().joinToStringIndented { ":${it.nameForUser} - ${it.desc}" } val magics = ReplLineMagic.values().asIterable().filter { it.visibleInHelp }.joinToStringIndented { diff --git a/src/main/kotlin/org/jetbrains/kotlinx/jupyter/repl.kt b/src/main/kotlin/org/jetbrains/kotlinx/jupyter/repl.kt index a18d1a531..eed5cfa3f 100644 --- a/src/main/kotlin/org/jetbrains/kotlinx/jupyter/repl.kt +++ b/src/main/kotlin/org/jetbrains/kotlinx/jupyter/repl.kt @@ -1,24 +1,10 @@ package org.jetbrains.kotlinx.jupyter -import jupyter.kotlin.CompilerArgs -import jupyter.kotlin.DependsOn -import jupyter.kotlin.KotlinContext -import jupyter.kotlin.KotlinKernelHostProvider -import jupyter.kotlin.Repository +import jupyter.kotlin.* import org.jetbrains.kotlin.config.KotlinCompilerVersion -import org.jetbrains.kotlinx.jupyter.api.Code -import org.jetbrains.kotlinx.jupyter.api.ExecutionCallback -import org.jetbrains.kotlinx.jupyter.api.KotlinKernelHost -import org.jetbrains.kotlinx.jupyter.api.KotlinKernelVersion -import org.jetbrains.kotlinx.jupyter.api.Renderable -import org.jetbrains.kotlinx.jupyter.codegen.ClassAnnotationsProcessor -import org.jetbrains.kotlinx.jupyter.codegen.ClassAnnotationsProcessorImpl -import org.jetbrains.kotlinx.jupyter.codegen.FieldsProcessor -import org.jetbrains.kotlinx.jupyter.codegen.FieldsProcessorImpl -import org.jetbrains.kotlinx.jupyter.codegen.FileAnnotationsProcessor -import org.jetbrains.kotlinx.jupyter.codegen.FileAnnotationsProcessorImpl -import org.jetbrains.kotlinx.jupyter.codegen.ResultsRenderersProcessor -import org.jetbrains.kotlinx.jupyter.codegen.RenderersProcessorImpl +import org.jetbrains.kotlinx.jupyter.api.* +import org.jetbrains.kotlinx.jupyter.codegen.* +import org.jetbrains.kotlinx.jupyter.common.ReplCommand import org.jetbrains.kotlinx.jupyter.common.looksLikeReplCommand import org.jetbrains.kotlinx.jupyter.compiler.CompilerArgsConfigurator import org.jetbrains.kotlinx.jupyter.compiler.DefaultCompilerArgsConfigurator @@ -34,41 +20,16 @@ import org.jetbrains.kotlinx.jupyter.dependencies.ScriptDependencyAnnotationHand import org.jetbrains.kotlinx.jupyter.exceptions.LibraryProblemPart import org.jetbrains.kotlinx.jupyter.exceptions.ReplException import org.jetbrains.kotlinx.jupyter.exceptions.rethrowAsLibraryException -import org.jetbrains.kotlinx.jupyter.libraries.LibrariesDir -import org.jetbrains.kotlinx.jupyter.libraries.LibrariesProcessorImpl -import org.jetbrains.kotlinx.jupyter.libraries.LibrariesScanner -import org.jetbrains.kotlinx.jupyter.libraries.LibraryResourcesProcessorImpl -import org.jetbrains.kotlinx.jupyter.libraries.ResolutionInfoProvider -import org.jetbrains.kotlinx.jupyter.libraries.ResolutionInfoSwitcher +import org.jetbrains.kotlinx.jupyter.libraries.* import org.jetbrains.kotlinx.jupyter.magics.CompoundCodePreprocessor import org.jetbrains.kotlinx.jupyter.magics.FullMagicsHandler import org.jetbrains.kotlinx.jupyter.magics.MagicsProcessor -import org.jetbrains.kotlinx.jupyter.repl.CellExecutor -import org.jetbrains.kotlinx.jupyter.repl.CompletionResult -import org.jetbrains.kotlinx.jupyter.repl.ContextUpdater -import org.jetbrains.kotlinx.jupyter.repl.InternalEvaluator -import org.jetbrains.kotlinx.jupyter.repl.KotlinCompleter -import org.jetbrains.kotlinx.jupyter.repl.ListErrorsResult -import org.jetbrains.kotlinx.jupyter.repl.impl.BaseKernelHost -import org.jetbrains.kotlinx.jupyter.repl.impl.CellExecutorImpl -import org.jetbrains.kotlinx.jupyter.repl.impl.InternalEvaluatorImpl -import org.jetbrains.kotlinx.jupyter.repl.impl.JupyterCompilerWithCompletion -import org.jetbrains.kotlinx.jupyter.repl.impl.ScriptImportsCollectorImpl -import org.jetbrains.kotlinx.jupyter.repl.impl.SharedReplContext +import org.jetbrains.kotlinx.jupyter.repl.* +import org.jetbrains.kotlinx.jupyter.repl.impl.* import java.io.File import java.net.URLClassLoader import java.util.concurrent.atomic.AtomicReference -import kotlin.script.experimental.api.ResultWithDiagnostics -import kotlin.script.experimental.api.ScriptCompilationConfiguration -import kotlin.script.experimental.api.ScriptConfigurationRefinementContext -import kotlin.script.experimental.api.ScriptEvaluationConfiguration -import kotlin.script.experimental.api.asSuccess -import kotlin.script.experimental.api.constructorArgs -import kotlin.script.experimental.api.dependencies -import kotlin.script.experimental.api.fileExtension -import kotlin.script.experimental.api.implicitReceivers -import kotlin.script.experimental.api.refineConfiguration -import kotlin.script.experimental.api.with +import kotlin.script.experimental.api.* import kotlin.script.experimental.jvm.BasicJvmReplEvaluator import kotlin.script.experimental.jvm.JvmDependency import kotlin.script.experimental.jvm.baseClassLoader @@ -360,6 +321,12 @@ class ReplForJupyterImpl( else context.compilationConfiguration.asSuccess() } + /** + * Used for debug purposes. + * @see ReplCommand + */ + private fun printVars() = log.debug(notebook.varsAsString()) + override fun eval(code: Code, displayHandler: DisplayHandler?, jupyterId: Int): EvalResult { return withEvalContext { rethrowAsLibraryException(LibraryProblemPart.BEFORE_CELL_CALLBACKS) { @@ -370,6 +337,7 @@ class ReplForJupyterImpl( val compiledData: SerializedCompiledScriptsData val newImports: List + val varsMap : Map val result = try { executor.execute(code, displayHandler) { internalId, codeToExecute -> cell = notebook.addCell(internalId, codeToExecute, EvalData(jupyterId, code)) @@ -377,6 +345,7 @@ class ReplForJupyterImpl( } finally { compiledData = internalEvaluator.popAddedCompiledScripts() newImports = importsCollector.popAddedImports() + varsMap = internalEvaluator.variablesMap } cell?.resultVal = result.result.value @@ -395,7 +364,10 @@ class ReplForJupyterImpl( updateClasspath() } ?: emptyList() - EvalResult(rendered, EvaluatedSnippetMetadata(newClasspath, compiledData, newImports)) + notebook.updateVarsState(varsMap) + printVars() + + EvalResult(rendered, EvaluatedSnippetMetadata(newClasspath, compiledData, newImports, varsMap)) } } diff --git a/src/main/kotlin/org/jetbrains/kotlinx/jupyter/repl/InternalEvaluator.kt b/src/main/kotlin/org/jetbrains/kotlinx/jupyter/repl/InternalEvaluator.kt index 3dc3246e4..10682d078 100644 --- a/src/main/kotlin/org/jetbrains/kotlinx/jupyter/repl/InternalEvaluator.kt +++ b/src/main/kotlin/org/jetbrains/kotlinx/jupyter/repl/InternalEvaluator.kt @@ -12,6 +12,7 @@ interface InternalEvaluator { val lastKClass: KClass<*> val lastClassLoader: ClassLoader + val variablesMap : MutableMap /** * Executes code snippet * @throws IllegalStateException if this method was invoked recursively diff --git a/src/main/kotlin/org/jetbrains/kotlinx/jupyter/repl/impl/InternalEvaluatorImpl.kt b/src/main/kotlin/org/jetbrains/kotlinx/jupyter/repl/impl/InternalEvaluatorImpl.kt index 32dc35e93..2e7ea8a28 100644 --- a/src/main/kotlin/org/jetbrains/kotlinx/jupyter/repl/impl/InternalEvaluatorImpl.kt +++ b/src/main/kotlin/org/jetbrains/kotlinx/jupyter/repl/impl/InternalEvaluatorImpl.kt @@ -12,6 +12,8 @@ import org.jetbrains.kotlinx.jupyter.exceptions.ReplCompilerException import org.jetbrains.kotlinx.jupyter.repl.ContextUpdater import org.jetbrains.kotlinx.jupyter.repl.InternalEvalResult import org.jetbrains.kotlinx.jupyter.repl.InternalEvaluator +import kotlin.reflect.KProperty1 +import kotlin.reflect.full.declaredMemberProperties import kotlin.script.experimental.api.ResultValue import kotlin.script.experimental.api.ResultWithDiagnostics import kotlin.script.experimental.jvm.BasicJvmReplEvaluator @@ -57,6 +59,8 @@ internal class InternalEvaluatorImpl( override val lastClassLoader get() = compiler.lastClassLoader + override val variablesMap = mutableMapOf() + private var isExecuting = false override fun eval(code: Code, onInternalIdGenerated: ((Int) -> Unit)?): InternalEvalResult { @@ -92,8 +96,12 @@ internal class InternalEvaluatorImpl( resultValue.error.message.orEmpty(), resultValue.error ) + //todo: unifying refactor is ResultValue.Unit -> { serializeAndRegisterScript(compiledScript) + updateVariablesMap(resultValue) + traverseSeenVarsNaive() + InternalEvalResult( FieldValue(Unit, null), resultValue.scriptInstance!! @@ -101,8 +109,11 @@ internal class InternalEvaluatorImpl( } is ResultValue.Value -> { serializeAndRegisterScript(compiledScript) + updateVariablesMap(resultValue) + traverseSeenVarsNaive() + InternalEvalResult( - FieldValue(resultValue.value, pureResult.compiledSnippet.resultField?.first), // TODO: replace with resultValue.name + FieldValue(resultValue.value, resultValue.name), resultValue.scriptInstance!! ) } @@ -124,4 +135,51 @@ internal class InternalEvaluatorImpl( isExecuting = false } } + + // should it be 1-way pass? + private fun updateVariablesMap(target: ResultValue, seenVars: MutableSet? = null) { + getVisibleVariables(target).forEach { + if (variablesMap.containsKey(it.key)) { + if (seenVars != null) { + if (seenVars.contains(it.key)) { + return + } + } + variablesMap.replace(it.key, it.value) + } else { + variablesMap[it.key] = it.value + seenVars?.add(it.key) + } + } + } + + fun getVisibleVariables(target: ResultValue) : Map { + val klass = target.scriptClass ?: return emptyMap() + val klassInstance = target.scriptInstance!! + + val fields = klass.declaredMemberProperties + val ans = mutableMapOf() + fields.forEach { + val casted = it as KProperty1 + ans[casted.name] = if (casted.get(klassInstance) == null) { + "null" + } else { + casted.get(klassInstance).toString() + } + } + return ans + } + + // this is the O(n) implementation + // todo: tree-structure? + private fun traverseSeenVarsNaive() { + var lastSnipped = evaluator.lastEvaluatedSnippet?.previous ?: return + val seenVars = mutableSetOf() + do { + val target = lastSnipped.get().result + updateVariablesMap(target, seenVars) + lastSnipped = lastSnipped.previous ?: break + } + while (true) + } } diff --git a/src/test/kotlin/org/jetbrains/kotlinx/jupyter/test/repl/ReplTests.kt b/src/test/kotlin/org/jetbrains/kotlinx/jupyter/test/repl/ReplTests.kt index de1b82655..c90e27a3d 100644 --- a/src/test/kotlin/org/jetbrains/kotlinx/jupyter/test/repl/ReplTests.kt +++ b/src/test/kotlin/org/jetbrains/kotlinx/jupyter/test/repl/ReplTests.kt @@ -19,6 +19,7 @@ import java.io.File import kotlin.script.experimental.api.SourceCode import kotlin.test.assertEquals import kotlin.test.assertFails +import kotlin.test.assertFalse import kotlin.test.fail class ReplTests : AbstractSingleReplTest() { @@ -438,4 +439,50 @@ class ReplTests : AbstractSingleReplTest() { assertEquals("org.RDKit.RWMol", res!!::class.qualifiedName) } + + @Test + fun testStateConsistency() { + assertTrue(repl.notebook.variablesMap.isEmpty()) + val varsUpdate = mutableMapOf( + "x" to "1", "y" to "0", + "z" to "47" + ) + repl.notebook.updateVarsState(varsUpdate) + assertFalse(repl.notebook.variablesMap.isEmpty()) + val varsState = repl.notebook.variablesMap + assertEquals("1", varsState["x"]) + assertEquals("0", varsState["y"]) + assertEquals("47", varsState["z"]) + + varsUpdate["z"] = "0" + repl.notebook.updateVarsState(varsUpdate) + assertEquals("0", varsState["z"]) + } + + @Test + fun testEmptyState() { + val res = eval("3+2") + val state = repl.notebook.variablesMap + assertTrue(state.isEmpty()) + assertEquals(res.metadata.variablesMap, state) + } + + @Test + fun testVarsCapture() { + val res = eval( + """ + val x = 1 + val y = "abc" + val z = x + """.trimIndent() + ) + val varsState = repl.notebook.variablesMap + assertTrue(varsState.isNotEmpty()) + + val returnedState = res.metadata.variablesMap + assertEquals(varsState, returnedState) + assertEquals("1", varsState["x"]) + assertEquals("abc", varsState["y"]) + assertEquals("1", varsState["z"]) + } } diff --git a/src/test/kotlin/org/jetbrains/kotlinx/jupyter/test/repl/TrackedCellExecutor.kt b/src/test/kotlin/org/jetbrains/kotlinx/jupyter/test/repl/TrackedCellExecutor.kt index 6a62fbd14..f6217b4f0 100644 --- a/src/test/kotlin/org/jetbrains/kotlinx/jupyter/test/repl/TrackedCellExecutor.kt +++ b/src/test/kotlin/org/jetbrains/kotlinx/jupyter/test/repl/TrackedCellExecutor.kt @@ -44,6 +44,8 @@ internal class MockedInternalEvaluator : TrackedInternalEvaluator { override val lastClassLoader: ClassLoader = ClassLoader.getSystemClassLoader() override val executedCodes = mutableListOf() + override val variablesMap = mutableMapOf() + override val results: List get() = executedCodes.map { null } diff --git a/src/test/kotlin/org/jetbrains/kotlinx/jupyter/test/testUtil.kt b/src/test/kotlin/org/jetbrains/kotlinx/jupyter/test/testUtil.kt index 3d2ff5ea1..c731505c2 100644 --- a/src/test/kotlin/org/jetbrains/kotlinx/jupyter/test/testUtil.kt +++ b/src/test/kotlin/org/jetbrains/kotlinx/jupyter/test/testUtil.kt @@ -147,6 +147,7 @@ object NotebookMock : Notebook { override val cellsList: Collection get() = emptyList() + override val variablesMap = mutableMapOf() override fun getCell(id: Int): CodeCellImpl { return cells[id] ?: throw ArrayIndexOutOfBoundsException( From 9da6cb5c9276ab444c8c822cafbbf5f300a6eca2 Mon Sep 17 00:00:00 2001 From: nikolay-egorov Date: Mon, 5 Jul 2021 13:14:37 +0300 Subject: [PATCH 2/8] Added VariableState.kt as a meta-storage, so String -> VariableState; html report generation --- .../jetbrains/kotlinx/jupyter/api/Notebook.kt | 2 +- .../kotlinx/jupyter/api/VariableState.kt | 9 +++ .../kotlinx/jupyter/common/ReplCommand.kt | 2 +- .../org/jetbrains/kotlinx/jupyter/apiImpl.kt | 10 ++- .../org/jetbrains/kotlinx/jupyter/commands.kt | 5 +- .../org/jetbrains/kotlinx/jupyter/repl.kt | 16 +++- .../kotlinx/jupyter/repl/InternalEvaluator.kt | 3 +- .../repl/impl/InternalEvaluatorImpl.kt | 73 +++++++++++-------- .../org/jetbrains/kotlinx/jupyter/util.kt | 65 +++++++++++++++++ .../jetbrains/kotlinx/jupyter/test/ApiTest.kt | 54 ++++++++++++++ .../kotlinx/jupyter/test/repl/ReplTests.kt | 38 +++++++--- .../jupyter/test/repl/TrackedCellExecutor.kt | 3 +- .../kotlinx/jupyter/test/testUtil.kt | 21 +++++- 13 files changed, 247 insertions(+), 54 deletions(-) create mode 100644 jupyter-lib/api/src/main/kotlin/org/jetbrains/kotlinx/jupyter/api/VariableState.kt diff --git a/jupyter-lib/api/src/main/kotlin/org/jetbrains/kotlinx/jupyter/api/Notebook.kt b/jupyter-lib/api/src/main/kotlin/org/jetbrains/kotlinx/jupyter/api/Notebook.kt index 371f01de3..6031c7da1 100644 --- a/jupyter-lib/api/src/main/kotlin/org/jetbrains/kotlinx/jupyter/api/Notebook.kt +++ b/jupyter-lib/api/src/main/kotlin/org/jetbrains/kotlinx/jupyter/api/Notebook.kt @@ -12,7 +12,7 @@ interface Notebook { /** * Current state of visible variables */ - val variablesMap: MutableMap + val variablesMap: MutableMap /** * Mapping allowing to get cell by execution number diff --git a/jupyter-lib/api/src/main/kotlin/org/jetbrains/kotlinx/jupyter/api/VariableState.kt b/jupyter-lib/api/src/main/kotlin/org/jetbrains/kotlinx/jupyter/api/VariableState.kt new file mode 100644 index 000000000..347bcad6d --- /dev/null +++ b/jupyter-lib/api/src/main/kotlin/org/jetbrains/kotlinx/jupyter/api/VariableState.kt @@ -0,0 +1,9 @@ +package org.jetbrains.kotlinx.jupyter.api + +import kotlin.reflect.KProperty1 + +data class VariableState ( + val propertyKlass: KProperty1, + val scriptInstance: Any, + var stringValue: String +) \ No newline at end of file diff --git a/kotlin-jupyter-plugin/common-dependencies/src/main/kotlin/org/jetbrains/kotlinx/jupyter/common/ReplCommand.kt b/kotlin-jupyter-plugin/common-dependencies/src/main/kotlin/org/jetbrains/kotlinx/jupyter/common/ReplCommand.kt index 79dc74276..bbf156966 100644 --- a/kotlin-jupyter-plugin/common-dependencies/src/main/kotlin/org/jetbrains/kotlinx/jupyter/common/ReplCommand.kt +++ b/kotlin-jupyter-plugin/common-dependencies/src/main/kotlin/org/jetbrains/kotlinx/jupyter/common/ReplCommand.kt @@ -3,7 +3,7 @@ package org.jetbrains.kotlinx.jupyter.common enum class ReplCommand(val desc: String) { HELP("display help"), CLASSPATH("show current classpath"), - GETVARS("get visible variables values"); + VARS("get visible variables values"); val nameForUser = getNameForUser(name) diff --git a/src/main/kotlin/org/jetbrains/kotlinx/jupyter/apiImpl.kt b/src/main/kotlin/org/jetbrains/kotlinx/jupyter/apiImpl.kt index bed940e97..0f1a8a06a 100644 --- a/src/main/kotlin/org/jetbrains/kotlinx/jupyter/apiImpl.kt +++ b/src/main/kotlin/org/jetbrains/kotlinx/jupyter/apiImpl.kt @@ -96,7 +96,7 @@ class NotebookImpl( override val cellsList: Collection get() = cells.values - override val variablesMap = mutableMapOf() + override val variablesMap = mutableMapOf() override fun getCell(id: Int): CodeCellImpl { return cells[id] ?: throw ArrayIndexOutOfBoundsException( @@ -126,15 +126,19 @@ class NotebookImpl( override val jreInfo: JREInfoProvider get() = JavaRuntime - fun updateVarsState(varsMap: Map) { + fun updateVarsState(varsMap: Map) { variablesMap += varsMap } + fun varsAsHtmlTable() : String { + return generateHTMLVarsReport(variablesMap) + } + fun varsAsString() : String { if (variablesMap.isEmpty()) return "" val stringBuilder = StringBuilder("Visible vars: \n") variablesMap.forEach { (t, u) -> - stringBuilder.append("\t$t : $u\n") + stringBuilder.append("\t$t : ${u.stringValue}\n") } return stringBuilder.append('\n').toString() } diff --git a/src/main/kotlin/org/jetbrains/kotlinx/jupyter/commands.kt b/src/main/kotlin/org/jetbrains/kotlinx/jupyter/commands.kt index 2d2ca162d..3e5bbbb9c 100644 --- a/src/main/kotlin/org/jetbrains/kotlinx/jupyter/commands.kt +++ b/src/main/kotlin/org/jetbrains/kotlinx/jupyter/commands.kt @@ -1,5 +1,6 @@ package org.jetbrains.kotlinx.jupyter +import org.jetbrains.kotlinx.jupyter.api.htmlResult import org.jetbrains.kotlinx.jupyter.api.textResult import org.jetbrains.kotlinx.jupyter.common.ReplCommand import org.jetbrains.kotlinx.jupyter.common.ReplLineMagic @@ -56,8 +57,8 @@ fun runCommand(code: String, repl: ReplForJupyter): Response { val cp = repl.currentClasspath OkResponseWithMessage(textResult("Current classpath (${cp.count()} paths):\n${cp.joinToString("\n")}")) } - ReplCommand.GETVARS -> { - OkResponseWithMessage(textResult(repl.notebook.varsAsString())) + ReplCommand.VARS -> { + OkResponseWithMessage(htmlResult(repl.notebook.varsAsHtmlTable())) } ReplCommand.HELP -> { val commands = ReplCommand.values().asIterable().joinToStringIndented { ":${it.nameForUser} - ${it.desc}" } diff --git a/src/main/kotlin/org/jetbrains/kotlinx/jupyter/repl.kt b/src/main/kotlin/org/jetbrains/kotlinx/jupyter/repl.kt index eed5cfa3f..9a0f74d31 100644 --- a/src/main/kotlin/org/jetbrains/kotlinx/jupyter/repl.kt +++ b/src/main/kotlin/org/jetbrains/kotlinx/jupyter/repl.kt @@ -325,7 +325,9 @@ class ReplForJupyterImpl( * Used for debug purposes. * @see ReplCommand */ - private fun printVars() = log.debug(notebook.varsAsString()) + private fun printVars(isHtmlFormat: Boolean = false) = log.debug( + if (isHtmlFormat) notebook.varsAsHtmlTable() else notebook.varsAsString() + ) override fun eval(code: Code, displayHandler: DisplayHandler?, jupyterId: Int): EvalResult { return withEvalContext { @@ -337,7 +339,7 @@ class ReplForJupyterImpl( val compiledData: SerializedCompiledScriptsData val newImports: List - val varsMap : Map + val varsMap : Map val result = try { executor.execute(code, displayHandler) { internalId, codeToExecute -> cell = notebook.addCell(internalId, codeToExecute, EvalData(jupyterId, code)) @@ -367,7 +369,15 @@ class ReplForJupyterImpl( notebook.updateVarsState(varsMap) printVars() - EvalResult(rendered, EvaluatedSnippetMetadata(newClasspath, compiledData, newImports, varsMap)) + //todo: perhaps redundant + // would send serializable version + val mapToSend = mutableMapOf() + varsMap.forEach { + val value = it.value.stringValue + mapToSend[it.key] = value + } + + EvalResult(rendered, EvaluatedSnippetMetadata(newClasspath, compiledData, newImports, mapToSend)) } } diff --git a/src/main/kotlin/org/jetbrains/kotlinx/jupyter/repl/InternalEvaluator.kt b/src/main/kotlin/org/jetbrains/kotlinx/jupyter/repl/InternalEvaluator.kt index 10682d078..d6f6e3114 100644 --- a/src/main/kotlin/org/jetbrains/kotlinx/jupyter/repl/InternalEvaluator.kt +++ b/src/main/kotlin/org/jetbrains/kotlinx/jupyter/repl/InternalEvaluator.kt @@ -1,6 +1,7 @@ package org.jetbrains.kotlinx.jupyter.repl import org.jetbrains.kotlinx.jupyter.api.Code +import org.jetbrains.kotlinx.jupyter.api.VariableState import org.jetbrains.kotlinx.jupyter.compiler.util.SerializedCompiledScriptsData import kotlin.reflect.KClass @@ -12,7 +13,7 @@ interface InternalEvaluator { val lastKClass: KClass<*> val lastClassLoader: ClassLoader - val variablesMap : MutableMap + val variablesMap : MutableMap /** * Executes code snippet * @throws IllegalStateException if this method was invoked recursively diff --git a/src/main/kotlin/org/jetbrains/kotlinx/jupyter/repl/impl/InternalEvaluatorImpl.kt b/src/main/kotlin/org/jetbrains/kotlinx/jupyter/repl/impl/InternalEvaluatorImpl.kt index 2e7ea8a28..63df066c6 100644 --- a/src/main/kotlin/org/jetbrains/kotlinx/jupyter/repl/impl/InternalEvaluatorImpl.kt +++ b/src/main/kotlin/org/jetbrains/kotlinx/jupyter/repl/impl/InternalEvaluatorImpl.kt @@ -4,6 +4,7 @@ import kotlinx.coroutines.runBlocking import org.jetbrains.kotlinx.jupyter.ReplEvalRuntimeException import org.jetbrains.kotlinx.jupyter.api.Code import org.jetbrains.kotlinx.jupyter.api.FieldValue +import org.jetbrains.kotlinx.jupyter.api.VariableState import org.jetbrains.kotlinx.jupyter.compiler.CompiledScriptsSerializer import org.jetbrains.kotlinx.jupyter.compiler.util.SerializedCompiledScript import org.jetbrains.kotlinx.jupyter.compiler.util.SerializedCompiledScriptsData @@ -59,7 +60,9 @@ internal class InternalEvaluatorImpl( override val lastClassLoader get() = compiler.lastClassLoader - override val variablesMap = mutableMapOf() + override val variablesMap = mutableMapOf() + + val variablesMap_ = mutableMapOf() private var isExecuting = false @@ -96,26 +99,25 @@ internal class InternalEvaluatorImpl( resultValue.error.message.orEmpty(), resultValue.error ) - //todo: unifying refactor - is ResultValue.Unit -> { - serializeAndRegisterScript(compiledScript) - updateVariablesMap(resultValue) - traverseSeenVarsNaive() - - InternalEvalResult( - FieldValue(Unit, null), - resultValue.scriptInstance!! - ) - } - is ResultValue.Value -> { + is ResultValue.Unit, is ResultValue.Value -> { serializeAndRegisterScript(compiledScript) - updateVariablesMap(resultValue) - traverseSeenVarsNaive() - InternalEvalResult( - FieldValue(resultValue.value, resultValue.name), - resultValue.scriptInstance!! - ) + variablesMap += getVisibleVariables(resultValue) + updateVariablesState() + + if (resultValue is ResultValue.Unit) { + InternalEvalResult( + FieldValue(Unit, null), + resultValue.scriptInstance!! + ) + } + else { + resultValue as ResultValue.Value + InternalEvalResult( + FieldValue(resultValue.value, resultValue.name), + resultValue.scriptInstance!! + ) + } } is ResultValue.NotEvaluated -> { throw ReplEvalRuntimeException( @@ -136,42 +138,55 @@ internal class InternalEvaluatorImpl( } } + private fun updateVariablesState() { + variablesMap.forEach { + val state = it.value + val property = state.propertyKlass + val newValue = property.get(state.scriptInstance) ?: return@forEach + + if (state.stringValue != newValue) { + state.stringValue = newValue.toString() + } + } + } + // should it be 1-way pass? private fun updateVariablesMap(target: ResultValue, seenVars: MutableSet? = null) { getVisibleVariables(target).forEach { - if (variablesMap.containsKey(it.key)) { + if (variablesMap_.containsKey(it.key)) { if (seenVars != null) { if (seenVars.contains(it.key)) { return } } - variablesMap.replace(it.key, it.value) + variablesMap_.replace(it.key, it.value.stringValue) } else { - variablesMap[it.key] = it.value + variablesMap_[it.key] = it.value.stringValue seenVars?.add(it.key) } } } - fun getVisibleVariables(target: ResultValue) : Map { + private fun getVisibleVariables(target: ResultValue) : Map { val klass = target.scriptClass ?: return emptyMap() - val klassInstance = target.scriptInstance!! + val cellKlassInstance = target.scriptInstance!! val fields = klass.declaredMemberProperties - val ans = mutableMapOf() + val ans = mutableMapOf() fields.forEach { val casted = it as KProperty1 - ans[casted.name] = if (casted.get(klassInstance) == null) { - "null" + ans[casted.name] = if (casted.get(cellKlassInstance) == null) { + return@forEach } else { - casted.get(klassInstance).toString() + val stringVal = casted.get(cellKlassInstance).toString() + VariableState(casted, cellKlassInstance, stringVal) } } return ans } // this is the O(n) implementation - // todo: tree-structure? + @Deprecated("Back-propagation def search", ReplaceWith("updateVariablesState")) private fun traverseSeenVarsNaive() { var lastSnipped = evaluator.lastEvaluatedSnippet?.previous ?: return val seenVars = mutableSetOf() diff --git a/src/main/kotlin/org/jetbrains/kotlinx/jupyter/util.kt b/src/main/kotlin/org/jetbrains/kotlinx/jupyter/util.kt index 745bbef29..688911809 100644 --- a/src/main/kotlin/org/jetbrains/kotlinx/jupyter/util.kt +++ b/src/main/kotlin/org/jetbrains/kotlinx/jupyter/util.kt @@ -1,5 +1,6 @@ package org.jetbrains.kotlinx.jupyter +import org.jetbrains.kotlinx.jupyter.api.VariableState import org.jetbrains.kotlinx.jupyter.api.bufferedImageRenderer import org.jetbrains.kotlinx.jupyter.codegen.ResultsRenderersProcessor import org.jetbrains.kotlinx.jupyter.compiler.util.SourceCodeImpl @@ -10,6 +11,70 @@ import kotlin.script.experimental.jvm.util.toSourceCodePosition fun List.joinToLines() = joinToString("\n") +//todo : perhaps, create factory class +fun generateHTMLVarsReport(variablesMap: Map) : String { + val stringBuilder = StringBuilder( + """ + + + + + """.trimIndent() + ) + stringBuilder.append(generateStyleSection()) + stringBuilder.append("\n\n\n") + stringBuilder.append("

Variables State

\n") + if (variablesMap.isEmpty()) { + stringBuilder.append("

Empty state

\n\n") + stringBuilder.append("\n") + return stringBuilder.toString() + } + + stringBuilder.append(generateVarsTable(variablesMap)) + + stringBuilder.append("\n") + return stringBuilder.toString() +} + +//todo: text is not aligning in a center +fun generateStyleSection(borderPx: Int = 1, paddingPx : Int = 5) : String { + return """ + + """.trimIndent() +} + +fun generateVarsTable(variablesMap: Map) : String { + val tableBuilder = StringBuilder(""" + + + + + + + """.trimIndent()) + + variablesMap.entries.forEach { + tableBuilder.append(""" + + + + + """.trimIndent()) + } + + return tableBuilder.append("\n
VariableValue
${it.key}${it.value.stringValue}
\n").toString() +} + + fun generateDiagnostic(fromLine: Int, fromCol: Int, toLine: Int, toCol: Int, message: String, severity: String) = ScriptDiagnostic( ScriptDiagnostic.unspecifiedError, diff --git a/src/test/kotlin/org/jetbrains/kotlinx/jupyter/test/ApiTest.kt b/src/test/kotlin/org/jetbrains/kotlinx/jupyter/test/ApiTest.kt index ee0de8375..8ab9236eb 100644 --- a/src/test/kotlin/org/jetbrains/kotlinx/jupyter/test/ApiTest.kt +++ b/src/test/kotlin/org/jetbrains/kotlinx/jupyter/test/ApiTest.kt @@ -1,6 +1,7 @@ package org.jetbrains.kotlinx.jupyter.test import org.jetbrains.kotlinx.jupyter.EvalResult +import org.jetbrains.kotlinx.jupyter.generateHTMLVarsReport import org.jetbrains.kotlinx.jupyter.repl.impl.getSimpleCompiler import org.jetbrains.kotlinx.jupyter.test.repl.AbstractSingleReplTest import org.junit.jupiter.api.Test @@ -41,4 +42,57 @@ class ApiTest : AbstractSingleReplTest() { val version = jCompiler.version assertTrue(version.major >= 0) } + + @Test + fun testVarsReportFormat() { + val res = eval(""" + val x = 1 + val y = "abc" + val z = 47 + """.trimIndent()) + + val varsUpdate = mutableMapOf( + "x" to "1", "y" to "abc", + "z" to "47" + ) + assertEquals(res.metadata.variablesMap, varsUpdate) + val htmlText = generateHTMLVarsReport(repl.notebook.variablesMap) + assertEquals( + """ + + + + + + +

Variables State

+ + + + + + + + + + + + + + + +
VariableValue
x1
yabc
z47
+ + + """.trimIndent(), htmlText) + } } diff --git a/src/test/kotlin/org/jetbrains/kotlinx/jupyter/test/repl/ReplTests.kt b/src/test/kotlin/org/jetbrains/kotlinx/jupyter/test/repl/ReplTests.kt index c90e27a3d..c05356d6b 100644 --- a/src/test/kotlin/org/jetbrains/kotlinx/jupyter/test/repl/ReplTests.kt +++ b/src/test/kotlin/org/jetbrains/kotlinx/jupyter/test/repl/ReplTests.kt @@ -443,28 +443,38 @@ class ReplTests : AbstractSingleReplTest() { @Test fun testStateConsistency() { assertTrue(repl.notebook.variablesMap.isEmpty()) + val res = eval (""" + val x = 1 + val y = 0 + val z = 47 + """.trimIndent()) + val varsUpdate = mutableMapOf( "x" to "1", "y" to "0", "z" to "47" ) - repl.notebook.updateVarsState(varsUpdate) + assertEquals(res.metadata.variablesMap, varsUpdate) assertFalse(repl.notebook.variablesMap.isEmpty()) val varsState = repl.notebook.variablesMap - assertEquals("1", varsState["x"]) - assertEquals("0", varsState["y"]) - assertEquals("47", varsState["z"]) + assertEquals("1", varsState["x"]!!.stringValue) + assertEquals("0", varsState["y"]!!.stringValue) + assertEquals("47", varsState["z"]!!.stringValue) - varsUpdate["z"] = "0" - repl.notebook.updateVarsState(varsUpdate) - assertEquals("0", varsState["z"]) + varsState["z"]!!.stringValue = "0" + repl.notebook.updateVarsState(varsState) + assertEquals("0", varsState["z"]!!.stringValue) } @Test fun testEmptyState() { val res = eval("3+2") val state = repl.notebook.variablesMap + val strState = mutableMapOf() + state.forEach { + strState[it.key] = it.value.stringValue + } assertTrue(state.isEmpty()) - assertEquals(res.metadata.variablesMap, state) + assertEquals(res.metadata.variablesMap, strState) } @Test @@ -478,11 +488,15 @@ class ReplTests : AbstractSingleReplTest() { ) val varsState = repl.notebook.variablesMap assertTrue(varsState.isNotEmpty()) + val strState = mutableMapOf() + varsState.forEach { + strState[it.key] = it.value.stringValue + } val returnedState = res.metadata.variablesMap - assertEquals(varsState, returnedState) - assertEquals("1", varsState["x"]) - assertEquals("abc", varsState["y"]) - assertEquals("1", varsState["z"]) + assertEquals(strState, returnedState) + assertEquals("1", varsState["x"]!!.stringValue) + assertEquals("abc", varsState["y"]!!.stringValue) + assertEquals("1", varsState["z"]!!.stringValue) } } diff --git a/src/test/kotlin/org/jetbrains/kotlinx/jupyter/test/repl/TrackedCellExecutor.kt b/src/test/kotlin/org/jetbrains/kotlinx/jupyter/test/repl/TrackedCellExecutor.kt index f6217b4f0..0031a6c46 100644 --- a/src/test/kotlin/org/jetbrains/kotlinx/jupyter/test/repl/TrackedCellExecutor.kt +++ b/src/test/kotlin/org/jetbrains/kotlinx/jupyter/test/repl/TrackedCellExecutor.kt @@ -3,6 +3,7 @@ package org.jetbrains.kotlinx.jupyter.test.repl import org.jetbrains.kotlinx.jupyter.ReplForJupyterImpl import org.jetbrains.kotlinx.jupyter.api.Code import org.jetbrains.kotlinx.jupyter.api.FieldValue +import org.jetbrains.kotlinx.jupyter.api.VariableState import org.jetbrains.kotlinx.jupyter.repl.CellExecutor import org.jetbrains.kotlinx.jupyter.repl.InternalEvalResult import org.jetbrains.kotlinx.jupyter.repl.InternalEvaluator @@ -44,7 +45,7 @@ internal class MockedInternalEvaluator : TrackedInternalEvaluator { override val lastClassLoader: ClassLoader = ClassLoader.getSystemClassLoader() override val executedCodes = mutableListOf() - override val variablesMap = mutableMapOf() + override val variablesMap = mutableMapOf() override val results: List get() = executedCodes.map { null } diff --git a/src/test/kotlin/org/jetbrains/kotlinx/jupyter/test/testUtil.kt b/src/test/kotlin/org/jetbrains/kotlinx/jupyter/test/testUtil.kt index c731505c2..4398e4765 100644 --- a/src/test/kotlin/org/jetbrains/kotlinx/jupyter/test/testUtil.kt +++ b/src/test/kotlin/org/jetbrains/kotlinx/jupyter/test/testUtil.kt @@ -31,6 +31,25 @@ import org.jetbrains.kotlinx.jupyter.libraries.parseLibraryDescriptors import org.jetbrains.kotlinx.jupyter.log import org.jetbrains.kotlinx.jupyter.repl.CompletionResult import java.io.File +import kotlin.collections.Collection +import kotlin.collections.List +import kotlin.collections.Map +import kotlin.collections.MutableList +import kotlin.collections.associate +import kotlin.collections.component1 +import kotlin.collections.component2 +import kotlin.collections.emptyList +import kotlin.collections.filter +import kotlin.collections.forEach +import kotlin.collections.hashMapOf +import kotlin.collections.map +import kotlin.collections.mapKeys +import kotlin.collections.mutableListOf +import kotlin.collections.mutableMapOf +import kotlin.collections.orEmpty +import kotlin.collections.set +import kotlin.collections.single +import kotlin.collections.toMap import kotlin.script.experimental.jvm.util.scriptCompilationClasspathFromContext import kotlin.test.assertEquals @@ -147,7 +166,7 @@ object NotebookMock : Notebook { override val cellsList: Collection get() = emptyList() - override val variablesMap = mutableMapOf() + override val variablesMap = mutableMapOf() override fun getCell(id: Int): CodeCellImpl { return cells[id] ?: throw ArrayIndexOutOfBoundsException( From 2538d8ebd549cb3ba5becd1d8bccbff12ebd41a0 Mon Sep 17 00:00:00 2001 From: nikolay-egorov Date: Tue, 6 Jul 2021 10:42:52 +0300 Subject: [PATCH 3/8] Add simple cache proto --- .../repl/impl/InternalEvaluatorImpl.kt | 62 +++++++++---------- .../kotlinx/jupyter/test/testUtil.kt | 1 + 2 files changed, 29 insertions(+), 34 deletions(-) diff --git a/src/main/kotlin/org/jetbrains/kotlinx/jupyter/repl/impl/InternalEvaluatorImpl.kt b/src/main/kotlin/org/jetbrains/kotlinx/jupyter/repl/impl/InternalEvaluatorImpl.kt index 63df066c6..0d125c626 100644 --- a/src/main/kotlin/org/jetbrains/kotlinx/jupyter/repl/impl/InternalEvaluatorImpl.kt +++ b/src/main/kotlin/org/jetbrains/kotlinx/jupyter/repl/impl/InternalEvaluatorImpl.kt @@ -13,6 +13,7 @@ import org.jetbrains.kotlinx.jupyter.exceptions.ReplCompilerException import org.jetbrains.kotlinx.jupyter.repl.ContextUpdater import org.jetbrains.kotlinx.jupyter.repl.InternalEvalResult import org.jetbrains.kotlinx.jupyter.repl.InternalEvaluator +import kotlin.reflect.KMutableProperty1 import kotlin.reflect.KProperty1 import kotlin.reflect.full.declaredMemberProperties import kotlin.script.experimental.api.ResultValue @@ -21,12 +22,12 @@ import kotlin.script.experimental.jvm.BasicJvmReplEvaluator import kotlin.script.experimental.jvm.impl.KJvmCompiledScript internal class InternalEvaluatorImpl( - val compiler: JupyterCompiler, - private val evaluator: BasicJvmReplEvaluator, - private val contextUpdater: ContextUpdater, - override var logExecution: Boolean, + val compiler: JupyterCompiler, + private val evaluator: BasicJvmReplEvaluator, + private val contextUpdater: ContextUpdater, + override var logExecution: Boolean, ) : - InternalEvaluator { + InternalEvaluator { private var classWriter: ClassWriter? = null @@ -34,6 +35,11 @@ internal class InternalEvaluatorImpl( private val registeredCompiledScripts = arrayListOf() + /** + * stores cache for val values + */ + private val variablesCache = mutableMapOf() + private fun serializeAndRegisterScript(compiledScript: KJvmCompiledScript) { val serializedData = scriptsSerializer.serialize(compiledScript) registeredCompiledScripts.addAll(serializedData.scripts) @@ -62,8 +68,6 @@ internal class InternalEvaluatorImpl( override val variablesMap = mutableMapOf() - val variablesMap_ = mutableMapOf() - private var isExecuting = false override fun eval(code: Code, onInternalIdGenerated: ((Int) -> Unit)?): InternalEvalResult { @@ -96,8 +100,8 @@ internal class InternalEvaluatorImpl( val pureResult = resultWithDiagnostics.value.get() return when (val resultValue = pureResult.result) { is ResultValue.Error -> throw ReplEvalRuntimeException( - resultValue.error.message.orEmpty(), - resultValue.error + resultValue.error.message.orEmpty(), + resultValue.error ) is ResultValue.Unit, is ResultValue.Value -> { serializeAndRegisterScript(compiledScript) @@ -110,8 +114,7 @@ internal class InternalEvaluatorImpl( FieldValue(Unit, null), resultValue.scriptInstance!! ) - } - else { + } else { resultValue as ResultValue.Value InternalEvalResult( FieldValue(resultValue.value, resultValue.name), @@ -121,8 +124,8 @@ internal class InternalEvaluatorImpl( } is ResultValue.NotEvaluated -> { throw ReplEvalRuntimeException( - "This snippet was not evaluated", - resultWithDiagnostics.reports.firstOrNull()?.exception + "This snippet was not evaluated", + resultWithDiagnostics.reports.firstOrNull()?.exception ) } else -> throw IllegalStateException("Unknown eval result type $this") @@ -150,24 +153,8 @@ internal class InternalEvaluatorImpl( } } - // should it be 1-way pass? - private fun updateVariablesMap(target: ResultValue, seenVars: MutableSet? = null) { - getVisibleVariables(target).forEach { - if (variablesMap_.containsKey(it.key)) { - if (seenVars != null) { - if (seenVars.contains(it.key)) { - return - } - } - variablesMap_.replace(it.key, it.value.stringValue) - } else { - variablesMap_[it.key] = it.value.stringValue - seenVars?.add(it.key) - } - } - } - private fun getVisibleVariables(target: ResultValue) : Map { + private fun getVisibleVariables(target: ResultValue): Map { val klass = target.scriptClass ?: return emptyMap() val cellKlassInstance = target.scriptInstance!! @@ -179,7 +166,14 @@ internal class InternalEvaluatorImpl( return@forEach } else { val stringVal = casted.get(cellKlassInstance).toString() - VariableState(casted, cellKlassInstance, stringVal) + val state = VariableState(casted, cellKlassInstance, stringVal) + // invariant with changes: cache --> varsMap, other way is not allowed + if (it !is KMutableProperty1) { + variablesCache[casted.name] = state + variablesMap[casted.name] = state + return@forEach + } + state } } return ans @@ -192,9 +186,9 @@ internal class InternalEvaluatorImpl( val seenVars = mutableSetOf() do { val target = lastSnipped.get().result - updateVariablesMap(target, seenVars) +// updateVariablesMap(target, seenVars) lastSnipped = lastSnipped.previous ?: break - } - while (true) + } while (true) } + } diff --git a/src/test/kotlin/org/jetbrains/kotlinx/jupyter/test/testUtil.kt b/src/test/kotlin/org/jetbrains/kotlinx/jupyter/test/testUtil.kt index 4398e4765..080cef6f2 100644 --- a/src/test/kotlin/org/jetbrains/kotlinx/jupyter/test/testUtil.kt +++ b/src/test/kotlin/org/jetbrains/kotlinx/jupyter/test/testUtil.kt @@ -13,6 +13,7 @@ import org.jetbrains.kotlinx.jupyter.api.JREInfoProvider import org.jetbrains.kotlinx.jupyter.api.KotlinKernelVersion import org.jetbrains.kotlinx.jupyter.api.Notebook import org.jetbrains.kotlinx.jupyter.api.RenderersProcessor +import org.jetbrains.kotlinx.jupyter.api.VariableState import org.jetbrains.kotlinx.jupyter.api.libraries.ExecutionHost import org.jetbrains.kotlinx.jupyter.api.libraries.JupyterIntegration import org.jetbrains.kotlinx.jupyter.api.libraries.LibraryDefinition From fef40d4d3b816ec197758bde2dc2bc52feb8717e Mon Sep 17 00:00:00 2001 From: nikolay-egorov Date: Tue, 6 Jul 2021 16:35:36 +0300 Subject: [PATCH 4/8] Variables usage per cell info added: declarations and modifying references --- .../kotlinx/jupyter/api/VariableState.kt | 10 +-- .../org/jetbrains/kotlinx/jupyter/apiImpl.kt | 14 +++- .../org/jetbrains/kotlinx/jupyter/repl.kt | 15 ++++ .../kotlinx/jupyter/repl/InternalEvaluator.kt | 8 ++- .../repl/impl/InternalEvaluatorImpl.kt | 72 +++++++++++++------ .../org/jetbrains/kotlinx/jupyter/util.kt | 25 ++++--- .../jetbrains/kotlinx/jupyter/test/ApiTest.kt | 17 +++-- .../kotlinx/jupyter/test/repl/ReplTests.kt | 15 ++-- .../jupyter/test/repl/TrackedCellExecutor.kt | 2 + 9 files changed, 125 insertions(+), 53 deletions(-) diff --git a/jupyter-lib/api/src/main/kotlin/org/jetbrains/kotlinx/jupyter/api/VariableState.kt b/jupyter-lib/api/src/main/kotlin/org/jetbrains/kotlinx/jupyter/api/VariableState.kt index 347bcad6d..9f11f7346 100644 --- a/jupyter-lib/api/src/main/kotlin/org/jetbrains/kotlinx/jupyter/api/VariableState.kt +++ b/jupyter-lib/api/src/main/kotlin/org/jetbrains/kotlinx/jupyter/api/VariableState.kt @@ -2,8 +2,8 @@ package org.jetbrains.kotlinx.jupyter.api import kotlin.reflect.KProperty1 -data class VariableState ( - val propertyKlass: KProperty1, - val scriptInstance: Any, - var stringValue: String -) \ No newline at end of file +data class VariableState( + val propertyKlass: KProperty1, + val scriptInstance: Any, + var stringValue: String +) diff --git a/src/main/kotlin/org/jetbrains/kotlinx/jupyter/apiImpl.kt b/src/main/kotlin/org/jetbrains/kotlinx/jupyter/apiImpl.kt index 0f1a8a06a..bb0b062d3 100644 --- a/src/main/kotlin/org/jetbrains/kotlinx/jupyter/apiImpl.kt +++ b/src/main/kotlin/org/jetbrains/kotlinx/jupyter/apiImpl.kt @@ -1,7 +1,15 @@ package org.jetbrains.kotlinx.jupyter import jupyter.kotlin.JavaRuntime -import org.jetbrains.kotlinx.jupyter.api.* +import org.jetbrains.kotlinx.jupyter.api.CodeCell +import org.jetbrains.kotlinx.jupyter.api.DisplayContainer +import org.jetbrains.kotlinx.jupyter.api.DisplayResult +import org.jetbrains.kotlinx.jupyter.api.DisplayResultWithCell +import org.jetbrains.kotlinx.jupyter.api.JREInfoProvider +import org.jetbrains.kotlinx.jupyter.api.KotlinKernelVersion +import org.jetbrains.kotlinx.jupyter.api.Notebook +import org.jetbrains.kotlinx.jupyter.api.RenderersProcessor +import org.jetbrains.kotlinx.jupyter.api.VariableState class DisplayResultWrapper private constructor( val display: DisplayResult, @@ -130,11 +138,11 @@ class NotebookImpl( variablesMap += varsMap } - fun varsAsHtmlTable() : String { + fun varsAsHtmlTable(): String { return generateHTMLVarsReport(variablesMap) } - fun varsAsString() : String { + fun varsAsString(): String { if (variablesMap.isEmpty()) return "" val stringBuilder = StringBuilder("Visible vars: \n") variablesMap.forEach { (t, u) -> diff --git a/src/main/kotlin/org/jetbrains/kotlinx/jupyter/repl.kt b/src/main/kotlin/org/jetbrains/kotlinx/jupyter/repl.kt index 9a0f74d31..f571d5070 100644 --- a/src/main/kotlin/org/jetbrains/kotlinx/jupyter/repl.kt +++ b/src/main/kotlin/org/jetbrains/kotlinx/jupyter/repl.kt @@ -316,6 +316,8 @@ class ReplForJupyterImpl( private val executor: CellExecutor = CellExecutorImpl(sharedContext) + internal var actualExecutionId: Int = 0 + private fun onAnnotationsHandler(context: ScriptConfigurationRefinementContext): ResultWithDiagnostics { return if (evalContextEnabled) fileAnnotationsProcessor.process(context, currentKernelHost!!) else context.compilationConfiguration.asSuccess() @@ -329,6 +331,14 @@ class ReplForJupyterImpl( if (isHtmlFormat) notebook.varsAsHtmlTable() else notebook.varsAsString() ) + // todo: causes some tests to fail. perhaps, due to exhausting log calling + private fun printUsage(cellId: Int, usedVars: Set) { + log.debug("Usages for cell $cellId:") + usedVars.forEach { + log.debug(it) + } + } + override fun eval(code: Code, displayHandler: DisplayHandler?, jupyterId: Int): EvalResult { return withEvalContext { rethrowAsLibraryException(LibraryProblemPart.BEFORE_CELL_CALLBACKS) { @@ -340,7 +350,10 @@ class ReplForJupyterImpl( val compiledData: SerializedCompiledScriptsData val newImports: List val varsMap : Map + val usageMap: Map> + internalEvaluator.lastExecutedCellId = jupyterId - 1 val result = try { + log.debug("jupyter id: $jupyterId") // == cell id executor.execute(code, displayHandler) { internalId, codeToExecute -> cell = notebook.addCell(internalId, codeToExecute, EvalData(jupyterId, code)) } @@ -348,6 +361,7 @@ class ReplForJupyterImpl( compiledData = internalEvaluator.popAddedCompiledScripts() newImports = importsCollector.popAddedImports() varsMap = internalEvaluator.variablesMap + usageMap = internalEvaluator.usageMap } cell?.resultVal = result.result.value @@ -368,6 +382,7 @@ class ReplForJupyterImpl( notebook.updateVarsState(varsMap) printVars() +// printUsage(jupyterId, usageMap[jupyterId - 1]!!) //todo: perhaps redundant // would send serializable version diff --git a/src/main/kotlin/org/jetbrains/kotlinx/jupyter/repl/InternalEvaluator.kt b/src/main/kotlin/org/jetbrains/kotlinx/jupyter/repl/InternalEvaluator.kt index d6f6e3114..f1041ad5d 100644 --- a/src/main/kotlin/org/jetbrains/kotlinx/jupyter/repl/InternalEvaluator.kt +++ b/src/main/kotlin/org/jetbrains/kotlinx/jupyter/repl/InternalEvaluator.kt @@ -13,7 +13,13 @@ interface InternalEvaluator { val lastKClass: KClass<*> val lastClassLoader: ClassLoader - val variablesMap : MutableMap + val variablesMap: MutableMap + + val usageMap: MutableMap> + + // todo: perhaps, better to be stateless + var lastExecutedCellId: Int + /** * Executes code snippet * @throws IllegalStateException if this method was invoked recursively diff --git a/src/main/kotlin/org/jetbrains/kotlinx/jupyter/repl/impl/InternalEvaluatorImpl.kt b/src/main/kotlin/org/jetbrains/kotlinx/jupyter/repl/impl/InternalEvaluatorImpl.kt index 0d125c626..f0fc33768 100644 --- a/src/main/kotlin/org/jetbrains/kotlinx/jupyter/repl/impl/InternalEvaluatorImpl.kt +++ b/src/main/kotlin/org/jetbrains/kotlinx/jupyter/repl/impl/InternalEvaluatorImpl.kt @@ -22,12 +22,12 @@ import kotlin.script.experimental.jvm.BasicJvmReplEvaluator import kotlin.script.experimental.jvm.impl.KJvmCompiledScript internal class InternalEvaluatorImpl( - val compiler: JupyterCompiler, - private val evaluator: BasicJvmReplEvaluator, - private val contextUpdater: ContextUpdater, - override var logExecution: Boolean, + val compiler: JupyterCompiler, + private val evaluator: BasicJvmReplEvaluator, + private val contextUpdater: ContextUpdater, + override var logExecution: Boolean, ) : - InternalEvaluator { + InternalEvaluator { private var classWriter: ClassWriter? = null @@ -36,7 +36,7 @@ internal class InternalEvaluatorImpl( private val registeredCompiledScripts = arrayListOf() /** - * stores cache for val values + * Stores cache for val values */ private val variablesCache = mutableMapOf() @@ -68,6 +68,15 @@ internal class InternalEvaluatorImpl( override val variablesMap = mutableMapOf() + override val usageMap = mutableMapOf>() + + override var lastExecutedCellId: Int = 0 + + /** + * Tells in which cell a variable was declared + */ + private val containingMap: MutableMap = mutableMapOf() + private var isExecuting = false override fun eval(code: Code, onInternalIdGenerated: ((Int) -> Unit)?): InternalEvalResult { @@ -100,32 +109,41 @@ internal class InternalEvaluatorImpl( val pureResult = resultWithDiagnostics.value.get() return when (val resultValue = pureResult.result) { is ResultValue.Error -> throw ReplEvalRuntimeException( - resultValue.error.message.orEmpty(), - resultValue.error + resultValue.error.message.orEmpty(), + resultValue.error ) is ResultValue.Unit, is ResultValue.Value -> { serializeAndRegisterScript(compiledScript) + if (!usageMap.containsKey(lastExecutedCellId)) { + usageMap[lastExecutedCellId] = mutableSetOf() + } + val visibleDeclarations = getVisibleVariables(resultValue, lastExecutedCellId) + visibleDeclarations.forEach { + val cellSet = usageMap[lastExecutedCellId] ?: return@forEach + containingMap[it.key] = lastExecutedCellId + cellSet += it.key + } + variablesMap += visibleDeclarations - variablesMap += getVisibleVariables(resultValue) - updateVariablesState() + updateVariablesState(lastExecutedCellId) if (resultValue is ResultValue.Unit) { InternalEvalResult( - FieldValue(Unit, null), - resultValue.scriptInstance!! + FieldValue(Unit, null), + resultValue.scriptInstance!! ) } else { resultValue as ResultValue.Value InternalEvalResult( - FieldValue(resultValue.value, resultValue.name), - resultValue.scriptInstance!! + FieldValue(resultValue.value, resultValue.name), + resultValue.scriptInstance!! ) } } is ResultValue.NotEvaluated -> { throw ReplEvalRuntimeException( - "This snippet was not evaluated", - resultWithDiagnostics.reports.firstOrNull()?.exception + "This snippet was not evaluated", + resultWithDiagnostics.reports.firstOrNull()?.exception ) } else -> throw IllegalStateException("Unknown eval result type $this") @@ -141,20 +159,25 @@ internal class InternalEvaluatorImpl( } } - private fun updateVariablesState() { + private fun updateVariablesState(cellId: Int) { + // remove known modifying usages in this cell + usageMap[cellId]?.removeIf { + containingMap[it] != cellId + } + variablesMap.forEach { val state = it.value val property = state.propertyKlass val newValue = property.get(state.scriptInstance) ?: return@forEach - if (state.stringValue != newValue) { + if (state.stringValue != newValue.toString()) { + usageMap[cellId]?.add(it.key) state.stringValue = newValue.toString() } } } - - private fun getVisibleVariables(target: ResultValue): Map { + private fun getVisibleVariables(target: ResultValue, cellId: Int): Map { val klass = target.scriptClass ?: return emptyMap() val cellKlassInstance = target.scriptInstance!! @@ -169,6 +192,14 @@ internal class InternalEvaluatorImpl( val state = VariableState(casted, cellKlassInstance, stringVal) // invariant with changes: cache --> varsMap, other way is not allowed if (it !is KMutableProperty1) { + // it is a replacement + if (variablesCache.containsKey(casted.name)) { + val oldCellID = containingMap[casted.name] + usageMap[oldCellID]?.remove(casted.name) + } + containingMap[casted.name] = cellId + usageMap[cellId]?.add(casted.name) + variablesCache[casted.name] = state variablesMap[casted.name] = state return@forEach @@ -190,5 +221,4 @@ internal class InternalEvaluatorImpl( lastSnipped = lastSnipped.previous ?: break } while (true) } - } diff --git a/src/main/kotlin/org/jetbrains/kotlinx/jupyter/util.kt b/src/main/kotlin/org/jetbrains/kotlinx/jupyter/util.kt index 688911809..d039c9cbd 100644 --- a/src/main/kotlin/org/jetbrains/kotlinx/jupyter/util.kt +++ b/src/main/kotlin/org/jetbrains/kotlinx/jupyter/util.kt @@ -11,8 +11,8 @@ import kotlin.script.experimental.jvm.util.toSourceCodePosition fun List.joinToLines() = joinToString("\n") -//todo : perhaps, create factory class -fun generateHTMLVarsReport(variablesMap: Map) : String { +// todo : perhaps, create factory class +fun generateHTMLVarsReport(variablesMap: Map): String { val stringBuilder = StringBuilder( """ @@ -36,9 +36,9 @@ fun generateHTMLVarsReport(variablesMap: Map) : String { return stringBuilder.toString() } -//todo: text is not aligning in a center -fun generateStyleSection(borderPx: Int = 1, paddingPx : Int = 5) : String { - return """ +// todo: text is not aligning in a center +fun generateStyleSection(borderPx: Int = 1, paddingPx: Int = 5): String { + return """ + """.trimIndent() + return styleSection +} + +fun generateVarsTable(variablesMap: Map): String { + return buildString { + this.append( + """ + + + + + + + """.trimIndent() + ) + + variablesMap.entries.forEach { + this.append( + """ + + + + + """.trimIndent() + ) + } + + this.append("\n
VariableValue
${it.key}${it.value.stringValue}
\n") + } +} diff --git a/src/main/kotlin/org/jetbrains/kotlinx/jupyter/repl.kt b/src/main/kotlin/org/jetbrains/kotlinx/jupyter/repl.kt index f571d5070..b4179699a 100644 --- a/src/main/kotlin/org/jetbrains/kotlinx/jupyter/repl.kt +++ b/src/main/kotlin/org/jetbrains/kotlinx/jupyter/repl.kt @@ -1,9 +1,25 @@ package org.jetbrains.kotlinx.jupyter -import jupyter.kotlin.* +import jupyter.kotlin.CompilerArgs +import jupyter.kotlin.DependsOn +import jupyter.kotlin.KotlinContext +import jupyter.kotlin.KotlinKernelHostProvider +import jupyter.kotlin.Repository import org.jetbrains.kotlin.config.KotlinCompilerVersion -import org.jetbrains.kotlinx.jupyter.api.* -import org.jetbrains.kotlinx.jupyter.codegen.* +import org.jetbrains.kotlinx.jupyter.api.Code +import org.jetbrains.kotlinx.jupyter.api.ExecutionCallback +import org.jetbrains.kotlinx.jupyter.api.KotlinKernelHost +import org.jetbrains.kotlinx.jupyter.api.KotlinKernelVersion +import org.jetbrains.kotlinx.jupyter.api.Renderable +import org.jetbrains.kotlinx.jupyter.api.VariableState +import org.jetbrains.kotlinx.jupyter.codegen.ClassAnnotationsProcessor +import org.jetbrains.kotlinx.jupyter.codegen.ClassAnnotationsProcessorImpl +import org.jetbrains.kotlinx.jupyter.codegen.FieldsProcessor +import org.jetbrains.kotlinx.jupyter.codegen.FieldsProcessorImpl +import org.jetbrains.kotlinx.jupyter.codegen.FileAnnotationsProcessor +import org.jetbrains.kotlinx.jupyter.codegen.FileAnnotationsProcessorImpl +import org.jetbrains.kotlinx.jupyter.codegen.RenderersProcessorImpl +import org.jetbrains.kotlinx.jupyter.codegen.ResultsRenderersProcessor import org.jetbrains.kotlinx.jupyter.common.ReplCommand import org.jetbrains.kotlinx.jupyter.common.looksLikeReplCommand import org.jetbrains.kotlinx.jupyter.compiler.CompilerArgsConfigurator @@ -20,16 +36,41 @@ import org.jetbrains.kotlinx.jupyter.dependencies.ScriptDependencyAnnotationHand import org.jetbrains.kotlinx.jupyter.exceptions.LibraryProblemPart import org.jetbrains.kotlinx.jupyter.exceptions.ReplException import org.jetbrains.kotlinx.jupyter.exceptions.rethrowAsLibraryException -import org.jetbrains.kotlinx.jupyter.libraries.* +import org.jetbrains.kotlinx.jupyter.libraries.LibrariesDir +import org.jetbrains.kotlinx.jupyter.libraries.LibrariesProcessorImpl +import org.jetbrains.kotlinx.jupyter.libraries.LibrariesScanner +import org.jetbrains.kotlinx.jupyter.libraries.LibraryResourcesProcessorImpl +import org.jetbrains.kotlinx.jupyter.libraries.ResolutionInfoProvider +import org.jetbrains.kotlinx.jupyter.libraries.ResolutionInfoSwitcher import org.jetbrains.kotlinx.jupyter.magics.CompoundCodePreprocessor import org.jetbrains.kotlinx.jupyter.magics.FullMagicsHandler import org.jetbrains.kotlinx.jupyter.magics.MagicsProcessor -import org.jetbrains.kotlinx.jupyter.repl.* -import org.jetbrains.kotlinx.jupyter.repl.impl.* +import org.jetbrains.kotlinx.jupyter.repl.CellExecutor +import org.jetbrains.kotlinx.jupyter.repl.CompletionResult +import org.jetbrains.kotlinx.jupyter.repl.ContextUpdater +import org.jetbrains.kotlinx.jupyter.repl.InternalEvaluator +import org.jetbrains.kotlinx.jupyter.repl.KotlinCompleter +import org.jetbrains.kotlinx.jupyter.repl.ListErrorsResult +import org.jetbrains.kotlinx.jupyter.repl.impl.BaseKernelHost +import org.jetbrains.kotlinx.jupyter.repl.impl.CellExecutorImpl +import org.jetbrains.kotlinx.jupyter.repl.impl.InternalEvaluatorImpl +import org.jetbrains.kotlinx.jupyter.repl.impl.JupyterCompilerWithCompletion +import org.jetbrains.kotlinx.jupyter.repl.impl.ScriptImportsCollectorImpl +import org.jetbrains.kotlinx.jupyter.repl.impl.SharedReplContext import java.io.File import java.net.URLClassLoader import java.util.concurrent.atomic.AtomicReference -import kotlin.script.experimental.api.* +import kotlin.script.experimental.api.ResultWithDiagnostics +import kotlin.script.experimental.api.ScriptCompilationConfiguration +import kotlin.script.experimental.api.ScriptConfigurationRefinementContext +import kotlin.script.experimental.api.ScriptEvaluationConfiguration +import kotlin.script.experimental.api.asSuccess +import kotlin.script.experimental.api.constructorArgs +import kotlin.script.experimental.api.dependencies +import kotlin.script.experimental.api.fileExtension +import kotlin.script.experimental.api.implicitReceivers +import kotlin.script.experimental.api.refineConfiguration +import kotlin.script.experimental.api.with import kotlin.script.experimental.jvm.BasicJvmReplEvaluator import kotlin.script.experimental.jvm.JvmDependency import kotlin.script.experimental.jvm.baseClassLoader @@ -316,8 +357,6 @@ class ReplForJupyterImpl( private val executor: CellExecutor = CellExecutorImpl(sharedContext) - internal var actualExecutionId: Int = 0 - private fun onAnnotationsHandler(context: ScriptConfigurationRefinementContext): ResultWithDiagnostics { return if (evalContextEnabled) fileAnnotationsProcessor.process(context, currentKernelHost!!) else context.compilationConfiguration.asSuccess() @@ -328,10 +367,10 @@ class ReplForJupyterImpl( * @see ReplCommand */ private fun printVars(isHtmlFormat: Boolean = false) = log.debug( - if (isHtmlFormat) notebook.varsAsHtmlTable() else notebook.varsAsString() + if (isHtmlFormat) notebook.varsAsHtmlTable() else notebook.varsAsString() ) - // todo: causes some tests to fail. perhaps, due to exhausting log calling + // TODO: causes some tests to fail. perhaps, due to exhausting log calling private fun printUsage(cellId: Int, usedVars: Set) { log.debug("Usages for cell $cellId:") usedVars.forEach { @@ -349,20 +388,18 @@ class ReplForJupyterImpl( val compiledData: SerializedCompiledScriptsData val newImports: List - val varsMap : Map - val usageMap: Map> - internalEvaluator.lastExecutedCellId = jupyterId - 1 val result = try { - log.debug("jupyter id: $jupyterId") // == cell id - executor.execute(code, displayHandler) { internalId, codeToExecute -> + log.debug("Current cell id: $jupyterId") + executor.execute(code, displayHandler, currentCellId = jupyterId - 1) { internalId, codeToExecute -> cell = notebook.addCell(internalId, codeToExecute, EvalData(jupyterId, code)) } } finally { compiledData = internalEvaluator.popAddedCompiledScripts() newImports = importsCollector.popAddedImports() - varsMap = internalEvaluator.variablesMap - usageMap = internalEvaluator.usageMap + } + val varsMap = internalEvaluator.variablesHolder + val usageMap = internalEvaluator.varsUsagePerCell cell?.resultVal = result.result.value @@ -381,18 +418,14 @@ class ReplForJupyterImpl( } ?: emptyList() notebook.updateVarsState(varsMap) - printVars() -// printUsage(jupyterId, usageMap[jupyterId - 1]!!) - - //todo: perhaps redundant - // would send serializable version - val mapToSend = mutableMapOf() - varsMap.forEach { - val value = it.value.stringValue - mapToSend[it.key] = value - } - - EvalResult(rendered, EvaluatedSnippetMetadata(newClasspath, compiledData, newImports, mapToSend)) + notebook.usageMap = usageMap + // printVars() + // printUsage(jupyterId, usageMap[jupyterId - 1]!!) + + // TODO: perhaps redundant + // send serializable version + val varsStateUpdate = varsMap.mapValues { it.value.stringValue } + EvalResult(rendered, EvaluatedSnippetMetadata(newClasspath, compiledData, newImports, varsStateUpdate)) } } diff --git a/src/main/kotlin/org/jetbrains/kotlinx/jupyter/repl/CellExecutor.kt b/src/main/kotlin/org/jetbrains/kotlinx/jupyter/repl/CellExecutor.kt index ffa0e03c5..2fc9ab41e 100644 --- a/src/main/kotlin/org/jetbrains/kotlinx/jupyter/repl/CellExecutor.kt +++ b/src/main/kotlin/org/jetbrains/kotlinx/jupyter/repl/CellExecutor.kt @@ -17,6 +17,7 @@ interface CellExecutor : ExecutionHost { processAnnotations: Boolean = true, processMagics: Boolean = true, invokeAfterCallbacks: Boolean = true, + currentCellId: Int = 1, callback: ExecutionStartedCallback? = null ): InternalEvalResult } diff --git a/src/main/kotlin/org/jetbrains/kotlinx/jupyter/repl/InternalEvaluator.kt b/src/main/kotlin/org/jetbrains/kotlinx/jupyter/repl/InternalEvaluator.kt index f1041ad5d..008afeac5 100644 --- a/src/main/kotlin/org/jetbrains/kotlinx/jupyter/repl/InternalEvaluator.kt +++ b/src/main/kotlin/org/jetbrains/kotlinx/jupyter/repl/InternalEvaluator.kt @@ -13,18 +13,15 @@ interface InternalEvaluator { val lastKClass: KClass<*> val lastClassLoader: ClassLoader - val variablesMap: MutableMap + val variablesHolder: MutableMap - val usageMap: MutableMap> - - // todo: perhaps, better to be stateless - var lastExecutedCellId: Int + val varsUsagePerCell: MutableMap> /** * Executes code snippet * @throws IllegalStateException if this method was invoked recursively */ - fun eval(code: Code, onInternalIdGenerated: ((Int) -> Unit)? = null): InternalEvalResult + fun eval(code: Code, cellId: Int = 1, onInternalIdGenerated: ((Int) -> Unit)? = null): InternalEvalResult /** * Pop a serialized form of recently added compiled scripts diff --git a/src/main/kotlin/org/jetbrains/kotlinx/jupyter/repl/impl/CellExecutorImpl.kt b/src/main/kotlin/org/jetbrains/kotlinx/jupyter/repl/impl/CellExecutorImpl.kt index 62a46b7db..7b5dc0942 100644 --- a/src/main/kotlin/org/jetbrains/kotlinx/jupyter/repl/impl/CellExecutorImpl.kt +++ b/src/main/kotlin/org/jetbrains/kotlinx/jupyter/repl/impl/CellExecutorImpl.kt @@ -38,6 +38,7 @@ internal class CellExecutorImpl(private val replContext: SharedReplContext) : Ce processAnnotations: Boolean, processMagics: Boolean, invokeAfterCallbacks: Boolean, + currentCellId: Int, callback: ExecutionStartedCallback? ): InternalEvalResult { with(replContext) { @@ -60,7 +61,7 @@ internal class CellExecutorImpl(private val replContext: SharedReplContext) : Ce } val result = baseHost.withHost(context) { - evaluator.eval(preprocessedCode) { internalId -> + evaluator.eval(preprocessedCode, currentCellId) { internalId -> if (callback != null) callback(internalId, preprocessedCode) } } diff --git a/src/main/kotlin/org/jetbrains/kotlinx/jupyter/repl/impl/InternalEvaluatorImpl.kt b/src/main/kotlin/org/jetbrains/kotlinx/jupyter/repl/impl/InternalEvaluatorImpl.kt index f0fc33768..9df12d709 100644 --- a/src/main/kotlin/org/jetbrains/kotlinx/jupyter/repl/impl/InternalEvaluatorImpl.kt +++ b/src/main/kotlin/org/jetbrains/kotlinx/jupyter/repl/impl/InternalEvaluatorImpl.kt @@ -36,7 +36,8 @@ internal class InternalEvaluatorImpl( private val registeredCompiledScripts = arrayListOf() /** - * Stores cache for val values + * Stores cache for val values. + * Its contents is a part of variablesHolder. */ private val variablesCache = mutableMapOf() @@ -66,20 +67,18 @@ internal class InternalEvaluatorImpl( override val lastClassLoader get() = compiler.lastClassLoader - override val variablesMap = mutableMapOf() + override val variablesHolder = mutableMapOf() - override val usageMap = mutableMapOf>() - - override var lastExecutedCellId: Int = 0 + override val varsUsagePerCell = mutableMapOf>() /** * Tells in which cell a variable was declared */ - private val containingMap: MutableMap = mutableMapOf() + private val varsDeclarationInfo: MutableMap = mutableMapOf() private var isExecuting = false - override fun eval(code: Code, onInternalIdGenerated: ((Int) -> Unit)?): InternalEvalResult { + override fun eval(code: Code, cellId: Int, onInternalIdGenerated: ((Int) -> Unit)?): InternalEvalResult { try { if (isExecuting) { error("Recursive execution is not supported") @@ -114,18 +113,7 @@ internal class InternalEvaluatorImpl( ) is ResultValue.Unit, is ResultValue.Value -> { serializeAndRegisterScript(compiledScript) - if (!usageMap.containsKey(lastExecutedCellId)) { - usageMap[lastExecutedCellId] = mutableSetOf() - } - val visibleDeclarations = getVisibleVariables(resultValue, lastExecutedCellId) - visibleDeclarations.forEach { - val cellSet = usageMap[lastExecutedCellId] ?: return@forEach - containingMap[it.key] = lastExecutedCellId - cellSet += it.key - } - variablesMap += visibleDeclarations - - updateVariablesState(lastExecutedCellId) + updateDataAfterExecution(cellId, resultValue) if (resultValue is ResultValue.Unit) { InternalEvalResult( @@ -161,64 +149,66 @@ internal class InternalEvaluatorImpl( private fun updateVariablesState(cellId: Int) { // remove known modifying usages in this cell - usageMap[cellId]?.removeIf { - containingMap[it] != cellId + varsUsagePerCell[cellId]?.removeIf { + varsDeclarationInfo[it] != cellId } - variablesMap.forEach { + variablesHolder.forEach { val state = it.value - val property = state.propertyKlass - val newValue = property.get(state.scriptInstance) ?: return@forEach + val oldValue = state.stringValue + state.update() - if (state.stringValue != newValue.toString()) { - usageMap[cellId]?.add(it.key) - state.stringValue = newValue.toString() + if (state.stringValue != oldValue) { + varsUsagePerCell[cellId]?.add(it.key) } } } private fun getVisibleVariables(target: ResultValue, cellId: Int): Map { - val klass = target.scriptClass ?: return emptyMap() + val kClass = target.scriptClass ?: return emptyMap() val cellKlassInstance = target.scriptInstance!! - val fields = klass.declaredMemberProperties + val fields = kClass.declaredMemberProperties val ans = mutableMapOf() - fields.forEach { - val casted = it as KProperty1 - ans[casted.name] = if (casted.get(cellKlassInstance) == null) { - return@forEach - } else { - val stringVal = casted.get(cellKlassInstance).toString() - val state = VariableState(casted, cellKlassInstance, stringVal) - // invariant with changes: cache --> varsMap, other way is not allowed - if (it !is KMutableProperty1) { - // it is a replacement - if (variablesCache.containsKey(casted.name)) { - val oldCellID = containingMap[casted.name] - usageMap[oldCellID]?.remove(casted.name) - } - containingMap[casted.name] = cellId - usageMap[cellId]?.add(casted.name) - - variablesCache[casted.name] = state - variablesMap[casted.name] = state - return@forEach + fields.forEach { property -> + property as KProperty1 + val state = VariableState(property, cellKlassInstance) + + // redeclaration of any type + if (varsDeclarationInfo.containsKey(property.name)) { + val oldCellId = varsDeclarationInfo[property.name] + if (oldCellId != cellId) { + varsUsagePerCell[oldCellId]?.remove(property.name) } - state } + // it was val, now it's var + if (property is KMutableProperty1) { + variablesCache.remove(property.name) + } + varsDeclarationInfo[property.name] = cellId + varsUsagePerCell[cellId]?.add(property.name) + // invariant with changes: cache --> varsMap, other way is not allowed + if (property !is KMutableProperty1) { + variablesCache[property.name] = state + variablesHolder[property.name] = state + return@forEach + } + ans[property.name] = state } return ans } - // this is the O(n) implementation - @Deprecated("Back-propagation def search", ReplaceWith("updateVariablesState")) - private fun traverseSeenVarsNaive() { - var lastSnipped = evaluator.lastEvaluatedSnippet?.previous ?: return - val seenVars = mutableSetOf() - do { - val target = lastSnipped.get().result -// updateVariablesMap(target, seenVars) - lastSnipped = lastSnipped.previous ?: break - } while (true) + private fun updateDataAfterExecution(lastExecutionCellId: Int, resultValue: ResultValue) { + varsUsagePerCell.putIfAbsent(lastExecutionCellId, mutableSetOf()) + + val visibleDeclarations = getVisibleVariables(resultValue, lastExecutionCellId) + visibleDeclarations.forEach { + val cellSet = varsUsagePerCell[lastExecutionCellId] ?: return@forEach + varsDeclarationInfo[it.key] = lastExecutionCellId + cellSet += it.key + } + variablesHolder += visibleDeclarations + + updateVariablesState(lastExecutionCellId) } } diff --git a/src/main/kotlin/org/jetbrains/kotlinx/jupyter/util.kt b/src/main/kotlin/org/jetbrains/kotlinx/jupyter/util.kt index d039c9cbd..745bbef29 100644 --- a/src/main/kotlin/org/jetbrains/kotlinx/jupyter/util.kt +++ b/src/main/kotlin/org/jetbrains/kotlinx/jupyter/util.kt @@ -1,6 +1,5 @@ package org.jetbrains.kotlinx.jupyter -import org.jetbrains.kotlinx.jupyter.api.VariableState import org.jetbrains.kotlinx.jupyter.api.bufferedImageRenderer import org.jetbrains.kotlinx.jupyter.codegen.ResultsRenderersProcessor import org.jetbrains.kotlinx.jupyter.compiler.util.SourceCodeImpl @@ -11,73 +10,6 @@ import kotlin.script.experimental.jvm.util.toSourceCodePosition fun List.joinToLines() = joinToString("\n") -// todo : perhaps, create factory class -fun generateHTMLVarsReport(variablesMap: Map): String { - val stringBuilder = StringBuilder( - """ - - - - - """.trimIndent() - ) - stringBuilder.append(generateStyleSection()) - stringBuilder.append("\n\n\n") - stringBuilder.append("

Variables State

\n") - if (variablesMap.isEmpty()) { - stringBuilder.append("

Empty state

\n\n") - stringBuilder.append("\n") - return stringBuilder.toString() - } - - stringBuilder.append(generateVarsTable(variablesMap)) - - stringBuilder.append("\n") - return stringBuilder.toString() -} - -// todo: text is not aligning in a center -fun generateStyleSection(borderPx: Int = 1, paddingPx: Int = 5): String { - return """ - - """.trimIndent() -} - -fun generateVarsTable(variablesMap: Map): String { - val tableBuilder = StringBuilder( - """ - - - - - - - """.trimIndent() - ) - - variablesMap.entries.forEach { - tableBuilder.append( - """ - - - - - """.trimIndent() - ) - } - - return tableBuilder.append("\n
VariableValue
${it.key}${it.value.stringValue}
\n").toString() -} - fun generateDiagnostic(fromLine: Int, fromCol: Int, toLine: Int, toCol: Int, message: String, severity: String) = ScriptDiagnostic( ScriptDiagnostic.unspecifiedError, diff --git a/src/test/kotlin/org/jetbrains/kotlinx/jupyter/test/ApiTest.kt b/src/test/kotlin/org/jetbrains/kotlinx/jupyter/test/ApiTest.kt index ec0bd8321..9df6e7941 100644 --- a/src/test/kotlin/org/jetbrains/kotlinx/jupyter/test/ApiTest.kt +++ b/src/test/kotlin/org/jetbrains/kotlinx/jupyter/test/ApiTest.kt @@ -83,7 +83,7 @@ class ApiTest : AbstractSingleReplTest() { Variable Value - + x 1 diff --git a/src/test/kotlin/org/jetbrains/kotlinx/jupyter/test/repl/CustomLibraryResolverTests.kt b/src/test/kotlin/org/jetbrains/kotlinx/jupyter/test/repl/CustomLibraryResolverTests.kt index 44b21553b..00c60c9b0 100644 --- a/src/test/kotlin/org/jetbrains/kotlinx/jupyter/test/repl/CustomLibraryResolverTests.kt +++ b/src/test/kotlin/org/jetbrains/kotlinx/jupyter/test/repl/CustomLibraryResolverTests.kt @@ -244,7 +244,7 @@ class CustomLibraryResolverTests : AbstractReplTest() { %use lib1 5 """.trimIndent() - repl.execute(code) + repl.execute(code = code) val expectedCodes = listOf( "import java.*", diff --git a/src/test/kotlin/org/jetbrains/kotlinx/jupyter/test/repl/IntegrationApiTests.kt b/src/test/kotlin/org/jetbrains/kotlinx/jupyter/test/repl/IntegrationApiTests.kt index bfdd92cd1..6766aed03 100644 --- a/src/test/kotlin/org/jetbrains/kotlinx/jupyter/test/repl/IntegrationApiTests.kt +++ b/src/test/kotlin/org/jetbrains/kotlinx/jupyter/test/repl/IntegrationApiTests.kt @@ -104,7 +104,7 @@ class IntegrationApiTests { } val repl = makeRepl(lib).trackExecution() - repl.execute("%use mylib\n1") + repl.execute(code = "%use mylib\n1") assertEquals(2, repl.executedCodes.size) assertEquals(1, repl.results[0]) diff --git a/src/test/kotlin/org/jetbrains/kotlinx/jupyter/test/repl/ReplTests.kt b/src/test/kotlin/org/jetbrains/kotlinx/jupyter/test/repl/ReplTests.kt index 4beada6e7..bcec4f626 100644 --- a/src/test/kotlin/org/jetbrains/kotlinx/jupyter/test/repl/ReplTests.kt +++ b/src/test/kotlin/org/jetbrains/kotlinx/jupyter/test/repl/ReplTests.kt @@ -439,9 +439,13 @@ class ReplTests : AbstractSingleReplTest() { assertEquals("org.RDKit.RWMol", res!!::class.qualifiedName) } +} + +class ReplVarsTest : AbstractSingleReplTest() { + override val repl = makeSimpleRepl() @Test - fun testStateConsistency() { + fun testVarsStateConsistency() { assertTrue(repl.notebook.variablesMap.isEmpty()) val res = eval( """ @@ -463,18 +467,18 @@ class ReplTests : AbstractSingleReplTest() { assertEquals("0", varsState["y"]!!.stringValue) assertEquals("47", varsState["z"]!!.stringValue) - varsState["z"]!!.stringValue = "0" + varsState["z"]!!.update() repl.notebook.updateVarsState(varsState) - assertEquals("0", varsState["z"]!!.stringValue) + assertEquals("47", varsState["z"]!!.stringValue) } @Test - fun testEmptyState() { + fun testVarsEmptyState() { val res = eval("3+2") val state = repl.notebook.variablesMap val strState = mutableMapOf() state.forEach { - strState[it.key] = it.value.stringValue + strState[it.key] = it.value.stringValue ?: return@forEach } assertTrue(state.isEmpty()) assertEquals(res.metadata.variablesMap, strState) @@ -493,7 +497,7 @@ class ReplTests : AbstractSingleReplTest() { assertTrue(varsState.isNotEmpty()) val strState = mutableMapOf() varsState.forEach { - strState[it.key] = it.value.stringValue + strState[it.key] = it.value.stringValue ?: return@forEach } val returnedState = res.metadata.variablesMap @@ -502,4 +506,151 @@ class ReplTests : AbstractSingleReplTest() { assertEquals("abc", varsState["y"]!!.stringValue) assertEquals("1", varsState["z"]!!.stringValue) } + + @Test + fun testVarsCaptureSeparateCells() { + eval( + """ + val x = 1 + val y = "abc" + val z = x + """.trimIndent() + ) + val varsState = repl.notebook.variablesMap + assertTrue(varsState.isNotEmpty()) + eval( + """ + val x = "abc" + var y = 123 + val z = x + """.trimIndent(), + jupyterId = 1 + ) + assertTrue(varsState.isNotEmpty()) + assertEquals(3, varsState.size) + assertEquals("abc", varsState["x"]!!.stringValue) + assertEquals("123", varsState["y"]!!.stringValue) + assertEquals("abc", varsState["z"]!!.stringValue) + + eval( + """ + val x = 1024 + y += 123 + """.trimIndent(), + jupyterId = 2 + ) + + assertTrue(varsState.isNotEmpty()) + assertEquals(3, varsState.size) + assertEquals("1024", varsState["x"]!!.stringValue) + assertEquals("${123 * 2}", varsState["y"]!!.stringValue) + assertEquals("abc", varsState["z"]!!.stringValue) + } + + @Test + fun testVarsUsageConsistency() { + eval("3+2") + val state = repl.notebook.usageMap + assertTrue(state.values.size == 1) + assertTrue(state.values.first().isEmpty()) + val setOfNextCell = setOf() + assertEquals(state.values.first(), setOfNextCell) + } + + @Test + fun testVarsDefsUsage() { + eval( + """ + val x = 1 + val z = "abcd" + var f = 47 + """.trimIndent() + ) + val state = repl.notebook.usageMap + assertTrue(state.isNotEmpty()) + assertTrue(state.values.first().isNotEmpty()) + val setOfCell = setOf("z", "f", "x") + assertTrue(state.containsValue(setOfCell)) + } + + @Test + fun testVarsDefNRefUsage() { + eval( + """ + val x = "abcd" + var f = 47 + """.trimIndent() + ) + val state = repl.notebook.usageMap + assertTrue(state.isNotEmpty()) + eval( + """ + val z = 1 + f += f + """.trimIndent() + ) + assertTrue(state.isNotEmpty()) + + val setOfCell = setOf("z", "f", "x") + assertTrue(state.containsValue(setOfCell)) + } + + @Test + fun testSeparateDefsUsage() { + eval( + """ + val x = "abcd" + var f = 47 + """.trimIndent(), + jupyterId = 1 + ) + val state = repl.notebook.usageMap + assertTrue(state[0]!!.contains("x")) + + eval( + """ + val x = 341 + var f = "abcd" + """.trimIndent(), + jupyterId = 2 + ) + assertTrue(state.isNotEmpty()) + assertTrue(state[0]!!.isEmpty()) + assertTrue(state[1]!!.contains("x")) + + val setOfPrevCell = setOf() + val setOfNextCell = setOf("x", "f") + assertEquals(state[0], setOfPrevCell) + assertEquals(state[1], setOfNextCell) + } + + @Test + fun testSeparateCellsUsage() { + eval( + """ + val x = "abcd" + var f = 47 + """.trimIndent(), + jupyterId = 1 + ) + val state = repl.notebook.usageMap + assertTrue(state[0]!!.contains("x")) + + eval( + """ + val x = 341 + f += x + """.trimIndent(), + jupyterId = 2 + ) + assertTrue(state.isNotEmpty()) + assertTrue(state[0]!!.isNotEmpty()) + assertFalse(state[0]!!.contains("x")) + assertTrue(state[1]!!.contains("x")) + + val setOfPrevCell = setOf("f") + val setOfNextCell = setOf("x", "f") + assertEquals(state[0], setOfPrevCell) + assertEquals(state[1], setOfNextCell) + } } diff --git a/src/test/kotlin/org/jetbrains/kotlinx/jupyter/test/repl/TrackedCellExecutor.kt b/src/test/kotlin/org/jetbrains/kotlinx/jupyter/test/repl/TrackedCellExecutor.kt index 30a869f01..33723e1b9 100644 --- a/src/test/kotlin/org/jetbrains/kotlinx/jupyter/test/repl/TrackedCellExecutor.kt +++ b/src/test/kotlin/org/jetbrains/kotlinx/jupyter/test/repl/TrackedCellExecutor.kt @@ -45,14 +45,13 @@ internal class MockedInternalEvaluator : TrackedInternalEvaluator { override val lastClassLoader: ClassLoader = ClassLoader.getSystemClassLoader() override val executedCodes = mutableListOf() - override val variablesMap = mutableMapOf() - override val usageMap = mutableMapOf>() - override var lastExecutedCellId: Int = 0 + override val variablesHolder = mutableMapOf() + override val varsUsagePerCell = mutableMapOf>() override val results: List get() = executedCodes.map { null } - override fun eval(code: Code, onInternalIdGenerated: ((Int) -> Unit)?): InternalEvalResult { + override fun eval(code: Code, cellId: Int, onInternalIdGenerated: ((Int) -> Unit)?): InternalEvalResult { executedCodes.add(code.trimIndent()) return InternalEvalResult(FieldValue(null, null), Unit) } @@ -64,9 +63,9 @@ internal class TrackedInternalEvaluatorImpl(private val baseEvaluator: InternalE override val results = mutableListOf() - override fun eval(code: Code, onInternalIdGenerated: ((Int) -> Unit)?): InternalEvalResult { + override fun eval(code: Code, cellId: Int, onInternalIdGenerated: ((Int) -> Unit)?): InternalEvalResult { executedCodes.add(code.trimIndent()) - val res = baseEvaluator.eval(code, onInternalIdGenerated) + val res = baseEvaluator.eval(code, onInternalIdGenerated = onInternalIdGenerated) results.add(res.result.value) return res } diff --git a/src/test/kotlin/org/jetbrains/kotlinx/jupyter/test/testUtil.kt b/src/test/kotlin/org/jetbrains/kotlinx/jupyter/test/testUtil.kt index 080cef6f2..9bb5a6657 100644 --- a/src/test/kotlin/org/jetbrains/kotlinx/jupyter/test/testUtil.kt +++ b/src/test/kotlin/org/jetbrains/kotlinx/jupyter/test/testUtil.kt @@ -32,25 +32,6 @@ import org.jetbrains.kotlinx.jupyter.libraries.parseLibraryDescriptors import org.jetbrains.kotlinx.jupyter.log import org.jetbrains.kotlinx.jupyter.repl.CompletionResult import java.io.File -import kotlin.collections.Collection -import kotlin.collections.List -import kotlin.collections.Map -import kotlin.collections.MutableList -import kotlin.collections.associate -import kotlin.collections.component1 -import kotlin.collections.component2 -import kotlin.collections.emptyList -import kotlin.collections.filter -import kotlin.collections.forEach -import kotlin.collections.hashMapOf -import kotlin.collections.map -import kotlin.collections.mapKeys -import kotlin.collections.mutableListOf -import kotlin.collections.mutableMapOf -import kotlin.collections.orEmpty -import kotlin.collections.set -import kotlin.collections.single -import kotlin.collections.toMap import kotlin.script.experimental.jvm.util.scriptCompilationClasspathFromContext import kotlin.test.assertEquals @@ -168,6 +149,7 @@ object NotebookMock : Notebook { override val cellsList: Collection get() = emptyList() override val variablesMap = mutableMapOf() + override var usageMap = mapOf>() override fun getCell(id: Int): CodeCellImpl { return cells[id] ?: throw ArrayIndexOutOfBoundsException( From 0b1fdcdce5caebbd503393b08b56e07c0b080846 Mon Sep 17 00:00:00 2001 From: nikolay-egorov Date: Mon, 12 Jul 2021 10:33:53 +0300 Subject: [PATCH 6/8] Add support for private/protected/internal vars; Make better naming and refactor --- .../jetbrains/kotlinx/jupyter/api/Notebook.kt | 6 +- .../kotlinx/jupyter/api/VariableState.kt | 24 --- .../kotlinx/jupyter/api/VariableStateImpl.kt | 29 ++++ .../compiler/util/serializedCompiledScript.kt | 2 +- .../org/jetbrains/kotlinx/jupyter/apiImpl.kt | 28 +-- .../org/jetbrains/kotlinx/jupyter/commands.kt | 2 +- .../org/jetbrains/kotlinx/jupyter/htmlUtil.kt | 28 +-- .../org/jetbrains/kotlinx/jupyter/repl.kt | 25 ++- .../kotlinx/jupyter/repl/CellExecutor.kt | 2 +- .../kotlinx/jupyter/repl/InternalEvaluator.kt | 6 +- .../repl/impl/InternalEvaluatorImpl.kt | 34 ++-- .../jetbrains/kotlinx/jupyter/test/ApiTest.kt | 4 +- .../test/repl/CustomLibraryResolverTests.kt | 2 +- .../jupyter/test/repl/IntegrationApiTests.kt | 2 +- .../kotlinx/jupyter/test/repl/ReplTests.kt | 160 +++++++++++++++--- .../jupyter/test/repl/TrackedCellExecutor.kt | 2 +- .../kotlinx/jupyter/test/testUtil.kt | 6 +- 17 files changed, 241 insertions(+), 121 deletions(-) delete mode 100644 jupyter-lib/api/src/main/kotlin/org/jetbrains/kotlinx/jupyter/api/VariableState.kt create mode 100644 jupyter-lib/api/src/main/kotlin/org/jetbrains/kotlinx/jupyter/api/VariableStateImpl.kt diff --git a/jupyter-lib/api/src/main/kotlin/org/jetbrains/kotlinx/jupyter/api/Notebook.kt b/jupyter-lib/api/src/main/kotlin/org/jetbrains/kotlinx/jupyter/api/Notebook.kt index 7f2a0c4af..f9acfb27a 100644 --- a/jupyter-lib/api/src/main/kotlin/org/jetbrains/kotlinx/jupyter/api/Notebook.kt +++ b/jupyter-lib/api/src/main/kotlin/org/jetbrains/kotlinx/jupyter/api/Notebook.kt @@ -12,15 +12,15 @@ interface Notebook { /** * Current state of visible variables */ - val variablesMap: Map + val variablesState: Map /** * Stores info about useful variables in a cell. * Key: cellId; - * Value: set of variables names. + * Value: set of variable names. * Useful <==> declarations + modifying references */ - var usageMap: Map> + var cellVariables: Map> /** * Mapping allowing to get cell by execution number diff --git a/jupyter-lib/api/src/main/kotlin/org/jetbrains/kotlinx/jupyter/api/VariableState.kt b/jupyter-lib/api/src/main/kotlin/org/jetbrains/kotlinx/jupyter/api/VariableState.kt deleted file mode 100644 index 705e8a30b..000000000 --- a/jupyter-lib/api/src/main/kotlin/org/jetbrains/kotlinx/jupyter/api/VariableState.kt +++ /dev/null @@ -1,24 +0,0 @@ -package org.jetbrains.kotlinx.jupyter.api - -import kotlin.reflect.KProperty -import kotlin.reflect.KProperty1 - -interface VarState { - val propertyKlass: KProperty<*> - val scriptInstance: Any? - val stringValue: String? -} - -data class VariableState( - override val propertyKlass: KProperty1, - override val scriptInstance: Any, -) : VarState { - private var _cachedValue: String? = null - - fun update() { - _cachedValue = propertyKlass.get(scriptInstance).toString() - } - - override val stringValue: String? - get() = _cachedValue -} diff --git a/jupyter-lib/api/src/main/kotlin/org/jetbrains/kotlinx/jupyter/api/VariableStateImpl.kt b/jupyter-lib/api/src/main/kotlin/org/jetbrains/kotlinx/jupyter/api/VariableStateImpl.kt new file mode 100644 index 000000000..c308dfd5e --- /dev/null +++ b/jupyter-lib/api/src/main/kotlin/org/jetbrains/kotlinx/jupyter/api/VariableStateImpl.kt @@ -0,0 +1,29 @@ +package org.jetbrains.kotlinx.jupyter.api + +import kotlin.reflect.KProperty +import kotlin.reflect.KProperty1 +import kotlin.reflect.jvm.isAccessible + +interface VariableState { + val propertyKlass: KProperty<*> + val scriptInstance: Any? + val stringValue: String? +} + +data class VariableStateImpl( + override val propertyKlass: KProperty1, + override val scriptInstance: Any, +) : VariableState { + private var cachedValue: String? = null + + fun update() { + val wasAccessible = propertyKlass.isAccessible + propertyKlass.isAccessible = true + val fieldValue = propertyKlass.get(scriptInstance) + propertyKlass.isAccessible = wasAccessible + cachedValue = fieldValue.toString() + } + + override val stringValue: String? + get() = cachedValue +} diff --git a/jupyter-lib/shared-compiler/src/main/kotlin/org/jetbrains/kotlinx/jupyter/compiler/util/serializedCompiledScript.kt b/jupyter-lib/shared-compiler/src/main/kotlin/org/jetbrains/kotlinx/jupyter/compiler/util/serializedCompiledScript.kt index 6252d77f4..7aa139a35 100644 --- a/jupyter-lib/shared-compiler/src/main/kotlin/org/jetbrains/kotlinx/jupyter/compiler/util/serializedCompiledScript.kt +++ b/jupyter-lib/shared-compiler/src/main/kotlin/org/jetbrains/kotlinx/jupyter/compiler/util/serializedCompiledScript.kt @@ -25,7 +25,7 @@ class EvaluatedSnippetMetadata( val newClasspath: Classpath = emptyList(), val compiledData: SerializedCompiledScriptsData = SerializedCompiledScriptsData.EMPTY, val newImports: List = emptyList(), - val variablesMap: Map = mutableMapOf() + val evaluatedVariablesState: Map = mutableMapOf() ) { companion object { val EMPTY = EvaluatedSnippetMetadata() diff --git a/src/main/kotlin/org/jetbrains/kotlinx/jupyter/apiImpl.kt b/src/main/kotlin/org/jetbrains/kotlinx/jupyter/apiImpl.kt index 6f092cc52..d93cb24e8 100644 --- a/src/main/kotlin/org/jetbrains/kotlinx/jupyter/apiImpl.kt +++ b/src/main/kotlin/org/jetbrains/kotlinx/jupyter/apiImpl.kt @@ -104,9 +104,9 @@ class NotebookImpl( override val cellsList: Collection get() = cells.values - override val variablesMap = mutableMapOf() + override val variablesState = mutableMapOf() - override var usageMap = mapOf>() + override var cellVariables = mapOf>() override fun getCell(id: Int): CodeCellImpl { return cells[id] ?: throw ArrayIndexOutOfBoundsException( @@ -136,21 +136,25 @@ class NotebookImpl( override val jreInfo: JREInfoProvider get() = JavaRuntime - fun updateVarsState(varsMap: Map) { - variablesMap += varsMap + fun updateVariablesState(varsStateUpdate: Map) { + variablesState += varsStateUpdate } - fun varsAsHtmlTable(): String { - return generateHTMLVarsReport(variablesMap) + fun variablesReportAsHTML(): String { + return generateHTMLVarsReport(variablesState) } - fun varsAsString(): String { - if (variablesMap.isEmpty()) return "" - val stringBuilder = StringBuilder("Visible vars: \n") - variablesMap.forEach { (t, u) -> - stringBuilder.append("\t$t : ${u.stringValue}\n") + fun variablesReport(): String { + return if (variablesState.isEmpty()) "" + else { + buildString { + append("Visible vars: \n") + variablesState.forEach { (name, currentState) -> + append("\t$name : ${currentState.stringValue}\n") + } + append('\n') + } } - return stringBuilder.append('\n').toString() } fun addCell( diff --git a/src/main/kotlin/org/jetbrains/kotlinx/jupyter/commands.kt b/src/main/kotlin/org/jetbrains/kotlinx/jupyter/commands.kt index 3e5bbbb9c..25e86d2e1 100644 --- a/src/main/kotlin/org/jetbrains/kotlinx/jupyter/commands.kt +++ b/src/main/kotlin/org/jetbrains/kotlinx/jupyter/commands.kt @@ -58,7 +58,7 @@ fun runCommand(code: String, repl: ReplForJupyter): Response { OkResponseWithMessage(textResult("Current classpath (${cp.count()} paths):\n${cp.joinToString("\n")}")) } ReplCommand.VARS -> { - OkResponseWithMessage(htmlResult(repl.notebook.varsAsHtmlTable())) + OkResponseWithMessage(htmlResult(repl.notebook.variablesReportAsHTML())) } ReplCommand.HELP -> { val commands = ReplCommand.values().asIterable().joinToStringIndented { ":${it.nameForUser} - ${it.desc}" } diff --git a/src/main/kotlin/org/jetbrains/kotlinx/jupyter/htmlUtil.kt b/src/main/kotlin/org/jetbrains/kotlinx/jupyter/htmlUtil.kt index 388b3f41c..b2f667ce1 100644 --- a/src/main/kotlin/org/jetbrains/kotlinx/jupyter/htmlUtil.kt +++ b/src/main/kotlin/org/jetbrains/kotlinx/jupyter/htmlUtil.kt @@ -3,9 +3,9 @@ package org.jetbrains.kotlinx.jupyter import org.jetbrains.kotlinx.jupyter.api.VariableState // TODO : perhaps, create factory class -fun generateHTMLVarsReport(variablesMap: Map): String { +fun generateHTMLVarsReport(variablesState: Map): String { return buildString { - this.append( + append( """ @@ -13,23 +13,23 @@ fun generateHTMLVarsReport(variablesMap: Map): String { """.trimIndent() ) - this.append(generateStyleSection()) - this.append("\n\n\n") - this.append("

Variables State

\n") + append(generateStyleSection()) + append("\n\n\n") + append("

Variables State

\n") - if (variablesMap.isEmpty()) { + if (variablesState.isEmpty()) { this.append("

Empty state

\n\n") this.append("\n") return this.toString() } - this.append(generateVarsTable(variablesMap)) + append(generateVarsTable(variablesState)) - this.append("\n") + append("\n") } } -// TODO: text is not aligning in a center +// TODO: text is not aligned in the center fun generateStyleSection(borderPx: Int = 1, paddingPx: Int = 5): String { //language=HTML val styleSection = """ @@ -47,9 +47,9 @@ fun generateStyleSection(borderPx: Int = 1, paddingPx: Int = 5): String { return styleSection } -fun generateVarsTable(variablesMap: Map): String { +fun generateVarsTable(variablesState: Map): String { return buildString { - this.append( + append( """ @@ -60,8 +60,8 @@ fun generateVarsTable(variablesMap: Map): String { """.trimIndent() ) - variablesMap.entries.forEach { - this.append( + variablesState.entries.forEach { + append( """ @@ -71,6 +71,6 @@ fun generateVarsTable(variablesMap: Map): String { ) } - this.append("\n
${it.key}
\n") + append("\n\n") } } diff --git a/src/main/kotlin/org/jetbrains/kotlinx/jupyter/repl.kt b/src/main/kotlin/org/jetbrains/kotlinx/jupyter/repl.kt index b4179699a..3dfa2cf38 100644 --- a/src/main/kotlin/org/jetbrains/kotlinx/jupyter/repl.kt +++ b/src/main/kotlin/org/jetbrains/kotlinx/jupyter/repl.kt @@ -11,7 +11,6 @@ import org.jetbrains.kotlinx.jupyter.api.ExecutionCallback import org.jetbrains.kotlinx.jupyter.api.KotlinKernelHost import org.jetbrains.kotlinx.jupyter.api.KotlinKernelVersion import org.jetbrains.kotlinx.jupyter.api.Renderable -import org.jetbrains.kotlinx.jupyter.api.VariableState import org.jetbrains.kotlinx.jupyter.codegen.ClassAnnotationsProcessor import org.jetbrains.kotlinx.jupyter.codegen.ClassAnnotationsProcessorImpl import org.jetbrains.kotlinx.jupyter.codegen.FieldsProcessor @@ -366,14 +365,14 @@ class ReplForJupyterImpl( * Used for debug purposes. * @see ReplCommand */ - private fun printVars(isHtmlFormat: Boolean = false) = log.debug( - if (isHtmlFormat) notebook.varsAsHtmlTable() else notebook.varsAsString() + private fun printVariables(isHtmlFormat: Boolean = false) = log.debug( + if (isHtmlFormat) notebook.variablesReportAsHTML() else notebook.variablesReport() ) // TODO: causes some tests to fail. perhaps, due to exhausting log calling - private fun printUsage(cellId: Int, usedVars: Set) { + private fun printUsagesInfo(cellId: Int, usedVariables: Set) { log.debug("Usages for cell $cellId:") - usedVars.forEach { + usedVariables.forEach { log.debug(it) } } @@ -396,10 +395,9 @@ class ReplForJupyterImpl( } finally { compiledData = internalEvaluator.popAddedCompiledScripts() newImports = importsCollector.popAddedImports() - } - val varsMap = internalEvaluator.variablesHolder - val usageMap = internalEvaluator.varsUsagePerCell + val variablesState = internalEvaluator.variablesHolder + val cellVariables = internalEvaluator.cellVariables cell?.resultVal = result.result.value @@ -417,15 +415,14 @@ class ReplForJupyterImpl( updateClasspath() } ?: emptyList() - notebook.updateVarsState(varsMap) - notebook.usageMap = usageMap + notebook.updateVariablesState(variablesState) + notebook.cellVariables = cellVariables // printVars() // printUsage(jupyterId, usageMap[jupyterId - 1]!!) - // TODO: perhaps redundant - // send serializable version - val varsStateUpdate = varsMap.mapValues { it.value.stringValue } - EvalResult(rendered, EvaluatedSnippetMetadata(newClasspath, compiledData, newImports, varsStateUpdate)) + + val variablesStateUpdate = variablesState.mapValues { it.value.stringValue } + EvalResult(rendered, EvaluatedSnippetMetadata(newClasspath, compiledData, newImports, variablesStateUpdate)) } } diff --git a/src/main/kotlin/org/jetbrains/kotlinx/jupyter/repl/CellExecutor.kt b/src/main/kotlin/org/jetbrains/kotlinx/jupyter/repl/CellExecutor.kt index 2fc9ab41e..dc4fccf80 100644 --- a/src/main/kotlin/org/jetbrains/kotlinx/jupyter/repl/CellExecutor.kt +++ b/src/main/kotlin/org/jetbrains/kotlinx/jupyter/repl/CellExecutor.kt @@ -17,7 +17,7 @@ interface CellExecutor : ExecutionHost { processAnnotations: Boolean = true, processMagics: Boolean = true, invokeAfterCallbacks: Boolean = true, - currentCellId: Int = 1, + currentCellId: Int = -1, callback: ExecutionStartedCallback? = null ): InternalEvalResult } diff --git a/src/main/kotlin/org/jetbrains/kotlinx/jupyter/repl/InternalEvaluator.kt b/src/main/kotlin/org/jetbrains/kotlinx/jupyter/repl/InternalEvaluator.kt index 008afeac5..aa546bf41 100644 --- a/src/main/kotlin/org/jetbrains/kotlinx/jupyter/repl/InternalEvaluator.kt +++ b/src/main/kotlin/org/jetbrains/kotlinx/jupyter/repl/InternalEvaluator.kt @@ -13,15 +13,15 @@ interface InternalEvaluator { val lastKClass: KClass<*> val lastClassLoader: ClassLoader - val variablesHolder: MutableMap + val variablesHolder: Map - val varsUsagePerCell: MutableMap> + val cellVariables: Map> /** * Executes code snippet * @throws IllegalStateException if this method was invoked recursively */ - fun eval(code: Code, cellId: Int = 1, onInternalIdGenerated: ((Int) -> Unit)? = null): InternalEvalResult + fun eval(code: Code, cellId: Int = -1, onInternalIdGenerated: ((Int) -> Unit)? = null): InternalEvalResult /** * Pop a serialized form of recently added compiled scripts diff --git a/src/main/kotlin/org/jetbrains/kotlinx/jupyter/repl/impl/InternalEvaluatorImpl.kt b/src/main/kotlin/org/jetbrains/kotlinx/jupyter/repl/impl/InternalEvaluatorImpl.kt index 9df12d709..7e5a010bd 100644 --- a/src/main/kotlin/org/jetbrains/kotlinx/jupyter/repl/impl/InternalEvaluatorImpl.kt +++ b/src/main/kotlin/org/jetbrains/kotlinx/jupyter/repl/impl/InternalEvaluatorImpl.kt @@ -5,6 +5,7 @@ import org.jetbrains.kotlinx.jupyter.ReplEvalRuntimeException import org.jetbrains.kotlinx.jupyter.api.Code import org.jetbrains.kotlinx.jupyter.api.FieldValue import org.jetbrains.kotlinx.jupyter.api.VariableState +import org.jetbrains.kotlinx.jupyter.api.VariableStateImpl import org.jetbrains.kotlinx.jupyter.compiler.CompiledScriptsSerializer import org.jetbrains.kotlinx.jupyter.compiler.util.SerializedCompiledScript import org.jetbrains.kotlinx.jupyter.compiler.util.SerializedCompiledScriptsData @@ -35,12 +36,6 @@ internal class InternalEvaluatorImpl( private val registeredCompiledScripts = arrayListOf() - /** - * Stores cache for val values. - * Its contents is a part of variablesHolder. - */ - private val variablesCache = mutableMapOf() - private fun serializeAndRegisterScript(compiledScript: KJvmCompiledScript) { val serializedData = scriptsSerializer.serialize(compiledScript) registeredCompiledScripts.addAll(serializedData.scripts) @@ -69,7 +64,7 @@ internal class InternalEvaluatorImpl( override val variablesHolder = mutableMapOf() - override val varsUsagePerCell = mutableMapOf>() + override val cellVariables = mutableMapOf>() /** * Tells in which cell a variable was declared @@ -149,47 +144,46 @@ internal class InternalEvaluatorImpl( private fun updateVariablesState(cellId: Int) { // remove known modifying usages in this cell - varsUsagePerCell[cellId]?.removeIf { + cellVariables[cellId]?.removeIf { varsDeclarationInfo[it] != cellId } variablesHolder.forEach { - val state = it.value + val state = it.value as VariableStateImpl val oldValue = state.stringValue state.update() if (state.stringValue != oldValue) { - varsUsagePerCell[cellId]?.add(it.key) + cellVariables[cellId]?.add(it.key) } } } - private fun getVisibleVariables(target: ResultValue, cellId: Int): Map { + private fun getVisibleVariables(target: ResultValue, cellId: Int): Map { val kClass = target.scriptClass ?: return emptyMap() - val cellKlassInstance = target.scriptInstance!! + val cellClassInstance = target.scriptInstance!! val fields = kClass.declaredMemberProperties - val ans = mutableMapOf() + val ans = mutableMapOf() fields.forEach { property -> property as KProperty1 - val state = VariableState(property, cellKlassInstance) + val state = VariableStateImpl(property, cellClassInstance) // redeclaration of any type if (varsDeclarationInfo.containsKey(property.name)) { val oldCellId = varsDeclarationInfo[property.name] if (oldCellId != cellId) { - varsUsagePerCell[oldCellId]?.remove(property.name) + cellVariables[oldCellId]?.remove(property.name) } } // it was val, now it's var if (property is KMutableProperty1) { - variablesCache.remove(property.name) + variablesHolder.remove(property.name) } varsDeclarationInfo[property.name] = cellId - varsUsagePerCell[cellId]?.add(property.name) + cellVariables[cellId]?.add(property.name) // invariant with changes: cache --> varsMap, other way is not allowed if (property !is KMutableProperty1) { - variablesCache[property.name] = state variablesHolder[property.name] = state return@forEach } @@ -199,11 +193,11 @@ internal class InternalEvaluatorImpl( } private fun updateDataAfterExecution(lastExecutionCellId: Int, resultValue: ResultValue) { - varsUsagePerCell.putIfAbsent(lastExecutionCellId, mutableSetOf()) + cellVariables.putIfAbsent(lastExecutionCellId, mutableSetOf()) val visibleDeclarations = getVisibleVariables(resultValue, lastExecutionCellId) visibleDeclarations.forEach { - val cellSet = varsUsagePerCell[lastExecutionCellId] ?: return@forEach + val cellSet = cellVariables[lastExecutionCellId] ?: return@forEach varsDeclarationInfo[it.key] = lastExecutionCellId cellSet += it.key } diff --git a/src/test/kotlin/org/jetbrains/kotlinx/jupyter/test/ApiTest.kt b/src/test/kotlin/org/jetbrains/kotlinx/jupyter/test/ApiTest.kt index 9df6e7941..ff308f141 100644 --- a/src/test/kotlin/org/jetbrains/kotlinx/jupyter/test/ApiTest.kt +++ b/src/test/kotlin/org/jetbrains/kotlinx/jupyter/test/ApiTest.kt @@ -58,8 +58,8 @@ class ApiTest : AbstractSingleReplTest() { "y" to "abc", "z" to "47" ) - assertEquals(res.metadata.variablesMap, varsUpdate) - val htmlText = generateHTMLVarsReport(repl.notebook.variablesMap) + assertEquals(res.metadata.evaluatedVariablesState, varsUpdate) + val htmlText = generateHTMLVarsReport(repl.notebook.variablesState) assertEquals( """ diff --git a/src/test/kotlin/org/jetbrains/kotlinx/jupyter/test/repl/CustomLibraryResolverTests.kt b/src/test/kotlin/org/jetbrains/kotlinx/jupyter/test/repl/CustomLibraryResolverTests.kt index 00c60c9b0..44b21553b 100644 --- a/src/test/kotlin/org/jetbrains/kotlinx/jupyter/test/repl/CustomLibraryResolverTests.kt +++ b/src/test/kotlin/org/jetbrains/kotlinx/jupyter/test/repl/CustomLibraryResolverTests.kt @@ -244,7 +244,7 @@ class CustomLibraryResolverTests : AbstractReplTest() { %use lib1 5 """.trimIndent() - repl.execute(code = code) + repl.execute(code) val expectedCodes = listOf( "import java.*", diff --git a/src/test/kotlin/org/jetbrains/kotlinx/jupyter/test/repl/IntegrationApiTests.kt b/src/test/kotlin/org/jetbrains/kotlinx/jupyter/test/repl/IntegrationApiTests.kt index 6766aed03..bfdd92cd1 100644 --- a/src/test/kotlin/org/jetbrains/kotlinx/jupyter/test/repl/IntegrationApiTests.kt +++ b/src/test/kotlin/org/jetbrains/kotlinx/jupyter/test/repl/IntegrationApiTests.kt @@ -104,7 +104,7 @@ class IntegrationApiTests { } val repl = makeRepl(lib).trackExecution() - repl.execute(code = "%use mylib\n1") + repl.execute("%use mylib\n1") assertEquals(2, repl.executedCodes.size) assertEquals(1, repl.results[0]) diff --git a/src/test/kotlin/org/jetbrains/kotlinx/jupyter/test/repl/ReplTests.kt b/src/test/kotlin/org/jetbrains/kotlinx/jupyter/test/repl/ReplTests.kt index bcec4f626..580228669 100644 --- a/src/test/kotlin/org/jetbrains/kotlinx/jupyter/test/repl/ReplTests.kt +++ b/src/test/kotlin/org/jetbrains/kotlinx/jupyter/test/repl/ReplTests.kt @@ -5,6 +5,7 @@ import kotlinx.coroutines.runBlocking import kotlinx.serialization.encodeToString import kotlinx.serialization.json.Json import org.jetbrains.kotlinx.jupyter.OutputConfig +import org.jetbrains.kotlinx.jupyter.api.VariableStateImpl import org.jetbrains.kotlinx.jupyter.exceptions.ReplCompilerException import org.jetbrains.kotlinx.jupyter.generateDiagnostic import org.jetbrains.kotlinx.jupyter.generateDiagnosticFromAbsolute @@ -446,7 +447,7 @@ class ReplVarsTest : AbstractSingleReplTest() { @Test fun testVarsStateConsistency() { - assertTrue(repl.notebook.variablesMap.isEmpty()) + assertTrue(repl.notebook.variablesState.isEmpty()) val res = eval( """ val x = 1 @@ -460,28 +461,28 @@ class ReplVarsTest : AbstractSingleReplTest() { "y" to "0", "z" to "47" ) - assertEquals(res.metadata.variablesMap, varsUpdate) - assertFalse(repl.notebook.variablesMap.isEmpty()) - val varsState = repl.notebook.variablesMap + assertEquals(res.metadata.evaluatedVariablesState, varsUpdate) + assertFalse(repl.notebook.variablesState.isEmpty()) + val varsState = repl.notebook.variablesState assertEquals("1", varsState["x"]!!.stringValue) assertEquals("0", varsState["y"]!!.stringValue) assertEquals("47", varsState["z"]!!.stringValue) - varsState["z"]!!.update() - repl.notebook.updateVarsState(varsState) + (varsState["z"]!! as VariableStateImpl).update() + repl.notebook.updateVariablesState(varsState) assertEquals("47", varsState["z"]!!.stringValue) } @Test fun testVarsEmptyState() { val res = eval("3+2") - val state = repl.notebook.variablesMap + val state = repl.notebook.variablesState val strState = mutableMapOf() state.forEach { strState[it.key] = it.value.stringValue ?: return@forEach } assertTrue(state.isEmpty()) - assertEquals(res.metadata.variablesMap, strState) + assertEquals(res.metadata.evaluatedVariablesState, strState) } @Test @@ -493,14 +494,14 @@ class ReplVarsTest : AbstractSingleReplTest() { val z = x """.trimIndent() ) - val varsState = repl.notebook.variablesMap + val varsState = repl.notebook.variablesState assertTrue(varsState.isNotEmpty()) val strState = mutableMapOf() varsState.forEach { strState[it.key] = it.value.stringValue ?: return@forEach } - val returnedState = res.metadata.variablesMap + val returnedState = res.metadata.evaluatedVariablesState assertEquals(strState, returnedState) assertEquals("1", varsState["x"]!!.stringValue) assertEquals("abc", varsState["y"]!!.stringValue) @@ -516,7 +517,7 @@ class ReplVarsTest : AbstractSingleReplTest() { val z = x """.trimIndent() ) - val varsState = repl.notebook.variablesMap + val varsState = repl.notebook.variablesState assertTrue(varsState.isNotEmpty()) eval( """ @@ -547,10 +548,74 @@ class ReplVarsTest : AbstractSingleReplTest() { assertEquals("abc", varsState["z"]!!.stringValue) } + @Test + fun testPrivateVarsCapture() { + val res = eval( + """ + private val x = 1 + private val y = "abc" + val z = x + """.trimIndent() + ) + val varsState = repl.notebook.variablesState + assertTrue(varsState.isNotEmpty()) + val strState = mutableMapOf() + varsState.forEach { + strState[it.key] = it.value.stringValue ?: return@forEach + } + + val returnedState = res.metadata.evaluatedVariablesState + assertEquals(strState, returnedState) + assertEquals("1", varsState["x"]!!.stringValue) + assertEquals("abc", varsState["y"]!!.stringValue) + assertEquals("1", varsState["z"]!!.stringValue) + } + + + @Test + fun testPrivateVarsCaptureSeparateCells() { + eval( + """ + private val x = 1 + private val y = "abc" + private val z = x + """.trimIndent() + ) + val varsState = repl.notebook.variablesState + assertTrue(varsState.isNotEmpty()) + eval( + """ + private val x = "abc" + var y = 123 + private val z = x + """.trimIndent(), + jupyterId = 1 + ) + assertTrue(varsState.isNotEmpty()) + assertEquals(3, varsState.size) + assertEquals("abc", varsState["x"]!!.stringValue) + assertEquals("123", varsState["y"]!!.stringValue) + assertEquals("abc", varsState["z"]!!.stringValue) + + eval( + """ + private val x = 1024 + y += x + """.trimIndent(), + jupyterId = 2 + ) + + assertTrue(varsState.isNotEmpty()) + assertEquals(3, varsState.size) + assertEquals("1024", varsState["x"]!!.stringValue) + assertEquals("${123 + 1024}", varsState["y"]!!.stringValue) + assertEquals("abc", varsState["z"]!!.stringValue) + } + @Test fun testVarsUsageConsistency() { eval("3+2") - val state = repl.notebook.usageMap + val state = repl.notebook.cellVariables assertTrue(state.values.size == 1) assertTrue(state.values.first().isEmpty()) val setOfNextCell = setOf() @@ -566,7 +631,7 @@ class ReplVarsTest : AbstractSingleReplTest() { var f = 47 """.trimIndent() ) - val state = repl.notebook.usageMap + val state = repl.notebook.cellVariables assertTrue(state.isNotEmpty()) assertTrue(state.values.first().isNotEmpty()) val setOfCell = setOf("z", "f", "x") @@ -581,7 +646,7 @@ class ReplVarsTest : AbstractSingleReplTest() { var f = 47 """.trimIndent() ) - val state = repl.notebook.usageMap + val state = repl.notebook.cellVariables assertTrue(state.isNotEmpty()) eval( """ @@ -595,6 +660,28 @@ class ReplVarsTest : AbstractSingleReplTest() { assertTrue(state.containsValue(setOfCell)) } + @Test + fun testPrivateVarsDefNRefUsage() { + eval( + """ + val x = 124 + private var f = "abcd" + """.trimIndent() + ) + val state = repl.notebook.cellVariables + assertTrue(state.isNotEmpty()) + eval( + """ + private var z = 1 + z += x + """.trimIndent() + ) + assertTrue(state.isNotEmpty()) + + val setOfCell = setOf("z", "f", "x") + assertTrue(state.containsValue(setOfCell)) + } + @Test fun testSeparateDefsUsage() { eval( @@ -604,7 +691,7 @@ class ReplVarsTest : AbstractSingleReplTest() { """.trimIndent(), jupyterId = 1 ) - val state = repl.notebook.usageMap + val state = repl.notebook.cellVariables assertTrue(state[0]!!.contains("x")) eval( @@ -625,31 +712,64 @@ class ReplVarsTest : AbstractSingleReplTest() { } @Test - fun testSeparateCellsUsage() { + fun testSeparatePrivateDefsUsage() { eval( """ - val x = "abcd" - var f = 47 + private val x = "abcd" + private var f = 47 """.trimIndent(), jupyterId = 1 ) - val state = repl.notebook.usageMap + val state = repl.notebook.cellVariables assertTrue(state[0]!!.contains("x")) eval( """ val x = 341 + private var f = "abcd" + """.trimIndent(), + jupyterId = 2 + ) + assertTrue(state.isNotEmpty()) + assertTrue(state[0]!!.isEmpty()) + assertTrue(state[1]!!.contains("x")) + + val setOfPrevCell = setOf() + val setOfNextCell = setOf("x", "f") + assertEquals(state[0], setOfPrevCell) + assertEquals(state[1], setOfNextCell) + } + + @Test + fun testSeparatePrivateCellsUsage() { + eval( + """ + private val x = "abcd" + var f = 47 + internal val z = 47 + """.trimIndent(), + jupyterId = 1 + ) + val state = repl.notebook.cellVariables + assertTrue(state[0]!!.contains("x")) + assertTrue(state[0]!!.contains("z")) + + eval( + """ + private val x = 341 f += x + protected val z = "abcd" """.trimIndent(), jupyterId = 2 ) assertTrue(state.isNotEmpty()) assertTrue(state[0]!!.isNotEmpty()) assertFalse(state[0]!!.contains("x")) + assertFalse(state[0]!!.contains("z")) assertTrue(state[1]!!.contains("x")) val setOfPrevCell = setOf("f") - val setOfNextCell = setOf("x", "f") + val setOfNextCell = setOf("x", "f", "z") assertEquals(state[0], setOfPrevCell) assertEquals(state[1], setOfNextCell) } diff --git a/src/test/kotlin/org/jetbrains/kotlinx/jupyter/test/repl/TrackedCellExecutor.kt b/src/test/kotlin/org/jetbrains/kotlinx/jupyter/test/repl/TrackedCellExecutor.kt index 33723e1b9..de2f6d249 100644 --- a/src/test/kotlin/org/jetbrains/kotlinx/jupyter/test/repl/TrackedCellExecutor.kt +++ b/src/test/kotlin/org/jetbrains/kotlinx/jupyter/test/repl/TrackedCellExecutor.kt @@ -46,7 +46,7 @@ internal class MockedInternalEvaluator : TrackedInternalEvaluator { override val executedCodes = mutableListOf() override val variablesHolder = mutableMapOf() - override val varsUsagePerCell = mutableMapOf>() + override val cellVariables = mutableMapOf>() override val results: List get() = executedCodes.map { null } diff --git a/src/test/kotlin/org/jetbrains/kotlinx/jupyter/test/testUtil.kt b/src/test/kotlin/org/jetbrains/kotlinx/jupyter/test/testUtil.kt index 9bb5a6657..81737eb15 100644 --- a/src/test/kotlin/org/jetbrains/kotlinx/jupyter/test/testUtil.kt +++ b/src/test/kotlin/org/jetbrains/kotlinx/jupyter/test/testUtil.kt @@ -13,7 +13,7 @@ import org.jetbrains.kotlinx.jupyter.api.JREInfoProvider import org.jetbrains.kotlinx.jupyter.api.KotlinKernelVersion import org.jetbrains.kotlinx.jupyter.api.Notebook import org.jetbrains.kotlinx.jupyter.api.RenderersProcessor -import org.jetbrains.kotlinx.jupyter.api.VariableState +import org.jetbrains.kotlinx.jupyter.api.VariableStateImpl import org.jetbrains.kotlinx.jupyter.api.libraries.ExecutionHost import org.jetbrains.kotlinx.jupyter.api.libraries.JupyterIntegration import org.jetbrains.kotlinx.jupyter.api.libraries.LibraryDefinition @@ -148,8 +148,8 @@ object NotebookMock : Notebook { override val cellsList: Collection get() = emptyList() - override val variablesMap = mutableMapOf() - override var usageMap = mapOf>() + override val variablesState = mutableMapOf() + override var cellVariables = mapOf>() override fun getCell(id: Int): CodeCellImpl { return cells[id] ?: throw ArrayIndexOutOfBoundsException( From 58d0a742ae8c92ca2aac2f90c750354d52843515 Mon Sep 17 00:00:00 2001 From: nikolay-egorov Date: Mon, 12 Jul 2021 13:42:19 +0300 Subject: [PATCH 7/8] Fix HTML table report; encapsulate logic about cell variables --- .../org/jetbrains/kotlinx/jupyter/htmlUtil.kt | 28 +++++-------- .../repl/impl/InternalEvaluatorImpl.kt | 39 +++++------------- .../org/jetbrains/kotlinx/jupyter/util.kt | 40 +++++++++++++++++++ .../jetbrains/kotlinx/jupyter/test/ApiTest.kt | 17 +++----- .../kotlinx/jupyter/test/repl/ReplTests.kt | 1 - 5 files changed, 65 insertions(+), 60 deletions(-) diff --git a/src/main/kotlin/org/jetbrains/kotlinx/jupyter/htmlUtil.kt b/src/main/kotlin/org/jetbrains/kotlinx/jupyter/htmlUtil.kt index b2f667ce1..3ea140b72 100644 --- a/src/main/kotlin/org/jetbrains/kotlinx/jupyter/htmlUtil.kt +++ b/src/main/kotlin/org/jetbrains/kotlinx/jupyter/htmlUtil.kt @@ -2,30 +2,19 @@ package org.jetbrains.kotlinx.jupyter import org.jetbrains.kotlinx.jupyter.api.VariableState +const val varsTableStyleClass = "variables_table" + // TODO : perhaps, create factory class fun generateHTMLVarsReport(variablesState: Map): String { return buildString { - append( - """ - - - - - """.trimIndent() - ) append(generateStyleSection()) - append("\n\n\n") - append("

Variables State

\n") - if (variablesState.isEmpty()) { - this.append("

Empty state

\n\n") - this.append("\n") - return this.toString() + append("

Variables State's Empty

\n") + return toString() } + append("

Variables State

\n") append(generateVarsTable(variablesState)) - - append("\n") } } @@ -34,15 +23,16 @@ fun generateStyleSection(borderPx: Int = 1, paddingPx: Int = 5): String { //language=HTML val styleSection = """ + """.trimIndent() return styleSection } @@ -51,7 +41,7 @@ fun generateVarsTable(variablesState: Map): String { return buildString { append( """ - +
diff --git a/src/main/kotlin/org/jetbrains/kotlinx/jupyter/repl/impl/InternalEvaluatorImpl.kt b/src/main/kotlin/org/jetbrains/kotlinx/jupyter/repl/impl/InternalEvaluatorImpl.kt index 7e5a010bd..97eb34b35 100644 --- a/src/main/kotlin/org/jetbrains/kotlinx/jupyter/repl/impl/InternalEvaluatorImpl.kt +++ b/src/main/kotlin/org/jetbrains/kotlinx/jupyter/repl/impl/InternalEvaluatorImpl.kt @@ -2,6 +2,7 @@ package org.jetbrains.kotlinx.jupyter.repl.impl import kotlinx.coroutines.runBlocking import org.jetbrains.kotlinx.jupyter.ReplEvalRuntimeException +import org.jetbrains.kotlinx.jupyter.VariablesUsagesPerCellWatcher import org.jetbrains.kotlinx.jupyter.api.Code import org.jetbrains.kotlinx.jupyter.api.FieldValue import org.jetbrains.kotlinx.jupyter.api.VariableState @@ -64,12 +65,10 @@ internal class InternalEvaluatorImpl( override val variablesHolder = mutableMapOf() - override val cellVariables = mutableMapOf>() + override val cellVariables: Map> + get() = variablesWatcher.cellVariables - /** - * Tells in which cell a variable was declared - */ - private val varsDeclarationInfo: MutableMap = mutableMapOf() + private val variablesWatcher: VariablesUsagesPerCellWatcher = VariablesUsagesPerCellWatcher() private var isExecuting = false @@ -143,10 +142,7 @@ internal class InternalEvaluatorImpl( } private fun updateVariablesState(cellId: Int) { - // remove known modifying usages in this cell - cellVariables[cellId]?.removeIf { - varsDeclarationInfo[it] != cellId - } + variablesWatcher.removeOldUsages(cellId) variablesHolder.forEach { val state = it.value as VariableStateImpl @@ -154,7 +150,7 @@ internal class InternalEvaluatorImpl( state.update() if (state.stringValue != oldValue) { - cellVariables[cellId]?.add(it.key) + variablesWatcher.addUsage(cellId, it.key) } } } @@ -168,21 +164,13 @@ internal class InternalEvaluatorImpl( fields.forEach { property -> property as KProperty1 val state = VariableStateImpl(property, cellClassInstance) + variablesWatcher.addDeclaration(cellId, property.name) - // redeclaration of any type - if (varsDeclarationInfo.containsKey(property.name)) { - val oldCellId = varsDeclarationInfo[property.name] - if (oldCellId != cellId) { - cellVariables[oldCellId]?.remove(property.name) - } - } // it was val, now it's var if (property is KMutableProperty1) { variablesHolder.remove(property.name) } - varsDeclarationInfo[property.name] = cellId - cellVariables[cellId]?.add(property.name) - // invariant with changes: cache --> varsMap, other way is not allowed + if (property !is KMutableProperty1) { variablesHolder[property.name] = state return@forEach @@ -193,15 +181,8 @@ internal class InternalEvaluatorImpl( } private fun updateDataAfterExecution(lastExecutionCellId: Int, resultValue: ResultValue) { - cellVariables.putIfAbsent(lastExecutionCellId, mutableSetOf()) - - val visibleDeclarations = getVisibleVariables(resultValue, lastExecutionCellId) - visibleDeclarations.forEach { - val cellSet = cellVariables[lastExecutionCellId] ?: return@forEach - varsDeclarationInfo[it.key] = lastExecutionCellId - cellSet += it.key - } - variablesHolder += visibleDeclarations + variablesWatcher.ensureStorageCreation(lastExecutionCellId) + variablesHolder += getVisibleVariables(resultValue, lastExecutionCellId) updateVariablesState(lastExecutionCellId) } diff --git a/src/main/kotlin/org/jetbrains/kotlinx/jupyter/util.kt b/src/main/kotlin/org/jetbrains/kotlinx/jupyter/util.kt index 745bbef29..b8d29e65f 100644 --- a/src/main/kotlin/org/jetbrains/kotlinx/jupyter/util.kt +++ b/src/main/kotlin/org/jetbrains/kotlinx/jupyter/util.kt @@ -68,3 +68,43 @@ fun Int.toSourceCodePositionWithNewAbsolute(code: SourceCode, newCode: SourceCod fun ResultsRenderersProcessor.registerDefaultRenderers() { register(bufferedImageRenderer) } + +/** + * Stores info about where a variable Y was declared and info about what are they at the address X. + * T: key, stands for a way of addressing variables, e.g. address. + * V: value, from Variable, choose any suitable type for your variable reference. + * Default: Int, String + */ +class VariablesUsagesPerCellWatcher { + val cellVariables = mutableMapOf>() + + /** + * Tells in which cell a variable was declared + */ + private val variablesDeclarationInfo: MutableMap = mutableMapOf() + + fun addDeclaration(address: T, variableRef: U) { + ensureStorageCreation(address) + + // redeclaration of any type + if (variablesDeclarationInfo.containsKey(variableRef)) { + val oldCellId = variablesDeclarationInfo[variableRef] + if (oldCellId != address) { + cellVariables[oldCellId]?.remove(variableRef) + } + } + variablesDeclarationInfo[variableRef] = address + cellVariables[address]?.add(variableRef) + } + + fun addUsage(address: T, variableRef: U) = cellVariables[address]?.add(variableRef) + + fun removeOldUsages(newAddress: T) { + // remove known modifying usages in this cell + cellVariables[newAddress]?.removeIf { + variablesDeclarationInfo[it] != newAddress + } + } + + fun ensureStorageCreation(address: T) = cellVariables.putIfAbsent(address, mutableSetOf()) +} diff --git a/src/test/kotlin/org/jetbrains/kotlinx/jupyter/test/ApiTest.kt b/src/test/kotlin/org/jetbrains/kotlinx/jupyter/test/ApiTest.kt index ff308f141..2bf659d3a 100644 --- a/src/test/kotlin/org/jetbrains/kotlinx/jupyter/test/ApiTest.kt +++ b/src/test/kotlin/org/jetbrains/kotlinx/jupyter/test/ApiTest.kt @@ -4,6 +4,7 @@ import org.jetbrains.kotlinx.jupyter.EvalResult import org.jetbrains.kotlinx.jupyter.generateHTMLVarsReport import org.jetbrains.kotlinx.jupyter.repl.impl.getSimpleCompiler import org.jetbrains.kotlinx.jupyter.test.repl.AbstractSingleReplTest +import org.jetbrains.kotlinx.jupyter.varsTableStyleClass import org.junit.jupiter.api.Test import kotlin.script.experimental.api.ScriptCompilationConfiguration import kotlin.script.experimental.api.ScriptEvaluationConfiguration @@ -62,23 +63,18 @@ class ApiTest : AbstractSingleReplTest() { val htmlText = generateHTMLVarsReport(repl.notebook.variablesState) assertEquals( """ - - - - - -

Variables State

-
Variable Value
+

Variables State

+
@@ -94,8 +90,7 @@ class ApiTest : AbstractSingleReplTest() {
Variable Value47
- - + """.trimIndent(), htmlText ) diff --git a/src/test/kotlin/org/jetbrains/kotlinx/jupyter/test/repl/ReplTests.kt b/src/test/kotlin/org/jetbrains/kotlinx/jupyter/test/repl/ReplTests.kt index 580228669..15593417c 100644 --- a/src/test/kotlin/org/jetbrains/kotlinx/jupyter/test/repl/ReplTests.kt +++ b/src/test/kotlin/org/jetbrains/kotlinx/jupyter/test/repl/ReplTests.kt @@ -571,7 +571,6 @@ class ReplVarsTest : AbstractSingleReplTest() { assertEquals("1", varsState["z"]!!.stringValue) } - @Test fun testPrivateVarsCaptureSeparateCells() { eval( From 6ec1281be9199aeab8d66db86cd4e8abc55391d6 Mon Sep 17 00:00:00 2001 From: nikolay-egorov Date: Tue, 13 Jul 2021 18:06:22 +0300 Subject: [PATCH 8/8] Add value property to VariableState; minor renaming --- .../jetbrains/kotlinx/jupyter/api/Notebook.kt | 2 +- .../kotlinx/jupyter/api/VariableState.kt | 33 +++++++++++++ .../kotlinx/jupyter/api/VariableStateImpl.kt | 29 ------------ .../org/jetbrains/kotlinx/jupyter/apiImpl.kt | 10 +++- .../org/jetbrains/kotlinx/jupyter/htmlUtil.kt | 20 ++++---- .../org/jetbrains/kotlinx/jupyter/repl.kt | 27 +++++------ .../repl/impl/InternalEvaluatorImpl.kt | 5 +- .../org/jetbrains/kotlinx/jupyter/util.kt | 18 ++++---- .../kotlinx/jupyter/test/repl/ReplTests.kt | 46 ++++++++++--------- .../kotlinx/jupyter/test/testUtil.kt | 11 ++++- 10 files changed, 111 insertions(+), 90 deletions(-) create mode 100644 jupyter-lib/api/src/main/kotlin/org/jetbrains/kotlinx/jupyter/api/VariableState.kt delete mode 100644 jupyter-lib/api/src/main/kotlin/org/jetbrains/kotlinx/jupyter/api/VariableStateImpl.kt diff --git a/jupyter-lib/api/src/main/kotlin/org/jetbrains/kotlinx/jupyter/api/Notebook.kt b/jupyter-lib/api/src/main/kotlin/org/jetbrains/kotlinx/jupyter/api/Notebook.kt index f9acfb27a..65caa72c6 100644 --- a/jupyter-lib/api/src/main/kotlin/org/jetbrains/kotlinx/jupyter/api/Notebook.kt +++ b/jupyter-lib/api/src/main/kotlin/org/jetbrains/kotlinx/jupyter/api/Notebook.kt @@ -20,7 +20,7 @@ interface Notebook { * Value: set of variable names. * Useful <==> declarations + modifying references */ - var cellVariables: Map> + val cellVariables: Map> /** * Mapping allowing to get cell by execution number diff --git a/jupyter-lib/api/src/main/kotlin/org/jetbrains/kotlinx/jupyter/api/VariableState.kt b/jupyter-lib/api/src/main/kotlin/org/jetbrains/kotlinx/jupyter/api/VariableState.kt new file mode 100644 index 000000000..d88ea7086 --- /dev/null +++ b/jupyter-lib/api/src/main/kotlin/org/jetbrains/kotlinx/jupyter/api/VariableState.kt @@ -0,0 +1,33 @@ +package org.jetbrains.kotlinx.jupyter.api + +import kotlin.reflect.KProperty +import kotlin.reflect.KProperty1 +import kotlin.reflect.jvm.isAccessible + +interface VariableState { + val property: KProperty<*> + val scriptInstance: Any? + val stringValue: String? + val value: Any? +} + +data class VariableStateImpl( + override val property: KProperty1, + override val scriptInstance: Any, +) : VariableState { + private var cachedValue: Any? = null + + fun update() { + val wasAccessible = property.isAccessible + property.isAccessible = true + val fieldValue = property.get(scriptInstance) + property.isAccessible = wasAccessible + cachedValue = fieldValue + } + + override val stringValue: String? + get() = cachedValue?.toString() + + override val value: Any? + get() = cachedValue +} diff --git a/jupyter-lib/api/src/main/kotlin/org/jetbrains/kotlinx/jupyter/api/VariableStateImpl.kt b/jupyter-lib/api/src/main/kotlin/org/jetbrains/kotlinx/jupyter/api/VariableStateImpl.kt deleted file mode 100644 index c308dfd5e..000000000 --- a/jupyter-lib/api/src/main/kotlin/org/jetbrains/kotlinx/jupyter/api/VariableStateImpl.kt +++ /dev/null @@ -1,29 +0,0 @@ -package org.jetbrains.kotlinx.jupyter.api - -import kotlin.reflect.KProperty -import kotlin.reflect.KProperty1 -import kotlin.reflect.jvm.isAccessible - -interface VariableState { - val propertyKlass: KProperty<*> - val scriptInstance: Any? - val stringValue: String? -} - -data class VariableStateImpl( - override val propertyKlass: KProperty1, - override val scriptInstance: Any, -) : VariableState { - private var cachedValue: String? = null - - fun update() { - val wasAccessible = propertyKlass.isAccessible - propertyKlass.isAccessible = true - val fieldValue = propertyKlass.get(scriptInstance) - propertyKlass.isAccessible = wasAccessible - cachedValue = fieldValue.toString() - } - - override val stringValue: String? - get() = cachedValue -} diff --git a/src/main/kotlin/org/jetbrains/kotlinx/jupyter/apiImpl.kt b/src/main/kotlin/org/jetbrains/kotlinx/jupyter/apiImpl.kt index d93cb24e8..f233ac330 100644 --- a/src/main/kotlin/org/jetbrains/kotlinx/jupyter/apiImpl.kt +++ b/src/main/kotlin/org/jetbrains/kotlinx/jupyter/apiImpl.kt @@ -10,6 +10,7 @@ import org.jetbrains.kotlinx.jupyter.api.KotlinKernelVersion import org.jetbrains.kotlinx.jupyter.api.Notebook import org.jetbrains.kotlinx.jupyter.api.RenderersProcessor import org.jetbrains.kotlinx.jupyter.api.VariableState +import org.jetbrains.kotlinx.jupyter.repl.InternalEvaluator class DisplayResultWrapper private constructor( val display: DisplayResult, @@ -106,7 +107,8 @@ class NotebookImpl( override val variablesState = mutableMapOf() - override var cellVariables = mapOf>() + override val cellVariables: Map> + get() = currentCellVariables override fun getCell(id: Int): CodeCellImpl { return cells[id] ?: throw ArrayIndexOutOfBoundsException( @@ -118,6 +120,7 @@ class NotebookImpl( return getCell(id).result } + private var currentCellVariables = mapOf>() private val history = arrayListOf() private var mainCellCreated = false @@ -136,6 +139,11 @@ class NotebookImpl( override val jreInfo: JREInfoProvider get() = JavaRuntime + fun updateVariablesState(evaluator: InternalEvaluator) { + variablesState += evaluator.variablesHolder + currentCellVariables = evaluator.cellVariables + } + fun updateVariablesState(varsStateUpdate: Map) { variablesState += varsStateUpdate } diff --git a/src/main/kotlin/org/jetbrains/kotlinx/jupyter/htmlUtil.kt b/src/main/kotlin/org/jetbrains/kotlinx/jupyter/htmlUtil.kt index 3ea140b72..943255f4f 100644 --- a/src/main/kotlin/org/jetbrains/kotlinx/jupyter/htmlUtil.kt +++ b/src/main/kotlin/org/jetbrains/kotlinx/jupyter/htmlUtil.kt @@ -4,7 +4,6 @@ import org.jetbrains.kotlinx.jupyter.api.VariableState const val varsTableStyleClass = "variables_table" -// TODO : perhaps, create factory class fun generateHTMLVarsReport(variablesState: Map): String { return buildString { append(generateStyleSection()) @@ -18,7 +17,6 @@ fun generateHTMLVarsReport(variablesState: Map): String { } } -// TODO: text is not aligned in the center fun generateStyleSection(borderPx: Int = 1, paddingPx: Int = 5): String { //language=HTML val styleSection = """ @@ -41,11 +39,11 @@ fun generateVarsTable(variablesState: Map): String { return buildString { append( """ - - - - - +
VariableValue
+ + + + """.trimIndent() ) @@ -53,10 +51,10 @@ fun generateVarsTable(variablesState: Map): String { variablesState.entries.forEach { append( """ - - - - + + + + """.trimIndent() ) } diff --git a/src/main/kotlin/org/jetbrains/kotlinx/jupyter/repl.kt b/src/main/kotlin/org/jetbrains/kotlinx/jupyter/repl.kt index 3dfa2cf38..96bebd0e9 100644 --- a/src/main/kotlin/org/jetbrains/kotlinx/jupyter/repl.kt +++ b/src/main/kotlin/org/jetbrains/kotlinx/jupyter/repl.kt @@ -369,12 +369,17 @@ class ReplForJupyterImpl( if (isHtmlFormat) notebook.variablesReportAsHTML() else notebook.variablesReport() ) - // TODO: causes some tests to fail. perhaps, due to exhausting log calling - private fun printUsagesInfo(cellId: Int, usedVariables: Set) { - log.debug("Usages for cell $cellId:") - usedVariables.forEach { - log.debug(it) - } + private fun printUsagesInfo(cellId: Int, usedVariables: Set?) { + log.debug(buildString { + if (usedVariables == null || usedVariables.isEmpty()) { + append("No usages for cell $cellId") + return@buildString + } + append("Usages for cell $cellId:\n") + usedVariables.forEach { + append(it + "\n") + } + }) } override fun eval(code: Code, displayHandler: DisplayHandler?, jupyterId: Int): EvalResult { @@ -396,9 +401,6 @@ class ReplForJupyterImpl( compiledData = internalEvaluator.popAddedCompiledScripts() newImports = importsCollector.popAddedImports() } - val variablesState = internalEvaluator.variablesHolder - val cellVariables = internalEvaluator.cellVariables - cell?.resultVal = result.result.value val rendered = result.result.let { @@ -415,13 +417,12 @@ class ReplForJupyterImpl( updateClasspath() } ?: emptyList() - notebook.updateVariablesState(variablesState) - notebook.cellVariables = cellVariables + notebook.updateVariablesState(internalEvaluator) // printVars() - // printUsage(jupyterId, usageMap[jupyterId - 1]!!) + // printUsagesInfo(jupyterId, cellVariables[jupyterId - 1]) - val variablesStateUpdate = variablesState.mapValues { it.value.stringValue } + val variablesStateUpdate = notebook.variablesState.mapValues { it.value.stringValue } EvalResult(rendered, EvaluatedSnippetMetadata(newClasspath, compiledData, newImports, variablesStateUpdate)) } } diff --git a/src/main/kotlin/org/jetbrains/kotlinx/jupyter/repl/impl/InternalEvaluatorImpl.kt b/src/main/kotlin/org/jetbrains/kotlinx/jupyter/repl/impl/InternalEvaluatorImpl.kt index 97eb34b35..0ea2f74a1 100644 --- a/src/main/kotlin/org/jetbrains/kotlinx/jupyter/repl/impl/InternalEvaluatorImpl.kt +++ b/src/main/kotlin/org/jetbrains/kotlinx/jupyter/repl/impl/InternalEvaluatorImpl.kt @@ -169,12 +169,11 @@ internal class InternalEvaluatorImpl( // it was val, now it's var if (property is KMutableProperty1) { variablesHolder.remove(property.name) - } - - if (property !is KMutableProperty1) { + } else { variablesHolder[property.name] = state return@forEach } + ans[property.name] = state } return ans diff --git a/src/main/kotlin/org/jetbrains/kotlinx/jupyter/util.kt b/src/main/kotlin/org/jetbrains/kotlinx/jupyter/util.kt index b8d29e65f..724cb6bce 100644 --- a/src/main/kotlin/org/jetbrains/kotlinx/jupyter/util.kt +++ b/src/main/kotlin/org/jetbrains/kotlinx/jupyter/util.kt @@ -71,19 +71,19 @@ fun ResultsRenderersProcessor.registerDefaultRenderers() { /** * Stores info about where a variable Y was declared and info about what are they at the address X. - * T: key, stands for a way of addressing variables, e.g. address. + * K: key, stands for a way of addressing variables, e.g. address. * V: value, from Variable, choose any suitable type for your variable reference. - * Default: Int, String + * Default: T=Int, V=String */ -class VariablesUsagesPerCellWatcher { - val cellVariables = mutableMapOf>() +class VariablesUsagesPerCellWatcher { + val cellVariables = mutableMapOf>() /** * Tells in which cell a variable was declared */ - private val variablesDeclarationInfo: MutableMap = mutableMapOf() + private val variablesDeclarationInfo: MutableMap = mutableMapOf() - fun addDeclaration(address: T, variableRef: U) { + fun addDeclaration(address: K, variableRef: V) { ensureStorageCreation(address) // redeclaration of any type @@ -97,14 +97,14 @@ class VariablesUsagesPerCellWatcher { cellVariables[address]?.add(variableRef) } - fun addUsage(address: T, variableRef: U) = cellVariables[address]?.add(variableRef) + fun addUsage(address: K, variableRef: V) = cellVariables[address]?.add(variableRef) - fun removeOldUsages(newAddress: T) { + fun removeOldUsages(newAddress: K) { // remove known modifying usages in this cell cellVariables[newAddress]?.removeIf { variablesDeclarationInfo[it] != newAddress } } - fun ensureStorageCreation(address: T) = cellVariables.putIfAbsent(address, mutableSetOf()) + fun ensureStorageCreation(address: K) = cellVariables.putIfAbsent(address, mutableSetOf()) } diff --git a/src/test/kotlin/org/jetbrains/kotlinx/jupyter/test/repl/ReplTests.kt b/src/test/kotlin/org/jetbrains/kotlinx/jupyter/test/repl/ReplTests.kt index 15593417c..19cce98c6 100644 --- a/src/test/kotlin/org/jetbrains/kotlinx/jupyter/test/repl/ReplTests.kt +++ b/src/test/kotlin/org/jetbrains/kotlinx/jupyter/test/repl/ReplTests.kt @@ -12,6 +12,8 @@ import org.jetbrains.kotlinx.jupyter.generateDiagnosticFromAbsolute import org.jetbrains.kotlinx.jupyter.repl.CompletionResult import org.jetbrains.kotlinx.jupyter.repl.ListErrorsResult import org.jetbrains.kotlinx.jupyter.test.getOrFail +import org.jetbrains.kotlinx.jupyter.test.getStringValue +import org.jetbrains.kotlinx.jupyter.test.getValue import org.jetbrains.kotlinx.jupyter.withPath import org.junit.jupiter.api.Assertions.assertTrue import org.junit.jupiter.api.Test @@ -464,13 +466,13 @@ class ReplVarsTest : AbstractSingleReplTest() { assertEquals(res.metadata.evaluatedVariablesState, varsUpdate) assertFalse(repl.notebook.variablesState.isEmpty()) val varsState = repl.notebook.variablesState - assertEquals("1", varsState["x"]!!.stringValue) - assertEquals("0", varsState["y"]!!.stringValue) - assertEquals("47", varsState["z"]!!.stringValue) + assertEquals("1", varsState.getStringValue("x")) + assertEquals("0", varsState.getStringValue("y")) + assertEquals(47, varsState.getValue("z")) (varsState["z"]!! as VariableStateImpl).update() repl.notebook.updateVariablesState(varsState) - assertEquals("47", varsState["z"]!!.stringValue) + assertEquals(47, varsState.getValue("z")) } @Test @@ -503,9 +505,9 @@ class ReplVarsTest : AbstractSingleReplTest() { val returnedState = res.metadata.evaluatedVariablesState assertEquals(strState, returnedState) - assertEquals("1", varsState["x"]!!.stringValue) - assertEquals("abc", varsState["y"]!!.stringValue) - assertEquals("1", varsState["z"]!!.stringValue) + assertEquals(1, varsState.getValue("x")) + assertEquals("abc", varsState.getStringValue("y")) + assertEquals("1", varsState.getStringValue("z")) } @Test @@ -529,9 +531,9 @@ class ReplVarsTest : AbstractSingleReplTest() { ) assertTrue(varsState.isNotEmpty()) assertEquals(3, varsState.size) - assertEquals("abc", varsState["x"]!!.stringValue) - assertEquals("123", varsState["y"]!!.stringValue) - assertEquals("abc", varsState["z"]!!.stringValue) + assertEquals("abc", varsState.getStringValue("x")) + assertEquals(123, varsState.getValue("y")) + assertEquals("abc", varsState.getStringValue("z")) eval( """ @@ -543,9 +545,9 @@ class ReplVarsTest : AbstractSingleReplTest() { assertTrue(varsState.isNotEmpty()) assertEquals(3, varsState.size) - assertEquals("1024", varsState["x"]!!.stringValue) - assertEquals("${123 * 2}", varsState["y"]!!.stringValue) - assertEquals("abc", varsState["z"]!!.stringValue) + assertEquals("1024", varsState.getStringValue("x")) + assertEquals("${123 * 2}", varsState.getStringValue("y")) + assertEquals("abc", varsState.getValue("z")) } @Test @@ -566,9 +568,9 @@ class ReplVarsTest : AbstractSingleReplTest() { val returnedState = res.metadata.evaluatedVariablesState assertEquals(strState, returnedState) - assertEquals("1", varsState["x"]!!.stringValue) - assertEquals("abc", varsState["y"]!!.stringValue) - assertEquals("1", varsState["z"]!!.stringValue) + assertEquals(1, varsState.getValue("x")) + assertEquals("abc", varsState.getStringValue("y")) + assertEquals("1", varsState.getStringValue("z")) } @Test @@ -592,9 +594,9 @@ class ReplVarsTest : AbstractSingleReplTest() { ) assertTrue(varsState.isNotEmpty()) assertEquals(3, varsState.size) - assertEquals("abc", varsState["x"]!!.stringValue) - assertEquals("123", varsState["y"]!!.stringValue) - assertEquals("abc", varsState["z"]!!.stringValue) + assertEquals("abc", varsState.getStringValue("x")) + assertEquals(123, varsState.getValue("y")) + assertEquals("abc", varsState.getStringValue("z")) eval( """ @@ -606,9 +608,9 @@ class ReplVarsTest : AbstractSingleReplTest() { assertTrue(varsState.isNotEmpty()) assertEquals(3, varsState.size) - assertEquals("1024", varsState["x"]!!.stringValue) - assertEquals("${123 + 1024}", varsState["y"]!!.stringValue) - assertEquals("abc", varsState["z"]!!.stringValue) + assertEquals("1024", varsState.getStringValue("x")) + assertEquals(123 + 1024, varsState.getValue("y")) + assertEquals("abc", varsState.getStringValue("z")) } @Test diff --git a/src/test/kotlin/org/jetbrains/kotlinx/jupyter/test/testUtil.kt b/src/test/kotlin/org/jetbrains/kotlinx/jupyter/test/testUtil.kt index 81737eb15..dfbae5cde 100644 --- a/src/test/kotlin/org/jetbrains/kotlinx/jupyter/test/testUtil.kt +++ b/src/test/kotlin/org/jetbrains/kotlinx/jupyter/test/testUtil.kt @@ -13,6 +13,7 @@ import org.jetbrains.kotlinx.jupyter.api.JREInfoProvider import org.jetbrains.kotlinx.jupyter.api.KotlinKernelVersion import org.jetbrains.kotlinx.jupyter.api.Notebook import org.jetbrains.kotlinx.jupyter.api.RenderersProcessor +import org.jetbrains.kotlinx.jupyter.api.VariableState import org.jetbrains.kotlinx.jupyter.api.VariableStateImpl import org.jetbrains.kotlinx.jupyter.api.libraries.ExecutionHost import org.jetbrains.kotlinx.jupyter.api.libraries.JupyterIntegration @@ -103,6 +104,14 @@ fun CompletionResult.getOrFail(): CompletionResult.Success = when (this) { else -> fail("Result should be success") } +fun Map.getStringValue(variableName: String): String? { + return get(variableName)?.stringValue +} + +fun Map.getValue(variableName: String): Any? { + return get(variableName)?.value +} + class InMemoryLibraryResolver( parent: LibraryResolver?, initialDescriptorsCache: Map? = null, @@ -149,7 +158,7 @@ object NotebookMock : Notebook { override val cellsList: Collection get() = emptyList() override val variablesState = mutableMapOf() - override var cellVariables = mapOf>() + override val cellVariables = mapOf>() override fun getCell(id: Int): CodeCellImpl { return cells[id] ?: throw ArrayIndexOutOfBoundsException(
VariableValue
${it.key}${it.value.stringValue}
${it.key}${it.value.stringValue}