Revise renaming of the name and Jvm*Signature in Kotlin property.
Bug: 70169921
Change-Id: I91456bcb89ce329719654ae1bfb6cdfcfb5937aa
diff --git a/src/main/java/com/android/tools/r8/kotlin/KotlinMetadataSynthesizer.java b/src/main/java/com/android/tools/r8/kotlin/KotlinMetadataSynthesizer.java
index 4935c24..6e959b6 100644
--- a/src/main/java/com/android/tools/r8/kotlin/KotlinMetadataSynthesizer.java
+++ b/src/main/java/com/android/tools/r8/kotlin/KotlinMetadataSynthesizer.java
@@ -329,18 +329,26 @@
KmType kmPropertyType = null;
KmType kmReceiverType = null;
+ // A flag to indicate we can rename the property name. This will become false if any member
+ // is pinned. Then, we conservatively assume that users want the property to be pinned too.
+ // That is, we won't rename the property even though some other members could be renamed.
+ boolean canChangePropertyName = true;
+ // A candidate property name. Not overwritten by the following members, hence the order of
+ // preference: a backing field, getter, and setter.
+ String renamedPropertyName = name;
if (field != null) {
- DexField renamedField = lens.lookupField(field.field, appView.dexItemFactory());
- if (renamedField == null) {
- // Bail out if we can't find a renamed backing field.
- return null;
+ if (appView.appInfo().isPinned(field.field)) {
+ canChangePropertyName = false;
}
- kmProperty.setName(renamedField.name.toString());
+ DexField renamedField = lens.lookupField(field.field, appView.dexItemFactory());
+ if (canChangePropertyName && renamedField.name != field.field.name) {
+ renamedPropertyName = renamedField.name.toString();
+ }
kmPropertyType = toRenamedKmType(field.field.type, appView, lens);
if (kmPropertyType != null) {
kmProperty.setReturnType(kmPropertyType);
}
- JvmExtensionsKt.setFieldSignature(kmProperty, toJvmFieldSignature(field.field));
+ JvmExtensionsKt.setFieldSignature(kmProperty, toJvmFieldSignature(renamedField));
}
GetterSetterCriteria criteria = checkGetterCriteria();
@@ -374,8 +382,17 @@
return null;
}
}
+ if (appView.appInfo().isPinned(getter.method)) {
+ canChangePropertyName = false;
+ }
+ DexMethod renamedGetter = lens.lookupMethod(getter.method, appView.dexItemFactory());
+ if (canChangePropertyName
+ && renamedGetter.name != getter.method.name
+ && renamedPropertyName.equals(name)) {
+ renamedPropertyName = renamedGetter.name.toString();
+ }
kmProperty.setGetterFlags(getter.accessFlags.getAsKotlinFlags());
- JvmExtensionsKt.setGetterSignature(kmProperty, toJvmMethodSignature(getter.method));
+ JvmExtensionsKt.setGetterSignature(kmProperty, toJvmMethodSignature(renamedGetter));
}
criteria = checkSetterCriteria();
@@ -430,8 +447,17 @@
if (kmValueParameter != null) {
kmProperty.setSetterParameter(kmValueParameter);
}
+ if (appView.appInfo().isPinned(setter.method)) {
+ canChangePropertyName = false;
+ }
+ DexMethod renamedSetter = lens.lookupMethod(setter.method, appView.dexItemFactory());
+ if (canChangePropertyName
+ && renamedSetter.name != setter.method.name
+ && renamedPropertyName.equals(name)) {
+ renamedPropertyName = renamedSetter.name.toString();
+ }
kmProperty.setSetterFlags(setter.accessFlags.getAsKotlinFlags());
- JvmExtensionsKt.setSetterSignature(kmProperty, toJvmMethodSignature(setter.method));
+ JvmExtensionsKt.setSetterSignature(kmProperty, toJvmMethodSignature(renamedSetter));
}
// If the property type remains null at the end, bail out to synthesize this property.
@@ -442,6 +468,11 @@
if (isExtension && kmReceiverType == null) {
return null;
}
+ // Rename the property name if and only if none of participating members is pinned, and
+ // any of them is indeed renamed (to a new name).
+ if (canChangePropertyName && !renamedPropertyName.equals(name)) {
+ kmProperty.setName(renamedPropertyName);
+ }
return kmProperty;
}
diff --git a/src/test/java/com/android/tools/r8/kotlin/metadata/MetadataRewriteInCompanionTest.java b/src/test/java/com/android/tools/r8/kotlin/metadata/MetadataRewriteInCompanionTest.java
index 16385b4..f77cdb9 100644
--- a/src/test/java/com/android/tools/r8/kotlin/metadata/MetadataRewriteInCompanionTest.java
+++ b/src/test/java/com/android/tools/r8/kotlin/metadata/MetadataRewriteInCompanionTest.java
@@ -94,6 +94,8 @@
.addProgramFiles(companionLibJarMap.get(targetVersion))
// Keep the B class and its interface (which has the doStuff method).
.addKeepRules("-keep class **.B")
+ // Property in companion with @JvmField is defined in the host class, without accessors.
+ .addKeepRules("-keepclassmembers class **.B { *** elt2; }")
.addKeepRules("-keep class **.I { <methods>; }")
// Keep getters for B$Companion.(eltN|foo) which will be referenced at the app.
.addKeepRules("-keepclassmembers class **.B$* { *** get*(...); }")
diff --git a/src/test/java/com/android/tools/r8/kotlin/metadata/MetadataRewriteInPropertyTest.java b/src/test/java/com/android/tools/r8/kotlin/metadata/MetadataRewriteInPropertyTest.java
index d8b230d..34abe12 100644
--- a/src/test/java/com/android/tools/r8/kotlin/metadata/MetadataRewriteInPropertyTest.java
+++ b/src/test/java/com/android/tools/r8/kotlin/metadata/MetadataRewriteInPropertyTest.java
@@ -9,6 +9,8 @@
import static com.android.tools.r8.utils.codeinspector.Matchers.isRenamed;
import static org.hamcrest.CoreMatchers.not;
import static org.hamcrest.MatcherAssert.assertThat;
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertNotEquals;
import static org.junit.Assert.assertNotNull;
import static org.junit.Assert.assertNull;
@@ -18,8 +20,10 @@
import com.android.tools.r8.shaking.ProguardKeepAttributes;
import com.android.tools.r8.utils.codeinspector.ClassSubject;
import com.android.tools.r8.utils.codeinspector.CodeInspector;
+import com.android.tools.r8.utils.codeinspector.FieldSubject;
import com.android.tools.r8.utils.codeinspector.KmClassSubject;
import com.android.tools.r8.utils.codeinspector.KmPropertySubject;
+import com.android.tools.r8.utils.codeinspector.MethodSubject;
import java.nio.file.Path;
import java.util.Collection;
import java.util.HashMap;
@@ -85,7 +89,7 @@
.addRunClasspathFiles(ToolHelper.getKotlinStdlibJar(), libJar)
.addClasspath(output)
.run(parameters.getRuntime(), PKG + ".fragile_property_only_getter.Getter_userKt")
- .assertSuccessWithOutputLines("true", "false", "Hey");
+ .assertSuccessWithOutputLines("true", "false", "Hey Jude");
}
private void inspectGetterOnly(CodeInspector inspector) {
@@ -94,6 +98,14 @@
assertThat(person, isPresent());
assertThat(person, not(isRenamed()));
+ FieldSubject backingField = person.uniqueFieldWithName("name");
+ assertThat(backingField, isRenamed());
+ MethodSubject getterForName = person.uniqueMethodWithName("getName");
+ assertThat(getterForName, isPresent());
+ assertThat(getterForName, not(isRenamed()));
+ MethodSubject setterForName = person.uniqueMethodWithName("setName");
+ assertThat(setterForName, not(isPresent()));
+
// API entry is kept, hence the presence of Metadata.
KmClassSubject kmClass = person.getKmClass();
assertThat(kmClass, isPresent());
@@ -101,8 +113,12 @@
KmPropertySubject name = kmClass.kmPropertyWithUniqueName("name");
assertThat(name, isPresent());
assertThat(name, not(isExtensionProperty()));
+ // Property name is not renamed, due to the kept getter.
+ assertEquals("name", name.name());
assertNotNull(name.fieldSignature());
+ assertEquals(backingField.getJvmFieldSignatureAsString(), name.fieldSignature().asString());
assertNotNull(name.getterSignature());
+ assertEquals(getterForName.getJvmMethodSignatureAsString(), name.getterSignature().asString());
assertNull(name.setterSignature());
KmPropertySubject familyName = kmClass.kmPropertyWithUniqueName("familyName");
@@ -159,6 +175,7 @@
ClassSubject person = inspector.clazz(personClassName);
assertThat(person, isPresent());
assertThat(person, not(isRenamed()));
+
// API entry is kept, hence the presence of Metadata.
KmClassSubject kmClass = person.getKmClass();
assertThat(kmClass, isPresent());
diff --git a/src/test/java/com/android/tools/r8/kotlin/metadata/fragile_property_only_getter/getter_user.kt b/src/test/java/com/android/tools/r8/kotlin/metadata/fragile_property_only_getter/getter_user.kt
index b85bd2e..b3aba2e 100644
--- a/src/test/java/com/android/tools/r8/kotlin/metadata/fragile_property_only_getter/getter_user.kt
+++ b/src/test/java/com/android/tools/r8/kotlin/metadata/fragile_property_only_getter/getter_user.kt
@@ -11,5 +11,5 @@
println(x.canDrink)
println(x.married)
- println(y.firstName)
+ println(y.name)
}
\ No newline at end of file
diff --git a/src/test/java/com/android/tools/r8/utils/codeinspector/AbsentFieldSubject.java b/src/test/java/com/android/tools/r8/utils/codeinspector/AbsentFieldSubject.java
index 20da5d0..f41135f 100644
--- a/src/test/java/com/android/tools/r8/utils/codeinspector/AbsentFieldSubject.java
+++ b/src/test/java/com/android/tools/r8/utils/codeinspector/AbsentFieldSubject.java
@@ -95,4 +95,9 @@
public AnnotationSubject annotation(String name) {
return new AbsentAnnotationSubject();
}
+
+ @Override
+ public String getJvmFieldSignatureAsString() {
+ return null;
+ }
}
diff --git a/src/test/java/com/android/tools/r8/utils/codeinspector/AbsentKmPropertySubject.java b/src/test/java/com/android/tools/r8/utils/codeinspector/AbsentKmPropertySubject.java
index 70f6e52..d9c7d55 100644
--- a/src/test/java/com/android/tools/r8/utils/codeinspector/AbsentKmPropertySubject.java
+++ b/src/test/java/com/android/tools/r8/utils/codeinspector/AbsentKmPropertySubject.java
@@ -29,6 +29,11 @@
}
@Override
+ public String name() {
+ return null;
+ }
+
+ @Override
public JvmFieldSignature fieldSignature() {
return null;
}
diff --git a/src/test/java/com/android/tools/r8/utils/codeinspector/AbsentMethodSubject.java b/src/test/java/com/android/tools/r8/utils/codeinspector/AbsentMethodSubject.java
index b3e5eec..e08c9b7 100644
--- a/src/test/java/com/android/tools/r8/utils/codeinspector/AbsentMethodSubject.java
+++ b/src/test/java/com/android/tools/r8/utils/codeinspector/AbsentMethodSubject.java
@@ -132,4 +132,9 @@
public AnnotationSubject annotation(String name) {
return new AbsentAnnotationSubject();
}
+
+ @Override
+ public String getJvmMethodSignatureAsString() {
+ return null;
+ }
}
diff --git a/src/test/java/com/android/tools/r8/utils/codeinspector/FieldSubject.java b/src/test/java/com/android/tools/r8/utils/codeinspector/FieldSubject.java
index dc9940f..957e78a 100644
--- a/src/test/java/com/android/tools/r8/utils/codeinspector/FieldSubject.java
+++ b/src/test/java/com/android/tools/r8/utils/codeinspector/FieldSubject.java
@@ -34,4 +34,6 @@
public boolean isFieldSubject() {
return true;
}
+
+ public abstract String getJvmFieldSignatureAsString();
}
diff --git a/src/test/java/com/android/tools/r8/utils/codeinspector/FoundFieldSubject.java b/src/test/java/com/android/tools/r8/utils/codeinspector/FoundFieldSubject.java
index 7c40d1e..6494955 100644
--- a/src/test/java/com/android/tools/r8/utils/codeinspector/FoundFieldSubject.java
+++ b/src/test/java/com/android/tools/r8/utils/codeinspector/FoundFieldSubject.java
@@ -154,4 +154,9 @@
public String toString() {
return dexField.toSourceString();
}
+
+ @Override
+ public String getJvmFieldSignatureAsString() {
+ return dexField.field.name.toString() + ":" + dexField.field.type.toDescriptorString();
+ }
}
diff --git a/src/test/java/com/android/tools/r8/utils/codeinspector/FoundKmPropertySubject.java b/src/test/java/com/android/tools/r8/utils/codeinspector/FoundKmPropertySubject.java
index 4f9cc94..3cfe20d 100644
--- a/src/test/java/com/android/tools/r8/utils/codeinspector/FoundKmPropertySubject.java
+++ b/src/test/java/com/android/tools/r8/utils/codeinspector/FoundKmPropertySubject.java
@@ -49,6 +49,11 @@
}
@Override
+ public String name() {
+ return kmProperty.getName();
+ }
+
+ @Override
public JvmFieldSignature fieldSignature() {
return fieldSignature;
}
diff --git a/src/test/java/com/android/tools/r8/utils/codeinspector/FoundMethodSubject.java b/src/test/java/com/android/tools/r8/utils/codeinspector/FoundMethodSubject.java
index 1051e28..c220f01 100644
--- a/src/test/java/com/android/tools/r8/utils/codeinspector/FoundMethodSubject.java
+++ b/src/test/java/com/android/tools/r8/utils/codeinspector/FoundMethodSubject.java
@@ -24,6 +24,7 @@
import com.android.tools.r8.graph.DexItemFactory;
import com.android.tools.r8.graph.DexMethod;
import com.android.tools.r8.graph.DexString;
+import com.android.tools.r8.graph.DexType;
import com.android.tools.r8.ir.code.IRCode;
import com.android.tools.r8.naming.MemberNaming;
import com.android.tools.r8.naming.MemberNaming.MethodSignature;
@@ -32,9 +33,11 @@
import com.android.tools.r8.references.MethodReference;
import com.android.tools.r8.references.Reference;
import com.android.tools.r8.utils.InternalOptions;
+import com.android.tools.r8.utils.StringUtils;
import com.android.tools.r8.utils.codeinspector.LocalVariableTable.LocalVariableTableEntry;
import com.google.common.base.Predicates;
import com.google.common.collect.ImmutableList;
+import com.google.common.collect.Streams;
import it.unimi.dsi.fastutil.objects.Object2IntMap;
import it.unimi.dsi.fastutil.objects.Object2IntOpenHashMap;
import java.util.Arrays;
@@ -356,4 +359,15 @@
.collect(Collectors.toList()),
Reference.returnTypeFromDescriptor(method.proto.returnType.toDescriptorString()));
}
+
+ @Override
+ public String getJvmMethodSignatureAsString() {
+ return dexMethod.method.name.toString()
+ + "("
+ + StringUtils.join(
+ Arrays.stream(dexMethod.method.proto.parameters.values)
+ .map(DexType::toDescriptorString).collect(Collectors.toList()), "")
+ + ")"
+ + dexMethod.method.proto.returnType.toDescriptorString();
+ }
}
diff --git a/src/test/java/com/android/tools/r8/utils/codeinspector/KmPropertySubject.java b/src/test/java/com/android/tools/r8/utils/codeinspector/KmPropertySubject.java
index 9fb5a0a..826c1f1 100644
--- a/src/test/java/com/android/tools/r8/utils/codeinspector/KmPropertySubject.java
+++ b/src/test/java/com/android/tools/r8/utils/codeinspector/KmPropertySubject.java
@@ -15,6 +15,8 @@
public abstract boolean isExtension();
+ public abstract String name();
+
public abstract JvmFieldSignature fieldSignature();
public abstract JvmMethodSignature getterSignature();
diff --git a/src/test/java/com/android/tools/r8/utils/codeinspector/MethodSubject.java b/src/test/java/com/android/tools/r8/utils/codeinspector/MethodSubject.java
index af6e1a4..50911f3 100644
--- a/src/test/java/com/android/tools/r8/utils/codeinspector/MethodSubject.java
+++ b/src/test/java/com/android/tools/r8/utils/codeinspector/MethodSubject.java
@@ -104,4 +104,6 @@
public boolean hasCode() {
return getMethod().getCode() != null;
}
+
+ public abstract String getJvmMethodSignatureAsString();
}
