Skylark stack traces are now displayed in Python format.
--
MOS_MIGRATED_REVID=101572295
diff --git a/src/main/java/com/google/devtools/build/lib/syntax/BaseFunction.java b/src/main/java/com/google/devtools/build/lib/syntax/BaseFunction.java
index 7574e1f..43d27a2 100644
--- a/src/main/java/com/google/devtools/build/lib/syntax/BaseFunction.java
+++ b/src/main/java/com/google/devtools/build/lib/syntax/BaseFunction.java
@@ -429,7 +429,7 @@
} catch (EvalExceptionWithStackTrace ex) {
throw updateStackTrace(ex, loc);
} catch (EvalException | RuntimeException | InterruptedException ex) {
- throw updateStackTrace(new EvalExceptionWithStackTrace(ex, Location.BUILTIN), loc);
+ throw updateStackTrace(new EvalExceptionWithStackTrace(ex, loc), loc);
}
}
@@ -438,8 +438,7 @@
*/
private EvalExceptionWithStackTrace updateStackTrace(
EvalExceptionWithStackTrace ex, Location location) {
- ex.registerFunction(this);
- ex.setLocation(location);
+ ex.registerFunction(this, location);
return ex;
}
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 5047521..d967962 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
@@ -13,53 +13,55 @@
// limitations under the License.
package com.google.devtools.build.lib.syntax;
+import com.google.common.base.Joiner;
+import com.google.common.base.Preconditions;
import com.google.devtools.build.lib.events.Location;
import com.google.devtools.build.lib.packages.Rule;
+import com.google.devtools.build.lib.vfs.PathFragment;
+
+import java.util.Deque;
+import java.util.LinkedList;
/**
* EvalException with a stack trace
*/
public class EvalExceptionWithStackTrace extends EvalException {
- private final StringBuilder builder = new StringBuilder();
- private Location location;
- private boolean printStackTrace;
+
+ private StackTraceElement mostRecentElement;
public EvalExceptionWithStackTrace(Exception original, Location callLocation) {
super(callLocation, original.getMessage(), original.getCause());
- setLocation(callLocation);
- builder.append(super.getMessage());
- printStackTrace = false;
}
/**
- * Adds a line for the given function to the stack trace. Requires that #setLocation() was called
- * previously.
+ * Adds an entry for the given statement to the stack trace.
*/
- public void registerFunction(BaseFunction function) {
- addStackFrame(function.getFullName());
+ public void registerStatement(Statement statement) {
+ Preconditions.checkState(
+ mostRecentElement == null, "Cannot add a statement to a non-empty stack trace.");
+ addStackFrame(statement.toString().trim(), statement.getLocation());
}
/**
- * Adds a line for the given rule to the stack trace.
+ * Adds an entry for the given function to the stack trace.
+ */
+ public void registerFunction(BaseFunction function, Location location) {
+ addStackFrame(function.getFullName(), location);
+ }
+
+ /**
+ * Adds an entry for the given rule to the stack trace.
*/
public void registerRule(Rule rule) {
- setLocation(rule.getLocation());
- addStackFrame(String.format("%s(name = '%s', ...)", rule.getRuleClass(), rule.getName()));
+ addStackFrame(
+ String.format("%s(name = '%s')", rule.getRuleClass(), rule.getName()), rule.getLocation());
}
/**
- * Adds a line for the given scope (function or rule).
+ * Adds a line for the given frame.
*/
- private void addStackFrame(String scope) {
- builder.append(String.format("\n\tin %s [%s]", scope, location));
- printStackTrace |= (location != Location.BUILTIN);
- }
-
- /**
- * Sets the location for the next function to be added via #registerFunction().
- */
- public void setLocation(Location callLocation) {
- this.location = callLocation;
+ private void addStackFrame(String label, Location location) {
+ mostRecentElement = new StackTraceElement(label, location, mostRecentElement);
}
/**
@@ -76,7 +78,141 @@
@Override
public String print() {
- // Only print the stack trace when it contains more than one built-in function.
- return printStackTrace ? builder.toString() : getOriginalMessage();
+ return print(StackTracePrinter.INSTANCE);
+ }
+
+ /**
+ * Prints the stack trace iff it contains more than just one built-in function.
+ */
+ public String print(StackTracePrinter printer) {
+ return canPrintStackTrace()
+ ? printer.print(getOriginalMessage(), mostRecentElement)
+ : getOriginalMessage();
+ }
+
+ /**
+ * Returns true when there is at least one non-built-in element.
+ */
+ protected boolean canPrintStackTrace() {
+ return mostRecentElement != null && mostRecentElement.getCause() != null;
+ }
+
+ /**
+ * An element in the stack trace which contains the name of the offending function / rule /
+ * statement and its location.
+ */
+ protected final class StackTraceElement {
+ private final String label;
+ private final Location location;
+ private final StackTraceElement cause;
+
+ StackTraceElement(String label, Location location, StackTraceElement cause) {
+ this.label = label;
+ this.location = location;
+ this.cause = cause;
+ }
+
+ String getLabel() {
+ return label;
+ }
+
+ Location getLocation() {
+ return location;
+ }
+
+ StackTraceElement getCause() {
+ return cause;
+ }
+ }
+
+ /**
+ * Singleton class that prints stack traces similar to Python.
+ */
+ public enum StackTracePrinter {
+ INSTANCE;
+
+ /**
+ * Turns the given message and StackTraceElements into a string.
+ */
+ public final String print(String message, StackTraceElement mostRecentElement) {
+ Deque<String> output = new LinkedList<>();
+
+ while (mostRecentElement != null) {
+ String entry = print(mostRecentElement);
+ if (entry != null && entry.length() > 0) {
+ addEntry(output, entry);
+ }
+
+ mostRecentElement = mostRecentElement.getCause();
+ }
+
+ addMessage(output, message);
+ return Joiner.on("\n").join(output);
+ }
+
+ /**
+ * Returns the location which should be shown on the same line as the label of the given
+ * element.
+ */
+ protected Location getDisplayLocation(StackTraceElement element) {
+ // If there is a rule definition in this element, it should print its own location in
+ // the BUILD file instead of using a location in a bzl file.
+ return describesRule(element) ? element.getLocation() : getLocation(element.getCause());
+ }
+
+ /**
+ * Returns the location of the given element or Location.BUILTIN if the element is null.
+ */
+ private Location getLocation(StackTraceElement element) {
+ return (element == null) ? Location.BUILTIN : element.getLocation();
+ }
+
+ /**
+ * Returns whether the given element describes the rule definition in a BUILD file.
+ */
+ protected boolean describesRule(StackTraceElement element) {
+ PathFragment pathFragment = element.getLocation().getPath();
+ return pathFragment != null && pathFragment.getPathString().contains("BUILD");
+ }
+
+ /**
+ * Returns the string representation of the given element.
+ */
+ protected String print(StackTraceElement element) {
+ // Similar to Python, the first (most-recent) entry in the stack frame is printed only once.
+ // Consequently, we skip it here.
+ if (element.getCause() == null) {
+ return "";
+ }
+
+ // Prints a two-line string, similar to Python.
+ Location location = getDisplayLocation(element);
+ return String.format(
+ "\tFile \"%s\", line %d, in %s%n\t\t%s",
+ printPath(location.getPath()),
+ location.getStartLineAndColumn().getLine(),
+ element.getLabel(),
+ element.getCause().getLabel());
+ }
+
+ private String printPath(PathFragment path) {
+ return (path == null) ? "<unknown>" : path.getPathString();
+ }
+
+ /**
+ * Adds the given string to the specified Deque.
+ */
+ protected void addEntry(Deque<String> output, String toAdd) {
+ output.addLast(toAdd);
+ }
+
+ /**
+ * Adds the given message to the given output dequeue after all stack trace elements have been
+ * added.
+ */
+ protected void addMessage(Deque<String> output, String message) {
+ output.addFirst("Traceback (most recent call last):");
+ output.addLast(message);
+ }
}
}
diff --git a/src/main/java/com/google/devtools/build/lib/syntax/FuncallExpression.java b/src/main/java/com/google/devtools/build/lib/syntax/FuncallExpression.java
index 5ebaa0e..2e90afd 100644
--- a/src/main/java/com/google/devtools/build/lib/syntax/FuncallExpression.java
+++ b/src/main/java/com/google/devtools/build/lib/syntax/FuncallExpression.java
@@ -481,15 +481,7 @@
@Override
Object eval(Environment env) throws EvalException, InterruptedException {
- try {
- return (obj != null) ? invokeObjectMethod(env) : invokeGlobalFunction(env);
- } catch (EvalException ex) {
- // BaseFunction will not get the exact location of some errors (such as exceptions thrown in
- // #invokeJavaMethod), so we create a new stack trace with the correct location here.
- throw (ex instanceof EvalExceptionWithStackTrace)
- ? ex
- : new EvalExceptionWithStackTrace(ex, ex.getLocation());
- }
+ return (obj != null) ? invokeObjectMethod(env) : invokeGlobalFunction(env);
}
/**
diff --git a/src/main/java/com/google/devtools/build/lib/syntax/UserDefinedFunction.java b/src/main/java/com/google/devtools/build/lib/syntax/UserDefinedFunction.java
index 334a5eb..750429d 100644
--- a/src/main/java/com/google/devtools/build/lib/syntax/UserDefinedFunction.java
+++ b/src/main/java/com/google/devtools/build/lib/syntax/UserDefinedFunction.java
@@ -57,12 +57,23 @@
env.update(name, arguments[i++]);
}
+ Statement lastStatement = null;
try {
for (Statement stmt : statements) {
+ lastStatement = stmt;
stmt.exec(env);
}
} catch (ReturnStatement.ReturnException e) {
return e.getValue();
+ } catch (EvalExceptionWithStackTrace ex) {
+ // We need this block since the next "catch" must only catch EvalExceptions that don't have a
+ // stack trace yet.
+ throw ex;
+ } catch (EvalException ex) {
+ EvalExceptionWithStackTrace real =
+ new EvalExceptionWithStackTrace(ex, lastStatement.getLocation());
+ real.registerStatement(lastStatement);
+ throw real;
}
return Environment.NONE;
}
diff --git a/src/test/java/com/google/devtools/build/lib/syntax/MethodLibraryTest.java b/src/test/java/com/google/devtools/build/lib/syntax/MethodLibraryTest.java
index 1a489d2..29a9459 100644
--- a/src/test/java/com/google/devtools/build/lib/syntax/MethodLibraryTest.java
+++ b/src/test/java/com/google/devtools/build/lib/syntax/MethodLibraryTest.java
@@ -42,18 +42,35 @@
}
@Test
+ public void testStackTraceSkipBuiltInOnly() throws Exception {
+ // The error message should not include the stack trace when there is
+ // only one built-in function.
+ new SkylarkTest()
+ .testIfExactError(
+ "Method string.index(sub: string, start: int, end: int or NoneType) is not applicable "
+ + "for arguments (int, int, NoneType): 'sub' is int, but should be string",
+ "'test'.index(1)");
+ }
+
+ @Test
public void testStackTrace() throws Exception {
- new SkylarkTest().testIfExactError(
- "Method string.index(sub: string, start: int, end: int or NoneType) is not "
- + "applicable for arguments (int, int, NoneType): 'sub' is int, but should be string\n"
- + "\tin string.index [Built-In]\n"
- + "\tin bar [4:4]\n"
- + "\tin foo [2:4]",
- "def foo():",
- " bar(1)",
- "def bar(x):",
- " 'test'.index(x)",
- "foo()");
+ // Unlike SkylarintegrationTests#testStackTraceErrorInFunction(), this test
+ // has neither a BUILD nor a bzl file.
+ new SkylarkTest()
+ .testIfExactError(
+ "Traceback (most recent call last):\n"
+ + "\tFile \"<unknown>\", line 2, in foo\n"
+ + "\t\tbar\n"
+ + "\tFile \"<unknown>\", line 4, in bar\n"
+ + "\t\tstring.index\n"
+ + "Method string.index(sub: string, start: int, end: int or NoneType) "
+ + "is not applicable "
+ + "for arguments (int, int, NoneType): 'sub' is int, but should be string",
+ "def foo():",
+ " bar(1)",
+ "def bar(x):",
+ " 'test'.index(x)",
+ "foo()");
}
@Test
diff --git a/src/test/java/com/google/devtools/build/lib/syntax/SkylarkEvaluationTest.java b/src/test/java/com/google/devtools/build/lib/syntax/SkylarkEvaluationTest.java
index 22c7153..b11866d 100644
--- a/src/test/java/com/google/devtools/build/lib/syntax/SkylarkEvaluationTest.java
+++ b/src/test/java/com/google/devtools/build/lib/syntax/SkylarkEvaluationTest.java
@@ -283,8 +283,11 @@
public void testForNotIterable() throws Exception {
new SkylarkTest()
.update("mock", new Mock())
- .testIfExactError("type 'int' is not iterable", "def func():",
- " for i in mock.value_of('1'): a = i", "func()\n");
+ .testIfErrorContains(
+ "type 'int' is not iterable",
+ "def func():",
+ " for i in mock.value_of('1'): a = i",
+ "func()\n");
}
@Test
@@ -777,14 +780,16 @@
@Test
public void testListIndexAsLValueAsLValue() throws Exception {
- new SkylarkTest().testIfExactError("unsupported operand type(s) for +: 'list' and 'dict'",
- "def id(l):",
- " return l",
- "def func():",
- " l = id([1])",
- " l[0] = 2",
- " return l",
- "l = func()");
+ new SkylarkTest()
+ .testIfErrorContains(
+ "unsupported operand type(s) for +: 'list' and 'dict'",
+ "def id(l):",
+ " return l",
+ "def func():",
+ " l = id([1])",
+ " l[0] = 2",
+ " return l",
+ "l = func()");
}
@Test
@@ -987,23 +992,32 @@
@Override
@Test
public void testListComprehensionsMultipleVariablesFail() throws Exception {
- new SkylarkTest().testIfExactError("lvalue has length 3, but rvalue has has length 2",
- "def foo (): return [x + y for x, y, z in [(1, 2), (3, 4)]]",
- "foo()");
+ new SkylarkTest()
+ .testIfErrorContains(
+ "lvalue has length 3, but rvalue has has length 2",
+ "def foo (): return [x + y for x, y, z in [(1, 2), (3, 4)]]",
+ "foo()");
- new SkylarkTest().testIfExactError("type 'int' is not a collection",
- "def bar (): return [x + y for x, y in (1, 2)]",
- "bar()");
+ new SkylarkTest()
+ .testIfErrorContains(
+ "type 'int' is not a collection",
+ "def bar (): return [x + y for x, y in (1, 2)]",
+ "bar()");
- new SkylarkTest().testIfExactError("lvalue has length 3, but rvalue has has length 2",
- "[x + y for x, y, z in [(1, 2), (3, 4)]]");
+ new SkylarkTest()
+ .testIfErrorContains(
+ "lvalue has length 3, but rvalue has has length 2",
+ "[x + y for x, y, z in [(1, 2), (3, 4)]]");
// can't reuse the same local variable twice(!)
- new SkylarkTest().testIfExactError("ERROR 2:1: Variable x is read only",
- "[x + y for x, y in (1, 2)]", "[x + y for x, y in (1, 2)]");
+ new SkylarkTest()
+ .testIfErrorContains(
+ "ERROR 2:1: Variable x is read only",
+ "[x + y for x, y in (1, 2)]",
+ "[x + y for x, y in (1, 2)]");
- new SkylarkTest().testIfExactError("type 'int' is not a collection",
- "[x2 + y2 for x2, y2 in (1, 2)]");
+ new SkylarkTest()
+ .testIfErrorContains("type 'int' is not a collection", "[x2 + y2 for x2, y2 in (1, 2)]");
}
@Override
