Fix eval -x of local members (#1760)

This fixes a regression where local members cannot be seen by the
expression evaluator.

Because variable are now parse-time resolved, this builds a truffle node
by first constructing a synthetic module of all defined local members,
and calls AstBuilder on this synthetic module first.

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.
-   *
-   * <p>This created module is never executed.
-   */
-  private Pair<String, Node> 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)));
     }
   }
 

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<StackFrame, StackFrame> frameTransformer;
 
+  private static final int PREAMBLE_LENGTH = VmUtils.EXPRESSION_PREAMBLE.length();
+
   public StackTraceRenderer(Function<StackFrame, StackFrame> frameTransformer) {
     this.frameTransformer = frameTransformer;
   }
@@ -36,9 +38,10 @@ public final class StackTraceRenderer {
       List<StackFrame> frames,
       @Nullable String hint,
       @Nullable BiConsumer<AnsiStringBuilder, Boolean> 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<AnsiStringBuilder, Boolean> 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);

pkl-core/src/main/java/org/pkl/core/runtime/VmException.java

@@ -37,6 +37,7 @@ public abstract class VmException extends AbstractTruffleException {
   private final List<StackFrame> leadingStackFrames;
   @Nullable private final BiConsumer<AnsiStringBuilder, Boolean> messageBuilder;
   @Nullable protected BiConsumer<AnsiStringBuilder, Boolean> 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,

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);

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.
+   *
+   * <p>Additionally, return a {@link org.pkl.parser.syntax.Node} with its span calculated in terms
+   * of this fake module.
+   *
+   * <p>This created module is never executed.
+   */
+  public static Pair<String, org.pkl.parser.syntax.Node> 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) {

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")

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)

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<PklException> { 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")
+  }
 }

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(