Skylark: make ConversionException an EvalException
This avoids using a RuntimeException (IllegalArgumentException)
to circumvent declaration issues, which when we were catching it too well
was hiding actual issues of RuntimeException.
--
MOS_MIGRATED_REVID=95767534
diff --git a/src/main/java/com/google/devtools/build/lib/packages/Type.java b/src/main/java/com/google/devtools/build/lib/packages/Type.java
index 9619162..593838b 100644
--- a/src/main/java/com/google/devtools/build/lib/packages/Type.java
+++ b/src/main/java/com/google/devtools/build/lib/packages/Type.java
@@ -23,6 +23,7 @@
import com.google.common.collect.Maps;
import com.google.devtools.build.lib.packages.License.DistributionType;
import com.google.devtools.build.lib.packages.License.LicenseParsingException;
+import com.google.devtools.build.lib.syntax.EvalException;
import com.google.devtools.build.lib.syntax.EvalUtils;
import com.google.devtools.build.lib.syntax.FilesetEntry;
import com.google.devtools.build.lib.syntax.GlobList;
@@ -359,7 +360,7 @@
* ConversionException is thrown when a type-conversion fails; it contains
* an explanatory error message.
*/
- public static class ConversionException extends Exception {
+ public static class ConversionException extends EvalException {
private static String message(Type<?> type, Object value, String what) {
StringBuilder builder = new StringBuilder();
builder.append("expected value of type '").append(type).append("'");
@@ -373,11 +374,11 @@
}
private ConversionException(Type<?> type, Object value, String what) {
- super(message(type, value, what));
+ super(null, message(type, value, what));
}
private ConversionException(String message) {
- super(message);
+ super(null, message);
}
}
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 af33276..3c041b7 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
@@ -15,7 +15,6 @@
import com.google.common.base.Preconditions;
import com.google.devtools.build.lib.events.Location;
-import com.google.devtools.build.lib.packages.Type.ConversionException;
import com.google.devtools.build.lib.syntax.SkylarkSignatureProcessor.HackHackEitherList;
import com.google.devtools.build.lib.syntax.SkylarkType.SkylarkFunctionType;
@@ -152,16 +151,14 @@
} catch (InvocationTargetException x) {
Throwable e = x.getCause();
if (e instanceof EvalException) {
- throw (EvalException) e;
+ throw ((EvalException) e).ensureLocation(loc);
} else if (e instanceof InterruptedException) {
throw (InterruptedException) e;
- } else if (e instanceof ConversionException
- || e instanceof ClassCastException
+ } else if (e instanceof ClassCastException
|| e instanceof ExecutionException
|| e instanceof IllegalStateException) {
- throw new EvalException(loc, e);
+ throw new EvalException(loc, "in call to " + getName(), e);
} else if (e instanceof IllegalArgumentException) {
- // Assume it was thrown by SkylarkType.cast and has a good message.
throw new EvalException(loc, "Illegal argument in call to " + getName(), e);
} else {
throw badCallException(loc, e, args);
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 ae9c04d..361ce9f 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
@@ -32,7 +32,7 @@
*/
public class EvalException extends Exception {
- private final Location location;
+ private Location location;
private final String message;
private final boolean dueToIncompleteAST;
@@ -118,6 +118,18 @@
}
/**
+ * Ensures that this EvalException has proper location information.
+ * Does nothing if the exception already had a location, or if no location is provided.
+ * @return this EvalException, in fluent style.
+ */
+ public EvalException ensureLocation(Location loc) {
+ if (location == null && loc != null) {
+ location = loc;
+ }
+ return this;
+ }
+
+ /**
* A class to support a special case of EvalException when the cause of the error is an
* Exception during a direct Java call. Allow the throwing code to provide context in a message.
*/
diff --git a/src/main/java/com/google/devtools/build/lib/syntax/SkylarkType.java b/src/main/java/com/google/devtools/build/lib/syntax/SkylarkType.java
index 23275ef..62ffcab 100644
--- a/src/main/java/com/google/devtools/build/lib/syntax/SkylarkType.java
+++ b/src/main/java/com/google/devtools/build/lib/syntax/SkylarkType.java
@@ -19,10 +19,8 @@
import com.google.common.collect.ImmutableMap;
import com.google.common.collect.Interner;
import com.google.common.collect.Interners;
-import com.google.common.collect.Iterables;
import com.google.devtools.build.lib.collect.nestedset.NestedSet;
import com.google.devtools.build.lib.events.Location;
-import com.google.devtools.build.lib.packages.Type.ConversionException;
import java.lang.reflect.Method;
import java.lang.reflect.ParameterizedType;
@@ -631,24 +629,22 @@
/** Cast a List or SkylarkList object into an Iterable of the given type. null means empty List */
public static <TYPE> Iterable<TYPE> castList(
- Object obj, final Class<TYPE> type, final String what) throws ConversionException {
+ Object obj, final Class<TYPE> type, final String what) throws EvalException {
if (obj == null) {
return ImmutableList.of();
}
- return Iterables.transform(com.google.devtools.build.lib.packages.Type.LIST.convert(obj, what),
- new com.google.common.base.Function<Object, TYPE>() {
- @Override
- public TYPE apply(Object input) {
- try {
- return type.cast(input);
- } catch (ClassCastException e) {
- throw new IllegalArgumentException(String.format(
- "expected %s type for '%s' but got %s instead",
- EvalUtils.getDataTypeNameFromClass(type), what,
- EvalUtils.getDataTypeName(input)));
- }
- }
- });
+ List<TYPE> results = new ArrayList<>();
+ for (Object object : com.google.devtools.build.lib.packages.Type.LIST.convert(obj, what)) {
+ try {
+ results.add(type.cast(object));
+ } catch (ClassCastException e) {
+ throw new EvalException(null, String.format(
+ "Illegal argument: expected %s type for '%s' but got %s instead",
+ EvalUtils.getDataTypeNameFromClass(type), what,
+ EvalUtils.getDataTypeName(object)));
+ }
+ }
+ return results;
}
/**
@@ -660,21 +656,22 @@
*/
@SuppressWarnings("unchecked")
public static <KEY_TYPE, VALUE_TYPE> Map<KEY_TYPE, VALUE_TYPE> castMap(Object obj,
- Class<KEY_TYPE> keyType, Class<VALUE_TYPE> valueType, String what) {
+ Class<KEY_TYPE> keyType, Class<VALUE_TYPE> valueType, String what)
+ throws EvalException {
if (obj == null) {
return ImmutableMap.of();
}
if (!(obj instanceof Map<?, ?>)) {
- throw new IllegalArgumentException(String.format(
- "expected a dictionary for %s but got %s instead",
+ throw new EvalException(null, String.format(
+ "Illegal argument: expected a dictionary for %s but got %s instead",
what, EvalUtils.getDataTypeName(obj)));
}
for (Map.Entry<?, ?> input : ((Map<?, ?>) obj).entrySet()) {
if (!keyType.isAssignableFrom(input.getKey().getClass())
|| !valueType.isAssignableFrom(input.getValue().getClass())) {
- throw new IllegalArgumentException(String.format(
- "expected <%s, %s> type for '%s' but got <%s, %s> instead",
+ throw new EvalException(null, String.format(
+ "Illegal argument: expected <%s, %s> type for '%s' but got <%s, %s> instead",
keyType.getSimpleName(), valueType.getSimpleName(), what,
EvalUtils.getDataTypeName(input.getKey()),
EvalUtils.getDataTypeName(input.getValue())));
