Do resourcestore lookups using R class based names
The resource model will rewrite names to match the R class names if they contain dots
This is conservative, in that we will potentially match resources not
reachable (actual resource string/foo_bar, string constant foo.bar)
I will apply this in AGP as well, but the regression test is much easier to write in our setup
Bug: b/309078004
Change-Id: I638a3bdb9c06e82ee79380d78be5ff601cc40f92
diff --git a/src/resourceshrinker/java/com/android/build/shrinker/PossibleResourcesMarker.java b/src/resourceshrinker/java/com/android/build/shrinker/PossibleResourcesMarker.java
index 5089061..b710916 100644
--- a/src/resourceshrinker/java/com/android/build/shrinker/PossibleResourcesMarker.java
+++ b/src/resourceshrinker/java/com/android/build/shrinker/PossibleResourcesMarker.java
@@ -16,6 +16,7 @@
package com.android.build.shrinker;
+import static com.android.ide.common.resources.ResourcesUtil.resourceNameToFieldName;
import static com.google.common.collect.ImmutableSet.toImmutableSet;
import static java.lang.Character.isDigit;
@@ -122,7 +123,7 @@
haveSlash |= c == '/';
formatting |= c == '%';
justName =
- justName && !(c == '.' || c == ':' || c == '%' || c == '/');
+ justName && !(c == ':' || c == '%' || c == '/');
}
Stream<Resource> reachable = Streams.concat(
@@ -170,7 +171,7 @@
// Check for a simple prefix match, e.g. as in
// getResources().getIdentifier("ic_video_codec_" + codecName, "drawable", ...)
return resourceStore.getResources().stream()
- .filter(resource -> resource.name.startsWith(string));
+ .filter(resource -> resource.name.startsWith(resourceNameToFieldName(string)));
}
private Stream<Resource> possibleFormatting(String string) {
@@ -191,7 +192,7 @@
// Try to pick out the resource name pieces; if we can find the
// resource type unambiguously; if not, just match on names
int slash = string.indexOf('/');
- String name = string.substring(slash + 1);
+ String name = resourceNameToFieldName(string.substring(slash + 1));
if (name.isEmpty() || !names.contains(name)) {
return Stream.empty();
}
diff --git a/src/test/java/com/android/tools/r8/androidresources/ResourceNameWithDotsRegressionTest.java b/src/test/java/com/android/tools/r8/androidresources/ResourceNameWithDotsRegressionTest.java
new file mode 100644
index 0000000..1e28ef0
--- /dev/null
+++ b/src/test/java/com/android/tools/r8/androidresources/ResourceNameWithDotsRegressionTest.java
@@ -0,0 +1,77 @@
+// Copyright (c) 2023, the R8 project authors. Please see the AUTHORS file
+// for details. All rights reserved. Use of this source code is governed by a
+// BSD-style license that can be found in the LICENSE file.
+package com.android.tools.r8.androidresources;
+
+import com.android.tools.r8.TestBase;
+import com.android.tools.r8.TestParameters;
+import com.android.tools.r8.androidresources.AndroidResourceTestingUtils.AndroidTestResource;
+import com.android.tools.r8.androidresources.AndroidResourceTestingUtils.AndroidTestResourceBuilder;
+import com.android.tools.r8.utils.BooleanUtils;
+import java.nio.file.Path;
+import java.util.List;
+import org.junit.Test;
+import org.junit.rules.TemporaryFolder;
+import org.junit.runner.RunWith;
+import org.junit.runners.Parameterized;
+import org.junit.runners.Parameterized.Parameter;
+import org.junit.runners.Parameterized.Parameters;
+
+@RunWith(Parameterized.class)
+public class ResourceNameWithDotsRegressionTest extends TestBase {
+
+ @Parameter(0)
+ public TestParameters parameters;
+
+ @Parameter(1)
+ public boolean debug;
+
+ @Parameter(2)
+ public boolean optimized;
+
+ @Parameters(name = "{0}, debug: {1}, optimized: {2}")
+ public static List<Object[]> data() {
+ return buildParameters(
+ getTestParameters().withDefaultDexRuntime().withAllApiLevels().build(),
+ BooleanUtils.values(),
+ BooleanUtils.values());
+ }
+
+ public static AndroidTestResource getTestResources(TemporaryFolder temp) throws Exception {
+ return new AndroidTestResourceBuilder()
+ .withSimpleManifestAndAppNameString()
+ .addStringValue("foo.bar", "the foobar string")
+ .build(temp);
+ }
+
+ @Test
+ public void testR8() throws Exception {
+ AndroidTestResource testResources = getTestResources(temp);
+ Path resourcesClass = AndroidResourceTestingUtils.resourcesClassAsDex(temp);
+ byte[] withAnroidResourcesReference =
+ AndroidResourceTestingUtils.transformResourcesReferences(FooBar.class);
+ testForR8(parameters.getBackend())
+ .setMinApi(parameters)
+ .addProgramClassFileData(withAnroidResourcesReference)
+ .addRunClasspathFiles(resourcesClass)
+ .addAndroidResources(testResources)
+ .addKeepMainRule(FooBar.class)
+ .compile()
+ .inspectShrunkenResources(
+ resourceTableInspector -> {
+ resourceTableInspector.assertContainsResourceWithName("string", "foo.bar");
+ })
+ .run(parameters.getRuntime(), FooBar.class)
+ .assertSuccess();
+ }
+
+ public static class FooBar {
+
+ public static void main(String[] args) {
+ if (System.currentTimeMillis() == 0) {
+ Resources resources = new Resources();
+ resources.getIdentifier("foo.bar", "string", "mypackage");
+ }
+ }
+ }
+}
diff --git a/src/test/java/com/android/tools/r8/androidresources/Resources.java b/src/test/java/com/android/tools/r8/androidresources/Resources.java
index ef01a99..e702f2a 100644
--- a/src/test/java/com/android/tools/r8/androidresources/Resources.java
+++ b/src/test/java/com/android/tools/r8/androidresources/Resources.java
@@ -13,4 +13,8 @@
public String getString(int id) {
return GET_STRING_VALUE;
}
+
+ public int getIdentifier(String name, String defType, String defPackage) {
+ return 42;
+ }
}
