Prevent using android libraries as resources if they have non-resource info

android_libraries can now be used in the resources attribute. However, if these
libraries contain non-resource information, it won't get picked up. To prevent
unexpected behavior, fail if such libraries are used as resources.

Adding a temporary boolean to AndroidResourcesProvider seems to be the most
straightforward way of doing this. The alternative, having the consuming target
check all relevant providers for non-resource information, would be much
messier.

RELNOTES: None
PiperOrigin-RevId: 165703578

7 files changed, 117 insertions, 29 deletions

diff --git a/src/main/java/com/google/devtools/build/lib/rules/android/AarImport.java b/src/main/java/com/google/devtools/build/lib/rules/android/AarImport.java
index 9143e4f5fa..05c667c76f 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/android/AarImport.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/android/AarImport.java
@@ -155,7 +155,8 @@ public class AarImport implements RuleConfiguredTargetFactory {
             JavaSkylarkApiProvider.NAME, JavaSkylarkApiProvider.fromRuleContext())
         .addProvider(RunfilesProvider.class, RunfilesProvider.EMPTY)
         .addProvider(
-            AndroidResourcesProvider.class, resourceApk.toResourceProvider(ruleContext.getLabel()))
+            AndroidResourcesProvider.class,
+            resourceApk.toResourceProvider(ruleContext.getLabel(), /* isResourcesOnly = */ false))
         .addProvider(
             NativeLibsZipsProvider.class,
             new NativeLibsZipsProvider(
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 f13b70e70c..1beadb87b7 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
@@ -808,7 +808,8 @@ public abstract class AndroidBinary implements RuleConfiguredTargetFactory {
         resourceApk,
         zipAlignedApk,
         apksUnderTest,
-        nativeLibs);
+        nativeLibs,
+        /* isResourcesOnly = */ false);
 
     if (proguardOutput.getMapping() != null) {
       builder.add(
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 3c938067c1..c7ca121ab2 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
@@ -743,7 +743,8 @@ public class AndroidCommon {
       ResourceApk resourceApk,
       Artifact zipAlignedApk,
       Iterable<Artifact> apksUnderTest,
-      NativeLibs nativeLibs) {
+      NativeLibs nativeLibs,
+      boolean isResourcesOnly) {
 
     idlHelper.addTransitiveInfoProviders(builder, classJar, manifestProtoOutput);
 
@@ -777,14 +778,16 @@ public class AndroidCommon {
         .setFilesToBuild(filesToBuild)
         .addSkylarkTransitiveInfo(
             JavaSkylarkApiProvider.NAME, JavaSkylarkApiProvider.fromRuleContext())
-        .add(JavaRuleOutputJarsProvider.class, ruleOutputJarsProvider)
-        .add(JavaSourceJarsProvider.class, sourceJarsProvider)
-        .add(
+        .addProvider(JavaRuleOutputJarsProvider.class, ruleOutputJarsProvider)
+        .addProvider(JavaSourceJarsProvider.class, sourceJarsProvider)
+        .addProvider(
             JavaRuntimeJarProvider.class,
             new JavaRuntimeJarProvider(javaCommon.getJavaCompilationArtifacts().getRuntimeJars()))
-        .add(RunfilesProvider.class, RunfilesProvider.simple(getRunfiles()))
-        .add(AndroidResourcesProvider.class, resourceApk.toResourceProvider(ruleContext.getLabel()))
-        .add(
+        .addProvider(RunfilesProvider.class, RunfilesProvider.simple(getRunfiles()))
+        .addProvider(
+            AndroidResourcesProvider.class,
+            resourceApk.toResourceProvider(ruleContext.getLabel(), isResourcesOnly))
+        .addProvider(
             AndroidIdeInfoProvider.class,
             createAndroidIdeInfoProvider(
                 ruleContext,
@@ -796,7 +799,7 @@ public class AndroidCommon {
                 zipAlignedApk,
                 apksUnderTest,
                 nativeLibs))
-        .add(JavaCompilationArgsProvider.class, compilationArgsProvider)
+        .addProvider(JavaCompilationArgsProvider.class, compilationArgsProvider)
         .addSkylarkTransitiveInfo(AndroidSkylarkApiProvider.NAME, new AndroidSkylarkApiProvider())
         .addOutputGroup(
             OutputGroupProvider.HIDDEN_TOP_LEVEL, collectHiddenTopLevelArtifacts(ruleContext))
@@ -831,10 +834,24 @@ public class AndroidCommon {
     if (prerequisite == null) {
       return null;
     }
+
+    AndroidResourcesProvider provider = prerequisite.getProvider(AndroidResourcesProvider.class);
+
+    if (!provider.getIsResourcesOnly()) {
+      ruleContext.attributeError(
+          "resources",
+          "android_library target "
+              + prerequisite.getLabel()
+              + " cannot be used in the 'resources' attribute as it specifies information (probably"
+              + " 'srcs' or 'deps') not directly related to android_resources. Consider moving this"
+              + " target from 'resources' to 'deps'.");
+      return null;
+    }
+
     ruleContext.ruleWarning(
         "The use of the android_resources rule and the resources attribute is deprecated. "
             + "Please use the resource_files, assets, and manifest attributes of android_library.");
-    return prerequisite.getProvider(AndroidResourcesProvider.class);
+    return provider;
   }
 
   /**
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 b9a1471ac0..71f8847e12 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
@@ -14,6 +14,7 @@
 package com.google.devtools.build.lib.rules.android;
 
 import com.google.common.collect.ImmutableList;
+import com.google.common.collect.ImmutableSet;
 import com.google.common.collect.Iterables;
 import com.google.devtools.build.lib.actions.Artifact;
 import com.google.devtools.build.lib.analysis.ConfiguredTarget;
@@ -57,6 +58,48 @@ public abstract class AndroidLibrary implements RuleConfiguredTargetFactory {
     }
   }
 
+  /**
+   * Attributes provided by android_library targets that provide information also supported by
+   * android_resources targets.
+   *
+   * <p>As part of migrating away from android_resources, we are allowing android_library targets to
+   * be used in the 'resources' attribute of android_binary, android_library, and android_test
+   * targets. However, android_library targets can specify information that cannot be propagated by
+   * the 'resources' attribute. By enumerating those attributes which can be propagated by
+   * 'resources' and having the {@link AndroidResourcesProvider} specify whether any other
+   * attributes are used, we can error out if an android_library is specified in a resources
+   * attribute despite having information incompatible with that output.
+   *
+   * TODO(b/30307842): Remove this support once the resources attribute is completely removed.
+   *
+   * <p>With the exception of 'resource_files', these attributes are simply those provided by both
+   * android_library and android_resources. android_resources does provide the 'resources'
+   * attribute, but its behavior is like the android_library 'resource_files' attribute, not the
+   * android_library 'resources' attribute (which indicates a dependency on an android_resources
+   * target).
+   */
+  private static final ImmutableSet<String> ATTRS_COMPATIBLE_WITH_ANDROID_RESOURCES =
+      ImmutableSet.of(
+          "assets",
+          "assets_dir",
+          "compatible_with",
+          "custom_package",
+          "deprecation",
+          "distribs",
+          "exports_manifest",
+          "features",
+          "inline_constants",
+          "javacopts",
+          "licenses",
+          "manifest",
+          "name",
+          "plugins",
+          "resource_files",
+          "restricted_to",
+          "tags",
+          "testonly",
+          "visibility");
+
   @Override
   public ConfiguredTarget create(RuleContext ruleContext)
       throws InterruptedException, RuleErrorException {
@@ -197,6 +240,15 @@ public abstract class AndroidLibrary implements RuleConfiguredTargetFactory {
           ruleContext.getFragment(AndroidConfiguration.class).throwOnResourceConflict())
       .build(ruleContext);
 
+    boolean isResourcesOnly = true;
+    for (String attr : ruleContext.attributes().getAttributeNames()) {
+      if (ruleContext.attributes().isAttributeValueExplicitlySpecified(attr)
+          && !ATTRS_COMPATIBLE_WITH_ANDROID_RESOURCES.contains(attr)) {
+        isResourcesOnly = false;
+        break;
+      }
+    }
+
     RuleConfiguredTargetBuilder builder = new RuleConfiguredTargetBuilder(ruleContext);
     androidCommon.addTransitiveInfoProviders(
         builder,
@@ -205,7 +257,8 @@ public abstract class AndroidLibrary implements RuleConfiguredTargetFactory {
         resourceApk,
         null,
         ImmutableList.<Artifact>of(),
-        NativeLibs.EMPTY);
+        NativeLibs.EMPTY,
+        isResourcesOnly);
 
     NestedSetBuilder<Artifact> transitiveResourcesJars = collectTransitiveResourceJars(ruleContext);
     if (androidCommon.getResourceClassJar() != null) {
diff --git a/src/main/java/com/google/devtools/build/lib/rules/android/AndroidResourcesProvider.java b/src/main/java/com/google/devtools/build/lib/rules/android/AndroidResourcesProvider.java
index 0bc2391640..2c1a217ab7 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/android/AndroidResourcesProvider.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/android/AndroidResourcesProvider.java
@@ -27,14 +27,13 @@ public abstract class AndroidResourcesProvider implements TransitiveInfoProvider
   public static AndroidResourcesProvider create(
       Label label,
       NestedSet<ResourceContainer> transitiveAndroidResources,
-      NestedSet<ResourceContainer> directAndroidResources) {
+      NestedSet<ResourceContainer> directAndroidResources,
+      boolean isResourcesOnly) {
     return new AutoValue_AndroidResourcesProvider(
-        label, transitiveAndroidResources, directAndroidResources);
+        label, transitiveAndroidResources, directAndroidResources, isResourcesOnly);
   }
 
-  /**
-   * Returns the label that is associated with this piece of information.
-   */
+  /** Returns the label that is associated with this piece of information. */
   public abstract Label getLabel();
 
   /** Returns the transitive ResourceContainers for the label. */
@@ -43,5 +42,13 @@ public abstract class AndroidResourcesProvider implements TransitiveInfoProvider
   /** Returns the immediate ResourceContainers for the label. */
   public abstract NestedSet<ResourceContainer> getDirectAndroidResources();
 
+  /**
+   * Returns whether the targets contained within this provider only represent android resources or
+   * also contain other information.
+   *
+   * TODO(b/30307842): Remove this once android_resources is fully removed.
+   */
+  public abstract boolean getIsResourcesOnly();
+
   AndroidResourcesProvider() {}
 }
diff --git a/src/main/java/com/google/devtools/build/lib/rules/android/ResourceApk.java b/src/main/java/com/google/devtools/build/lib/rules/android/ResourceApk.java
index 136c404bf4..4f244bd683 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/android/ResourceApk.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/android/ResourceApk.java
@@ -112,10 +112,10 @@ public final class ResourceApk {
    * <p>If the ResourceApk was generated from local resources, that will be the direct dependencies and
    * the rest will be transitive.
    */
-  public AndroidResourcesProvider toResourceProvider(Label label) {
+  public AndroidResourcesProvider toResourceProvider(Label label, boolean isResourcesOnly) {
     if (primaryResource == null) {
-      return resourceDeps.toProvider(label);
+      return resourceDeps.toProvider(label, isResourcesOnly);
     }
-    return resourceDeps.toProvider(label, primaryResource);
+    return resourceDeps.toProvider(label, primaryResource, isResourcesOnly);
   }
 }
diff --git a/src/main/java/com/google/devtools/build/lib/rules/android/ResourceDependencies.java b/src/main/java/com/google/devtools/build/lib/rules/android/ResourceDependencies.java
index 945fd1a603..92d4eebdbb 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/android/ResourceDependencies.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/android/ResourceDependencies.java
@@ -164,17 +164,21 @@ public final class ResourceDependencies {
    * Creates a new AndroidResourcesProvider with the supplied ResourceContainer as the direct dep.
    *
    * <p>When a library produces a new resource container the AndroidResourcesProvider should use
-   * that container as a the direct dependency for that library. This makes the consuming rule
-   * to identify the new container and merge appropriately. The  previous direct dependencies are
-   * then added to the transitive dependencies.
+   * that container as a the direct dependency for that library. This makes the consuming rule to
+   * identify the new container and merge appropriately. The previous direct dependencies are then
+   * added to the transitive dependencies.
    *
    * @param label The label of the library exporting this provider.
    * @param newDirectResource The new direct dependency for AndroidResourcesProvider
+   * @param isResourcesOnly if the direct dependency is either an android_resources
+   *     target or an android_library target with no fields that android_resources targets do not
+   *     provide.
    * @return A provider with the current resources and label.
    */
-  public AndroidResourcesProvider toProvider(Label label, ResourceContainer newDirectResource) {
+  public AndroidResourcesProvider toProvider(
+      Label label, ResourceContainer newDirectResource, boolean isResourcesOnly) {
     if (neverlink) {
-      return ResourceDependencies.empty().toProvider(label);
+      return ResourceDependencies.empty().toProvider(label, isResourcesOnly);
     }
     return AndroidResourcesProvider.create(
         label,
@@ -182,7 +186,8 @@ public final class ResourceDependencies {
             .addTransitive(transitiveResources)
             .addTransitive(directResources)
             .build(),
-        NestedSetBuilder.<ResourceContainer>naiveLinkOrder().add(newDirectResource).build());
+        NestedSetBuilder.<ResourceContainer>naiveLinkOrder().add(newDirectResource).build(),
+        isResourcesOnly);
   }
 
   /**
@@ -193,13 +198,17 @@ public final class ResourceDependencies {
    * the resource merging as if this library didn't exist.
    *
    * @param label The label of the library exporting this provider.
+   * @param isResourcesOnly if the direct dependency is either an android_resources
+   *     target or an android_library target with no fields that android_resources targets do not
+   *     provide.
    * @return A provider with the current resources and label.
    */
-  public AndroidResourcesProvider toProvider(Label label) {
+  public AndroidResourcesProvider toProvider(Label label, boolean isResourcesOnly) {
     if (neverlink) {
-      return ResourceDependencies.empty().toProvider(label);
+      return ResourceDependencies.empty().toProvider(label, isResourcesOnly);
     }
-    return AndroidResourcesProvider.create(label, transitiveResources, directResources);
+    return AndroidResourcesProvider.create(
+        label, transitiveResources, directResources, isResourcesOnly);
   }
 
   /** Provides an NestedSet of the direct and transitive resources. */