MethodDescriptor.call: don't convert unchecked exceptions into EvalException.
Also, move some code that doesn't need to be in the try block out of it.
Closes #8266.
PiperOrigin-RevId: 251673653
diff --git a/src/main/java/com/google/devtools/build/lib/bazel/repository/skylark/SkylarkRepositoryFunction.java b/src/main/java/com/google/devtools/build/lib/bazel/repository/skylark/SkylarkRepositoryFunction.java
index 839e228..c251bac 100644
--- a/src/main/java/com/google/devtools/build/lib/bazel/repository/skylark/SkylarkRepositoryFunction.java
+++ b/src/main/java/com/google/devtools/build/lib/bazel/repository/skylark/SkylarkRepositoryFunction.java
@@ -146,12 +146,11 @@
// all label-arguments can be resolved to paths.
try {
skylarkRepositoryContext.enforceLabelAttributes();
+ } catch (RepositoryMissingDependencyException e) {
+ // Missing values are expected; just restart before we actually start the rule
+ return null;
} catch (EvalException e) {
- if (e instanceof RepositoryMissingDependencyException) {
- // Missing values are expected; just restart before we actually start the rule
- return null;
- }
- // Other EvalExceptions indicate labels not referring to existing files. This is fine,
+ // EvalExceptions indicate labels not referring to existing files. This is fine,
// as long as they are never resolved to files in the execution of the rule; we allow
// non-strict rules. So now we have to start evaluating the actual rule, even if that
// means the rule might get restarted for legitimate reasons.
@@ -193,18 +192,17 @@
}
}
env.getListener().post(resolved);
- } catch (EvalException e) {
- if (e.getCause() instanceof RepositoryMissingDependencyException) {
- // A dependency is missing, cleanup and returns null
- try {
- if (outputDirectory.exists()) {
- outputDirectory.deleteTree();
- }
- } catch (IOException e1) {
- throw new RepositoryFunctionException(e1, Transience.TRANSIENT);
+ } catch (RepositoryMissingDependencyException e) {
+ // A dependency is missing, cleanup and returns null
+ try {
+ if (outputDirectory.exists()) {
+ outputDirectory.deleteTree();
}
- return null;
+ } catch (IOException e1) {
+ throw new RepositoryFunctionException(e1, Transience.TRANSIENT);
}
+ return null;
+ } catch (EvalException e) {
env.getListener()
.handle(
Event.error(
diff --git a/src/main/java/com/google/devtools/build/lib/exec/local/XcodeLocalEnvProvider.java b/src/main/java/com/google/devtools/build/lib/exec/local/XcodeLocalEnvProvider.java
index c2c5ea6..f64291e 100644
--- a/src/main/java/com/google/devtools/build/lib/exec/local/XcodeLocalEnvProvider.java
+++ b/src/main/java/com/google/devtools/build/lib/exec/local/XcodeLocalEnvProvider.java
@@ -93,7 +93,7 @@
String developerDir = "";
if (containsXcodeVersion) {
String version = env.get(AppleConfiguration.XCODE_VERSION_ENV_NAME);
- developerDir = getDeveloperDir(binTools, DottedVersion.fromString(version));
+ developerDir = getDeveloperDir(binTools, DottedVersion.fromStringUnchecked(version));
newEnvBuilder.put("DEVELOPER_DIR", developerDir);
}
if (containsAppleSdkVersion) {
diff --git a/src/main/java/com/google/devtools/build/lib/packages/RuleClass.java b/src/main/java/com/google/devtools/build/lib/packages/RuleClass.java
index 86730ac..4528501 100644
--- a/src/main/java/com/google/devtools/build/lib/packages/RuleClass.java
+++ b/src/main/java/com/google/devtools/build/lib/packages/RuleClass.java
@@ -211,7 +211,15 @@
* messaging should be done via {@link RuleErrorConsumer}; this exception only interrupts
* configured target creation in cases where it can no longer continue.
*/
- public static final class RuleErrorException extends Exception {}
+ public static final class RuleErrorException extends Exception {
+ public RuleErrorException() {
+ super();
+ }
+
+ public RuleErrorException(String message) {
+ super(message);
+ }
+ }
}
/**
diff --git a/src/main/java/com/google/devtools/build/lib/packages/RuleErrorConsumer.java b/src/main/java/com/google/devtools/build/lib/packages/RuleErrorConsumer.java
index 8b63425..68dc985 100644
--- a/src/main/java/com/google/devtools/build/lib/packages/RuleErrorConsumer.java
+++ b/src/main/java/com/google/devtools/build/lib/packages/RuleErrorConsumer.java
@@ -55,7 +55,7 @@
*/
default RuleErrorException throwWithRuleError(String message) throws RuleErrorException {
ruleError(message);
- throw new RuleErrorException();
+ throw new RuleErrorException(message);
}
/**
@@ -70,7 +70,7 @@
default RuleErrorException throwWithAttributeError(String attrName, String message)
throws RuleErrorException {
attributeError(attrName, message);
- throw new RuleErrorException();
+ throw new RuleErrorException(message);
}
/**
diff --git a/src/main/java/com/google/devtools/build/lib/rules/android/AndroidAssets.java b/src/main/java/com/google/devtools/build/lib/rules/android/AndroidAssets.java
index 6f8da8a..5da1090 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/android/AndroidAssets.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/android/AndroidAssets.java
@@ -91,12 +91,11 @@
PathFragment path = file.getExecPath();
assetRoots.add(path.subFragment(0, path.segmentCount() - relativePath.segmentCount()));
} else {
- errorConsumer.attributeError(
+ errorConsumer.throwWithAttributeError(
ASSETS_ATTR,
String.format(
"'%s' (generated by '%s') is not beneath '%s'",
file.getRootRelativePath(), target.getLabel(), assetsDir));
- throw new RuleErrorException();
}
assets.add(file);
}
diff --git a/src/main/java/com/google/devtools/build/lib/rules/apple/ApplePlatform.java b/src/main/java/com/google/devtools/build/lib/rules/apple/ApplePlatform.java
index d33ebe6..959c05e 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/apple/ApplePlatform.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/apple/ApplePlatform.java
@@ -192,6 +192,13 @@
printer.append(toString());
}
+ /** Exception indicating an unknown or unsupported Apple platform type. */
+ public static class UnsupportedPlatformTypeException extends Exception {
+ public UnsupportedPlatformTypeException(String msg) {
+ super(msg);
+ }
+ }
+
/**
* Value used to describe Apple platform "type". A {@link ApplePlatform} is implied from a
* platform type (for example, watchOS) together with a cpu value (for example, armv7).
@@ -222,15 +229,16 @@
/**
* Returns the {@link PlatformType} with given name (case insensitive).
*
- * @throws IllegalArgumentException if the name does not match a valid platform type.
+ * @throws UnsupportedPlatformTypeException if the name does not match a valid platform type.
*/
- public static PlatformType fromString(String name) {
+ public static PlatformType fromString(String name) throws UnsupportedPlatformTypeException {
for (PlatformType platformType : PlatformType.values()) {
if (name.equalsIgnoreCase(platformType.toString())) {
return platformType;
}
}
- throw new IllegalArgumentException(String.format("Unsupported platform type \"%s\"", name));
+ throw new UnsupportedPlatformTypeException(
+ String.format("Unsupported platform type \"%s\"", name));
}
/** Returns a Skylark struct that contains the instances of this enum. */
diff --git a/src/main/java/com/google/devtools/build/lib/rules/apple/AppleToolchain.java b/src/main/java/com/google/devtools/build/lib/rules/apple/AppleToolchain.java
index a19fa35..99a47c7 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/apple/AppleToolchain.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/apple/AppleToolchain.java
@@ -106,7 +106,7 @@
case IOS_SIMULATOR:
if (xcodeConfig
.getSdkVersionForPlatform(targetPlatform)
- .compareTo(DottedVersion.fromString("9.0"))
+ .compareTo(DottedVersion.fromStringUnchecked("9.0"))
>= 0) {
relativePath = SYSTEM_FRAMEWORK_PATH;
} else {
diff --git a/src/main/java/com/google/devtools/build/lib/rules/apple/DottedVersion.java b/src/main/java/com/google/devtools/build/lib/rules/apple/DottedVersion.java
index b4cc5ff..3ab3aea 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/apple/DottedVersion.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/apple/DottedVersion.java
@@ -129,14 +129,37 @@
private static final String NO_ALPHA_SEQUENCE = null;
private static final Component ZERO_COMPONENT = new Component(0, NO_ALPHA_SEQUENCE, 0, "0");
+ /** Exception thrown when parsing an invalid dotted version. */
+ public static class InvalidDottedVersionException extends Exception {
+ InvalidDottedVersionException(String msg) {
+ super(msg);
+ }
+
+ InvalidDottedVersionException(String msg, Throwable cause) {
+ super(msg, cause);
+ }
+ }
+
+ /**
+ * Create a dotted version by parsing the given version string. Throws an unchecked exception if
+ * the argument is malformed.
+ */
+ public static DottedVersion fromStringUnchecked(String version) {
+ try {
+ return fromString(version);
+ } catch (InvalidDottedVersionException e) {
+ throw new IllegalArgumentException(e);
+ }
+ }
+
/**
* Generates a new dotted version from the given version string.
*
- * @throws IllegalArgumentException if the passed string is not a valid dotted version
+ * @throws InvalidDottedVersionException if the passed string is not a valid dotted version
*/
- public static DottedVersion fromString(String version) {
+ public static DottedVersion fromString(String version) throws InvalidDottedVersionException {
if (Strings.isNullOrEmpty(version)) {
- throw new IllegalArgumentException(String.format(ILLEGAL_VERSION, version));
+ throw new InvalidDottedVersionException(String.format(ILLEGAL_VERSION, version));
}
ArrayList<Component> components = new ArrayList<>();
for (String component : DOT_SPLITTER.split(version)) {
@@ -158,10 +181,11 @@
return new DottedVersion(ImmutableList.copyOf(components), version, numOriginalComponents);
}
- private static Component toComponent(String component, String version) {
+ private static Component toComponent(String component, String version)
+ throws InvalidDottedVersionException {
Matcher parsedComponent = COMPONENT_PATTERN.matcher(component);
if (!parsedComponent.matches()) {
- throw new IllegalArgumentException(String.format(ILLEGAL_VERSION, version));
+ throw new InvalidDottedVersionException(String.format(ILLEGAL_VERSION, version));
}
int firstNumber;
@@ -180,12 +204,13 @@
return new Component(firstNumber, alphaSequence, secondNumber, component);
}
- private static int parseNumber(Matcher parsedComponent, int group, String version) {
+ private static int parseNumber(Matcher parsedComponent, int group, String version)
+ throws InvalidDottedVersionException {
int firstNumber;
try {
firstNumber = Integer.parseInt(parsedComponent.group(group));
} catch (NumberFormatException e) {
- throw new IllegalArgumentException(String.format(ILLEGAL_VERSION, version), e);
+ throw new InvalidDottedVersionException(String.format(ILLEGAL_VERSION, version), e);
}
return firstNumber;
}
diff --git a/src/main/java/com/google/devtools/build/lib/rules/apple/DottedVersionConverter.java b/src/main/java/com/google/devtools/build/lib/rules/apple/DottedVersionConverter.java
index 44ba329..cb26b3e 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/apple/DottedVersionConverter.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/apple/DottedVersionConverter.java
@@ -26,7 +26,7 @@
public DottedVersion.Option convert(String input) throws OptionsParsingException {
try {
return DottedVersion.option(DottedVersion.fromString(input));
- } catch (IllegalArgumentException e) {
+ } catch (DottedVersion.InvalidDottedVersionException e) {
throw new OptionsParsingException(e.getMessage());
}
}
diff --git a/src/main/java/com/google/devtools/build/lib/rules/apple/XcodeConfig.java b/src/main/java/com/google/devtools/build/lib/rules/apple/XcodeConfig.java
index 445db9b..19a2a99 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/apple/XcodeConfig.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/apple/XcodeConfig.java
@@ -37,7 +37,8 @@
* Implementation for the {@code xcode_config} rule.
*/
public class XcodeConfig implements RuleConfiguredTargetFactory {
- private static final DottedVersion MINIMUM_BITCODE_XCODE_VERSION = DottedVersion.fromString("7");
+ private static final DottedVersion MINIMUM_BITCODE_XCODE_VERSION =
+ DottedVersion.fromStringUnchecked("7");
/**
* An exception that signals that an Xcode config setup was invalid.
diff --git a/src/main/java/com/google/devtools/build/lib/rules/apple/XcodeVersionProperties.java b/src/main/java/com/google/devtools/build/lib/rules/apple/XcodeVersionProperties.java
index 83e13ef..0d9bd86 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/apple/XcodeVersionProperties.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/apple/XcodeVersionProperties.java
@@ -79,20 +79,20 @@
this.xcodeVersion = Optional.fromNullable(xcodeVersion);
this.defaultIosSdkVersion =
Strings.isNullOrEmpty(defaultIosSdkVersion)
- ? DottedVersion.fromString(DEFAULT_IOS_SDK_VERSION)
- : DottedVersion.fromString(defaultIosSdkVersion);
+ ? DottedVersion.fromStringUnchecked(DEFAULT_IOS_SDK_VERSION)
+ : DottedVersion.fromStringUnchecked(defaultIosSdkVersion);
this.defaultWatchosSdkVersion =
Strings.isNullOrEmpty(defaultWatchosSdkVersion)
- ? DottedVersion.fromString(DEFAULT_WATCHOS_SDK_VERSION)
- : DottedVersion.fromString(defaultWatchosSdkVersion);
+ ? DottedVersion.fromStringUnchecked(DEFAULT_WATCHOS_SDK_VERSION)
+ : DottedVersion.fromStringUnchecked(defaultWatchosSdkVersion);
this.defaultTvosSdkVersion =
Strings.isNullOrEmpty(defaultTvosSdkVersion)
- ? DottedVersion.fromString(DEFAULT_TVOS_SDK_VERSION)
- : DottedVersion.fromString(defaultTvosSdkVersion);
+ ? DottedVersion.fromStringUnchecked(DEFAULT_TVOS_SDK_VERSION)
+ : DottedVersion.fromStringUnchecked(defaultTvosSdkVersion);
this.defaultMacosSdkVersion =
Strings.isNullOrEmpty(defaultMacosSdkVersion)
- ? DottedVersion.fromString(DEFAULT_MACOS_SDK_VERSION)
- : DottedVersion.fromString(defaultMacosSdkVersion);
+ ? DottedVersion.fromStringUnchecked(DEFAULT_MACOS_SDK_VERSION)
+ : DottedVersion.fromStringUnchecked(defaultMacosSdkVersion);
}
/** Returns the xcode version, or null if the xcode version is unknown. */
diff --git a/src/main/java/com/google/devtools/build/lib/rules/apple/XcodeVersionRuleData.java b/src/main/java/com/google/devtools/build/lib/rules/apple/XcodeVersionRuleData.java
index e98bb23..a482abe 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/apple/XcodeVersionRuleData.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/apple/XcodeVersionRuleData.java
@@ -45,8 +45,9 @@
NonconfigurableAttributeMapper.of(rule);
this.label = label;
- DottedVersion xcodeVersion = DottedVersion.fromString(
- attrMapper.get(XcodeVersionRule.VERSION_ATTR_NAME, Type.STRING));
+ DottedVersion xcodeVersion =
+ DottedVersion.fromStringUnchecked(
+ attrMapper.get(XcodeVersionRule.VERSION_ATTR_NAME, Type.STRING));
String iosSdkVersionString =
attrMapper.get(XcodeVersionRule.DEFAULT_IOS_SDK_VERSION_ATTR_NAME, Type.STRING);
String watchosSdkVersionString =
diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppCompileActionBuilder.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppCompileActionBuilder.java
index 02a8d61..51a066b 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppCompileActionBuilder.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppCompileActionBuilder.java
@@ -218,7 +218,7 @@
ruleErrorConsumer.ruleError(errorMessage);
}
- throw new RuleErrorException();
+ throw new RuleErrorException(errorMessages.get(0));
}
return result;
diff --git a/src/main/java/com/google/devtools/build/lib/rules/objc/AppleSkylarkCommon.java b/src/main/java/com/google/devtools/build/lib/rules/objc/AppleSkylarkCommon.java
index ea014b2..f25fdb3 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/objc/AppleSkylarkCommon.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/objc/AppleSkylarkCommon.java
@@ -182,9 +182,7 @@
@Override
// This method is registered statically for skylark, and never called directly.
public ObjcProvider newObjcProvider(
- Boolean usesSwift,
- SkylarkDict<?, ?> kwargs,
- Environment environment) {
+ Boolean usesSwift, SkylarkDict<?, ?> kwargs, Environment environment) throws EvalException {
ObjcProvider.Builder resultBuilder = new ObjcProvider.Builder(environment.getSemantics());
if (usesSwift) {
resultBuilder.add(ObjcProvider.FLAG, ObjcProvider.Flag.USES_SWIFT);
@@ -198,7 +196,7 @@
} else if (entry.getKey().equals("direct_dep_providers")) {
resultBuilder.addDirectDepProvidersFromSkylark(entry.getValue());
} else {
- throw new IllegalArgumentException(String.format(BAD_KEY_ERROR, entry.getKey()));
+ throw new EvalException(null, String.format(BAD_KEY_ERROR, entry.getKey()));
}
}
return resultBuilder.build();
@@ -253,8 +251,12 @@
}
@Override
- public DottedVersion dottedVersion(String version) {
- return DottedVersion.fromString(version);
+ public DottedVersion dottedVersion(String version) throws EvalException {
+ try {
+ return DottedVersion.fromString(version);
+ } catch (DottedVersion.InvalidDottedVersionException e) {
+ throw new EvalException(null, e.getMessage());
+ }
}
@Override
diff --git a/src/main/java/com/google/devtools/build/lib/rules/objc/MultiArchSplitTransitionProvider.java b/src/main/java/com/google/devtools/build/lib/rules/objc/MultiArchSplitTransitionProvider.java
index dd6646f..762d64f 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/objc/MultiArchSplitTransitionProvider.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/objc/MultiArchSplitTransitionProvider.java
@@ -76,7 +76,9 @@
ruleContext.attributes().get(PlatformRule.PLATFORM_TYPE_ATTR_NAME, STRING);
try {
return getPlatformType(attributeValue);
- } catch (IllegalArgumentException exception) {
+ } catch (
+ @SuppressWarnings("UnusedException")
+ ApplePlatform.UnsupportedPlatformTypeException exception) {
throw ruleContext.throwWithAttributeError(
PlatformRule.PLATFORM_TYPE_ATTR_NAME,
String.format(UNSUPPORTED_PLATFORM_TYPE_ERROR_FORMAT, attributeValue));
@@ -87,13 +89,14 @@
* Returns the apple platform type for the given platform type string (corresponding directly with
* platform type attribute value).
*
- * @throws IllegalArgumentException if the given platform type string is not a valid type
+ * @throws UnsupportedPlatformTypeException if the given platform type string is not a valid type
*/
- public static PlatformType getPlatformType(String platformTypeString) {
+ private static PlatformType getPlatformType(String platformTypeString)
+ throws ApplePlatform.UnsupportedPlatformTypeException {
PlatformType platformType = PlatformType.fromString(platformTypeString);
if (!SUPPORTED_PLATFORM_TYPES.contains(platformType)) {
- throw new IllegalArgumentException(
+ throw new ApplePlatform.UnsupportedPlatformTypeException(
String.format(UNSUPPORTED_PLATFORM_TYPE_ERROR_FORMAT, platformTypeString));
} else {
return platformType;
@@ -122,7 +125,7 @@
PlatformRule.MINIMUM_OS_VERSION,
String.format(INVALID_VERSION_STRING_ERROR_FORMAT, attributeValue));
}
- } catch (IllegalArgumentException exception) {
+ } catch (DottedVersion.InvalidDottedVersionException exception) {
ruleContext.throwWithAttributeError(
PlatformRule.MINIMUM_OS_VERSION,
String.format(INVALID_VERSION_STRING_ERROR_FORMAT, attributeValue));
@@ -144,7 +147,8 @@
} else {
minimumOsVersion = Optional.of(DottedVersion.fromString(minimumOsVersionString));
}
- } catch (IllegalArgumentException exception) {
+ } catch (ApplePlatform.UnsupportedPlatformTypeException
+ | DottedVersion.InvalidDottedVersionException exception) {
// There's no opportunity to propagate exception information up cleanly at the transition
// provider level. This should later be registered as a rule error during the initialization
// of the rule.
@@ -208,7 +212,7 @@
AppleConfiguration.iosCpuFromCpu(buildOptions.get(CoreOptions.class).cpu));
}
if (actualMinimumOsVersion != null
- && actualMinimumOsVersion.compareTo(DottedVersion.fromString("11.0")) >= 0) {
+ && actualMinimumOsVersion.compareTo(DottedVersion.fromStringUnchecked("11.0")) >= 0) {
List<String> non32BitCpus =
cpus.stream()
.filter(cpu -> !ApplePlatform.is32Bit(PlatformType.IOS, cpu))
diff --git a/src/main/java/com/google/devtools/build/lib/rules/objc/ObjcProvider.java b/src/main/java/com/google/devtools/build/lib/rules/objc/ObjcProvider.java
index ad82eb2..973c6ef 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/objc/ObjcProvider.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/objc/ObjcProvider.java
@@ -39,6 +39,7 @@
import com.google.devtools.build.lib.rules.cpp.LibraryToLink;
import com.google.devtools.build.lib.rules.cpp.LibraryToLink.CcLinkingContext;
import com.google.devtools.build.lib.skylarkbuildapi.apple.ObjcProviderApi;
+import com.google.devtools.build.lib.syntax.EvalException;
import com.google.devtools.build.lib.syntax.EvalUtils;
import com.google.devtools.build.lib.syntax.SkylarkList;
import com.google.devtools.build.lib.syntax.SkylarkNestedSet;
@@ -1128,7 +1129,7 @@
* Add elements in toAdd with the given key from skylark. An error is thrown if toAdd is not an
* appropriate SkylarkNestedSet.
*/
- void addElementsFromSkylark(Key<?> key, Object skylarkToAdd) {
+ void addElementsFromSkylark(Key<?> key, Object skylarkToAdd) throws EvalException {
NestedSet<?> toAdd = ObjcProviderSkylarkConverters.convertToJava(key, skylarkToAdd);
uncheckedAddTransitive(key, toAdd, this.items);
if (ObjcProvider.KEYS_FOR_DIRECT.contains(key)) {
@@ -1141,16 +1142,18 @@
* ObjcProvider instances.
*/
@SuppressWarnings("unchecked")
- void addProvidersFromSkylark(Object toAdd) {
+ void addProvidersFromSkylark(Object toAdd) throws EvalException {
if (!(toAdd instanceof Iterable)) {
- throw new IllegalArgumentException(
+ throw new EvalException(
+ null,
String.format(
AppleSkylarkCommon.BAD_PROVIDERS_ITER_ERROR, EvalUtils.getDataTypeName(toAdd)));
} else {
Iterable<Object> toAddIterable = (Iterable<Object>) toAdd;
for (Object toAddObject : toAddIterable) {
if (!(toAddObject instanceof ObjcProvider)) {
- throw new IllegalArgumentException(
+ throw new EvalException(
+ null,
String.format(
AppleSkylarkCommon.BAD_PROVIDERS_ELEM_ERROR,
EvalUtils.getDataTypeName(toAddObject)));
@@ -1162,21 +1165,22 @@
}
/**
- * Adds the given providers from skylark, but propagate any normally-propagated items
- * only to direct dependers. An error is thrown if toAdd is not an iterable of ObjcProvider
- * instances.
+ * Adds the given providers from skylark, but propagate any normally-propagated items only to
+ * direct dependers. An error is thrown if toAdd is not an iterable of ObjcProvider instances.
*/
@SuppressWarnings("unchecked")
- void addDirectDepProvidersFromSkylark(Object toAdd) {
+ void addDirectDepProvidersFromSkylark(Object toAdd) throws EvalException {
if (!(toAdd instanceof Iterable)) {
- throw new IllegalArgumentException(
+ throw new EvalException(
+ null,
String.format(
AppleSkylarkCommon.BAD_PROVIDERS_ITER_ERROR, EvalUtils.getDataTypeName(toAdd)));
} else {
Iterable<Object> toAddIterable = (Iterable<Object>) toAdd;
for (Object toAddObject : toAddIterable) {
if (!(toAddObject instanceof ObjcProvider)) {
- throw new IllegalArgumentException(
+ throw new EvalException(
+ null,
String.format(
AppleSkylarkCommon.BAD_PROVIDERS_ELEM_ERROR,
EvalUtils.getDataTypeName(toAddObject)));
diff --git a/src/main/java/com/google/devtools/build/lib/rules/objc/ObjcProviderSkylarkConverters.java b/src/main/java/com/google/devtools/build/lib/rules/objc/ObjcProviderSkylarkConverters.java
index 1cde2ee..a1b3d4e 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/objc/ObjcProviderSkylarkConverters.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/objc/ObjcProviderSkylarkConverters.java
@@ -22,6 +22,7 @@
import com.google.devtools.build.lib.collect.nestedset.NestedSet;
import com.google.devtools.build.lib.collect.nestedset.NestedSetBuilder;
import com.google.devtools.build.lib.rules.objc.ObjcProvider.Key;
+import com.google.devtools.build.lib.syntax.EvalException;
import com.google.devtools.build.lib.syntax.EvalUtils;
import com.google.devtools.build.lib.syntax.SkylarkNestedSet;
import com.google.devtools.build.lib.syntax.SkylarkType;
@@ -52,7 +53,8 @@
}
/** Returns a value for a java ObjcProvider given a key and a corresponding skylark value. */
- public static NestedSet<?> convertToJava(Key<?> javaKey, Object skylarkValue) {
+ public static NestedSet<?> convertToJava(Key<?> javaKey, Object skylarkValue)
+ throws EvalException {
return CONVERTERS.get(javaKey.getType()).valueForJava(javaKey, skylarkValue);
}
@@ -74,7 +76,7 @@
Object valueForSkylark(Key<?> javaKey, NestedSet<?> javaValue);
/** Translates a skylark ObjcProvider value to a java ObjcProvider value. */
- NestedSet<?> valueForJava(Key<?> javaKey, Object skylarkValue);
+ NestedSet<?> valueForJava(Key<?> javaKey, Object skylarkValue) throws EvalException;
}
/**
@@ -89,7 +91,7 @@
}
@Override
- public NestedSet<?> valueForJava(Key<?> javaKey, Object skylarkValue) {
+ public NestedSet<?> valueForJava(Key<?> javaKey, Object skylarkValue) throws EvalException {
validateTypes(skylarkValue, javaKey.getType(), javaKey.getSkylarkKeyName());
return ((SkylarkNestedSet) skylarkValue).getSet(javaKey.getType());
}
@@ -108,7 +110,7 @@
@SuppressWarnings("unchecked")
@Override
- public NestedSet<?> valueForJava(Key<?> javaKey, Object skylarkValue) {
+ public NestedSet<?> valueForJava(Key<?> javaKey, Object skylarkValue) throws EvalException {
validateTypes(skylarkValue, String.class, javaKey.getSkylarkKeyName());
NestedSetBuilder<PathFragment> result = NestedSetBuilder.stableOrder();
for (String path : ((SkylarkNestedSet) skylarkValue).toCollection(String.class)) {
@@ -117,7 +119,7 @@
return result.build();
}
}
-
+
/**
* A converter that that translates between a java {@link SdkFramework} and a skylark string.
*/
@@ -135,7 +137,7 @@
@SuppressWarnings("unchecked")
@Override
- public NestedSet<?> valueForJava(Key<?> javaKey, Object skylarkValue) {
+ public NestedSet<?> valueForJava(Key<?> javaKey, Object skylarkValue) throws EvalException {
validateTypes(skylarkValue, String.class, javaKey.getSkylarkKeyName());
NestedSetBuilder<SdkFramework> result = NestedSetBuilder.stableOrder();
for (String path : ((SkylarkNestedSet) skylarkValue).toCollection(String.class)) {
@@ -145,15 +147,15 @@
}
}
- /**
- * Throws an error if the given object is not a nested set of the given type.
- */
- private static void validateTypes(Object toCheck, Class<?> expectedSetType, String keyName) {
+ /** Throws an error if the given object is not a nested set of the given type. */
+ private static void validateTypes(Object toCheck, Class<?> expectedSetType, String keyName)
+ throws EvalException {
if (!(toCheck instanceof SkylarkNestedSet)) {
- throw new IllegalArgumentException(
- String.format(NOT_SET_ERROR, keyName, EvalUtils.getDataTypeName(toCheck)));
+ throw new EvalException(
+ null, String.format(NOT_SET_ERROR, keyName, EvalUtils.getDataTypeName(toCheck)));
} else if (!((SkylarkNestedSet) toCheck).getContentType().canBeCastTo(expectedSetType)) {
- throw new IllegalArgumentException(
+ throw new EvalException(
+ null,
String.format(
BAD_SET_TYPE_ERROR,
keyName,
diff --git a/src/main/java/com/google/devtools/build/lib/rules/repository/RepositoryFunction.java b/src/main/java/com/google/devtools/build/lib/rules/repository/RepositoryFunction.java
index a996df9..2bab9f5 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/repository/RepositoryFunction.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/repository/RepositoryFunction.java
@@ -138,6 +138,11 @@
RepositoryMissingDependencyException() {
super(Location.BUILTIN, "Internal exception");
}
+
+ @Override
+ public boolean canBeAddedToStackTrace() {
+ return false;
+ }
}
/**
* repository functions can throw the result of this function to notify the RepositoryFunction
diff --git a/src/main/java/com/google/devtools/build/lib/skylarkbuildapi/apple/AppleCommonApi.java b/src/main/java/com/google/devtools/build/lib/skylarkbuildapi/apple/AppleCommonApi.java
index 28d62b0..b5d9a23 100644
--- a/src/main/java/com/google/devtools/build/lib/skylarkbuildapi/apple/AppleCommonApi.java
+++ b/src/main/java/com/google/devtools/build/lib/skylarkbuildapi/apple/AppleCommonApi.java
@@ -279,32 +279,27 @@
public SplitTransitionProviderApi getMultiArchSplitProvider();
@SkylarkCallable(
- name = "new_objc_provider",
- doc = "Creates a new ObjcProvider instance.",
- parameters = {
- @Param(
- name = "uses_swift",
- type = Boolean.class,
- defaultValue = "False",
- named = true,
- positional = false,
- doc = "Whether this provider should enable Swift support."
- )
- },
- extraKeywords =
+ name = "new_objc_provider",
+ doc = "Creates a new ObjcProvider instance.",
+ parameters = {
@Param(
- name = "kwargs",
- type = SkylarkDict.class,
- defaultValue = "{}",
- doc = "Dictionary of arguments."
- ),
- useEnvironment = true
- )
+ name = "uses_swift",
+ type = Boolean.class,
+ defaultValue = "False",
+ named = true,
+ positional = false,
+ doc = "Whether this provider should enable Swift support.")
+ },
+ extraKeywords =
+ @Param(
+ name = "kwargs",
+ type = SkylarkDict.class,
+ defaultValue = "{}",
+ doc = "Dictionary of arguments."),
+ useEnvironment = true)
// This method is registered statically for skylark, and never called directly.
public ObjcProviderApi<?> newObjcProvider(
- Boolean usesSwift,
- SkylarkDict<?, ?> kwargs,
- Environment environment);
+ Boolean usesSwift, SkylarkDict<?, ?> kwargs, Environment environment) throws EvalException;
@SkylarkCallable(
name = "new_dynamic_framework_provider",
@@ -403,17 +398,15 @@
throws EvalException, InterruptedException;
@SkylarkCallable(
- name = "dotted_version",
- doc = "Creates a new <a href=\"DottedVersion.html\">DottedVersion</a> instance.",
- parameters = {
- @Param(
- name = "version",
- type = String.class,
- doc = "The string representation of the DottedVersion."
- )
- }
- )
- public DottedVersionApi<?> dottedVersion(String version);
+ name = "dotted_version",
+ doc = "Creates a new <a href=\"DottedVersion.html\">DottedVersion</a> instance.",
+ parameters = {
+ @Param(
+ name = "version",
+ type = String.class,
+ doc = "The string representation of the DottedVersion.")
+ })
+ public DottedVersionApi<?> dottedVersion(String version) throws EvalException;
@SkylarkCallable(
name = "objc_proto_aspect",
diff --git a/src/main/java/com/google/devtools/build/lib/syntax/MethodDescriptor.java b/src/main/java/com/google/devtools/build/lib/syntax/MethodDescriptor.java
index b322d23..3ae488f 100644
--- a/src/main/java/com/google/devtools/build/lib/syntax/MethodDescriptor.java
+++ b/src/main/java/com/google/devtools/build/lib/syntax/MethodDescriptor.java
@@ -130,46 +130,54 @@
public Object call(Object obj, Object[] args, Location loc, Environment env)
throws EvalException, InterruptedException {
Preconditions.checkNotNull(obj);
+ Object result;
try {
- Object result = method.invoke(obj, args);
- if (method.getReturnType().equals(Void.TYPE)) {
- return Runtime.NONE;
- }
- if (result == null) {
- if (isAllowReturnNones()) {
- return Runtime.NONE;
- } else {
- throw new EvalException(
- loc,
- "method invocation returned None, please file a bug report: "
- + getName()
- + Printer.printAbbreviatedList(ImmutableList.copyOf(args), "(", ", ", ")", null));
- }
- }
- // TODO(bazel-team): get rid of this, by having everyone use the Skylark data structures
- result = SkylarkType.convertToSkylark(result, method, env);
- if (result != null && !EvalUtils.isSkylarkAcceptable(result.getClass())) {
- throw new EvalException(
- loc,
- Printer.format(
- "method '%s' returns an object of invalid type %r", getName(), result.getClass()));
- }
- return result;
+ result = method.invoke(obj, args);
} catch (IllegalAccessException e) {
// TODO(bazel-team): Print a nice error message. Maybe the method exists
// and an argument is missing or has the wrong type.
throw new EvalException(loc, "Method invocation failed: " + e);
- } catch (InvocationTargetException e) {
- if (e.getCause() instanceof FuncallExpression.FuncallException) {
- throw new EvalException(loc, e.getCause().getMessage());
- } else if (e.getCause() != null) {
- Throwables.throwIfInstanceOf(e.getCause(), InterruptedException.class);
- throw new EvalException.EvalExceptionWithJavaCause(loc, e.getCause());
+ } catch (InvocationTargetException x) {
+ Throwable e = x.getCause();
+ if (e == null) {
+ // This is unlikely to happen.
+ throw new IllegalStateException(
+ String.format(
+ "causeless InvocationTargetException when calling %s with arguments %s at %s",
+ obj, Arrays.toString(args), loc),
+ x);
+ }
+ Throwables.propagateIfPossible(e, InterruptedException.class);
+ if (e instanceof FuncallExpression.FuncallException) {
+ throw new EvalException(loc, e.getMessage());
+ }
+ if (e instanceof EvalException) {
+ throw ((EvalException) e).ensureLocation(loc);
+ }
+ throw new EvalException.EvalExceptionWithJavaCause(loc, e);
+ }
+ if (method.getReturnType().equals(Void.TYPE)) {
+ return Runtime.NONE;
+ }
+ if (result == null) {
+ if (isAllowReturnNones()) {
+ return Runtime.NONE;
} else {
- // This is unlikely to happen
- throw new EvalException(loc, "method invocation failed: " + e);
+ throw new IllegalStateException(
+ "method invocation returned None "
+ + getName()
+ + Printer.printAbbreviatedList(ImmutableList.copyOf(args), "(", ", ", ")", null));
}
}
+ // TODO(bazel-team): get rid of this, by having everyone use the Skylark data structures
+ result = SkylarkType.convertToSkylark(result, method, env);
+ if (result != null && !EvalUtils.isSkylarkAcceptable(result.getClass())) {
+ throw new EvalException(
+ loc,
+ Printer.format(
+ "method '%s' returns an object of invalid type %r", getName(), result.getClass()));
+ }
+ return result;
}
/** @see SkylarkCallable#name() */
diff --git a/src/test/java/com/google/devtools/build/lib/rules/apple/DottedVersionTest.java b/src/test/java/com/google/devtools/build/lib/rules/apple/DottedVersionTest.java
index 311de1e..a62d83d 100644
--- a/src/test/java/com/google/devtools/build/lib/rules/apple/DottedVersionTest.java
+++ b/src/test/java/com/google/devtools/build/lib/rules/apple/DottedVersionTest.java
@@ -124,28 +124,33 @@
@Test
public void testIllegalVersion_noLeadingInteger() throws Exception {
- IllegalArgumentException expected =
- assertThrows(IllegalArgumentException.class, () -> DottedVersion.fromString("a"));
+ Throwable expected =
+ assertThrows(
+ DottedVersion.InvalidDottedVersionException.class, () -> DottedVersion.fromString("a"));
assertThat(expected).hasMessageThat().contains("a");
}
@Test
public void testIllegalVersion_empty() throws Exception {
- assertThrows(IllegalArgumentException.class, () -> DottedVersion.fromString(""));
+ assertThrows(
+ DottedVersion.InvalidDottedVersionException.class, () -> DottedVersion.fromString(""));
}
@Test
public void testIllegalVersion_punctuation() throws Exception {
- assertThrows(IllegalArgumentException.class, () -> DottedVersion.fromString("2:3"));
+ assertThrows(
+ DottedVersion.InvalidDottedVersionException.class, () -> DottedVersion.fromString("2:3"));
}
@Test
public void testIllegalVersion_emptyComponent() throws Exception {
- assertThrows(IllegalArgumentException.class, () -> DottedVersion.fromString("1..3"));
+ assertThrows(
+ DottedVersion.InvalidDottedVersionException.class, () -> DottedVersion.fromString("1..3"));
}
@Test
public void testIllegalVersion_negativeComponent() throws Exception {
- assertThrows(IllegalArgumentException.class, () -> DottedVersion.fromString("1.-1"));
+ assertThrows(
+ DottedVersion.InvalidDottedVersionException.class, () -> DottedVersion.fromString("1.-1"));
}
}
diff --git a/src/test/java/com/google/devtools/build/lib/rules/objc/ObjcProviderTest.java b/src/test/java/com/google/devtools/build/lib/rules/objc/ObjcProviderTest.java
index b4e96a2..b612509 100644
--- a/src/test/java/com/google/devtools/build/lib/rules/objc/ObjcProviderTest.java
+++ b/src/test/java/com/google/devtools/build/lib/rules/objc/ObjcProviderTest.java
@@ -112,7 +112,7 @@
}
@Test
- public void directFieldsAddFromSkylark() {
+ public void directFieldsAddFromSkylark() throws Exception {
ImmutableList<Artifact> artifacts =
ImmutableList.of(createArtifact("/foo"), createArtifact("/bar"));
SkylarkNestedSet set =
diff --git a/src/test/java/com/google/devtools/build/lib/rules/objc/ObjcRuleTestCase.java b/src/test/java/com/google/devtools/build/lib/rules/objc/ObjcRuleTestCase.java
index 33f4148..133c6ae 100644
--- a/src/test/java/com/google/devtools/build/lib/rules/objc/ObjcRuleTestCase.java
+++ b/src/test/java/com/google/devtools/build/lib/rules/objc/ObjcRuleTestCase.java
@@ -85,8 +85,7 @@
protected static final ImmutableList<String> FASTBUILD_COPTS = ImmutableList.of("-O0", "-DDEBUG");
protected static final DottedVersion DEFAULT_IOS_SDK_VERSION =
- DottedVersion.fromString(AppleCommandLineOptions.DEFAULT_IOS_SDK_VERSION);
-
+ DottedVersion.fromStringUnchecked(AppleCommandLineOptions.DEFAULT_IOS_SDK_VERSION);
/**
* Returns the configuration obtained by applying the apple crosstool configuration transtion to
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 2e8fdc7..bbcde18 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
@@ -511,6 +511,11 @@
return "with_args_and_kwargs(" + foo + ", " + argsString + ", " + kwargsString + ")";
}
+ @SkylarkCallable(name = "raise_unchecked_exception", documented = false)
+ public void raiseUncheckedException() {
+ throw new InternalError("buggy code");
+ }
+
@Override
public String toString() {
return "<mock>";
@@ -1309,6 +1314,12 @@
}
@Test
+ public void testCallingMethodThatRaisesUncheckedException() throws Exception {
+ update("mock", new Mock());
+ assertThrows(InternalError.class, () -> eval("mock.raise_unchecked_exception()"));
+ }
+
+ @Test
public void testJavaFunctionWithExtraInterpreterParams() throws Exception {
new SkylarkTest()
.update("mock", new Mock())
@@ -1470,12 +1481,10 @@
@Test
public void testJavaFunctionReturnsNullFails() throws Exception {
- new SkylarkTest()
- .update("mock", new Mock())
- .testIfErrorContains(
- "method invocation returned None,"
- + " please file a bug report: nullfunc_failing(\"abc\", 1)",
- "mock.nullfunc_failing('abc', 1)");
+ update("mock", new Mock());
+ IllegalStateException e =
+ assertThrows(IllegalStateException.class, () -> eval("mock.nullfunc_failing('abc', 1)"));
+ assertThat(e).hasMessageThat().contains("method invocation returned None");
}
@Test
@@ -1947,6 +1956,7 @@
"nullfunc_failing",
"nullfunc_working",
"proxy_methods_object",
+ "raise_unchecked_exception",
"return_bad",
"string",
"string_list",
