Skylark error messages now include a stack trace.
This means that some tests had to be changed from using exact equality of error messages to working with contains() / startsWith().
--
MOS_MIGRATED_REVID=100923593
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 5145f4b..0309af2 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
@@ -396,21 +396,31 @@
}
/**
+ * Returns the environment for the scope of this function.
+ *
+ * <p>Since this is a BaseFunction, we don't create a new environment.
+ */
+ @SuppressWarnings("unused") // For the exception
+ protected Environment getOrCreateChildEnvironment(Environment parent) throws EvalException {
+ return parent;
+ }
+
+ /**
* The outer calling convention to a BaseFunction.
*
* @param args a list of all positional arguments (as in *starArg)
* @param kwargs a map for key arguments (as in **kwArgs)
* @param ast the expression for this function's definition
- * @param env the lexical Environment for the function call
+ * @param parentEnv the lexical Environment for the function call
* @return the value resulting from evaluating the function with the given arguments
* @throws construction of EvalException-s containing source information.
*/
public Object call(@Nullable List<Object> args,
@Nullable Map<String, Object> kwargs,
@Nullable FuncallExpression ast,
- @Nullable Environment env)
+ @Nullable Environment parentEnv)
throws EvalException, InterruptedException {
-
+ Environment env = getOrCreateChildEnvironment(parentEnv);
Preconditions.checkState(isConfigured(), "Function %s was not configured", getName());
// ast is null when called from Java (as there's no Skylark call site).
@@ -421,24 +431,37 @@
try {
return call(arguments, ast, env);
- } catch (ConversionException e) {
- throw new EvalException(loc, e.getMessage());
+ } catch (EvalExceptionWithStackTrace ex) {
+ throw updateStackTrace(ex, loc);
+ } catch (EvalException | RuntimeException | InterruptedException ex) {
+ throw updateStackTrace(new EvalExceptionWithStackTrace(ex, Location.BUILTIN), loc);
}
}
/**
+ * Adds an entry for the current function to the stack trace of the exception.
+ */
+ private EvalExceptionWithStackTrace updateStackTrace(
+ EvalExceptionWithStackTrace ex, Location location) {
+ ex.registerFunction(this);
+ ex.setLocation(location);
+ return ex;
+ }
+
+ /**
* Inner call to a BaseFunction
* subclasses need to @Override this method.
*
* @param args an array of argument values sorted as per the signature.
* @param ast the source code for the function if user-defined
- * @param ast the lexical environment of the function call
+ * @param env the lexical environment of the function call
*/
// Don't make it abstract, so that subclasses may be defined that @Override the outer call() only.
- public Object call(Object[] args,
+ protected Object call(Object[] args,
@Nullable FuncallExpression ast, @Nullable Environment env)
throws EvalException, ConversionException, InterruptedException {
- throw new EvalException(ast.getLocation(),
+ throw new EvalException(
+ (ast == null) ? Location.BUILTIN : ast.getLocation(),
String.format("function %s not implemented", getName()));
}
@@ -498,14 +521,22 @@
}
/**
- * Returns the signature as "[className.]methodName(paramType1, paramType2, ...)"
+ * Returns [class.]function (depending on whether func belongs to a class).
*/
- public String getShortSignature() {
+ public String getFullName() {
+ return String.format("%s%s", getObjectTypeString(), getName());
+ }
+
+ /**
+ * Returns the signature as "[className.]methodName(name1: paramType1, name2: paramType2, ...)"
+ * or "[className.]methodName(paramType1, paramType2, ...)", depending on the value of showNames.
+ */
+ public String getShortSignature(boolean showNames) {
StringBuilder builder = new StringBuilder();
boolean hasSelf = hasSelfArgument();
- builder.append(getObjectTypeString()).append(getName()).append("(");
- signature.toStringBuilder(builder, true, false, false, hasSelf);
+ builder.append(getFullName()).append("(");
+ signature.toStringBuilder(builder, showNames, false, false, hasSelf);
builder.append(")");
return builder.toString();
diff --git a/src/main/java/com/google/devtools/build/lib/syntax/BuildFileAST.java b/src/main/java/com/google/devtools/build/lib/syntax/BuildFileAST.java
index e8f901b..e2a333a 100644
--- a/src/main/java/com/google/devtools/build/lib/syntax/BuildFileAST.java
+++ b/src/main/java/com/google/devtools/build/lib/syntax/BuildFileAST.java
@@ -190,12 +190,9 @@
// BUILD file (as it is the most probable cause for the error).
Location exnLoc = e.getLocation();
Location nodeLoc = stmt.getLocation();
- if (exnLoc == null || !nodeLoc.getPath().equals(exnLoc.getPath())) {
- eventHandler.handle(Event.error(nodeLoc,
- e.getMessage() + " (raised from " + exnLoc + ")"));
- } else {
- eventHandler.handle(Event.error(exnLoc, e.getMessage()));
- }
+ eventHandler.handle(Event.error(
+ (exnLoc == null || !nodeLoc.getPath().equals(exnLoc.getPath())) ? nodeLoc : exnLoc,
+ e.getMessage()));
}
}
return ok;
diff --git a/src/main/java/com/google/devtools/build/lib/syntax/BuiltinFunction.java b/src/main/java/com/google/devtools/build/lib/syntax/BuiltinFunction.java
index 7fc85ba..70173ce 100644
--- a/src/main/java/com/google/devtools/build/lib/syntax/BuiltinFunction.java
+++ b/src/main/java/com/google/devtools/build/lib/syntax/BuiltinFunction.java
@@ -177,7 +177,7 @@
loc,
String.format(
"Method %s is not applicable for arguments %s: '%s' is %s, but should be %s",
- getShortSignature(), printTypeString(args, args.length - extraArgsCount),
+ getShortSignature(true), printTypeString(args, args.length - extraArgsCount),
paramName, EvalUtils.getDataTypeName(args[i]),
EvalUtils.getDataTypeNameFromClass(types[i])));
}
diff --git a/src/main/java/com/google/devtools/build/lib/syntax/DotExpression.java b/src/main/java/com/google/devtools/build/lib/syntax/DotExpression.java
index 1b98a27..e6ae8a0 100644
--- a/src/main/java/com/google/devtools/build/lib/syntax/DotExpression.java
+++ b/src/main/java/com/google/devtools/build/lib/syntax/DotExpression.java
@@ -70,8 +70,12 @@
*/
public static Object eval(Object objValue, String name, Location loc) throws EvalException {
if (objValue instanceof ClassObject) {
- Object result = ((ClassObject) objValue).getValue(name);
-
+ Object result = null;
+ try {
+ result = ((ClassObject) objValue).getValue(name);
+ } catch (IllegalArgumentException ex) {
+ throw new EvalException(loc, ex);
+ }
// ClassObjects may have fields that are annotated with @SkylarkCallable.
// Since getValue() does not know about those, we cannot expect that result is a valid object.
if (result != null) {
diff --git a/src/main/java/com/google/devtools/build/lib/syntax/EvalException.java b/src/main/java/com/google/devtools/build/lib/syntax/EvalException.java
index 361ce9f..32f289b 100644
--- a/src/main/java/com/google/devtools/build/lib/syntax/EvalException.java
+++ b/src/main/java/com/google/devtools/build/lib/syntax/EvalException.java
@@ -70,7 +70,7 @@
if (message == null) {
message = "";
}
- if (cause != null) {
+ if (cause != null && !message.contains(cause.getMessage())) {
message = message + (message.isEmpty() ? "" : ": ") + cause.getMessage();
}
if (message.isEmpty()) {
@@ -92,7 +92,16 @@
return (getLocation() == null ? "" : getLocation()) + ": "
+ (message == null ? "" : message + "\n")
+ (dueToIncompleteAST ? "due to incomplete AST\n" : "")
- + (getCause() != null && getCause().getMessage() != null ? getCause().getMessage() : "");
+ + getCauseMessage();
+ }
+
+ private String getCauseMessage() {
+ Throwable cause = getCause();
+ if (cause == null) {
+ return "";
+ }
+ String causeMessage = cause.getMessage();
+ return (causeMessage == null || message.contains(causeMessage)) ? "" : causeMessage;
}
/**
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
new file mode 100644
index 0000000..5047521
--- /dev/null
+++ b/src/main/java/com/google/devtools/build/lib/syntax/EvalExceptionWithStackTrace.java
@@ -0,0 +1,82 @@
+// Copyright 2015 Google Inc. All rights reserved.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+package com.google.devtools.build.lib.syntax;
+
+import com.google.devtools.build.lib.events.Location;
+import com.google.devtools.build.lib.packages.Rule;
+
+/**
+ * EvalException with a stack trace
+ */
+public class EvalExceptionWithStackTrace extends EvalException {
+ private final StringBuilder builder = new StringBuilder();
+ private Location location;
+ private boolean printStackTrace;
+
+ 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.
+ */
+ public void registerFunction(BaseFunction function) {
+ addStackFrame(function.getFullName());
+ }
+
+ /**
+ * Adds a line 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()));
+ }
+
+ /**
+ * Adds a line for the given scope (function or rule).
+ */
+ 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;
+ }
+
+ /**
+ * Returns the exception message without the stack trace.
+ */
+ public String getOriginalMessage() {
+ return super.getMessage();
+ }
+
+ @Override
+ public String getMessage() {
+ return print();
+ }
+
+ @Override
+ public String print() {
+ // Only print the stack trace when it contains more than one built-in function.
+ return printStackTrace ? builder.toString() : getOriginalMessage();
+ }
+}
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 a448325..32fda83 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,64 +481,91 @@
@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());
+ }
+ }
+
+ /**
+ * Invokes obj.func() and returns the result.
+ */
+ private Object invokeObjectMethod(Environment env) throws EvalException, InterruptedException {
+ Object objValue = obj.eval(env);
ImmutableList.Builder<Object> posargs = new ImmutableList.Builder<>();
// We copy this into an ImmutableMap in the end, but we can't use an ImmutableMap.Builder, or
// we'd still have to have a HashMap on the side for the sake of properly handling duplicates.
Map<String, Object> kwargs = new HashMap<>();
- Object returnValue;
- BaseFunction function;
- if (obj != null) { // obj.func(...)
- Object objValue = obj.eval(env);
- // Strings, lists and dictionaries (maps) have functions that we want to use in MethodLibrary.
- // For other classes, we can call the Java methods.
- function =
- env.getFunction(EvalUtils.getSkylarkType(objValue.getClass()), func.getName());
- if (function != null) {
- if (!isNamespace(objValue.getClass())) {
- // Add self as an implicit parameter in front.
- posargs.add(objValue);
- }
- evalArguments(posargs, kwargs, env, function);
- returnValue = function.call(
- posargs.build(), ImmutableMap.<String, Object>copyOf(kwargs), this, env);
- } else if (env.isSkylarkEnabled()) {
- // Only allow native Java calls when using Skylark
- // When calling a Java method, the name is not in the Environment,
- // so evaluating 'func' would fail.
- evalArguments(posargs, kwargs, env, null);
- if (!kwargs.isEmpty()) {
- throw new EvalException(func.getLocation(),
- String.format("Keyword arguments are not allowed when calling a java method"
- + "\nwhile calling method '%s' on object %s of type %s",
- func.getName(), objValue, EvalUtils.getDataTypeName(objValue)));
- }
- if (objValue instanceof Class<?>) {
- // Static Java method call. We can return the value from here directly because
- // invokeJavaMethod() has special checks.
- return invokeJavaMethod(null, (Class<?>) objValue, func.getName(), posargs.build());
- } else {
- return invokeJavaMethod(objValue, objValue.getClass(), func.getName(), posargs.build());
- }
- } else {
- throw new EvalException(getLocation(), String.format(
- "%s is not defined on object of type '%s'",
- functionName(), EvalUtils.getDataTypeName(objValue)));
+ // Strings, lists and dictionaries (maps) have functions that we want to use in
+ // MethodLibrary.
+ // For other classes, we can call the Java methods.
+ BaseFunction function =
+ env.getFunction(EvalUtils.getSkylarkType(objValue.getClass()), func.getName());
+ if (function != null) {
+ if (!isNamespace(objValue.getClass())) {
+ // Add self as an implicit parameter in front.
+ posargs.add(objValue);
}
- } else { // func(...)
- Object funcValue = func.eval(env);
- if ((funcValue instanceof BaseFunction)) {
- function = (BaseFunction) funcValue;
- evalArguments(posargs, kwargs, env, function);
- returnValue = function.call(
- posargs.build(), ImmutableMap.<String, Object>copyOf(kwargs), this, env);
- } else {
- throw new EvalException(getLocation(),
- "'" + EvalUtils.getDataTypeName(funcValue)
- + "' object is not callable");
+ evalArguments(posargs, kwargs, env, function);
+ return convertFromSkylark(
+ function.call(posargs.build(), ImmutableMap.<String, Object>copyOf(kwargs), this, env),
+ env);
+ } else if (env.isSkylarkEnabled()) {
+ // Only allow native Java calls when using Skylark
+ // When calling a Java method, the name is not in the Environment,
+ // so evaluating 'func' would fail.
+ evalArguments(posargs, kwargs, env, null);
+ if (!kwargs.isEmpty()) {
+ throw new EvalException(
+ func.getLocation(),
+ String.format(
+ "Keyword arguments are not allowed when calling a java method"
+ + "\nwhile calling method '%s' on object %s of type %s",
+ func.getName(), objValue, EvalUtils.getDataTypeName(objValue)));
}
+ if (objValue instanceof Class<?>) {
+ // Static Java method call. We can return the value from here directly because
+ // invokeJavaMethod() has special checks.
+ return invokeJavaMethod(null, (Class<?>) objValue, func.getName(), posargs.build());
+ } else {
+ return invokeJavaMethod(objValue, objValue.getClass(), func.getName(), posargs.build());
+ }
+ } else {
+ throw new EvalException(
+ getLocation(),
+ String.format("%s is not defined on object of type '%s'", functionName(),
+ EvalUtils.getDataTypeName(objValue)));
}
+ }
+ /**
+ * Invokes func() and returns the result.
+ */
+ private Object invokeGlobalFunction(Environment env) throws EvalException, InterruptedException {
+ Object funcValue = func.eval(env);
+ ImmutableList.Builder<Object> posargs = new ImmutableList.Builder<>();
+ // We copy this into an ImmutableMap in the end, but we can't use an ImmutableMap.Builder, or
+ // we'd still have to have a HashMap on the side for the sake of properly handling duplicates.
+ Map<String, Object> kwargs = new HashMap<>();
+ if ((funcValue instanceof BaseFunction)) {
+ BaseFunction function = (BaseFunction) funcValue;
+ evalArguments(posargs, kwargs, env, function);
+ return convertFromSkylark(
+ function.call(posargs.build(), ImmutableMap.<String, Object>copyOf(kwargs), this, env),
+ env);
+ } else {
+ throw new EvalException(
+ getLocation(), "'" + EvalUtils.getDataTypeName(funcValue) + "' object is not callable");
+ }
+ }
+
+ protected Object convertFromSkylark(Object returnValue, Environment env) throws EvalException {
EvalUtils.checkNotNull(this, returnValue);
if (!env.isSkylarkEnabled()) {
// The call happens in the BUILD language. Note that accessing "BUILD language" functions in
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 5831a28..334a5eb 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
@@ -46,35 +46,33 @@
return location;
}
- /**
- * Since the types of parameters of user defined functions are unknown, we just return
- * "name(parameterCount)"
- */
- @Override
- public String getShortSignature() {
- return String.format("%s(%d)", getName(), getArgArraySize());
- }
-
@Override
public Object call(Object[] arguments, FuncallExpression ast, Environment env)
throws EvalException, InterruptedException {
- SkylarkEnvironment functionEnv = SkylarkEnvironment.createEnvironmentForFunctionCalling(
- env, definitionEnv, this);
ImmutableList<String> names = signature.getSignature().getNames();
// Registering the functions's arguments as variables in the local Environment
int i = 0;
for (String name : names) {
- functionEnv.update(name, arguments[i++]);
+ env.update(name, arguments[i++]);
}
try {
for (Statement stmt : statements) {
- stmt.exec(functionEnv);
+ stmt.exec(env);
}
} catch (ReturnStatement.ReturnException e) {
return e.getValue();
}
return Environment.NONE;
}
+
+ /**
+ * Creates a new environment for the execution of this function.
+ */
+ @Override
+ protected Environment getOrCreateChildEnvironment(Environment parent) throws EvalException {
+ return SkylarkEnvironment.createEnvironmentForFunctionCalling(
+ parent, definitionEnv, this);
+ }
}
