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

4 files changed, 51 insertions, 3 deletions

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 368f373d27..c979b440ac 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.PackageGroupsRuleVisibility;
 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 @@ public final class ConfiguredTargetFactory {
       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 20ff73c903..f7671a2655 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.PackageSpecification;
 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 final class RuleContext extends TargetContext
   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 final class RuleContext extends TargetContext
   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 @@ public final class RuleContext extends TargetContext
   }
 
   /**
+   * 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 331c47eba4..99f998c1d4 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.RuleConfiguredTargetBuilder;
 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 @@ import java.util.Map;
 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 c8be4087a5..52e10c8e48 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.RuleContext;
 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 @@ import com.google.devtools.build.lib.rules.RuleConfiguredTargetFactory;
 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.