Stop Bazel from crashing on data-bound Android libraries with no resources.

These libraries don't produce data binding outputs on resource processing (since
there's nothing to process). Their only role is to pass through the outputs of
their deps.

--
PiperOrigin-RevId: 150251921
MOS_MIGRATED_REVID=150251921

5 files changed, 76 insertions, 36 deletions

diff --git a/src/main/java/com/google/devtools/build/lib/rules/android/AndroidBinary.java b/src/main/java/com/google/devtools/build/lib/rules/android/AndroidBinary.java
index 79457e3e8e..e6153fe198 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/android/AndroidBinary.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/android/AndroidBinary.java
@@ -123,23 +123,17 @@ public abstract class AndroidBinary implements RuleConfiguredTargetFactory {
     return builder.build();
   }
 
-  private static RuleConfiguredTargetBuilder init(
-      RuleContext ruleContext,
-      NestedSetBuilder<Artifact> filesBuilder,
-      ResourceDependencies resourceDeps,
-      JavaCommon javaCommon,
-      AndroidCommon androidCommon,
-      JavaSemantics javaSemantics,
-      AndroidSemantics androidSemantics)
+  /**
+   * Checks expected rule invariants, throws rule errors if anything is set wrong.
+   */
+  private static void validateRuleContext(RuleContext ruleContext)
       throws InterruptedException, RuleErrorException {
-
     if (getMultidexMode(ruleContext) != MultidexMode.LEGACY
         && ruleContext.attributes().isAttributeValueExplicitlySpecified(
             "main_dex_proguard_specs")) {
       ruleContext.throwWithAttributeError("main_dex_proguard_specs", "The "
           + "'main_dex_proguard_specs' attribute is only allowed if 'multidex' is set to 'legacy'");
     }
-
     if (ruleContext.attributes().isAttributeValueExplicitlySpecified("proguard_apply_mapping")
         && ruleContext.attributes()
             .get(ProguardHelper.PROGUARD_SPECS, BuildType.LABEL_LIST)
@@ -147,14 +141,12 @@ public abstract class AndroidBinary implements RuleConfiguredTargetFactory {
       ruleContext.throwWithAttributeError("proguard_apply_mapping",
           "'proguard_apply_mapping' can only be used when 'proguard_specs' is also set");
     }
-
     if (ruleContext.attributes().isAttributeValueExplicitlySpecified("rex_package_map")
         && !ruleContext.attributes().get("rewrite_dexes_with_rex", Type.BOOLEAN)) {
       ruleContext.throwWithAttributeError(
           "rex_package_map",
           "'rex_package_map' can only be used when 'rewrite_dexes_with_rex' is also set");
     }
-
     if (ruleContext.attributes().isAttributeValueExplicitlySpecified("rex_package_map")
         && ruleContext.attributes()
         .get(ProguardHelper.PROGUARD_SPECS, BuildType.LABEL_LIST)
@@ -162,6 +154,24 @@ public abstract class AndroidBinary implements RuleConfiguredTargetFactory {
       ruleContext.throwWithAttributeError("rex_package_map",
           "'rex_package_map' can only be used when 'proguard_specs' is also set");
     }
+    if (ruleContext.attributes().isAttributeValueExplicitlySpecified("resources")
+      && DataBinding.isEnabled(ruleContext)) {
+      ruleContext.throwWithRuleError("Data binding doesn't work with the \"resources\" attribute. "
+          + "Use \"resource_files\" instead.");
+    }
+  }
+
+  private static RuleConfiguredTargetBuilder init(
+      RuleContext ruleContext,
+      NestedSetBuilder<Artifact> filesBuilder,
+      ResourceDependencies resourceDeps,
+      JavaCommon javaCommon,
+      AndroidCommon androidCommon,
+      JavaSemantics javaSemantics,
+      AndroidSemantics androidSemantics)
+      throws InterruptedException, RuleErrorException {
+
+    validateRuleContext(ruleContext);
 
     // TODO(bazel-team): Find a way to simplify this code.
     // treeKeys() means that the resulting map sorts the entries by key, which is necessary to
diff --git a/src/main/java/com/google/devtools/build/lib/rules/android/AndroidCommon.java b/src/main/java/com/google/devtools/build/lib/rules/android/AndroidCommon.java
index 72de4a2aa7..2c837e9ea7 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/android/AndroidCommon.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/android/AndroidCommon.java
@@ -944,7 +944,11 @@ public class AndroidCommon {
 
     ImmutableList<Artifact> srcs =
         ruleContext.getPrerequisiteArtifacts("srcs", RuleConfiguredTarget.Mode.TARGET).list();
-    if (useDataBinding) {
+    if (useDataBinding
+        && LocalResourceContainer.definesAndroidResources(ruleContext.attributes())) {
+      // Add this rule's annotation processor input if this rule has direct resources. If it
+      // doesn't have direct resources, it doesn't produce data binding output so there's no
+      // input for the annotation processor.
       srcs = ImmutableList.<Artifact>builder().addAll(srcs)
           .add(DataBinding.createAnnotationFile(ruleContext, isLibrary)).build();
     }
diff --git a/src/main/java/com/google/devtools/build/lib/rules/android/AndroidLibrary.java b/src/main/java/com/google/devtools/build/lib/rules/android/AndroidLibrary.java
index a3ab7878b9..f194aa559c 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/android/AndroidLibrary.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/android/AndroidLibrary.java
@@ -46,9 +46,22 @@ public abstract class AndroidLibrary implements RuleConfiguredTargetFactory {
   protected abstract JavaSemantics createJavaSemantics();
   protected abstract AndroidSemantics createAndroidSemantics();
 
+  /**
+   * Checks expected rule invariants, throws rule errors if anything is set wrong.
+   */
+  private static void validateRuleContext(RuleContext ruleContext)
+      throws InterruptedException, RuleErrorException {
+    if (ruleContext.attributes().isAttributeValueExplicitlySpecified("resources")
+      && DataBinding.isEnabled(ruleContext)) {
+      ruleContext.throwWithRuleError("Data binding doesn't work with the \"resources\" attribute. "
+          + "Use \"resource_files\" instead.");
+    }
+  }
+
   @Override
   public ConfiguredTarget create(RuleContext ruleContext)
       throws InterruptedException, RuleErrorException {
+    validateRuleContext(ruleContext);
     JavaSemantics javaSemantics = createJavaSemantics();
     AndroidSemantics androidSemantics = createAndroidSemantics();
     if (!AndroidSdkProvider.verifyPresence(ruleContext)) {
diff --git a/src/main/java/com/google/devtools/build/lib/rules/android/DataBinding.java b/src/main/java/com/google/devtools/build/lib/rules/android/DataBinding.java
index c52bc88938..9c9c3882ac 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/android/DataBinding.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/android/DataBinding.java
@@ -205,21 +205,34 @@ public final class DataBinding {
     if (DataBinding.isEnabled(ruleContext)) {
       dataBindingMetadataOutputs.addAll(getMetadataOutputs(ruleContext));
     }
-    if (ruleContext.attributes().has("exports", BuildType.LABEL_LIST)) {
-      for (UsesDataBindingProvider provider : ruleContext.getPrerequisites("exports",
-          RuleConfiguredTarget.Mode.TARGET, UsesDataBindingProvider.class)) {
-        dataBindingMetadataOutputs.addAll(provider.getMetadataOutputs());
-      }
+    dataBindingMetadataOutputs.addAll(getTransitiveMetadata(ruleContext, "exports"));
+    if (!LocalResourceContainer.definesAndroidResources(ruleContext.attributes())) {
+      // If this rule doesn't declare direct resources, no resource processing is run so no data
+      // binding outputs are produced. In that case, we need to explicitly propagate data binding
+      // outputs from the deps to make sure they continue up the build graph.
+      dataBindingMetadataOutputs.addAll(getTransitiveMetadata(ruleContext, "deps"));
     }
     if (!dataBindingMetadataOutputs.isEmpty()) {
-      // QUESTION(gregce): does a rule need to propagate the metadata outputs of its deps, or do
-      // they get integrated automatically into its own outputs?
       builder.addProvider(UsesDataBindingProvider.class,
           new UsesDataBindingProvider(dataBindingMetadataOutputs));
     }
   }
 
   /**
+   * Returns the data binding resource processing output from deps under the given attribute.
+   */
+  private static List<Artifact> getTransitiveMetadata(RuleContext ruleContext, String attr) {
+    ImmutableList.Builder<Artifact> dataBindingMetadataOutputs = ImmutableList.builder();
+    if (ruleContext.attributes().has(attr, BuildType.LABEL_LIST)) {
+      for (UsesDataBindingProvider provider : ruleContext.getPrerequisites(attr,
+          RuleConfiguredTarget.Mode.TARGET, UsesDataBindingProvider.class)) {
+        dataBindingMetadataOutputs.addAll(provider.getMetadataOutputs());
+      }
+    }
+    return dataBindingMetadataOutputs.build();
+  }
+
+  /**
    * Annotation processing creates the following metadata files that describe how data binding is
    * applied. The full file paths include prefixes as implemented in {@link #getMetadataOutputs}.
    */
@@ -235,6 +248,11 @@ public final class DataBinding {
    * jar, even though (due to resource merging) both modules compile against their own instances.
    */
   public static List<Artifact> getMetadataOutputs(RuleContext ruleContext) {
+    if (!LocalResourceContainer.definesAndroidResources(ruleContext.attributes())) {
+      // If this rule doesn't define local resources, no resource processing was done, so it
+      // doesn't produce data binding output.
+      return ImmutableList.<Artifact>of();
+    }
     ImmutableList.Builder<Artifact> outputs = ImmutableList.<Artifact>builder();
     String javaPackage = AndroidCommon.getJavaPackage(ruleContext);
     Label ruleLabel = ruleContext.getRule().getLabel();
@@ -263,13 +281,12 @@ public final class DataBinding {
   static ImmutableList<Artifact> processDeps(RuleContext ruleContext,
       JavaTargetAttributes.Builder attributes) {
     ImmutableList.Builder<Artifact> dataBindingJavaInputs = ImmutableList.<Artifact>builder();
-    dataBindingJavaInputs.add(DataBinding.getLayoutInfoFile(ruleContext));
-    for (UsesDataBindingProvider p : ruleContext.getPrerequisites("deps",
-        RuleConfiguredTarget.Mode.TARGET, UsesDataBindingProvider.class)) {
-      for (Artifact dataBindingDepMetadata : p.getMetadataOutputs()) {
-        attributes.addProcessorPathDir(dataBindingDepMetadata.getExecPath().getParentDirectory());
-        dataBindingJavaInputs.add(dataBindingDepMetadata);
-      }
+    if (LocalResourceContainer.definesAndroidResources(ruleContext.attributes())) {
+      dataBindingJavaInputs.add(DataBinding.getLayoutInfoFile(ruleContext));
+    }
+    for (Artifact dataBindingDepMetadata : getTransitiveMetadata(ruleContext, "deps")) {
+      attributes.addProcessorPathDir(dataBindingDepMetadata.getExecPath().getParentDirectory());
+      dataBindingJavaInputs.add(dataBindingDepMetadata);
     }
     return dataBindingJavaInputs.build();
   }
diff --git a/src/main/java/com/google/devtools/build/lib/rules/android/LocalResourceContainer.java b/src/main/java/com/google/devtools/build/lib/rules/android/LocalResourceContainer.java
index d4a15834e8..56f8b37b38 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/android/LocalResourceContainer.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/android/LocalResourceContainer.java
@@ -77,22 +77,20 @@ public final class LocalResourceContainer {
       throws RuleErrorException {
     if (ruleContext.attributes().isAttributeValueExplicitlySpecified("assets")
         ^ ruleContext.attributes().isAttributeValueExplicitlySpecified("assets_dir")) {
-      ruleContext.ruleError(
+      ruleContext.throwWithRuleError(
           "'assets' and 'assets_dir' should be either both empty or both non-empty");
-      throw new RuleErrorException();
     }
   }
 
   /**
-   * Validates that there are no resources defined if there are resource attribute defined.
+   * Validates that there are no resources defined if there are resource attributes defined.
    */
   private static void validateNoResourcesAttribute(RuleContext ruleContext)
       throws RuleErrorException {
     if (ruleContext.attributes().isAttributeValueExplicitlySpecified("resources")) {
-      ruleContext.attributeError("resources",
+      ruleContext.throwWithAttributeError("resources",
           String.format("resources cannot be set when any of %s are defined.",
               Joiner.on(", ").join(RESOURCES_ATTRIBUTES)));
-      throw new RuleErrorException();
     }
   }
 
@@ -105,17 +103,15 @@ public final class LocalResourceContainer {
     Iterable<AndroidResourcesProvider> resources =
         ruleContext.getPrerequisites("srcs", Mode.TARGET, AndroidResourcesProvider.class);
     for (AndroidResourcesProvider provider : resources) {
-      ruleContext.attributeError("srcs",
+      ruleContext.throwWithAttributeError("srcs",
           String.format("srcs should not contain android_resource label %s", provider.getLabel()));
-      throw new RuleErrorException();
     }
   }
 
   private static void validateManifest(RuleContext ruleContext) throws RuleErrorException {
     if (ruleContext.getPrerequisiteArtifact("manifest", Mode.TARGET) == null) {
-      ruleContext.attributeError("manifest",
+      ruleContext.throwWithAttributeError("manifest",
           "manifest is required when resource_files or assets are defined.");
-      throw new RuleErrorException();
     }
   }