From 00819c421f650d3da48867d48d359676854c5bc4 Mon Sep 17 00:00:00 2001 From: Ilya Muradyan Date: Tue, 17 May 2022 17:41:10 +0300 Subject: [PATCH] [REPL] Report script codegen errors in advance --- .../experimental/jvmhost/test/ReplTest.kt | 71 ++++++++++++++++--- .../plugin/impl/KJvmReplCompilerBase.kt | 46 ++++++------ .../compiler/plugin/impl/errorReporting.kt | 40 +++++++++++ 3 files changed, 126 insertions(+), 31 deletions(-) diff --git a/libraries/scripting/jvm-host-test/test/kotlin/script/experimental/jvmhost/test/ReplTest.kt b/libraries/scripting/jvm-host-test/test/kotlin/script/experimental/jvmhost/test/ReplTest.kt index a93ee3a44eb..adfb9445cc2 100644 --- a/libraries/scripting/jvm-host-test/test/kotlin/script/experimental/jvmhost/test/ReplTest.kt +++ b/libraries/scripting/jvm-host-test/test/kotlin/script/experimental/jvmhost/test/ReplTest.kt @@ -201,6 +201,31 @@ class ReplTest : TestCase() { ) } + @Test + fun testCodegenErrors() { + checkEvaluateInReplDiags( + sequenceOf( + """ + val x = 1 + class C { + companion object { + fun f() = x + } + } + """.trimIndent() + ), + sequenceOf( + makeFailureResult( + "Object Companion captures the script class instance. Try to use class or anonymous object instead", + location = SourceCode.Location( + SourceCode.Position(3, 15), + SourceCode.Position(3, 21) + ) + ), + ) + ) + } + @Test // TODO: make it covering more cases fun testIrReceiverOvewrite() { @@ -217,7 +242,11 @@ class ReplTest : TestCase() { } @Test - fun testNoEvaluationError() { + fun testNoErrorAfterBrokenCodegenSnippet() { + val errorMessage = "Platform declaration clash: The following declarations have the same JVM signature (getX()I):\n" + + " fun ``(): Int defined in Line_0_simplescript\n" + + " fun ``(): Int defined in Line_0_simplescript" + checkEvaluateInReplDiags( sequenceOf( """ @@ -225,19 +254,27 @@ class ReplTest : TestCase() { val X = 1 val x = stack(1, X) """.trimIndent(), - "val y = 42" + """ + val y = 42 + y + """.trimIndent() ), sequenceOf( - ResultValue.NotEvaluated.asSuccess( - listOf( - ScriptDiagnostic( - ScriptDiagnostic.unspecifiedError, - "Unable to instantiate class Line_0_simplescript: java.lang.ClassFormatError: " + - "Duplicate method name \"getX\" with signature \"()I\" in class file Line_0_simplescript" + ResultWithDiagnostics.Failure( + errorMessage.asErrorDiagnostics( + location = SourceCode.Location( + SourceCode.Position(2, 1), + SourceCode.Position(2, 6) + ) + ), + errorMessage.asErrorDiagnostics( + location = SourceCode.Location( + SourceCode.Position(3, 1), + SourceCode.Position(3, 6) ) ) ), - makeFailureResult("Snippet cannot be evaluated due to history mismatch") + 42.asSuccess() ) ) } @@ -333,6 +370,20 @@ class ReplTest : TestCase() { } companion object { + private fun positionsEqual(a: SourceCode.Position?, b: SourceCode.Position?): Boolean { + if (a == null || b == null) { + return a == null && b == null + } + return a.col == b.col && a.line == b.line + } + + private fun locationsEqual(a: SourceCode.Location?, b: SourceCode.Location?): Boolean { + if (a == null || b == null) { + return a == null && b == null + } + return positionsEqual(a.start, b.start) && positionsEqual(a.end, b.end) + } + private fun evaluateInRepl( snippets: Sequence, compilationConfiguration: ScriptCompilationConfiguration = simpleScriptCompilationConfiguration, @@ -389,7 +440,7 @@ class ReplTest : TestCase() { Assert.assertTrue( "#$index: Expected $expectedRes, got $res. Locations are different", resReports.map { it.location }.zip(expectedRes.reports.map { it.location }).all { - it.second == null || it.second == it.first + it.second == null || locationsEqual(it.first, it.second) } ) } diff --git a/plugins/scripting/scripting-compiler/src/org/jetbrains/kotlin/scripting/compiler/plugin/impl/KJvmReplCompilerBase.kt b/plugins/scripting/scripting-compiler/src/org/jetbrains/kotlin/scripting/compiler/plugin/impl/KJvmReplCompilerBase.kt index 37f25b6059e..1357b284cf0 100644 --- a/plugins/scripting/scripting-compiler/src/org/jetbrains/kotlin/scripting/compiler/plugin/impl/KJvmReplCompilerBase.kt +++ b/plugins/scripting/scripting-compiler/src/org/jetbrains/kotlin/scripting/compiler/plugin/impl/KJvmReplCompilerBase.kt @@ -24,6 +24,7 @@ import org.jetbrains.kotlin.codegen.state.GenerationState import org.jetbrains.kotlin.config.JVMConfigurationKeys import org.jetbrains.kotlin.config.languageVersionSettings import org.jetbrains.kotlin.descriptors.ScriptDescriptor +import org.jetbrains.kotlin.diagnostics.impl.SimpleDiagnosticsCollector import org.jetbrains.kotlin.idea.MainFunctionDetector import org.jetbrains.kotlin.ir.backend.jvm.serialization.JvmDescriptorMangler import org.jetbrains.kotlin.ir.declarations.impl.IrFactoryImpl @@ -60,6 +61,7 @@ open class KJvmReplCompilerBase( configuration: ScriptCompilationConfiguration ): ResultWithDiagnostics> = snippets.map { snippet -> + // TODO: get rid of messageCollector to avoid creation of additional entities withMessageCollector(snippet) { messageCollector -> val initialConfiguration = configuration.refineBeforeParsing(snippet).valueOr { return it @@ -124,10 +126,25 @@ open class KJvmReplCompilerBase( val isIr = context.environment.configuration.getBoolean(JVMConfigurationKeys.IR) + val codegenDiagnosticsCollector = SimpleDiagnosticsCollector() + + val genStateBuilder = GenerationState.Builder( + snippetKtFile.project, + ClassBuilderFactories.BINARIES, + compilationState.analyzerEngine.module, + compilationState.analyzerEngine.trace.bindingContext, + compilationState.environment.configuration + ).diagnosticReporter(codegenDiagnosticsCollector) + val generationState = if (isIr) { - generateWithBackendIr(compilationState, snippetKtFile, sourceFiles) + generateWithBackendIr(compilationState, sourceFiles, genStateBuilder) } else { - generateWithOldBackend(compilationState, snippetKtFile, sourceFiles) + generateWithOldBackend(snippetKtFile, sourceFiles, genStateBuilder) + } + + if (codegenDiagnosticsCollector.hasErrors) { + val scriptDiagnostics = codegenDiagnosticsCollector.scriptDiagnostics(snippet) + return failure(messageCollector, *scriptDiagnostics.toTypedArray()) } state.history.push(LineId(snippetNo, 0, snippet.hashCode()), scriptDescriptor) @@ -156,17 +173,11 @@ open class KJvmReplCompilerBase( }.last() private fun generateWithOldBackend( - compilationState: ReplCompilationState, snippetKtFile: KtFile, - sourceFiles: List + sourceFiles: List, + prebuiltState: GenerationState.Builder, ): GenerationState { - val generationState = GenerationState.Builder( - snippetKtFile.project, - ClassBuilderFactories.BINARIES, - compilationState.analyzerEngine.module, - compilationState.analyzerEngine.trace.bindingContext, - compilationState.environment.configuration - ).build().also { generationState -> + val generationState = prebuiltState.build().also { generationState -> generationState.scriptSpecific.earlierScriptsForReplInterpreter = state.history.map { it.item } generationState.beforeCompile() generationState.oldBEInitTrace(sourceFiles) @@ -178,8 +189,8 @@ open class KJvmReplCompilerBase( private fun generateWithBackendIr( compilationState: ReplCompilationState, - snippetKtFile: KtFile, - sourceFiles: List + sourceFiles: List, + prebuiltState: GenerationState.Builder, ): GenerationState { val generatorExtensions = object : JvmGeneratorExtensionsImpl(compilationState.environment.configuration) { override fun getPreviousScripts() = state.history.map { compilationState.symbolTable.referenceScript(it.item) } @@ -189,14 +200,7 @@ open class KJvmReplCompilerBase( compilationState.environment.configuration.get(CLIConfigurationKeys.PHASE_CONFIG), compilationState.mangler, compilationState.symbolTable, generatorExtensions ) - val generationState = GenerationState.Builder( - snippetKtFile.project, - ClassBuilderFactories.BINARIES, - compilationState.analyzerEngine.module, - compilationState.analyzerEngine.trace.bindingContext, - compilationState.environment.configuration - ) - .build() + val generationState = prebuiltState.build() codegenFactory.generateModule( generationState, diff --git a/plugins/scripting/scripting-compiler/src/org/jetbrains/kotlin/scripting/compiler/plugin/impl/errorReporting.kt b/plugins/scripting/scripting-compiler/src/org/jetbrains/kotlin/scripting/compiler/plugin/impl/errorReporting.kt index 574cc798673..61795b67964 100644 --- a/plugins/scripting/scripting-compiler/src/org/jetbrains/kotlin/scripting/compiler/plugin/impl/errorReporting.kt +++ b/plugins/scripting/scripting-compiler/src/org/jetbrains/kotlin/scripting/compiler/plugin/impl/errorReporting.kt @@ -11,6 +11,11 @@ import org.jetbrains.kotlin.cli.common.messages.CompilerMessageLocation import org.jetbrains.kotlin.cli.common.messages.CompilerMessageSeverity import org.jetbrains.kotlin.cli.common.messages.CompilerMessageSourceLocation import org.jetbrains.kotlin.cli.common.messages.MessageCollector +import org.jetbrains.kotlin.diagnostics.KtDiagnostic +import org.jetbrains.kotlin.diagnostics.Severity +import org.jetbrains.kotlin.diagnostics.impl.BaseDiagnosticsCollector +import org.jetbrains.kotlin.diagnostics.rendering.RootDiagnosticRendererFactory +import org.jetbrains.kotlin.psi import org.jetbrains.kotlin.scripting.definitions.MessageReporter import org.jetbrains.kotlin.utils.addToStdlib.firstIsInstanceOrNull import kotlin.reflect.KMutableProperty1 @@ -18,6 +23,7 @@ import kotlin.script.experimental.api.ResultWithDiagnostics import kotlin.script.experimental.api.ScriptDiagnostic import kotlin.script.experimental.api.SourceCode import kotlin.script.experimental.api.asErrorDiagnostics +import kotlin.script.experimental.jvm.util.toSourceCodePosition class ScriptDiagnosticsMessageCollector(private val parentMessageCollector: MessageCollector?) : MessageCollector { @@ -205,3 +211,37 @@ val MessageCollector.reporter: MessageReporter this.report(severity.toCompilerMessageSeverity(), message) } +fun KtDiagnostic.asScriptDiagnostic(sourceCode: SourceCode): ScriptDiagnostic { + val (diagnosticCode, scriptSeverity) = when (severity) { + Severity.INFO -> ScriptDiagnostic.unspecifiedInfo to ScriptDiagnostic.Severity.INFO + Severity.ERROR -> ScriptDiagnostic.unspecifiedError to ScriptDiagnostic.Severity.ERROR + Severity.WARNING -> ScriptDiagnostic.unspecifiedInfo to ScriptDiagnostic.Severity.WARNING + } + + val renderer = RootDiagnosticRendererFactory(this) + + val location = if (textRanges.isEmpty()) { + null + } else { + val firstRange = textRanges.first() + val lastRange = textRanges.last() + SourceCode.LocationWithId( + element.psi?.containingFile?.virtualFile?.path.orEmpty(), + SourceCode.Location( + firstRange.startOffset.toSourceCodePosition(sourceCode), + lastRange.endOffset.toSourceCodePosition(sourceCode) + ) + ) + } + + return ScriptDiagnostic( + diagnosticCode, + renderer.render(this), + scriptSeverity, + location + ) +} + +fun BaseDiagnosticsCollector.scriptDiagnostics(sourceCode: SourceCode) = diagnostics.map { + it.asScriptDiagnostic(sourceCode) +}