Get resource processing settings from method params instead of rule attrs

In a previous review, I left some setting controlled only by rule attributes.
Now actually specify them in Skylark method params.

Data binding and should mimic behavior in existing android_* rules.

For library, aapt_version cannot be passed in; we use the flag value.

I left the SDK attribute intact as it's private. People should be setting
defaults in rules but otherwise overriding SDK with flags, not parameters. This
mimics existing behavior.

RELNOTES: none
PiperOrigin-RevId: 195970351

2 files changed, 60 insertions, 21 deletions

diff --git a/src/test/java/com/google/devtools/build/lib/rules/android/AndroidBuildViewTestCase.java b/src/test/java/com/google/devtools/build/lib/rules/android/AndroidBuildViewTestCase.java
index 2848520e2f..a02601fced 100644
--- a/src/test/java/com/google/devtools/build/lib/rules/android/AndroidBuildViewTestCase.java
+++ b/src/test/java/com/google/devtools/build/lib/rules/android/AndroidBuildViewTestCase.java
@@ -405,7 +405,7 @@ public abstract class AndroidBuildViewTestCase extends BuildViewTestCase {
    * <p>You'll need to use a configuration pointing to it, such as "--android_sdk=//sdk:sdk", to use
    * it.
    */
-  void mockAndroidSdkWithAapt2() throws Exception {
+  public void mockAndroidSdkWithAapt2() throws Exception {
     scratch.file(
         "sdk/BUILD",
         "android_sdk(",
diff --git a/src/test/java/com/google/devtools/build/lib/rules/android/AndroidResourcesTest.java b/src/test/java/com/google/devtools/build/lib/rules/android/AndroidResourcesTest.java
index 1ab8083626..db71439629 100644
--- a/src/test/java/com/google/devtools/build/lib/rules/android/AndroidResourcesTest.java
+++ b/src/test/java/com/google/devtools/build/lib/rules/android/AndroidResourcesTest.java
@@ -23,6 +23,7 @@ import com.google.devtools.build.lib.actions.Artifact;
 import com.google.devtools.build.lib.analysis.ConfiguredTarget;
 import com.google.devtools.build.lib.analysis.RuleContext;
 import com.google.devtools.build.lib.packages.RuleClass.ConfiguredTargetFactory.RuleErrorException;
+import com.google.devtools.build.lib.rules.android.AndroidConfiguration.AndroidAaptVersion;
 import com.google.devtools.build.lib.vfs.PathFragment;
 import java.util.Optional;
 import org.junit.Before;
@@ -141,7 +142,7 @@ public class AndroidResourcesTest extends ResourceTestBase {
   private void assertFilterValidated(
       ImmutableList<Artifact> unfilteredResources, ImmutableList<Artifact> filteredResources)
       throws Exception {
-    RuleContext ruleContext = getRuleContext(/* useDataBinding = */ false);
+    RuleContext ruleContext = getRuleContext();
     ValidatedAndroidResources unfiltered =
         new AndroidResources(unfilteredResources, getResourceRoots(unfilteredResources))
             .process(ruleContext, getManifest(), /* neverlink = */ false);
@@ -217,8 +218,8 @@ public class AndroidResourcesTest extends ResourceTestBase {
   public void testParseNoCompile() throws Exception {
     useConfiguration("--android_aapt=aapt");
 
-    RuleContext ruleContext = getRuleContext(/* useDataBinding = */ true);
-    ParsedAndroidResources parsed = assertParse(ruleContext);
+    RuleContext ruleContext = getRuleContext();
+    ParsedAndroidResources parsed = assertParse(ruleContext, /* enableDataBinding = */ true);
 
     // Since we are not using aapt2, there should be no compiled symbols
     assertThat(parsed.getCompiledSymbols()).isNull();
@@ -235,7 +236,7 @@ public class AndroidResourcesTest extends ResourceTestBase {
     mockAndroidSdkWithAapt2();
     useConfiguration("--android_sdk=//sdk:sdk", "--android_aapt=aapt2");
 
-    RuleContext ruleContext = getRuleContext(/* useDataBinding = */ false);
+    RuleContext ruleContext = getRuleContext();
     ParsedAndroidResources parsed = assertParse(ruleContext);
 
     assertThat(parsed.getCompiledSymbols()).isNotNull();
@@ -259,9 +260,9 @@ public class AndroidResourcesTest extends ResourceTestBase {
     mockAndroidSdkWithAapt2();
     useConfiguration("--android_sdk=//sdk:sdk", "--android_aapt=aapt2");
 
-    RuleContext ruleContext = getRuleContext(/* useDataBinding = */ true);
+    RuleContext ruleContext = getRuleContext();
 
-    ParsedAndroidResources parsed = assertParse(ruleContext);
+    ParsedAndroidResources parsed = assertParse(ruleContext, /* enableDataBinding = */ true);
 
     // The parse action should take resources and busybox artifacts in and output symbols
     assertActionArtifacts(
@@ -285,9 +286,14 @@ public class AndroidResourcesTest extends ResourceTestBase {
   public void testMergeDataBinding() throws Exception {
     useConfiguration("--android_aapt=aapt");
 
-    RuleContext ruleContext = getRuleContext(/* useDataBinding = */ true);
+    RuleContext ruleContext = getRuleContext();
     ParsedAndroidResources parsed = assertParse(ruleContext);
-    MergedAndroidResources merged = parsed.merge(ruleContext, /* neverlink = */ false);
+    MergedAndroidResources merged =
+        parsed.merge(
+            ruleContext,
+            ResourceDependencies.empty(),
+            /* enableDataBinding = */ true,
+            AndroidAaptVersion.chooseTargetAaptVersion(ruleContext));
 
     // Besides processed manifest, inherited values should be equal
     assertThat(parsed).isEqualTo(new ParsedAndroidResources(merged, parsed.getStampedManifest()));
@@ -317,9 +323,14 @@ public class AndroidResourcesTest extends ResourceTestBase {
     useConfiguration(
         "--android_sdk=//sdk:sdk", "--android_aapt=aapt2", "--experimental_skip_parsing_action");
 
-    RuleContext ruleContext = getRuleContext(/* useDataBinding = */ false);
+    RuleContext ruleContext = getRuleContext();
     ParsedAndroidResources parsed = assertParse(ruleContext);
-    MergedAndroidResources merged = parsed.merge(ruleContext, /* neverlink = */ false);
+    MergedAndroidResources merged =
+        parsed.merge(
+            ruleContext,
+            ResourceDependencies.fromRuleDeps(ruleContext, /* neverlink = */ false),
+            DataBinding.isEnabled(ruleContext),
+            AndroidAaptVersion.chooseTargetAaptVersion(ruleContext));
 
     // Besides processed manifest, inherited values should be equal
     assertThat(parsed).isEqualTo(new ParsedAndroidResources(merged, parsed.getStampedManifest()));
@@ -354,10 +365,11 @@ public class AndroidResourcesTest extends ResourceTestBase {
   @Test
   public void testValidateAapt() throws Exception {
     useConfiguration("--android_aapt=aapt");
-    RuleContext ruleContext = getRuleContext(/* useDataBinding = */ false);
+    RuleContext ruleContext = getRuleContext();
 
     MergedAndroidResources merged = makeMergedResources(ruleContext);
-    ValidatedAndroidResources validated = merged.validate(ruleContext);
+    ValidatedAndroidResources validated =
+        merged.validate(ruleContext, AndroidAaptVersion.chooseTargetAaptVersion(ruleContext));
 
     // Inherited values should be equal
     assertThat(merged).isEqualTo(new MergedAndroidResources(validated));
@@ -380,10 +392,11 @@ public class AndroidResourcesTest extends ResourceTestBase {
   public void testValidateAapt2() throws Exception {
     mockAndroidSdkWithAapt2();
     useConfiguration("--android_sdk=//sdk:sdk", "--android_aapt=aapt2");
-    RuleContext ruleContext = getRuleContext(/* useDataBinding = */ false);
+    RuleContext ruleContext = getRuleContext();
 
     MergedAndroidResources merged = makeMergedResources(ruleContext);
-    ValidatedAndroidResources validated = merged.validate(ruleContext);
+    ValidatedAndroidResources validated =
+        merged.validate(ruleContext, AndroidAaptVersion.chooseTargetAaptVersion(ruleContext));
 
     // Inherited values should be equal
     assertThat(merged).isEqualTo(new MergedAndroidResources(validated));
@@ -417,7 +430,7 @@ public class AndroidResourcesTest extends ResourceTestBase {
 
   @Test
   public void testGenerateRClass() throws Exception {
-    RuleContext ruleContext = getRuleContext(/* useDataBinding = */ false);
+    RuleContext ruleContext = getRuleContext();
     Artifact rTxt = ruleContext.getImplicitOutputArtifact(AndroidRuleClasses.ANDROID_R_TXT);
     ProcessedAndroidManifest manifest = getManifest();
 
@@ -461,13 +474,24 @@ public class AndroidResourcesTest extends ResourceTestBase {
    * for further validation.
    */
   private ParsedAndroidResources assertParse(RuleContext ruleContext) throws Exception {
+    return assertParse(ruleContext, /* enableDataBinding = */ false);
+  }
+
+  private ParsedAndroidResources assertParse(RuleContext ruleContext, boolean enableDataBinding)
+      throws Exception {
+
     ImmutableList<Artifact> resources = getResources("values-en/foo.xml", "drawable-hdpi/bar.png");
     AndroidResources raw =
         new AndroidResources(
             resources, AndroidResources.getResourceRoots(ruleContext, resources, "resource_files"));
     StampedAndroidManifest manifest = getManifest();
 
-    ParsedAndroidResources parsed = raw.parse(ruleContext, manifest);
+    ParsedAndroidResources parsed =
+        raw.parse(
+            ruleContext,
+            manifest,
+            enableDataBinding,
+            AndroidAaptVersion.chooseTargetAaptVersion(ruleContext));
 
     // Inherited values should be equal
     assertThat(raw).isEqualTo(new AndroidResources(parsed));
@@ -480,15 +504,30 @@ public class AndroidResourcesTest extends ResourceTestBase {
 
   private MergedAndroidResources makeMergedResources(RuleContext ruleContext)
       throws RuleErrorException, InterruptedException {
-    return makeParsedResources(ruleContext).merge(ruleContext, /* neverlink = */ true);
+    return makeParsedResources(ruleContext)
+        .merge(
+            ruleContext,
+            ResourceDependencies.fromRuleDeps(ruleContext, /* neverlink = */ false),
+            DataBinding.isEnabled(ruleContext),
+            AndroidAaptVersion.chooseTargetAaptVersion(ruleContext));
   }
 
   private ParsedAndroidResources makeParsedResources(RuleContext ruleContext)
       throws RuleErrorException, InterruptedException {
+    return makeParsedResources(ruleContext, /* enableDataBinding = */ false);
+  }
+
+  private ParsedAndroidResources makeParsedResources(
+      RuleContext ruleContext, boolean enableDataBinding)
+      throws RuleErrorException, InterruptedException {
     ImmutableList<Artifact> resources = getResources("values-en/foo.xml", "drawable-hdpi/bar.png");
     return new AndroidResources(
             resources, AndroidResources.getResourceRoots(ruleContext, resources, "resource_files"))
-        .parse(ruleContext, getManifest());
+        .parse(
+            ruleContext,
+            getManifest(),
+            enableDataBinding,
+            AndroidAaptVersion.chooseTargetAaptVersion(ruleContext));
   }
 
   private ProcessedAndroidManifest getManifest() {
@@ -497,8 +536,8 @@ public class AndroidResourcesTest extends ResourceTestBase {
   }
 
   /** Gets a dummy rule context object by creating a dummy target. */
-  private RuleContext getRuleContext(boolean useDataBinding) throws Exception {
-    return getRuleContext("android_library", useDataBinding ? "  enable_data_binding = True" : "");
+  private RuleContext getRuleContext() throws Exception {
+    return getRuleContext("android_library");
   }
 
   /** Gets a dummy rule context object by creating a dummy target. */