Handle problematic resource qualifiers in references to individual resources

ResourceFilter already handles problematic qualifiers in the
resource_configuration_filters attribute. Also handle them in references to
individual resources.

RELNOTES: none
PiperOrigin-RevId: 160969753

2 files changed, 81 insertions, 7 deletions

diff --git a/src/test/java/com/google/devtools/build/lib/rules/android/ResourceFilterTest.java b/src/test/java/com/google/devtools/build/lib/rules/android/ResourceFilterTest.java
index b7aaf7d1d0..5ad26b7637 100644
--- a/src/test/java/com/google/devtools/build/lib/rules/android/ResourceFilterTest.java
+++ b/src/test/java/com/google/devtools/build/lib/rules/android/ResourceFilterTest.java
@@ -154,7 +154,11 @@ public class ResourceFilterTest extends ResourceTestBase {
         ImmutableList.of("sr-Latn", "sr-rLatn", "sr_Latn", "b+sr+Latn"),
         ImmutableList.of(),
         FilterBehavior.FILTER_IN_ANALYSIS,
-        ImmutableList.of("values-sr/foo.xml", "values-b+sr+Latn/foo.xml"),
+        ImmutableList.of(
+            "values-sr/foo.xml",
+            "values-b+sr+Latn/foo.xml",
+            "values-sr-Latn/foo.xml",
+            "values-sr-rLatn/foo.xml"),
         // Serbian in explicitly Cyrillic characters should be filtered out.
         ImmutableList.of("values-b+sr+Cyrl/foo.xml"));
 
@@ -171,7 +175,11 @@ public class ResourceFilterTest extends ResourceTestBase {
         ImmutableList.of(),
         FilterBehavior.FILTER_IN_ANALYSIS,
         ImmutableList.of(
-            "values-sr/foo.xml", "values-b+sr+Latn/foo.xml", "values-b+sr+Cyrl/foo.xml"));
+            "values-sr/foo.xml",
+            "values-b+sr+Latn/foo.xml",
+            "values-b+sr+Cyrl/foo.xml",
+            "values-sr-Latn/foo.xml",
+            "values-sr-rLatn/foo.xml"));
 
     errorConsumer.assertNoAttributeWarnings(ResourceFilter.RESOURCE_CONFIGURATION_FILTERS_NAME);
   }
@@ -182,7 +190,7 @@ public class ResourceFilterTest extends ResourceTestBase {
         ImmutableList.of("es-419", "es_419", "b+es+419"),
         ImmutableList.of(),
         FilterBehavior.FILTER_IN_ANALYSIS,
-        ImmutableList.of("values-es/foo.xml", "values-b+es+419/foo.xml"),
+        ImmutableList.of("values-es/foo.xml", "values-b+es+419/foo.xml", "values-es-419/foo.xml"),
         // Spanish with another region specified should be filtered out.
         ImmutableList.of("values-es-rUS/foo.xml"));
 
@@ -198,7 +206,11 @@ public class ResourceFilterTest extends ResourceTestBase {
         ImmutableList.of("es"),
         ImmutableList.of(),
         FilterBehavior.FILTER_IN_ANALYSIS,
-        ImmutableList.of("values-es/foo.xml", "values-b+es+419/foo.xml", "values-es-rUS/foo.xml"));
+        ImmutableList.of(
+            "values-es/foo.xml",
+            "values-b+es+419/foo.xml",
+            "values-es-rUS/foo.xml",
+            "values-es-419/foo.xml"));
 
     errorConsumer.assertNoAttributeWarnings(ResourceFilter.RESOURCE_CONFIGURATION_FILTERS_NAME);
   }
@@ -208,9 +220,9 @@ public class ResourceFilterTest extends ResourceTestBase {
    * problem in each resource filter object.
    */
   @Test
-  public void testFilterDeprecatedQualifierFormatsWarnings() {
+  public void testFilterDeprecatedQualifierFormatsAttributeWarnings() {
     ImmutableList<String> badQualifiers =
-        ImmutableList.of("en_US", "sr-Latn", "es-419", "mcc310-en_US", "sr_Latn", "es_419");
+        ImmutableList.of("en_US", "sr-Latn", "es-419", "mcc310-en_US", "sr_rLatn", "es_419");
 
     ImmutableList<String> expectedWarnings =
         ImmutableList.of(
@@ -229,10 +241,12 @@ public class ResourceFilterTest extends ResourceTestBase {
             errorConsumer.getAndClearAttributeWarnings(
                 ResourceFilter.RESOURCE_CONFIGURATION_FILTERS_NAME))
         .containsExactlyElementsIn(expectedWarnings);
+    errorConsumer.assertNoRuleWarnings();
 
     // Filtering again with this filter should not produce additional warnings
     filter.filter(errorConsumer, ImmutableList.of());
     errorConsumer.assertNoAttributeWarnings(ResourceFilter.RESOURCE_CONFIGURATION_FILTERS_NAME);
+    errorConsumer.assertNoRuleWarnings();
 
     // Filtering with a new filter should produce warnings again, since it is working on a different
     // target
@@ -243,6 +257,50 @@ public class ResourceFilterTest extends ResourceTestBase {
             errorConsumer.getAndClearAttributeWarnings(
                 ResourceFilter.RESOURCE_CONFIGURATION_FILTERS_NAME))
         .containsExactlyElementsIn(expectedWarnings);
+    errorConsumer.assertNoRuleWarnings();
+  }
+
+  @Test
+  public void testFilterDeprecatedQualifierFormatsRuleWarnings() {
+    ImmutableList<Artifact> badResources =
+        getResources(
+            "values-es_US/foo.xml",
+            "drawables-sr-Latn/foo.xml",
+            "stylables-es-419/foo.xml",
+            "values-mcc310-es_US/foo.xml",
+            "values-sr_rLatn/foo.xml",
+            "drawables-es_419/foo.xml");
+
+    ImmutableList<String> expectedWarnings =
+        ImmutableList.of(
+            "For resource folder drawables-sr-Latn, when referring to Serbian in Latin characters, "
+                + "use of qualifier 'sr-Latn' is deprecated. Use 'b+sr+Latn' instead.",
+            "For resource folder stylables-es-419, when referring to Spanish for Latin America and "
+                + "the Caribbean, use of qualifier 'es-419' is deprecated. Use 'b+es+419' instead.",
+            "For resource folder values-es_US, when referring to locale qualifiers with regions, "
+                + "use of qualifier 'es_US' is deprecated. Use 'es-rUS' instead.");
+
+    ResourceFilter filter =
+        new ResourceFilter(
+            ImmutableList.of("en"), ImmutableList.of(), FilterBehavior.FILTER_IN_ANALYSIS);
+
+    filter.filter(errorConsumer, badResources);
+    assertThat(errorConsumer.getAndClearRuleWarnings()).containsExactlyElementsIn(expectedWarnings);
+    errorConsumer.assertNoAttributeWarnings(ResourceFilter.RESOURCE_CONFIGURATION_FILTERS_NAME);
+
+    // Filtering again with this filter should not produce additional warnings
+    filter.filter(errorConsumer, badResources);
+    errorConsumer.assertNoRuleWarnings();
+    errorConsumer.assertNoAttributeWarnings(ResourceFilter.RESOURCE_CONFIGURATION_FILTERS_NAME);
+
+    // Filtering with a new filter should produce warnings again, since it is working on a different
+    // target
+    filter =
+        new ResourceFilter(
+            ImmutableList.of("en"), ImmutableList.of(), FilterBehavior.FILTER_IN_ANALYSIS);
+    filter.filter(errorConsumer, badResources);
+    assertThat(errorConsumer.getAndClearRuleWarnings()).containsExactlyElementsIn(expectedWarnings);
+    errorConsumer.assertNoAttributeWarnings(ResourceFilter.RESOURCE_CONFIGURATION_FILTERS_NAME);
   }
 
   private void testNoopFilter(
diff --git a/src/test/java/com/google/devtools/build/lib/rules/android/ResourceTestBase.java b/src/test/java/com/google/devtools/build/lib/rules/android/ResourceTestBase.java
index cada5518d9..27ca3510df 100644
--- a/src/test/java/com/google/devtools/build/lib/rules/android/ResourceTestBase.java
+++ b/src/test/java/com/google/devtools/build/lib/rules/android/ResourceTestBase.java
@@ -28,7 +28,9 @@ import com.google.devtools.build.lib.packages.RuleErrorConsumer;
 import com.google.devtools.build.lib.vfs.FileSystem;
 import com.google.devtools.build.lib.vfs.Path;
 import com.google.devtools.build.lib.vfs.inmemoryfs.InMemoryFileSystem;
+import java.util.ArrayList;
 import java.util.Collection;
+import java.util.List;
 import org.junit.After;
 import org.junit.Before;
 
@@ -51,12 +53,16 @@ public abstract class ResourceTestBase {
     private String attributeErrorAttribute = null;
     private String attributeErrorMessage = null;
 
+    private final List<String> ruleWarnings = new ArrayList<>();
+
     // Use an ArrayListMultimap since it allows duplicates - we'll want to know if a warning is
     // reported twice.
     private final Multimap<String, String> attributeWarnings = ArrayListMultimap.create();
 
     @Override
-    public void ruleWarning(String message) {}
+    public void ruleWarning(String message) {
+      ruleWarnings.add(message);
+    }
 
     @Override
     public void ruleError(String message) {
@@ -74,6 +80,16 @@ public abstract class ResourceTestBase {
       attributeErrorMessage = message;
     }
 
+    public Collection<String> getAndClearRuleWarnings() {
+      Collection<String> warnings = ImmutableList.copyOf(ruleWarnings);
+      ruleWarnings.clear();
+      return warnings;
+    }
+
+    public void assertNoRuleWarnings() {
+      assertThat(ruleWarnings).isEmpty();
+    }
+
     public Collection<String> getAndClearAttributeWarnings(String attrName) {
       if (!attributeWarnings.containsKey(attrName)) {
         return ImmutableList.of();