Cleanups for Skylark tracebacks

In Skylark.java, fix line numbers to start at 1 instead of 2.

In Eval.java, go through maybeTransformException for consistency with expressions, even though no statement nodes use this feature.

In EvalExceptionWithStackTrace, fix style violation (non-consecutive overloads) and add TODO explaining an open issue.

RELNOTES: None
PiperOrigin-RevId: 169769928

3 files changed, 22 insertions, 10 deletions

diff --git a/src/main/java/com/google/devtools/build/lib/syntax/Eval.java b/src/main/java/com/google/devtools/build/lib/syntax/Eval.java
index b7b91d44f4..6fd1f281d1 100644
--- a/src/main/java/com/google/devtools/build/lib/syntax/Eval.java
+++ b/src/main/java/com/google/devtools/build/lib/syntax/Eval.java
@@ -174,6 +174,14 @@ public class Eval {
    * @throws InterruptedException may be thrown in a sub class.
    */
   public void exec(Statement st) throws EvalException, InterruptedException {
+    try {
+      execDispatch(st);
+    } catch (EvalException ex) {
+      throw st.maybeTransformException(ex);
+    }
+  }
+
+  void execDispatch(Statement st) throws EvalException, InterruptedException {
     switch (st.kind()) {
       case ASSIGNMENT:
         execAssignment((AssignmentStatement) st);
diff --git a/src/main/java/com/google/devtools/build/lib/syntax/EvalExceptionWithStackTrace.java b/src/main/java/com/google/devtools/build/lib/syntax/EvalExceptionWithStackTrace.java
index 173f2f7ec9..c76de98bc7 100644
--- a/src/main/java/com/google/devtools/build/lib/syntax/EvalExceptionWithStackTrace.java
+++ b/src/main/java/com/google/devtools/build/lib/syntax/EvalExceptionWithStackTrace.java
@@ -110,18 +110,24 @@ public class EvalExceptionWithStackTrace extends EvalException {
     addStackFrame(funcallDescription, location, false);
   }
 
-  /**
-   * Adds a line for the given frame.
-   */
+  /** Adds a line for the given frame. */
   private void addStackFrame(String label, Location location, boolean canPrint) {
-    // We have to watch out for duplicate since ExpressionStatements add themselves twice:
-    // Statement#exec() calls Expression#eval(), both of which call this method.
+    // TODO(bazel-team): This check was originally created to weed out duplicates in case the same
+    // node is added twice, but it's not clear if that is still a possibility. In any case, it would
+    // be better to eliminate the check and not create unwanted duplicates in the first place.
+    //
+    // The check is problematic because it suppresses tracebacks in the REPL, where line numbers
+    // can be reset within a single session.
     if (mostRecentElement != null && isSameLocation(location, mostRecentElement.getLocation())) {
       return;
     }
     mostRecentElement = new StackFrame(label, location, mostRecentElement, canPrint);
   }
 
+  private void addStackFrame(String label, Location location)   {
+    addStackFrame(label, location, true);
+  }
+
   /**
    * Checks two locations for equality in paths and start offsets.
    *
@@ -136,10 +142,6 @@ public class EvalExceptionWithStackTrace extends EvalException {
     }
   }
 
-  private void addStackFrame(String label, Location location)   {
-    addStackFrame(label, location, true);
-  }
-
   /**
    * Returns the exception message without the stack trace.
    */
diff --git a/src/main/java/com/google/devtools/skylark/Skylark.java b/src/main/java/com/google/devtools/skylark/Skylark.java
index f7c7d11628..df7932e16b 100644
--- a/src/main/java/com/google/devtools/skylark/Skylark.java
+++ b/src/main/java/com/google/devtools/skylark/Skylark.java
@@ -64,6 +64,7 @@ class Skylark {
     StringBuilder input = new StringBuilder();
     System.out.print(START_PROMPT);
     try {
+      String lineSeparator = "";
       while (true) {
         String line = reader.readLine();
         if (line == null) {
@@ -72,7 +73,8 @@ class Skylark {
         if (line.isEmpty()) {
           return input.toString();
         }
-        input.append("\n").append(line);
+        input.append(lineSeparator).append(line);
+        lineSeparator = "\n";
         System.out.print(CONTINUATION_PROMPT);
       }
     } catch (IOException io) {