Allow use of Exceptions to exit early out of configured-target creation, instead of passing and checking null in all helpers.
Demonstrates this pattern usage in a few select rules (e.g. AndroidBinary) where this was particularly egregious.
There are many places which can benefit from this pattern -- this change doesn't try to fix them all at once.
--
MOS_MIGRATED_REVID=123012378
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/ConfiguredTargetFactory.java b/src/main/java/com/google/devtools/build/lib/analysis/ConfiguredTargetFactory.java
index 368f373..c979b44 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/ConfiguredTargetFactory.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/ConfiguredTargetFactory.java
@@ -45,6 +45,7 @@
import com.google.devtools.build.lib.packages.PackageSpecification;
import com.google.devtools.build.lib.packages.Rule;
import com.google.devtools.build.lib.packages.RuleClass;
+import com.google.devtools.build.lib.packages.RuleClass.ConfiguredTargetFactory.RuleErrorException;
import com.google.devtools.build.lib.packages.RuleVisibility;
import com.google.devtools.build.lib.packages.Target;
import com.google.devtools.build.lib.rules.SkylarkRuleConfiguredTargetBuilder;
@@ -266,7 +267,15 @@
RuleClass.ConfiguredTargetFactory<ConfiguredTarget, RuleContext> factory =
rule.getRuleClassObject().<ConfiguredTarget, RuleContext>getConfiguredTargetFactory();
Preconditions.checkNotNull(factory, rule.getRuleClassObject());
- return factory.create(ruleContext);
+ try {
+ return factory.create(ruleContext);
+ } catch (RuleErrorException ruleErrorException) {
+ // Returning null in this method is an indication an error occurred. Exceptions are not
+ // propagated, as this would show a nasty stack trace to users, and only provide info
+ // on one specific failure with poor messaging. By returning null, the caller can
+ // inspect ruleContext for multiple errors and output thorough messaging on each.
+ return null;
+ }
}
}
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/RuleContext.java b/src/main/java/com/google/devtools/build/lib/analysis/RuleContext.java
index 20ff73c..f7671a2 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/RuleContext.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/RuleContext.java
@@ -65,6 +65,7 @@
import com.google.devtools.build.lib.packages.RawAttributeMapper;
import com.google.devtools.build.lib.packages.Rule;
import com.google.devtools.build.lib.packages.RuleClass;
+import com.google.devtools.build.lib.packages.RuleClass.ConfiguredTargetFactory.RuleErrorException;
import com.google.devtools.build.lib.packages.RuleErrorConsumer;
import com.google.devtools.build.lib.packages.Target;
import com.google.devtools.build.lib.packages.TargetUtils;
@@ -289,6 +290,16 @@
public boolean hasErrors() {
return getAnalysisEnvironment().hasErrors();
}
+
+ /**
+ * No-op if {@link #hasErrors} is false, throws {@link RuleErrorException} if it is true.
+ * This provides a convenience to early-exit of configured target creation if there are errors.
+ */
+ public void assertNoErrors() throws RuleErrorException {
+ if (hasErrors()) {
+ throw new RuleErrorException();
+ }
+ }
/**
* Returns an immutable map from attribute name to list of configured targets for that attribute.
@@ -406,6 +417,17 @@
public void ruleError(String message) {
reporter.ruleError(message);
}
+
+ /**
+ * Convenience function to report non-attribute-specific errors in the current rule and then
+ * throw a {@link RuleErrorException}, immediately exiting the build invocation. Alternatively,
+ * invoke {@link #ruleError} instead to collect additional error information before ending the
+ * invocation.
+ */
+ public void throwWithRuleError(String message) throws RuleErrorException {
+ reporter.ruleError(message);
+ throw new RuleErrorException();
+ }
/**
* Convenience function for subclasses to report non-attribute-specific
@@ -429,6 +451,20 @@
}
/**
+ * Convenience function to report attribute-specific errors in the current rule, and then throw a
+ * {@link RuleErrorException}, immediately exiting the build invocation. Alternatively, invoke
+ * {@link #attributeError} instead to collect additional error information before ending the
+ * invocation.
+ *
+ * <p>If the name of the attribute starts with <code>$</code>
+ * it is replaced with a string <code>(an implicit dependency)</code>.
+ */
+ public void throwWithAttributeError(String attrName, String message) throws RuleErrorException {
+ reporter.attributeError(attrName, message);
+ throw new RuleErrorException();
+ }
+
+ /**
* Like attributeError, but does not mark the configured target as errored.
*
* <p>If the name of the attribute starts with <code>$</code>
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/config/ConfigSetting.java b/src/main/java/com/google/devtools/build/lib/analysis/config/ConfigSetting.java
index 331c47e..99f998c 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/config/ConfigSetting.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/config/ConfigSetting.java
@@ -23,6 +23,7 @@
import com.google.devtools.build.lib.analysis.RuleContext;
import com.google.devtools.build.lib.analysis.RunfilesProvider;
import com.google.devtools.build.lib.packages.NonconfigurableAttributeMapper;
+import com.google.devtools.build.lib.packages.RuleClass.ConfiguredTargetFactory.RuleErrorException;
import com.google.devtools.build.lib.rules.RuleConfiguredTargetFactory;
import com.google.devtools.build.lib.syntax.Type;
import com.google.devtools.build.lib.util.Preconditions;
@@ -44,7 +45,8 @@
public class ConfigSetting implements RuleConfiguredTargetFactory {
@Override
- public ConfiguredTarget create(RuleContext ruleContext) throws InterruptedException {
+ public ConfiguredTarget create(RuleContext ruleContext)
+ throws InterruptedException, RuleErrorException {
// Get the required flag=value settings for this rule.
Map<String, String> settings = NonconfigurableAttributeMapper.of(ruleContext.getRule())
.get(ConfigRuleClasses.ConfigSettingRule.SETTINGS_ATTRIBUTE, Type.STRING_DICT);
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/constraints/Environment.java b/src/main/java/com/google/devtools/build/lib/analysis/constraints/Environment.java
index c8be408..52e10c8 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/constraints/Environment.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/constraints/Environment.java
@@ -22,6 +22,7 @@
import com.google.devtools.build.lib.analysis.RunfilesProvider;
import com.google.devtools.build.lib.cmdline.Label;
import com.google.devtools.build.lib.packages.EnvironmentGroup;
+import com.google.devtools.build.lib.packages.RuleClass.ConfiguredTargetFactory.RuleErrorException;
import com.google.devtools.build.lib.rules.RuleConfiguredTargetFactory;
/**
@@ -30,7 +31,7 @@
public class Environment implements RuleConfiguredTargetFactory {
@Override
- public ConfiguredTarget create(RuleContext ruleContext) {
+ public ConfiguredTarget create(RuleContext ruleContext) throws RuleErrorException {
// The main analysis work to do here is to simply fill in SupportedEnvironmentsProvider to
// pass the environment itself to depending rules.
