Fix incorrect split on $ in DescriptorUtils#getSimpleClassNameFromDescriptor.
Bug: 120811478
Change-Id: Icee657bcb359a60970917425d8dda5954d8dacaa
diff --git a/src/main/java/com/android/tools/r8/ir/desugar/Java8MethodRewriter.java b/src/main/java/com/android/tools/r8/ir/desugar/Java8MethodRewriter.java
index 1245520..1c345a2 100644
--- a/src/main/java/com/android/tools/r8/ir/desugar/Java8MethodRewriter.java
+++ b/src/main/java/com/android/tools/r8/ir/desugar/Java8MethodRewriter.java
@@ -629,8 +629,9 @@
if (dexMethod != null) {
return dexMethod;
}
- String clazzDescriptor = DescriptorUtils.getSimpleClassNameFromDescriptor(clazz.toString());
- String postFix = "$" + clazzDescriptor + "$" + method + "$" + proto.shorty.toString();
+ String unqualifiedName =
+ DescriptorUtils.getUnqualifiedClassNameFromDescriptor(clazz.toString());
+ String postFix = "$" + unqualifiedName + "$" + method + "$" + proto.shorty.toString();
DexType clazz = factory.createType(UTILITY_CLASS_DESCRIPTOR_PREFIX + postFix + ";");
dexMethod = factory.createMethod(clazz, proto, method);
return dexMethod;
diff --git a/src/main/java/com/android/tools/r8/ir/optimize/ReflectionOptimizer.java b/src/main/java/com/android/tools/r8/ir/optimize/ReflectionOptimizer.java
index e78280f..d61758b 100644
--- a/src/main/java/com/android/tools/r8/ir/optimize/ReflectionOptimizer.java
+++ b/src/main/java/com/android/tools/r8/ir/optimize/ReflectionOptimizer.java
@@ -6,7 +6,7 @@
import static com.android.tools.r8.ir.analysis.type.Nullability.definitelyNotNull;
import static com.android.tools.r8.utils.DescriptorUtils.getCanonicalNameFromDescriptor;
import static com.android.tools.r8.utils.DescriptorUtils.getClassNameFromDescriptor;
-import static com.android.tools.r8.utils.DescriptorUtils.getSimpleClassNameFromDescriptor;
+import static com.android.tools.r8.utils.DescriptorUtils.getUnqualifiedClassNameFromDescriptor;
import com.android.tools.r8.errors.Unreachable;
import com.android.tools.r8.graph.AppView;
@@ -195,7 +195,7 @@
if (!renamed && needsToRetrieveInnerName) {
name = holder.getInnerClassAttributeForThisClass().getInnerName().toString();
} else {
- name = getSimpleClassNameFromDescriptor(descriptor);
+ name = getUnqualifiedClassNameFromDescriptor(descriptor);
}
if (arrayDepth > 0) {
name = name + Strings.repeat("[]", arrayDepth);
diff --git a/src/main/java/com/android/tools/r8/naming/MinifiedRenaming.java b/src/main/java/com/android/tools/r8/naming/MinifiedRenaming.java
index a30860f..c40fbc3 100644
--- a/src/main/java/com/android/tools/r8/naming/MinifiedRenaming.java
+++ b/src/main/java/com/android/tools/r8/naming/MinifiedRenaming.java
@@ -65,8 +65,8 @@
}
// The Java reflection library assumes that that inner-class names are separated by a $ and
// thus we allow the mapping of an inner name to rely on that too. If the dollar is not
- // present after pulling off the original inner-name, then we revert to using the simple name
- // of the inner class as its name.
+ // present after pulling off the original inner-name, then we revert to using the unqualified
+ // name of the inner class as its name.
DexType innerType = attribute.getInner();
String inner = DescriptorUtils.descriptorToInternalName(innerType.descriptor.toString());
String innerName = attribute.getInnerName().toString();
@@ -74,7 +74,9 @@
if (lengthOfPrefix < 0
|| inner.lastIndexOf(DescriptorUtils.INNER_CLASS_SEPARATOR, lengthOfPrefix - 1) < 0
|| !inner.endsWith(innerName)) {
- return lookupSimpleName(innerType, options.itemFactory);
+ String descriptor = lookupDescriptor(innerType).toString();
+ return options.itemFactory.createString(
+ DescriptorUtils.getUnqualifiedClassNameFromDescriptor(descriptor));
}
// At this point we assume the input was of the form: <OuterType>$<index><InnerName>
@@ -90,7 +92,9 @@
// Hitting means we have converted a proper Outer$Inner relationship to an invalid one.
assert !options.testing.allowFailureOnInnerClassErrors
: "Outer$Inner class was remapped without keeping the dollar separator";
- return lookupSimpleName(innerType, options.itemFactory);
+ String descriptor = lookupDescriptor(innerType).toString();
+ return options.itemFactory.createString(
+ DescriptorUtils.getUnqualifiedClassNameFromDescriptor(descriptor));
}
return options.itemFactory.createString(innerTypeMapped.substring(index + 1));
}
diff --git a/src/main/java/com/android/tools/r8/naming/NamingLens.java b/src/main/java/com/android/tools/r8/naming/NamingLens.java
index 8488f09..54a7e31 100644
--- a/src/main/java/com/android/tools/r8/naming/NamingLens.java
+++ b/src/main/java/com/android/tools/r8/naming/NamingLens.java
@@ -3,9 +3,6 @@
// BSD-style license that can be found in the LICENSE file.
package com.android.tools.r8.naming;
-import static com.android.tools.r8.utils.DescriptorUtils.DESCRIPTOR_PACKAGE_SEPARATOR;
-import static com.android.tools.r8.utils.DescriptorUtils.descriptorToJavaType;
-
import com.android.tools.r8.graph.DexCallSite;
import com.android.tools.r8.graph.DexEncodedField;
import com.android.tools.r8.graph.DexEncodedMethod;
@@ -48,15 +45,6 @@
public abstract DexString lookupDescriptor(DexType type);
- public DexString lookupSimpleName(DexType type, DexItemFactory factory) {
- String descriptor = lookupDescriptor(type).toString();
- int lastSeparator = descriptor.lastIndexOf(DESCRIPTOR_PACKAGE_SEPARATOR);
- String simpleName =
- descriptor.substring(lastSeparator > 0 ? lastSeparator + 1 : 1, descriptor.length() - 1);
- // TODO(b/120811478): Replace by DescriptorUtils.getSimpleClassNameFromDescriptor once fixed.
- return factory.createString(simpleName);
- }
-
public abstract DexString lookupInnerName(InnerClassAttribute attribute, InternalOptions options);
public abstract DexString lookupName(DexMethod method);
@@ -68,7 +56,7 @@
public final DexString lookupName(DexReference reference, DexItemFactory dexItemFactory) {
if (reference.isDexType()) {
DexString renamed = lookupDescriptor(reference.asDexType());
- return dexItemFactory.createString(descriptorToJavaType(renamed.toString()));
+ return dexItemFactory.createString(DescriptorUtils.descriptorToJavaType(renamed.toString()));
}
if (reference.isDexMethod()) {
return lookupName(reference.asDexMethod());
diff --git a/src/main/java/com/android/tools/r8/utils/DescriptorUtils.java b/src/main/java/com/android/tools/r8/utils/DescriptorUtils.java
index 2499ccb..09b0e54 100644
--- a/src/main/java/com/android/tools/r8/utils/DescriptorUtils.java
+++ b/src/main/java/com/android/tools/r8/utils/DescriptorUtils.java
@@ -207,13 +207,13 @@
}
/**
- * Get simple class name from its descriptor.
+ * Get unqualified class name from its descriptor.
*
- * @param classDescriptor a class descriptor i.e. "Ljava/lang/Object;"
- * @return class name i.e. "Object"
+ * @param classDescriptor a class descriptor i.e. "Ljava/lang/Object;" or "La/b/C$D;"
+ * @return class name i.e. "Object" or "C$D" (not "D")
*/
- public static String getSimpleClassNameFromDescriptor(String classDescriptor) {
- return getSimpleClassNameFromBinaryName(getClassBinaryNameFromDescriptor(classDescriptor));
+ public static String getUnqualifiedClassNameFromDescriptor(String classDescriptor) {
+ return getUnqualifiedClassNameFromBinaryName(getClassBinaryNameFromDescriptor(classDescriptor));
}
/**
@@ -293,16 +293,18 @@
}
/**
- * Get class name from its binary name.
+ * Get unqualified class name from its binary name.
*
* @param classBinaryName a class binary name i.e. "java/lang/Object" or "a/b/C$Inner"
- * @return class name i.e. "Object" or "Inner"
+ * @return class name i.e. "Object" or "C$Inner" (not "Inner")
+ *
+ * Note that we cannot rely on $ separator in binary name or descriptor because a class, which is
+ * not a member or local class, can still contain $ in its name. For the correct retrieval of the
+ * simple name of member or local classes, use the inner name in the inner-class attribute (or
+ * refer to ReflectionOptimizer#computeClassName as an example).
*/
- public static String getSimpleClassNameFromBinaryName(String classBinaryName) {
- int simpleNameIndex = classBinaryName.lastIndexOf(INNER_CLASS_SEPARATOR);
- if (simpleNameIndex < 0) {
- simpleNameIndex = classBinaryName.lastIndexOf(DESCRIPTOR_PACKAGE_SEPARATOR);
- }
+ public static String getUnqualifiedClassNameFromBinaryName(String classBinaryName) {
+ int simpleNameIndex = classBinaryName.lastIndexOf(DESCRIPTOR_PACKAGE_SEPARATOR);
return (simpleNameIndex < 0) ? classBinaryName : classBinaryName.substring(simpleNameIndex + 1);
}
diff --git a/src/test/java/com/android/tools/r8/naming/AvoidRTest.java b/src/test/java/com/android/tools/r8/naming/AvoidRTest.java
index 2eda1cd..5c2d882 100644
--- a/src/test/java/com/android/tools/r8/naming/AvoidRTest.java
+++ b/src/test/java/com/android/tools/r8/naming/AvoidRTest.java
@@ -3,14 +3,15 @@
// BSD-style license that can be found in the LICENSE file.
package com.android.tools.r8.naming;
-import static com.android.tools.r8.utils.DescriptorUtils.getSimpleClassNameFromDescriptor;
+import static com.android.tools.r8.utils.DescriptorUtils.getUnqualifiedClassNameFromDescriptor;
import static com.android.tools.r8.utils.codeinspector.Matchers.isRenamed;
+import static org.hamcrest.MatcherAssert.assertThat;
import static org.junit.Assert.assertNotEquals;
-import static org.junit.Assert.assertThat;
import static org.junit.Assert.assertTrue;
import com.android.tools.r8.R8FullTestBuilder;
-import com.android.tools.r8.ToolHelper;
+import com.android.tools.r8.TestParameters;
+import com.android.tools.r8.TestParametersCollection;
import com.android.tools.r8.jasmin.JasminBuilder;
import com.android.tools.r8.jasmin.JasminTestBase;
import com.android.tools.r8.utils.FileUtils;
@@ -26,15 +27,15 @@
@RunWith(Parameterized.class)
public class AvoidRTest extends JasminTestBase {
- private Backend backend;
+ private final TestParameters parameters;
- @Parameters(name = "Backend: {0}")
- public static Backend[] data() {
- return ToolHelper.getBackends();
+ @Parameters(name = "{0}")
+ public static TestParametersCollection data() {
+ return getTestParameters().withAllRuntimes().build();
}
- public AvoidRTest(Backend backend) {
- this.backend = backend;
+ public AvoidRTest(TestParameters parameters) {
+ this.parameters = parameters;
}
@Test
@@ -44,7 +45,7 @@
Set<String> expectedNames = ImmutableSet.of("P", "Q", "S", "T");
JasminBuilder jasminBuilder = new JasminBuilder();
- R8FullTestBuilder builder = testForR8(backend);
+ R8FullTestBuilder builder = testForR8(parameters.getBackend());
for (int i = 0; i < 4; i++) {
jasminBuilder.addClass("TopLevel" + Integer.toString(i));
}
@@ -67,8 +68,9 @@
assertThat(classSubject, isRenamed());
String renamedDescriptor = classSubject.getFinalDescriptor();
assertTrue(usedDescriptors.add(renamedDescriptor));
- assertNotEquals("R", getSimpleClassNameFromDescriptor(renamedDescriptor));
- assertTrue(expectedNames.contains(getSimpleClassNameFromDescriptor(renamedDescriptor)));
+ assertNotEquals("R", getUnqualifiedClassNameFromDescriptor(renamedDescriptor));
+ assertTrue(expectedNames.contains(
+ getUnqualifiedClassNameFromDescriptor(renamedDescriptor)));
});
});
}
@@ -76,7 +78,7 @@
@Test
public void test_withoutPackageHierarchy() throws Exception {
JasminBuilder jasminBuilder = new JasminBuilder();
- R8FullTestBuilder builder = testForR8(backend);
+ R8FullTestBuilder builder = testForR8(parameters.getBackend());
for (int i = 0; i < 26 * 2; i++) {
jasminBuilder.addClass("TestClass" + Integer.toString(i));
}
@@ -96,7 +98,7 @@
}
private void test_withPackageHierarchy(String keepRule) throws Exception {
- R8FullTestBuilder builder = testForR8(backend);
+ R8FullTestBuilder builder = testForR8(parameters.getBackend());
JasminBuilder jasminBuilder = new JasminBuilder();
for (int i = 0; i < 26 * 2; i++) {
jasminBuilder.addClass("TopLevel" + Integer.toString(i));
@@ -120,7 +122,7 @@
assertThat(classSubject, isRenamed());
String renamedDescriptor = classSubject.getFinalDescriptor();
assertTrue(usedDescriptors.add(renamedDescriptor));
- assertNotEquals("R", getSimpleClassNameFromDescriptor(renamedDescriptor));
+ assertNotEquals("R", getUnqualifiedClassNameFromDescriptor(renamedDescriptor));
});
});
}
diff --git a/src/test/java/com/android/tools/r8/shaking/examples/TreeShaking1Test.java b/src/test/java/com/android/tools/r8/shaking/examples/TreeShaking1Test.java
index 044d21d..6189f73 100644
--- a/src/test/java/com/android/tools/r8/shaking/examples/TreeShaking1Test.java
+++ b/src/test/java/com/android/tools/r8/shaking/examples/TreeShaking1Test.java
@@ -170,7 +170,7 @@
String descriptor = clazz.getFinalDescriptor();
Assert.assertTrue(
descriptor,
- DescriptorUtils.getSimpleClassNameFromDescriptor(descriptor).equals("Shaking")
+ DescriptorUtils.getUnqualifiedClassNameFromDescriptor(descriptor).equals("Shaking")
|| DescriptorUtils.getPackageNameFromDescriptor(descriptor).equals("repackaged"));
});
}
diff --git a/src/test/java/com/android/tools/r8/utils/DescriptorUtilsTest.java b/src/test/java/com/android/tools/r8/utils/DescriptorUtilsTest.java
index 7ad9163..badc5c4 100644
--- a/src/test/java/com/android/tools/r8/utils/DescriptorUtilsTest.java
+++ b/src/test/java/com/android/tools/r8/utils/DescriptorUtilsTest.java
@@ -7,15 +7,13 @@
import static org.junit.Assert.assertEquals;
import java.io.File;
-import java.io.IOException;
-import java.nio.file.Path;
import java.nio.file.Paths;
import org.junit.Test;
public class DescriptorUtilsTest {
@Test
- public void toShorty() throws IOException {
+ public void toShorty() {
assertEquals("Z", DescriptorUtils.javaTypeToShorty("boolean"));
assertEquals("B", DescriptorUtils.javaTypeToShorty("byte"));
assertEquals("S", DescriptorUtils.javaTypeToShorty("short"));
@@ -31,7 +29,7 @@
}
@Test
- public void toDescriptor() throws IOException {
+ public void toDescriptor() {
assertEquals("Z", DescriptorUtils.javaTypeToDescriptor("boolean"));
assertEquals("B", DescriptorUtils.javaTypeToDescriptor("byte"));
assertEquals("S", DescriptorUtils.javaTypeToDescriptor("short"));
@@ -47,16 +45,16 @@
}
@Test
- public void fromDescriptor() throws IOException {
+ public void fromDescriptor() {
String obj = "Ljava/lang/Object;";
- assertEquals("Object", DescriptorUtils.getSimpleClassNameFromDescriptor(obj));
+ assertEquals("Object", DescriptorUtils.getUnqualifiedClassNameFromDescriptor(obj));
assertEquals("java.lang.Object", DescriptorUtils.getClassNameFromDescriptor(obj));
assertEquals("java.lang", DescriptorUtils.getPackageNameFromDescriptor(obj));
assertEquals("java/lang/Object", DescriptorUtils.getClassBinaryNameFromDescriptor(obj));
}
@Test
- public void toJavaType() throws IOException {
+ public void toJavaType() {
assertEquals("boolean", DescriptorUtils.descriptorToJavaType("Z"));
assertEquals("byte", DescriptorUtils.descriptorToJavaType("B"));
assertEquals("short", DescriptorUtils.descriptorToJavaType("S"));
