Skip to content

Commit 9e6cbfd

Browse files
ileasileileasile
committed
Filter out magics in listErrors and complete
Fixes #57
1 parent 53c9e24 commit 9e6cbfd

File tree

7 files changed

+245
-45
lines changed

7 files changed

+245
-45
lines changed

‎src/main/kotlin/org/jetbrains/kotlin/jupyter/magics.kt

+21-8
Original file line numberDiff line numberDiff line change
@@ -12,10 +12,20 @@ enum class ReplLineMagics(val desc: String, val argumentsUsage: String? = null,
1212
trackClasspath("log current classpath changes"),
1313
trackExecution("log code that is going to be executed in repl", visibleInHelp = false),
1414
dumpClassesForSpark("stores compiled repl classes in special folder for Spark integration", visibleInHelp = false),
15-
output("setup output settings", "--max-cell-size=1000 --no-stdout --max-time=100 --max-buffer=400")
15+
output("setup output settings", "--max-cell-size=1000 --no-stdout --max-time=100 --max-buffer=400");
16+
17+
companion object {
18+
fun valueOfOrNull(name: String): ReplLineMagics? {
19+
return try {
20+
valueOf(name)
21+
} catch (e: IllegalArgumentException) {
22+
null
23+
}
24+
}
25+
}
1626
}
1727

18-
data class MagicProcessingResult(val code: String, val libraries: List<LibraryDefinition> = emptyList())
28+
data class MagicProcessingResult(val code: String, val libraries: List<LibraryDefinition>)
1929

2030
class MagicsProcessor(val repl: ReplOptions, private val libraries: LibrariesProcessor) {
2131

@@ -44,7 +54,7 @@ class MagicsProcessor(val repl: ReplOptions, private val libraries: LibrariesPro
4454
}
4555
}
4656

47-
fun processMagics(code: String): MagicProcessingResult {
57+
fun processMagics(code: String, ignoreMagicsErrors: Boolean = false): MagicProcessingResult {
4858

4959
val sb = StringBuilder()
5060
var nextSearchIndex = 0
@@ -71,9 +81,8 @@ class MagicsProcessor(val repl: ReplOptions, private val libraries: LibrariesPro
7181
val keyword = parts[0]
7282
val arg = if (parts.count() > 1) parts[1] else null
7383

74-
val magic = try {
75-
ReplLineMagics.valueOf(keyword)
76-
} catch (e: IllegalArgumentException) {
84+
val magic = ReplLineMagics.valueOfOrNull(keyword)
85+
if(magic == null && !ignoreMagicsErrors) {
7786
throw ReplCompilerException("Unknown line magic keyword: '$keyword'")
7887
}
7988

@@ -92,8 +101,12 @@ class MagicsProcessor(val repl: ReplOptions, private val libraries: LibrariesPro
92101
ReplLineMagics.dumpClassesForSpark -> repl.writeCompiledClasses = true
93102

94103
ReplLineMagics.use -> {
95-
if (arg == null) throw ReplCompilerException("Need some arguments for 'use' command")
96-
newLibraries.addAll(libraries.processNewLibraries(arg))
104+
try {
105+
if (arg == null) throw ReplCompilerException("Need some arguments for 'use' command")
106+
newLibraries.addAll(libraries.processNewLibraries(arg))
107+
} catch (e: ReplCompilerException) {
108+
if (!ignoreMagicsErrors) throw e
109+
}
97110
}
98111
ReplLineMagics.output -> {
99112
repl.outputConfig = updateOutputConfig(repl.outputConfig, (arg ?: "").split(" "))

‎src/main/kotlin/org/jetbrains/kotlin/jupyter/repl.kt

+4-4
Original file line numberDiff line numberDiff line change
@@ -427,14 +427,14 @@ class ReplForJupyterImpl(private val scriptClasspath: List<File> = emptyList(),
427427

428428
private val completionQueue = LockQueue<CompletionResult, CompletionArgs>()
429429
override suspend fun complete(code: String, cursor: Int, callback: (CompletionResult) -> Unit) = doWithLock(CompletionArgs(code, cursor, callback), completionQueue, CompletionResult.Empty(code, cursor)) {
430-
//val preprocessed = preprocessCode(code)
431-
completer.complete(compiler, compilerConfiguration, code, executionCounter++, cursor)
430+
val preprocessed = magics.processMagics(code, true).code
431+
completer.complete(compiler, compilerConfiguration, code, preprocessed, executionCounter++, cursor)
432432
}
433433

434434
private val listErrorsQueue = LockQueue<ListErrorsResult, ListErrorsArgs>()
435435
override suspend fun listErrors(code: String, callback: (ListErrorsResult) -> Unit) = doWithLock(ListErrorsArgs(code, callback), listErrorsQueue, ListErrorsResult(code)) {
436-
//val preprocessed = preprocessCode(code)
437-
val codeLine = SourceCodeImpl(executionCounter++, code)
436+
val preprocessed = magics.processMagics(code, true).code
437+
val codeLine = SourceCodeImpl(executionCounter++, preprocessed)
438438
val errorsList = runBlocking { compiler.analyze(codeLine, 0.toSourceCodePosition(codeLine), compilerConfiguration) }
439439
ListErrorsResult(code, errorsList.valueOrThrow()[ReplAnalyzerResult.analysisDiagnostics]!!)
440440
}

‎src/main/kotlin/org/jetbrains/kotlin/jupyter/repl/completion/KotlinCompleter.kt

+8-4
Original file line numberDiff line numberDiff line change
@@ -4,9 +4,11 @@ import com.beust.klaxon.JsonObject
44
import kotlinx.coroutines.runBlocking
55
import org.jetbrains.annotations.TestOnly
66
import org.jetbrains.kotlin.jupyter.jsonObject
7+
import org.jetbrains.kotlin.jupyter.toSourceCodePositionWithNewAbsolute
78
import java.io.PrintWriter
89
import java.io.StringWriter
910
import kotlin.script.experimental.api.*
11+
import kotlin.script.experimental.jvm.util.calcAbsolute
1012
import kotlin.script.experimental.jvm.util.toSourceCodePosition
1113

1214
enum class CompletionStatus(private val value: String) {
@@ -119,12 +121,14 @@ internal class SourceCodeImpl(number: Int, override val text: String) : SourceCo
119121
}
120122

121123
class KotlinCompleter {
122-
fun complete(compiler: ReplCompleter, configuration: ScriptCompilationConfiguration, code: String, id: Int, cursor: Int): CompletionResult {
124+
fun complete(compiler: ReplCompleter, configuration: ScriptCompilationConfiguration, code: String, preprocessedCode: String, id: Int, cursor: Int): CompletionResult {
123125
return try {
124126
val codeLine = SourceCodeImpl(id, code)
125-
val completionResult = runBlocking { compiler.complete(codeLine, cursor.toSourceCodePosition(codeLine), configuration) }
127+
val preprocessedCodeLine = SourceCodeImpl(id, preprocessedCode)
128+
val codePos = cursor.toSourceCodePositionWithNewAbsolute(codeLine, preprocessedCodeLine)
129+
val completionResult = codePos?.let { runBlocking { compiler.complete(preprocessedCodeLine, codePos, configuration) } }
126130

127-
completionResult.valueOrNull()?.toList()?.let { completionList ->
131+
completionResult?.valueOrNull()?.toList()?.let { completionList ->
128132
val bounds = getTokenBounds(code, cursor)
129133
CompletionResult.Success(completionList.map { it.text }, bounds, completionList, code, cursor)
130134
} ?: CompletionResult.Empty(code, cursor)
@@ -137,7 +141,7 @@ class KotlinCompleter {
137141
}
138142

139143
companion object {
140-
fun getTokenBounds(buf: String, cursor: Int): CompletionTokenBounds {
144+
private fun getTokenBounds(buf: String, cursor: Int): CompletionTokenBounds {
141145
require(cursor <= buf.length) { "Position $cursor does not exist in code snippet <$buf>" }
142146

143147
val startSubstring = buf.substring(0, cursor)

‎src/main/kotlin/org/jetbrains/kotlin/jupyter/util.kt

+34
Original file line numberDiff line numberDiff line change
@@ -2,11 +2,14 @@ package org.jetbrains.kotlin.jupyter
22

33
import kotlinx.coroutines.*
44
import org.jetbrains.kotlin.cli.common.messages.CompilerMessageLocationWithRange
5+
import org.jetbrains.kotlin.utils.addToStdlib.min
56
import org.slf4j.Logger
67
import java.io.File
78
import kotlin.script.experimental.api.ResultWithDiagnostics
89
import kotlin.script.experimental.api.ScriptDiagnostic
910
import kotlin.script.experimental.api.SourceCode
11+
import kotlin.script.experimental.jvm.util.determineSep
12+
import kotlin.script.experimental.jvm.util.toSourceCodePosition
1013

1114
fun <T> catchAll(body: () -> T): T? = try {
1215
body()
@@ -84,3 +87,34 @@ fun CompilerMessageLocationWithRange.toExtString(): String {
8487
val loc = if (start.isEmpty() && end.isEmpty()) "" else " ($start$end)"
8588
return path + loc
8689
}
90+
91+
fun String.findNthSubstring(s: String, n: Int, start: Int = 0): Int {
92+
if (n < 1 || start == -1) return -1
93+
94+
var i = start
95+
96+
for (k in 1..n) {
97+
i = indexOf(s, i)
98+
if (i == -1) return -1
99+
i += s.length
100+
}
101+
102+
return i - s.length
103+
}
104+
105+
fun Int.toSourceCodePositionWithNewAbsolute(code: SourceCode, newCode: SourceCode): SourceCode.Position? {
106+
val pos = toSourceCodePosition(code)
107+
val sep = code.text.determineSep()
108+
val absLineStart =
109+
if (pos.line == 1) 0
110+
else newCode.text.findNthSubstring(sep, pos.line - 1) + sep.length
111+
112+
var nextNewLinePos = newCode.text.indexOf(sep, absLineStart)
113+
if (nextNewLinePos == -1) nextNewLinePos = newCode.text.length
114+
115+
val abs = absLineStart + pos.col - 1
116+
if (abs > nextNewLinePos)
117+
return null
118+
119+
return SourceCode.Position(pos.line, abs - absLineStart + 1, abs)
120+
}

‎src/test/kotlin/org/jetbrains/kotlin/jupyter/test/parseMagicsTests.kt

+125-1
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,17 @@
11
package org.jetbrains.kotlin.jupyter.test
22

3+
import org.jetbrains.kotlin.jupyter.ExecutedCodeLogging
4+
import org.jetbrains.kotlin.jupyter.LibrariesProcessor
5+
import org.jetbrains.kotlin.jupyter.LibraryDefinition
6+
import org.jetbrains.kotlin.jupyter.MagicsProcessor
7+
import org.jetbrains.kotlin.jupyter.OutputConfig
8+
import org.jetbrains.kotlin.jupyter.ReplOptions
39
import org.jetbrains.kotlin.jupyter.parseLibraryName
10+
import org.jetbrains.kotlin.jupyter.repl.completion.SourceCodeImpl
11+
import org.jetbrains.kotlin.jupyter.toSourceCodePositionWithNewAbsolute
412
import org.junit.jupiter.api.Assertions.assertEquals
13+
import org.junit.jupiter.api.Assertions.assertNull
14+
import org.junit.jupiter.api.Assertions.assertTrue
515
import org.junit.jupiter.api.Test
616

717
class ParseArgumentsTests {
@@ -32,4 +42,118 @@ class ParseArgumentsTests {
3242
assertEquals("arg2", args[1].name)
3343
assertEquals("val2", args[1].value)
3444
}
35-
}
45+
}
46+
47+
class ParseMagicsTests {
48+
49+
private class TestReplOptions : ReplOptions {
50+
override var trackClasspath = false
51+
override var executedCodeLogging = ExecutedCodeLogging.Off
52+
override var writeCompiledClasses = false
53+
override var outputConfig = OutputConfig()
54+
}
55+
56+
private val options = TestReplOptions()
57+
58+
private fun test(code: String, expectedProcessedCode: String, librariesChecker: (List<LibraryDefinition>) -> Unit = {}) {
59+
val processor = MagicsProcessor(options, LibrariesProcessor(testResolverConfig.libraries))
60+
with(processor.processMagics(code, true)) {
61+
assertEquals(expectedProcessedCode, this.code)
62+
librariesChecker(libraries)
63+
}
64+
}
65+
66+
@Test
67+
fun `single magic`() {
68+
test("%use krangl", "") { libs ->
69+
assertEquals(1, libs.size)
70+
}
71+
}
72+
73+
@Test
74+
fun `trailing newlines should be left`() {
75+
test("\n%use krangl\n\n", "\n\n\n"){ libs ->
76+
assertEquals(1, libs.size)
77+
}
78+
}
79+
80+
@Test
81+
fun `multiple magics`() {
82+
test(
83+
"""
84+
%use lets-plot, krangl
85+
86+
fun f() = 42
87+
%trackClasspath
88+
val x = 9
89+
90+
""".trimIndent(),
91+
"""
92+
93+
94+
fun f() = 42
95+
96+
val x = 9
97+
98+
""".trimIndent()
99+
){ libs ->
100+
assertEquals(2, libs.size)
101+
}
102+
103+
assertTrue(options.trackClasspath)
104+
}
105+
106+
@Test
107+
fun `wrong magics should be tolerated`() {
108+
test(
109+
"""
110+
%use lets-plot
111+
%use wrongLib
112+
val x = 9
113+
%wrongMagic
114+
fun f() = 42
115+
%trackExecution -generated
116+
""".trimIndent(),
117+
"""
118+
119+
120+
val x = 9
121+
122+
fun f() = 42
123+
124+
""".trimIndent()
125+
){ libs ->
126+
assertEquals(1, libs.size)
127+
}
128+
129+
assertEquals(ExecutedCodeLogging.Generated, options.executedCodeLogging)
130+
}
131+
132+
@Test
133+
fun `source location is correctly transformed`() {
134+
val sourceText = """
135+
fun g() = 99
136+
%use lets-plot
137+
%use wrongLib
138+
val x = 9
139+
""".trimIndent()
140+
141+
val resultText = """
142+
fun g() = 99
143+
144+
145+
val x = 9
146+
""".trimIndent()
147+
148+
test(sourceText, resultText) { libs ->
149+
assertEquals(1, libs.size)
150+
}
151+
152+
val source = SourceCodeImpl(1, sourceText)
153+
val result = SourceCodeImpl(1, resultText)
154+
val cursor = sourceText.indexOf("lets-plot")
155+
156+
val actualPos = cursor.toSourceCodePositionWithNewAbsolute(source, result)
157+
assertNull(actualPos)
158+
}
159+
}

0 commit comments

Comments
 (0)