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();
 }