diff --git a/pkl-core/src/main/java/org/pkl/core/repl/ReplServer.java b/pkl-core/src/main/java/org/pkl/core/repl/ReplServer.java index ce861d29..3d950391 100644 --- a/pkl-core/src/main/java/org/pkl/core/repl/ReplServer.java +++ b/pkl-core/src/main/java/org/pkl/core/repl/ReplServer.java @@ -50,7 +50,6 @@ import org.pkl.core.util.AnsiStringBuilder; import org.pkl.core.util.EconomicMaps; import org.pkl.core.util.IoUtils; import org.pkl.core.util.MutableReference; -import org.pkl.core.util.Pair; import org.pkl.core.util.SyntaxHighlighter; import org.pkl.parser.Parser; import org.pkl.parser.ParserError; @@ -183,51 +182,6 @@ public class ReplServer implements AutoCloseable { .collect(Collectors.toList()); } - /** - * Create a fake module that declares all the local properties that exist in ReplState, so that - * AstBuilder can correctly resolve variables. Additionally, return a {@link Node} with its span - * calculated in terms of this fake module. - * - *

This created module is never executed. - */ - private Pair buildSyntheticModuleText(Node syntaxNode, String srcText) { - if (syntaxNode instanceof ModuleDecl) { - return Pair.of(srcText, syntaxNode); - } - var sb = new StringBuilder(); - var nodeText = syntaxNode.text(srcText.toCharArray()); - var adjustedNode = syntaxNode; - if (syntaxNode instanceof Expr) { - sb.append(expressionPreamble).append(nodeText).append('\n'); - } else { - sb.append(nodeText).append('\n'); - } - var mod = parser.parseModule(sb.toString()); - if (syntaxNode instanceof Expr) { - adjustedNode = mod.getProperties().get(0).getExpr(); - } else if (syntaxNode instanceof ImportClause) { - adjustedNode = mod.getImports().get(0); - } else if (syntaxNode instanceof ClassProperty) { - adjustedNode = mod.getProperties().get(0); - } else if (syntaxNode instanceof Class) { - adjustedNode = mod.getClasses().get(0); - } else if (syntaxNode instanceof org.pkl.parser.syntax.TypeAlias) { - adjustedNode = mod.getTypeAliases().get(0); - } else if (syntaxNode instanceof org.pkl.parser.syntax.ClassMethod) { - adjustedNode = mod.getMethods().get(0); - } - var cursor = EconomicMaps.getEntries(replState.module.getMembers()); - while (cursor.advance()) { - var key = cursor.getKey(); - var value = cursor.getValue(); - if (value.isLocal()) { - sb.append("local ").append(key).append(" = Undefined()\n\n"); - } - } - assert adjustedNode != null; - return Pair.of(sb.toString(), adjustedNode); - } - @SuppressWarnings("StatementWithEmptyBody") private void handleNode( ReplState replState, @@ -237,7 +191,7 @@ public class ReplServer implements AutoCloseable { String srcText, boolean evalDefinitions, boolean forceResults) { - var pair = buildSyntheticModuleText(node, srcText); + var pair = VmUtils.buildSyntheticModuleText(node, replState.module, srcText); var syntheticModuleText = pair.first; var adjustedNode = pair.second; var module = ModuleKeys.synthetic(uri, workingDir.toUri(), uri, syntheticModuleText, false); @@ -283,8 +237,9 @@ public class ReplServer implements AutoCloseable { new ReplResponse.InternalError(new IllegalStateException("Unexpected parse result"))); } } catch (VmException e) { + e.setForExpressionInput(true); // TODO: patch stack trace for constants - results.add(new EvalError(renderException(e))); + results.add(new EvalError(errorRenderer.render(e))); } } diff --git a/pkl-core/src/main/java/org/pkl/core/runtime/StackTraceRenderer.java b/pkl-core/src/main/java/org/pkl/core/runtime/StackTraceRenderer.java index b13c8dea..a7dca0c5 100644 --- a/pkl-core/src/main/java/org/pkl/core/runtime/StackTraceRenderer.java +++ b/pkl-core/src/main/java/org/pkl/core/runtime/StackTraceRenderer.java @@ -28,6 +28,8 @@ import org.pkl.core.util.SyntaxHighlighter; public final class StackTraceRenderer { private final Function frameTransformer; + private static final int PREAMBLE_LENGTH = VmUtils.EXPRESSION_PREAMBLE.length(); + public StackTraceRenderer(Function frameTransformer) { this.frameTransformer = frameTransformer; } @@ -36,9 +38,10 @@ public final class StackTraceRenderer { List frames, @Nullable String hint, @Nullable BiConsumer hintBuilder, - AnsiStringBuilder out) { + AnsiStringBuilder out, + boolean forExpressionInput) { var compressed = compressFrames(frames); - doRender(compressed, hint, hintBuilder, out, "", true); + doRender(compressed, hint, hintBuilder, out, "", true, forExpressionInput); } // non-private for testing @@ -48,12 +51,13 @@ public final class StackTraceRenderer { @Nullable BiConsumer hintBuilder, AnsiStringBuilder out, String leftMargin, - boolean isFirstElement) { + boolean isFirstElement, + boolean forExpressionInput) { for (var frame : frames) { if (frame instanceof StackFrameLoop loop) { // ensure a cycle of length 1 doesn't get rendered as a loop if (loop.count == 1) { - doRender(loop.frames, null, null, out, leftMargin, isFirstElement); + doRender(loop.frames, null, null, out, leftMargin, isFirstElement, forExpressionInput); } else { if (!isFirstElement) { out.append(AnsiTheme.STACK_TRACE_MARGIN, leftMargin).append('\n'); @@ -63,7 +67,7 @@ public final class StackTraceRenderer { .append(AnsiTheme.STACK_TRACE_LOOP_COUNT, loop.count) .append(" repetitions of:\n"); var newLeftMargin = leftMargin + "│ "; - doRender(loop.frames, null, null, out, newLeftMargin, isFirstElement); + doRender(loop.frames, null, null, out, newLeftMargin, isFirstElement, forExpressionInput); if (isFirstElement) { renderHint(hint, hintBuilder, out, newLeftMargin); isFirstElement = false; @@ -74,7 +78,7 @@ public final class StackTraceRenderer { if (!isFirstElement) { out.append(AnsiTheme.STACK_TRACE_MARGIN, leftMargin).append('\n'); } - renderFrame((StackFrame) frame, out, leftMargin); + renderFrame((StackFrame) frame, out, leftMargin, forExpressionInput); } if (isFirstElement) { @@ -84,9 +88,10 @@ public final class StackTraceRenderer { } } - private void renderFrame(StackFrame frame, AnsiStringBuilder out, String leftMargin) { + private void renderFrame( + StackFrame frame, AnsiStringBuilder out, String leftMargin, boolean forExpressionInput) { var transformed = frameTransformer.apply(frame); - renderSourceLine(transformed, out, leftMargin); + renderSourceLine(transformed, out, leftMargin, forExpressionInput); renderSourceLocation(transformed, out, leftMargin); } @@ -113,16 +118,27 @@ public final class StackTraceRenderer { out.append('\n'); } - private void renderSourceLine(StackFrame frame, AnsiStringBuilder out, String leftMargin) { + private void renderSourceLine( + StackFrame frame, AnsiStringBuilder out, String leftMargin, boolean forExpressionInput) { var originalSourceLine = frame.getSourceLines().get(0); var leadingWhitespace = VmUtils.countLeadingWhitespace(originalSourceLine); var sourceLine = originalSourceLine.strip(); + var hasPreamble = false; + if (forExpressionInput && sourceLine.startsWith(VmUtils.EXPRESSION_PREAMBLE)) { + sourceLine = sourceLine.substring(PREAMBLE_LENGTH); + hasPreamble = true; + } var startColumn = frame.getStartColumn() - leadingWhitespace; var endColumn = frame.getStartLine() == frame.getEndLine() ? frame.getEndColumn() - leadingWhitespace : sourceLine.length(); + if (hasPreamble) { + startColumn -= PREAMBLE_LENGTH; + endColumn -= PREAMBLE_LENGTH; + } + var prefix = frame.getStartLine() + " | "; out.append(AnsiTheme.STACK_TRACE_MARGIN, leftMargin) .append(AnsiTheme.STACK_TRACE_LINE_NUMBER, prefix); diff --git a/pkl-core/src/main/java/org/pkl/core/runtime/VmException.java b/pkl-core/src/main/java/org/pkl/core/runtime/VmException.java index d518bd32..e11912e1 100644 --- a/pkl-core/src/main/java/org/pkl/core/runtime/VmException.java +++ b/pkl-core/src/main/java/org/pkl/core/runtime/VmException.java @@ -37,6 +37,7 @@ public abstract class VmException extends AbstractTruffleException { private final List leadingStackFrames; @Nullable private final BiConsumer messageBuilder; @Nullable protected BiConsumer hintBuilder; + private boolean forExpressionInput; public VmException( @Nullable String message, @@ -62,6 +63,7 @@ public abstract class VmException extends AbstractTruffleException { this.insertedStackFrames = insertedStackFrames; this.leadingStackFrames = leadingStackFrames; this.hintBuilder = hintBuilder; + this.forExpressionInput = forExpressionInput; } public final boolean isExternalMessage() { @@ -112,6 +114,14 @@ public abstract class VmException extends AbstractTruffleException { this.hintBuilder = ((builder, aBoolean) -> builder.append(hint)); } + public void setForExpressionInput(boolean forExpressionInput) { + this.forExpressionInput = forExpressionInput; + } + + public boolean isForExpressionInput() { + return forExpressionInput; + } + public enum Kind { EVAL_ERROR, UNDEFINED_VALUE, diff --git a/pkl-core/src/main/java/org/pkl/core/runtime/VmExceptionRenderer.java b/pkl-core/src/main/java/org/pkl/core/runtime/VmExceptionRenderer.java index 37e5f3c7..701b06c6 100644 --- a/pkl-core/src/main/java/org/pkl/core/runtime/VmExceptionRenderer.java +++ b/pkl-core/src/main/java/org/pkl/core/runtime/VmExceptionRenderer.java @@ -144,7 +144,8 @@ public final class VmExceptionRenderer { var hintBuilder = exception.getHintBuilder(); if (!frames.isEmpty()) { - stackTraceRenderer.render(frames, hint, hintBuilder, out.append('\n')); + stackTraceRenderer.render( + frames, hint, hintBuilder, out.append('\n'), exception.isForExpressionInput()); } else if (hint != null) { // render hint if there are no stack frames out.append('\n').append(AnsiTheme.ERROR_MESSAGE_HINT, hint); diff --git a/pkl-core/src/main/java/org/pkl/core/runtime/VmUtils.java b/pkl-core/src/main/java/org/pkl/core/runtime/VmUtils.java index 2bddcb41..7ddcbf5d 100644 --- a/pkl-core/src/main/java/org/pkl/core/runtime/VmUtils.java +++ b/pkl-core/src/main/java/org/pkl/core/runtime/VmUtils.java @@ -61,9 +61,12 @@ import org.pkl.core.module.ModuleKey; import org.pkl.core.module.ModuleKeys; import org.pkl.core.module.ResolvedModuleKey; import org.pkl.core.util.EconomicMaps; +import org.pkl.core.util.Pair; import org.pkl.parser.Parser; import org.pkl.parser.ParserError; import org.pkl.parser.syntax.Expr; +import org.pkl.parser.syntax.ImportClause; +import org.pkl.parser.syntax.ModuleDecl; public final class VmUtils { /** See {@link VmUtils#shouldRunTypeCheck(VirtualFrame)}. */ @@ -92,6 +95,10 @@ public final class VmUtils { private static final DecimalFormatSymbols ROOT_DECIMAL_FORMAT_SYMBOLS = DecimalFormatSymbols.getInstance(Locale.ROOT); + public static final String EXPRESSION_PREAMBLE = "`THE REPL TEXT EXPR` = "; + + private static final Parser parser = new Parser(); + private VmUtils() {} static VmTyped createEmptyModule() { @@ -930,12 +937,69 @@ public final class VmUtils { } } + /** + * Create a fake module that declares all the local properties that exist in {@code module}, so + * that AstBuilder can correctly resolve variables. + * + *

Additionally, return a {@link org.pkl.parser.syntax.Node} with its span calculated in terms + * of this fake module. + * + *

This created module is never executed. + */ + public static Pair buildSyntheticModuleText( + org.pkl.parser.syntax.Node syntaxNode, VmTyped module, String srcText) { + if (syntaxNode instanceof ModuleDecl) { + return Pair.of(srcText, syntaxNode); + } + var sb = new StringBuilder(); + var nodeText = syntaxNode.text(srcText.toCharArray()); + var adjustedNode = syntaxNode; + if (syntaxNode instanceof Expr) { + sb.append(EXPRESSION_PREAMBLE).append(nodeText).append('\n'); + } else { + sb.append(nodeText).append('\n'); + } + var mod = parser.parseModule(sb.toString()); + if (syntaxNode instanceof Expr) { + adjustedNode = mod.getProperties().get(0).getExpr(); + } else if (syntaxNode instanceof ImportClause) { + adjustedNode = mod.getImports().get(0); + } else if (syntaxNode instanceof org.pkl.parser.syntax.ClassProperty) { + adjustedNode = mod.getProperties().get(0); + } else if (syntaxNode instanceof org.pkl.parser.syntax.Class) { + adjustedNode = mod.getClasses().get(0); + } else if (syntaxNode instanceof org.pkl.parser.syntax.TypeAlias) { + adjustedNode = mod.getTypeAliases().get(0); + } else if (syntaxNode instanceof org.pkl.parser.syntax.ClassMethod) { + adjustedNode = mod.getMethods().get(0); + } + var cursor = EconomicMaps.getEntries(module.getMembers()); + while (cursor.advance()) { + var key = cursor.getKey(); + var value = cursor.getValue(); + if (value.isLocal()) { + sb.append("local ").append(key).append(" = Undefined()\n\n"); + } + } + assert adjustedNode != null; + return Pair.of(sb.toString(), adjustedNode); + } + public static Object evaluateExpression( VmTyped module, String expression, SecurityManager securityManager, ModuleResolver moduleResolver) { - var syntheticModule = ModuleKeys.synthetic(REPL_TEXT_URI, expression); + org.pkl.parser.syntax.Node node; + try { + node = parser.parseExpressionInput(expression); + } catch (ParserError e) { + throw toVmException(e, expression, REPL_TEXT_URI, ""); + } + var pair = buildSyntheticModuleText(node, module, expression); + var syntheticModuleText = pair.first; + var adjustedNode = pair.second; + var syntheticModule = ModuleKeys.synthetic(REPL_TEXT_URI, syntheticModuleText); ResolvedModuleKey resolvedModule; try { resolvedModule = syntheticModule.resolve(securityManager); @@ -955,9 +1019,10 @@ public final class VmUtils { resolvedModule, false); var language = VmLanguage.get(null); - var parsedExpression = parseExpressionNode(expression, source); var builder = new AstBuilder(source, language, moduleInfo, moduleResolver); - var exprNode = builder.visitExpr(parsedExpression); + var mod = parser.parseModule(syntheticModuleText); + builder.visitModule(mod); + var exprNode = builder.visitExpr((Expr) adjustedNode); var rootNode = new SimpleRootNode( language, @@ -966,7 +1031,12 @@ public final class VmUtils { "", exprNode); var callNode = Truffle.getRuntime().createIndirectCallNode(); - return callNode.call(rootNode.getCallTarget(), module, module); + try { + return callNode.call(rootNode.getCallTarget(), module, module); + } catch (VmException e) { + e.setForExpressionInput(true); + throw e; + } } public static int findCustomThisSlot(VirtualFrame frame) { diff --git a/pkl-core/src/test/files/LanguageSnippetTests/input/errors/replTextPreamble.pkl b/pkl-core/src/test/files/LanguageSnippetTests/input/errors/replTextPreamble.pkl new file mode 100644 index 00000000..32b79b2d --- /dev/null +++ b/pkl-core/src/test/files/LanguageSnippetTests/input/errors/replTextPreamble.pkl @@ -0,0 +1,3 @@ +// make sure that we don't trim this identifier when not using the expression evaluator +// see VmUtils.buildSyntheticModuleText() +`THE REPL TEXT EXPR` = throw("uh oh") diff --git a/pkl-core/src/test/files/LanguageSnippetTests/output/errors/replTextPreamble.err b/pkl-core/src/test/files/LanguageSnippetTests/output/errors/replTextPreamble.err new file mode 100644 index 00000000..52c0d20a --- /dev/null +++ b/pkl-core/src/test/files/LanguageSnippetTests/output/errors/replTextPreamble.err @@ -0,0 +1,14 @@ +–– Pkl Error –– +uh oh + +x | `THE REPL TEXT EXPR` = throw("uh oh") + ^^^^^^^^^^^^^^ +at replTextPreamble#`THE REPL TEXT EXPR` (file:///$snippetsDir/input/errors/replTextPreamble.pkl) + +xxx | renderer.renderDocument(value) + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ +at pkl.base#Module.output.text (pkl:base) + +xxx | if (renderer is BytesRenderer) renderer.renderDocument(value) else text.encodeToBytes("UTF-8") + ^^^^ +at pkl.base#Module.output.bytes (pkl:base) diff --git a/pkl-core/src/test/kotlin/org/pkl/core/EvaluateExpressionTest.kt b/pkl-core/src/test/kotlin/org/pkl/core/EvaluateExpressionTest.kt index 1c6e3b02..3e3f0574 100644 --- a/pkl-core/src/test/kotlin/org/pkl/core/EvaluateExpressionTest.kt +++ b/pkl-core/src/test/kotlin/org/pkl/core/EvaluateExpressionTest.kt @@ -134,4 +134,43 @@ class EvaluateExpressionTest { assertThat(error).hasMessageContaining("Expected value of type `String`, but got type `Int`") } + + @Test + fun `evaluate local property`() { + val result = evaluate("local foo = 1", "foo") + + assertThat(result).isEqualTo(1L) + } + + @Test + fun `evaluate throwing local property trims expression preamble when rendering stack frames`() { + val error = assertThrows { evaluate("local foo = throw(\"uh oh\")", "foo") } + + assertThat(error.message) + .isEqualTo( + """ + –– Pkl Error –– + uh oh + + 1 | local foo = throw("uh oh") + ^^^^^^^^^^^^^^ + at text#foo (repl:text) + + 1 | foo + ^^^ + at (repl:text) + + """ + .trimIndent() + ) + } + + @Test + fun `evaluate import`() { + val result = evaluate("import \"pkl:base\"", "base") + + assertThat(result).isInstanceOf(PModule::class.java) + result as PModule + assertThat(result.moduleName).isEqualTo("pkl.base") + } } diff --git a/pkl-core/src/test/kotlin/org/pkl/core/runtime/StackTraceRendererTest.kt b/pkl-core/src/test/kotlin/org/pkl/core/runtime/StackTraceRendererTest.kt index 9d9727a1..6d356571 100644 --- a/pkl-core/src/test/kotlin/org/pkl/core/runtime/StackTraceRendererTest.kt +++ b/pkl-core/src/test/kotlin/org/pkl/core/runtime/StackTraceRendererTest.kt @@ -192,7 +192,7 @@ class StackTraceRendererTest { val loop = StackTraceRenderer.StackFrameLoop(loopFrames, 1) val frames = listOf(createFrame("bar", 1), createFrame("baz", 2), loop) val formatter = AnsiStringBuilder(false) - renderer.doRender(frames, null, null, formatter, "", true) + renderer.doRender(frames, null, null, formatter, "", true, true) val renderedFrames = formatter.toString() assertThat(renderedFrames) .isEqualTo(