Reland "Check for inner class attribute before writing kotlin class name"

This reverts commit b8e7e2aa317c2fb6f3f79495912e9efac11a1d8b.

Bug: 188690067
Change-Id: Iaa4e5b92cde9fc3c9f75cf9f59b921f6107d7278
diff --git a/src/main/java/com/android/tools/r8/kotlin/KotlinClassInfo.java b/src/main/java/com/android/tools/r8/kotlin/KotlinClassInfo.java
index e440049..08236e2 100644
--- a/src/main/java/com/android/tools/r8/kotlin/KotlinClassInfo.java
+++ b/src/main/java/com/android/tools/r8/kotlin/KotlinClassInfo.java
@@ -16,6 +16,7 @@
 import com.android.tools.r8.graph.DexItemFactory;
 import com.android.tools.r8.graph.DexString;
 import com.android.tools.r8.naming.NamingLens;
+import com.android.tools.r8.utils.Box;
 import com.android.tools.r8.utils.DescriptorUtils;
 import com.android.tools.r8.utils.Pair;
 import com.android.tools.r8.utils.Reporter;
@@ -37,6 +38,7 @@
 
   private final int flags;
   private final String name;
+  private final boolean nameCanBeSynthesizedFromClassOrAnonymousObjectOrigin;
   private final String moduleName;
   private final List<KotlinConstructorInfo> constructorsWithNoBacking;
   private final KotlinDeclarationContainerInfo declarationContainerInfo;
@@ -54,6 +56,7 @@
   private KotlinClassInfo(
       int flags,
       String name,
+      boolean nameCanBeSynthesizedFromClassOrAnonymousObjectOrigin,
       String moduleName,
       KotlinDeclarationContainerInfo declarationContainerInfo,
       List<KotlinTypeParameterInfo> typeParameters,
@@ -69,6 +72,8 @@
       int[] metadataVersion) {
     this.flags = flags;
     this.name = name;
+    this.nameCanBeSynthesizedFromClassOrAnonymousObjectOrigin =
+        nameCanBeSynthesizedFromClassOrAnonymousObjectOrigin;
     this.moduleName = moduleName;
     this.declarationContainerInfo = declarationContainerInfo;
     this.typeParameters = typeParameters;
@@ -127,9 +132,17 @@
         KotlinDeclarationContainerInfo.create(
             kmClass, methodMap, fieldMap, factory, reporter, keepByteCode, extensionInformation);
     setCompanionObject(kmClass, hostClass, reporter);
+    KotlinTypeReference anonymousObjectOrigin = getAnonymousObjectOrigin(kmClass, factory);
+    boolean nameCanBeDeducedFromClassOrOrigin =
+        kmClass.name.equals(
+                KotlinMetadataUtils.getKotlinClassName(
+                    hostClass, hostClass.getType().toDescriptorString()))
+            || (anonymousObjectOrigin != null
+                && kmClass.name.equals(anonymousObjectOrigin.toKotlinClassifier(true)));
     return new KotlinClassInfo(
         kmClass.getFlags(),
         kmClass.name,
+        nameCanBeDeducedFromClassOrOrigin,
         JvmExtensionsKt.getModuleName(kmClass),
         container,
         KotlinTypeParameterInfo.create(kmClass.getTypeParameters(), factory, reporter),
@@ -139,7 +152,7 @@
         getNestedClasses(hostClass, kmClass.getNestedClasses(), factory),
         kmClass.getEnumEntries(),
         KotlinVersionRequirementInfo.create(kmClass.getVersionRequirements()),
-        getAnonymousObjectOrigin(kmClass, factory),
+        anonymousObjectOrigin,
         packageName,
         KotlinLocalDelegatedPropertyInfo.create(
             JvmExtensionsKt.getLocalDelegatedProperties(kmClass), factory, reporter),
@@ -221,14 +234,29 @@
     // Set potentially renamed class name.
     DexString originalDescriptor = clazz.type.descriptor;
     DexString rewrittenDescriptor = namingLens.lookupDescriptor(clazz.type);
-    // If the original descriptor equals the rewritten descriptor, we pick the original name
-    // to preserve potential errors in the original name. As an example, the kotlin stdlib has
-    // name: .kotlin/collections/CollectionsKt___CollectionsKt$groupingBy$1, which seems incorrect.
     boolean rewritten = !originalDescriptor.equals(rewrittenDescriptor);
-    kmClass.setName(
-        !rewritten
-            ? this.name
-            : DescriptorUtils.getBinaryNameFromDescriptor(rewrittenDescriptor.toString()));
+    if (!nameCanBeSynthesizedFromClassOrAnonymousObjectOrigin) {
+      kmClass.setName(this.name);
+    } else {
+      String rewrittenName = null;
+      // When the class has an anonymousObjectOrigin and the name equals the identifier there, we
+      // keep the name tied to the anonymousObjectOrigin.
+      if (anonymousObjectOrigin != null
+          && name.equals(anonymousObjectOrigin.toKotlinClassifier(true))) {
+        Box<String> rewrittenOrigin = new Box<>();
+        anonymousObjectOrigin.toRenamedBinaryNameOrDefault(
+            rewrittenOrigin::set, appView, namingLens, null);
+        if (rewrittenOrigin.isSet()) {
+          rewrittenName = "." + rewrittenOrigin.get();
+        }
+      }
+      if (rewrittenName == null) {
+        rewrittenName =
+            KotlinMetadataUtils.getKotlinClassName(clazz, rewrittenDescriptor.toString());
+      }
+      kmClass.setName(rewrittenName);
+      rewritten |= !name.equals(rewrittenName);
+    }
     // Find a companion object.
     for (DexEncodedField field : clazz.fields()) {
       if (field.getKotlinInfo().isCompanion()) {
diff --git a/src/main/java/com/android/tools/r8/kotlin/KotlinClassifierInfo.java b/src/main/java/com/android/tools/r8/kotlin/KotlinClassifierInfo.java
index 5f5b696..9b2939e 100644
--- a/src/main/java/com/android/tools/r8/kotlin/KotlinClassifierInfo.java
+++ b/src/main/java/com/android/tools/r8/kotlin/KotlinClassifierInfo.java
@@ -4,6 +4,8 @@
 
 package com.android.tools.r8.kotlin;
 
+import static com.android.tools.r8.kotlin.KotlinMetadataUtils.getKotlinLocalOrAnonymousNameFromDescriptor;
+
 import com.android.tools.r8.graph.AppView;
 import com.android.tools.r8.graph.DexDefinitionSupplier;
 import com.android.tools.r8.graph.DexItemFactory;
@@ -68,13 +70,8 @@
     boolean rewrite(KmTypeVisitor visitor, AppView<?> appView, NamingLens namingLens) {
       return type.toRenamedDescriptorOrDefault(
           descriptor -> {
-            // For local or anonymous classes, the classifier is prefixed with '.' and inner classes
-            // are separated with '$'.
-            if (isLocalOrAnonymous) {
-              visitor.visitClass("." + DescriptorUtils.getBinaryNameFromDescriptor(descriptor));
-            } else {
-              visitor.visitClass(DescriptorUtils.descriptorToKotlinClassifier(descriptor));
-            }
+            visitor.visitClass(
+                getKotlinLocalOrAnonymousNameFromDescriptor(descriptor, isLocalOrAnonymous));
           },
           appView,
           namingLens,
diff --git a/src/main/java/com/android/tools/r8/kotlin/KotlinMetadataUtils.java b/src/main/java/com/android/tools/r8/kotlin/KotlinMetadataUtils.java
index 7eb45f6..2d6f39a 100644
--- a/src/main/java/com/android/tools/r8/kotlin/KotlinMetadataUtils.java
+++ b/src/main/java/com/android/tools/r8/kotlin/KotlinMetadataUtils.java
@@ -13,6 +13,7 @@
 import com.android.tools.r8.graph.DexItemFactory;
 import com.android.tools.r8.graph.DexMethod;
 import com.android.tools.r8.graph.DexType;
+import com.android.tools.r8.graph.InnerClassAttribute;
 import com.android.tools.r8.naming.NamingLens;
 import com.android.tools.r8.shaking.ProguardConfiguration;
 import com.android.tools.r8.shaking.ProguardConfigurationRule;
@@ -201,4 +202,27 @@
     // Check if the type is matched
     return proguardKeepRule.getClassNames().matches(factory.kotlinMetadataType);
   }
+
+  static String getKotlinClassName(DexClass clazz, String descriptor) {
+    InnerClassAttribute innerClassAttribute = clazz.getInnerClassAttributeForThisClass();
+    if (innerClassAttribute != null && innerClassAttribute.getOuter() != null) {
+      return DescriptorUtils.descriptorToKotlinClassifier(descriptor);
+    } else if (clazz.isLocalClass() || clazz.isAnonymousClass()) {
+      return getKotlinLocalOrAnonymousNameFromDescriptor(descriptor, true);
+    } else {
+      // We no longer have an innerclass relationship to maintain and we therefore return a binary
+      // name.
+      return DescriptorUtils.getBinaryNameFromDescriptor(descriptor);
+    }
+  }
+
+  static String getKotlinLocalOrAnonymousNameFromDescriptor(
+      String descriptor, boolean isLocalOrAnonymous) {
+    // For local or anonymous classes, the classifier is prefixed with '.' and inner classes
+    // are separated with '$'.
+    if (isLocalOrAnonymous) {
+      return "." + DescriptorUtils.getBinaryNameFromDescriptor(descriptor);
+    }
+    return DescriptorUtils.descriptorToKotlinClassifier(descriptor);
+  }
 }
diff --git a/src/main/java/com/android/tools/r8/kotlin/KotlinTypeReference.java b/src/main/java/com/android/tools/r8/kotlin/KotlinTypeReference.java
index 4d0d990..c166103 100644
--- a/src/main/java/com/android/tools/r8/kotlin/KotlinTypeReference.java
+++ b/src/main/java/com/android/tools/r8/kotlin/KotlinTypeReference.java
@@ -4,6 +4,8 @@
 
 package com.android.tools.r8.kotlin;
 
+import static com.android.tools.r8.kotlin.KotlinMetadataUtils.getKotlinLocalOrAnonymousNameFromDescriptor;
+
 import com.android.tools.r8.graph.AppView;
 import com.android.tools.r8.graph.DexDefinitionSupplier;
 import com.android.tools.r8.graph.DexItemFactory;
@@ -81,6 +83,14 @@
     return !known.toDescriptorString().equals(renamedString);
   }
 
+  String toKotlinClassifier(boolean isLocalOrAnonymous) {
+    if (known == null) {
+      return unknown;
+    }
+    return getKotlinLocalOrAnonymousNameFromDescriptor(
+        known.toDescriptorString(), isLocalOrAnonymous);
+  }
+
   boolean toRenamedBinaryNameOrDefault(
       Consumer<String> rewrittenConsumer,
       AppView<?> appView,
diff --git a/src/test/java/com/android/tools/r8/kotlin/metadata/KotlinMetadataTestBase.java b/src/test/java/com/android/tools/r8/kotlin/metadata/KotlinMetadataTestBase.java
index 996dc79..55a048e 100644
--- a/src/test/java/com/android/tools/r8/kotlin/metadata/KotlinMetadataTestBase.java
+++ b/src/test/java/com/android/tools/r8/kotlin/metadata/KotlinMetadataTestBase.java
@@ -106,10 +106,8 @@
             if (rewrittenString.equals("L;") || rewrittenString.equals("(L;)V")) {
               return;
             }
-            System.out.println(clazzSubject.toString() + ": " + rewrittenString);
             addedNonInitStrings.increment();
           });
-      System.out.flush();
       assertEquals(originalHeader.getPackageName(), rewrittenHeader.getPackageName());
 
       String expected = KotlinMetadataWriter.kotlinMetadataToString("", originalMetadata);
diff --git a/src/test/java/com/android/tools/r8/kotlin/metadata/MetadataRewriteInnerClassTest.java b/src/test/java/com/android/tools/r8/kotlin/metadata/MetadataRewriteInnerClassTest.java
index c38a3d7..48007f1 100644
--- a/src/test/java/com/android/tools/r8/kotlin/metadata/MetadataRewriteInnerClassTest.java
+++ b/src/test/java/com/android/tools/r8/kotlin/metadata/MetadataRewriteInnerClassTest.java
@@ -26,17 +26,19 @@
 @RunWith(Parameterized.class)
 public class MetadataRewriteInnerClassTest extends KotlinMetadataTestBase {
 
+  private static final String PKG_NESTED_REFLECT = PKG + ".nested_reflect";
   private static final String EXPECTED =
       StringUtils.lines(
-          "fun <init>(kotlin.Int):"
-              + " com.android.tools.r8.kotlin.metadata.nested_reflect.Outer.Nested",
-          "fun com.android.tools.r8.kotlin.metadata.nested_reflect.Outer.Inner.<init>(kotlin.Int):"
-              + " com.android.tools.r8.kotlin.metadata.nested_reflect.Outer.Inner");
+          "fun <init>(kotlin.Int): " + PKG_NESTED_REFLECT + ".Outer.Nested",
+          "fun "
+              + PKG_NESTED_REFLECT
+              + ".Outer.Inner.<init>(kotlin.Int): "
+              + PKG_NESTED_REFLECT
+              + ".Outer.Inner");
   private static final String EXPECTED_OUTER_RENAMED =
       StringUtils.lines(
-          "fun <init>(kotlin.Int): com.android.tools.r8.kotlin.metadata.nested_reflect.Nested",
-          "fun <init>(kotlin.Int): com.android.tools.r8.kotlin.metadata.nested_reflect.Inner");
-  private static final String PKG_NESTED_REFLECT = PKG + ".nested_reflect";
+          "fun <init>(kotlin.Int): " + PKG_NESTED_REFLECT + ".`Outer$Nested`",
+          "fun <init>(kotlin.Int): " + PKG_NESTED_REFLECT + ".`Outer$Inner`");
 
   private final TestParameters parameters;
 
diff --git a/src/test/java/com/android/tools/r8/kotlin/metadata/MetadataRewriteJvmStaticTest.java b/src/test/java/com/android/tools/r8/kotlin/metadata/MetadataRewriteJvmStaticTest.java
index 605b16c..2c378c6 100644
--- a/src/test/java/com/android/tools/r8/kotlin/metadata/MetadataRewriteJvmStaticTest.java
+++ b/src/test/java/com/android/tools/r8/kotlin/metadata/MetadataRewriteJvmStaticTest.java
@@ -93,7 +93,10 @@
             .addProgramFiles(kotlincLibJar.getForConfiguration(kotlinc, targetVersion))
             .addClasspathFiles(getKotlinStdlibJar(kotlinc), getKotlinAnnotationJar(kotlinc))
             .addKeepAllClassesRule()
-            .addKeepAttributes(ProguardKeepAttributes.RUNTIME_VISIBLE_ANNOTATIONS)
+            .addKeepAttributes(
+                ProguardKeepAttributes.RUNTIME_VISIBLE_ANNOTATIONS,
+                ProguardKeepAttributes.INNER_CLASSES,
+                ProguardKeepAttributes.ENCLOSING_METHOD)
             .compile()
             .inspect(this::inspect)
             .writeToZip();
diff --git a/src/test/java/com/android/tools/r8/kotlin/metadata/MetadataRewritePassThroughTest.java b/src/test/java/com/android/tools/r8/kotlin/metadata/MetadataRewritePassThroughTest.java
index 80578a3..44092e7 100644
--- a/src/test/java/com/android/tools/r8/kotlin/metadata/MetadataRewritePassThroughTest.java
+++ b/src/test/java/com/android/tools/r8/kotlin/metadata/MetadataRewritePassThroughTest.java
@@ -77,7 +77,10 @@
         .setMinApi(parameters.getApiLevel())
         .addKeepAllClassesRule()
         .addKeepKotlinMetadata()
-        .addKeepAttributes(ProguardKeepAttributes.RUNTIME_VISIBLE_ANNOTATIONS)
+        .addKeepAttributes(
+            ProguardKeepAttributes.RUNTIME_VISIBLE_ANNOTATIONS,
+            ProguardKeepAttributes.INNER_CLASSES,
+            ProguardKeepAttributes.ENCLOSING_METHOD)
         .allowDiagnosticWarningMessages()
         .compile()
         .assertAllWarningMessagesMatch(equalTo("Resource 'META-INF/MANIFEST.MF' already exists."))