Merge "Support to keep nesting structure of class names."

7 files changed, 178 insertions, 67 deletions

diff --git a/src/main/java/com/android/tools/r8/graph/DexAnnotation.java b/src/main/java/com/android/tools/r8/graph/DexAnnotation.java
index 55aa9868a..1afd77ea3 100644
--- a/src/main/java/com/android/tools/r8/graph/DexAnnotation.java
+++ b/src/main/java/com/android/tools/r8/graph/DexAnnotation.java
@@ -16,19 +16,6 @@ import java.util.ArrayList;
 import java.util.List;
 
 public class DexAnnotation extends DexItem {
-  // Dex system annotations.
-  // See https://source.android.com/devices/tech/dalvik/dex-format.html#system-annotation
-  private static final String ANNOTATION_DEFAULT_DESCRIPTOR =
-      "Ldalvik/annotation/AnnotationDefault;";
-  private static final String ENCLOSING_CLASS_DESCRIPTOR = "Ldalvik/annotation/EnclosingClass;";
-  private static final String ENCLOSING_METHOD_DESCRIPTOR = "Ldalvik/annotation/EnclosingMethod;";
-  private static final String INNER_CLASS_DESCRIPTOR = "Ldalvik/annotation/InnerClass;";
-  private static final String MEMBER_CLASSES_DESCRIPTOR = "Ldalvik/annotation/MemberClasses;";
-  private static final String METHOD_PARAMETERS_DESCRIPTOR = "Ldalvik/annotation/MethodParameters;";
-  private static final String SIGNATURE_DESCRIPTOR = "Ldalvik/annotation/Signature;";
-  private static final String SOURCE_DEBUG_EXTENSION = "Ldalvik/annotation/SourceDebugExtension;";
-  private static final String THROWS_DESCRIPTOR = "Ldalvik/annotation/Throws;";
-
   public static final int VISIBILITY_BUILD = 0x00;
   public static final int VISIBILITY_RUNTIME = 0x01;
   public static final int VISIBILITY_SYSTEM = 0x02;
@@ -74,37 +61,36 @@ public class DexAnnotation extends DexItem {
 
   public static DexAnnotation createEnclosingClassAnnotation(DexType enclosingClass,
       DexItemFactory factory) {
-    return createSystemValueAnnotation(ENCLOSING_CLASS_DESCRIPTOR, factory,
+    return createSystemValueAnnotation(factory.annotationEnclosingClass, factory,
         new DexValueType(enclosingClass));
   }
 
   public static DexAnnotation createEnclosingMethodAnnotation(DexMethod enclosingMethod,
       DexItemFactory factory) {
-    return createSystemValueAnnotation(ENCLOSING_METHOD_DESCRIPTOR, factory,
+    return createSystemValueAnnotation(factory.annotationEnclosingMethod, factory,
         new DexValueMethod(enclosingMethod));
   }
 
-  public static boolean isEnclosingClassAnnotation(DexAnnotation annotation) {
-    return annotation.annotation.type.toDescriptorString().equals(ENCLOSING_CLASS_DESCRIPTOR);
-  }
-
-  public static boolean isEnclosingMethodAnnotation(DexAnnotation annotation) {
-    return annotation.annotation.type.toDescriptorString().equals(ENCLOSING_METHOD_DESCRIPTOR);
+  public static boolean isEnclosingClassAnnotation(DexAnnotation annotation,
+      DexItemFactory factory) {
+    return annotation.annotation.type == factory.annotationEnclosingClass;
   }
 
-  public static boolean isEnclosingAnnotation(DexAnnotation annotation) {
-    return isEnclosingClassAnnotation(annotation) || isEnclosingMethodAnnotation(annotation);
+  public static boolean isEnclosingMethodAnnotation(DexAnnotation annotation,
+      DexItemFactory factory) {
+    return annotation.annotation.type == factory.annotationEnclosingMethod;
   }
 
-  public static boolean isInnerClassesAnnotation(DexAnnotation annotation) {
-    return annotation.annotation.type.toDescriptorString().equals(MEMBER_CLASSES_DESCRIPTOR)
-        || annotation.annotation.type.toDescriptorString().equals(INNER_CLASS_DESCRIPTOR);
+  public static boolean isInnerClassesAnnotation(DexAnnotation annotation,
+      DexItemFactory factory) {
+    return annotation.annotation.type == factory.annotationMemberClasses
+        || annotation.annotation.type == factory.annotationInnerClass;
   }
 
   public static DexAnnotation createInnerClassAnnotation(String clazz, int access,
       DexItemFactory factory) {
     return new DexAnnotation(VISIBILITY_SYSTEM,
-        new DexEncodedAnnotation(factory.createType(INNER_CLASS_DESCRIPTOR),
+        new DexEncodedAnnotation(factory.annotationInnerClass,
             new DexAnnotationElement[]{
                 new DexAnnotationElement(
                     factory.createString("accessFlags"),
@@ -123,14 +109,14 @@ public class DexAnnotation extends DexItem {
     for (int i = 0; i < classes.size(); i++) {
       values[i] = new DexValueType(classes.get(i));
     }
-    return createSystemValueAnnotation(MEMBER_CLASSES_DESCRIPTOR, factory,
+    return createSystemValueAnnotation(factory.annotationMemberClasses, factory,
         new DexValueArray(values));
   }
 
   public static DexAnnotation createSourceDebugExtensionAnnotation(DexValue value,
       DexItemFactory factory) {
     return new DexAnnotation(VISIBILITY_SYSTEM,
-        new DexEncodedAnnotation(factory.createType(SOURCE_DEBUG_EXTENSION),
+        new DexEncodedAnnotation(factory.annotationSourceDebugExtension,
             new DexAnnotationElement[] {
               new DexAnnotationElement(factory.createString("value"), value)
             }));
@@ -140,7 +126,7 @@ public class DexAnnotation extends DexItem {
       DexValue[] accessFlags, DexItemFactory factory) {
     assert names.length == accessFlags.length;
     return new DexAnnotation(VISIBILITY_SYSTEM,
-        new DexEncodedAnnotation(factory.createType(METHOD_PARAMETERS_DESCRIPTOR),
+        new DexEncodedAnnotation(factory.annotationMethodParameters,
             new DexAnnotationElement[]{
                 new DexAnnotationElement(
                     factory.createString("names"),
@@ -153,7 +139,7 @@ public class DexAnnotation extends DexItem {
 
   public static DexAnnotation createAnnotationDefaultAnnotation(DexType type,
       List<DexAnnotationElement> defaults, DexItemFactory factory) {
-    return createSystemValueAnnotation(ANNOTATION_DEFAULT_DESCRIPTOR, factory,
+    return createSystemValueAnnotation(factory.annotationDefault, factory,
         new DexValueAnnotation(
             new DexEncodedAnnotation(type,
                 defaults.toArray(new DexAnnotationElement[defaults.size()])))
@@ -161,34 +147,38 @@ public class DexAnnotation extends DexItem {
   }
 
   public static DexAnnotation createSignatureAnnotation(String signature, DexItemFactory factory) {
-    return createSystemValueAnnotation(SIGNATURE_DESCRIPTOR, factory,
+    return createSystemValueAnnotation(factory.annotationSignature, factory,
         compressSignature(signature, factory));
   }
 
   public static DexAnnotation createThrowsAnnotation(DexValue[] exceptions,
       DexItemFactory factory) {
-    return createSystemValueAnnotation(THROWS_DESCRIPTOR, factory, new DexValueArray(exceptions));
+    return createSystemValueAnnotation(factory.annotationThrows, factory,
+        new DexValueArray(exceptions));
   }
 
-  private static DexAnnotation createSystemValueAnnotation(String desc, DexItemFactory factory,
+  private static DexAnnotation createSystemValueAnnotation(DexType type, DexItemFactory factory,
       DexValue value) {
     return new DexAnnotation(VISIBILITY_SYSTEM,
-        new DexEncodedAnnotation(factory.createType(desc), new DexAnnotationElement[] {
+        new DexEncodedAnnotation(type, new DexAnnotationElement[]{
             new DexAnnotationElement(factory.createString("value"), value)
         }));
   }
 
-  public static boolean isThrowingAnnotation(DexAnnotation annotation) {
-    return annotation.annotation.type.toDescriptorString().equals(THROWS_DESCRIPTOR);
+  public static boolean isThrowingAnnotation(DexAnnotation annotation,
+      DexItemFactory factory) {
+    return annotation.annotation.type == factory.annotationThrows;
   }
 
-  public static boolean isSignatureAnnotation(DexAnnotation annotation) {
-    return annotation.annotation.type.toDescriptorString().equals(SIGNATURE_DESCRIPTOR);
+  public static boolean isSignatureAnnotation(DexAnnotation annotation,
+      DexItemFactory factory) {
+    return annotation.annotation.type == factory.annotationSignature;
   }
 
 
-  public static boolean isSourceDebugExtension(DexAnnotation annotation) {
-    return annotation.annotation.type.toDescriptorString().equals(SOURCE_DEBUG_EXTENSION);
+  public static boolean isSourceDebugExtension(DexAnnotation annotation,
+      DexItemFactory factory) {
+    return annotation.annotation.type == factory.annotationSourceDebugExtension;
   }
 
   /**
diff --git a/src/main/java/com/android/tools/r8/graph/DexAnnotationSet.java b/src/main/java/com/android/tools/r8/graph/DexAnnotationSet.java
index 04e94d7ea..e91e1f009 100644
--- a/src/main/java/com/android/tools/r8/graph/DexAnnotationSet.java
+++ b/src/main/java/com/android/tools/r8/graph/DexAnnotationSet.java
@@ -68,6 +68,15 @@ public class DexAnnotationSet extends DexItem {
     sorted = hashCode();
   }
 
+  public DexAnnotation getFirstMatching(DexType type) {
+    for (DexAnnotation annotation : annotations) {
+      if (annotation.annotation.type == type) {
+        return annotation;
+      }
+    }
+    return null;
+  }
+
   private int sortedHashCode() {
     int hashCode = hashCode();
     return hashCode == UNSORTED ? 1 : hashCode;
diff --git a/src/main/java/com/android/tools/r8/graph/DexItemFactory.java b/src/main/java/com/android/tools/r8/graph/DexItemFactory.java
index 5dff4b432..36d732d8d 100644
--- a/src/main/java/com/android/tools/r8/graph/DexItemFactory.java
+++ b/src/main/java/com/android/tools/r8/graph/DexItemFactory.java
@@ -129,8 +129,8 @@ public class DexItemFactory {
   public DexType annotationType = createType(annotationDescriptor);
   public DexType throwableType = createType(throwableDescriptor);
 
-  public DexType stringBuilderType = createType(createString("Ljava/lang/StringBuilder;"));
-  public DexType stringBufferType = createType(createString("Ljava/lang/StringBuffer;"));
+  public DexType stringBuilderType = createType("Ljava/lang/StringBuilder;");
+  public DexType stringBufferType = createType("Ljava/lang/StringBuffer;");
 
   public StringBuildingMethods stringBuilderMethods = new StringBuildingMethods(stringBuilderType);
   public StringBuildingMethods stringBufferMethods = new StringBuildingMethods(stringBufferType);
@@ -140,6 +140,21 @@ public class DexItemFactory {
   public ThrowableMethods throwableMethods = new ThrowableMethods();
   public ClassMethods classMethods = new ClassMethods();
 
+  // Dex system annotations.
+  // See https://source.android.com/devices/tech/dalvik/dex-format.html#system-annotation
+  public final DexType annotationDefault = createType("Ldalvik/annotation/AnnotationDefault;");
+  public final DexType annotationEnclosingClass = createType("Ldalvik/annotation/EnclosingClass;");
+  public final DexType annotationEnclosingMethod = createType(
+      "Ldalvik/annotation/EnclosingMethod;");
+  public final DexType annotationInnerClass = createType("Ldalvik/annotation/InnerClass;");
+  public final DexType annotationMemberClasses = createType("Ldalvik/annotation/MemberClasses;");
+  public final DexType annotationMethodParameters = createType(
+      "Ldalvik/annotation/MethodParameters;");
+  public final DexType annotationSignature = createType("Ldalvik/annotation/Signature;");
+  public final DexType annotationSourceDebugExtension = createType(
+      "Ldalvik/annotation/SourceDebugExtension;");
+  public final DexType annotationThrows = createType("Ldalvik/annotation/Throws;");
+
   public void clearSubtypeInformation() {
     types.values().forEach(DexType::clearSubtypeInformation);
   }
diff --git a/src/main/java/com/android/tools/r8/naming/ClassNameMinifier.java b/src/main/java/com/android/tools/r8/naming/ClassNameMinifier.java
index 7c12590b3..9f26fb51f 100644
--- a/src/main/java/com/android/tools/r8/naming/ClassNameMinifier.java
+++ b/src/main/java/com/android/tools/r8/naming/ClassNameMinifier.java
@@ -3,14 +3,17 @@
 // BSD-style license that can be found in the LICENSE file.
 package com.android.tools.r8.naming;
 
+import com.android.tools.r8.graph.DexAnnotation;
 import com.android.tools.r8.graph.DexClass;
-import com.android.tools.r8.graph.DexMethod;
+import com.android.tools.r8.graph.DexProgramClass;
 import com.android.tools.r8.graph.DexString;
 import com.android.tools.r8.graph.DexType;
+import com.android.tools.r8.graph.DexValue;
+import com.android.tools.r8.graph.DexValue.DexValueType;
 import com.android.tools.r8.shaking.Enqueuer.AppInfoWithLiveness;
 import com.android.tools.r8.shaking.RootSetBuilder.RootSet;
+import com.android.tools.r8.utils.DescriptorUtils;
 import com.android.tools.r8.utils.StringUtils;
-import com.google.common.collect.ImmutableSet;
 import com.google.common.collect.Maps;
 import com.google.common.collect.Sets;
 import java.util.Collections;
@@ -29,30 +32,31 @@ public class ClassNameMinifier {
 
   private final Map<DexType, DexString> renaming = Maps.newIdentityHashMap();
   private final Map<String, NamingState> states = new HashMap<>();
-  final List<String> dictionary;
+  private final List<String> dictionary;
+  private final boolean keepInnerClassStructure;
 
   public ClassNameMinifier(AppInfoWithLiveness appInfo, RootSet rootSet, String packagePrefix,
-      List<String> dictionary) {
+      List<String> dictionary, boolean keepInnerClassStructure) {
     this.appInfo = appInfo;
     this.rootSet = rootSet;
     this.packagePrefix = packagePrefix;
     this.dictionary = dictionary;
+    this.keepInnerClassStructure = keepInnerClassStructure;
   }
 
   public Map<DexType, DexString> computeRenaming() {
+    Iterable<DexProgramClass> classes = appInfo.classes();
     // Collect names we have to keep.
     for (DexClass clazz : appInfo.classes()) {
       if (rootSet.noObfuscation.contains(clazz)) {
         assert !renaming.containsKey(clazz.type);
-        renaming.put(clazz.type, clazz.type.descriptor);
-        usedTypeNames.add(clazz.type.descriptor);
+        registerClassAsUsed(clazz.type);
       }
     }
     for (DexClass clazz : appInfo.classes()) {
       if (!renaming.containsKey(clazz.type)) {
-        String packageName = getPackageNameFor(clazz);
-        NamingState state = getStateFor(packageName);
-        renaming.put(clazz.type, state.nextTypeName());
+        DexString renamed = computeName(clazz);
+        renaming.put(clazz.type, renamed);
       }
     }
     appInfo.dexItemFactory.forAllTypes(this::renameArrayTypeIfNeeded);
@@ -60,6 +64,63 @@ public class ClassNameMinifier {
     return Collections.unmodifiableMap(renaming);
   }
 
+  /**
+   * Registers the given type as used.
+   * <p>
+   * When {@link #keepInnerClassStructure} is true, keeping the name of an inner class will
+   * automatically also keep the name of the outer class, as otherwise the structure would be
+   * invalidated.
+   */
+  private void registerClassAsUsed(DexType type) {
+    renaming.put(type, type.descriptor);
+    usedTypeNames.add(type.descriptor);
+    if (keepInnerClassStructure) {
+      DexType outerClass = getOutClassForType(type);
+      if (outerClass != null) {
+        if (!renaming.containsKey(outerClass)) {
+          // The outer class was not previously kept. We have to do this now.
+          registerClassAsUsed(outerClass);
+        }
+      }
+    }
+  }
+
+  private DexType getOutClassForType(DexType type) {
+    DexClass clazz = appInfo.definitionFor(type);
+    if (clazz == null) {
+      return null;
+    }
+    DexAnnotation annotation =
+        clazz.annotations.getFirstMatching(appInfo.dexItemFactory.annotationEnclosingClass);
+    if (annotation != null) {
+      assert annotation.annotation.elements.length == 1;
+      DexValue value = annotation.annotation.elements[0].value;
+      return ((DexValueType) value).value;
+    }
+    // We do not need to preserve the names for local or anonymous classes, as they do not result
+    // in a member type declaration and hence cannot be referenced as nested classes in
+    // method signatures.
+    // See https://docs.oracle.com/javase/specs/jls/se7/html/jls-8.html#jls-8.5.
+    return null;
+  }
+
+  private DexString computeName(DexClass clazz) {
+    NamingState state = null;
+    if (keepInnerClassStructure) {
+      // When keeping the nesting structure of inner classes, we have to insert the name
+      // of the outer class for the $ prefix.
+      DexType outerClass = getOutClassForType(clazz.type);
+      if (outerClass != null) {
+        state = getStateForOuterClass(outerClass);
+      }
+    }
+    if (state == null) {
+      String packageName = getPackageNameFor(clazz);
+      state = getStateFor(packageName);
+    }
+    return state.nextTypeName();
+  }
+
   private String getPackageNameFor(DexClass clazz) {
     if ((packagePrefix == null) || rootSet.keepPackageName.contains(clazz)) {
       return clazz.type.getPackageDescriptor();
@@ -72,6 +133,27 @@ public class ClassNameMinifier {
     return states.computeIfAbsent(packageName, NamingState::new);
   }
 
+  private NamingState getStateForOuterClass(DexType outer) {
+    String prefix = DescriptorUtils
+        .getClassBinaryNameFromDescriptor(outer.toDescriptorString());
+    return states.computeIfAbsent(prefix, k -> {
+      // Create a naming state with this classes renaming as prefix.
+      DexString renamed = renaming.get(outer);
+      if (renamed == null) {
+        // The outer class has not been renamed yet, so rename the outer class first.
+        DexClass outerClass = appInfo.definitionFor(outer);
+        if (outerClass == null) {
+          renamed = outer.descriptor;
+        } else {
+          renamed = computeName(outerClass);
+          renaming.put(outer, renamed);
+        }
+      }
+      String binaryName = DescriptorUtils.getClassBinaryNameFromDescriptor(renamed.toString());
+      return new NamingState(binaryName, "$");
+    });
+  }
+
   private void renameArrayTypeIfNeeded(DexType type) {
     if (type.isArrayType()) {
       DexType base = type.toBaseType(appInfo.dexItemFactory);
@@ -92,11 +174,18 @@ public class ClassNameMinifier {
   private class NamingState {
 
     private final char[] packagePrefix;
+    private final String separator;
     private int typeCounter = 1;
     private Iterator<String> dictionaryIterator;
 
     NamingState(String packageName) {
-      this.packagePrefix = ("L" + packageName + (packageName.isEmpty() ? "" : "/")).toCharArray();
+      this(packageName, "/");
+    }
+
+    NamingState(String packageName, String separator) {
+      this.packagePrefix = ("L" + packageName + (packageName.isEmpty() ? "" : separator))
+          .toCharArray();
+      this.separator = separator;
       this.dictionaryIterator = dictionary.iterator();
     }
 
diff --git a/src/main/java/com/android/tools/r8/naming/Minifier.java b/src/main/java/com/android/tools/r8/naming/Minifier.java
index 9e8110278..025002dbf 100644
--- a/src/main/java/com/android/tools/r8/naming/Minifier.java
+++ b/src/main/java/com/android/tools/r8/naming/Minifier.java
@@ -42,7 +42,8 @@ public class Minifier {
     timing.begin("MinifyClasses");
     Map<DexType, DexString> classRenaming =
         new ClassNameMinifier(
-            appInfo, rootSet, options.packagePrefix, options.classObfuscationDictionary)
+            appInfo, rootSet, options.packagePrefix, options.classObfuscationDictionary,
+            options.attributeRemoval.signature)
             .computeRenaming();
     timing.end();
     timing.begin("MinifyMethods");
diff --git a/src/main/java/com/android/tools/r8/shaking/AnnotationRemover.java b/src/main/java/com/android/tools/r8/shaking/AnnotationRemover.java
index e60f241c9..3ab231cb0 100644
--- a/src/main/java/com/android/tools/r8/shaking/AnnotationRemover.java
+++ b/src/main/java/com/android/tools/r8/shaking/AnnotationRemover.java
@@ -9,6 +9,7 @@ import com.android.tools.r8.graph.DexAnnotationSet;
 import com.android.tools.r8.graph.DexAnnotationSetRefList;
 import com.android.tools.r8.graph.DexEncodedField;
 import com.android.tools.r8.graph.DexEncodedMethod;
+import com.android.tools.r8.graph.DexItemFactory;
 import com.android.tools.r8.graph.DexProgramClass;
 import com.android.tools.r8.shaking.Enqueuer.AppInfoWithLiveness;
 import com.android.tools.r8.utils.InternalOptions;
@@ -42,21 +43,24 @@ public class AnnotationRemover {
       case DexAnnotation.VISIBILITY_SYSTEM:
         // EnclosingClass and InnerClass are used in Dalvik to represent the InnerClasses
         // attribute section from class files.
-        if (keep.innerClasses &&
-            (DexAnnotation.isInnerClassesAnnotation(annotation) ||
-                DexAnnotation.isEnclosingClassAnnotation(annotation))) {
+        DexItemFactory factory = appInfo.dexItemFactory;
+        if (keep.innerClasses
+            && (DexAnnotation.isInnerClassesAnnotation(annotation, factory) ||
+                DexAnnotation.isEnclosingClassAnnotation(annotation, factory))) {
           return true;
         }
-        if (keep.enclosingMethod && DexAnnotation.isEnclosingMethodAnnotation(annotation)) {
+        if (keep.enclosingMethod
+            && DexAnnotation.isEnclosingMethodAnnotation(annotation, factory)) {
           return true;
         }
-        if (keep.exceptions && DexAnnotation.isThrowingAnnotation(annotation)) {
+        if (keep.exceptions && DexAnnotation.isThrowingAnnotation(annotation, factory)) {
           return true;
         }
-        if (keep.signature && DexAnnotation.isSignatureAnnotation(annotation)) {
+        if (keep.signature && DexAnnotation.isSignatureAnnotation(annotation, factory)) {
           return true;
         }
-        if (keep.sourceDebugExtension && DexAnnotation.isSourceDebugExtension(annotation)) {
+        if (keep.sourceDebugExtension
+            && DexAnnotation.isSourceDebugExtension(annotation, factory)) {
           return true;
         }
         return false;
diff --git a/src/main/java/com/android/tools/r8/utils/InternalOptions.java b/src/main/java/com/android/tools/r8/utils/InternalOptions.java
index 9d6cfa5ea..72f5fbc4e 100644
--- a/src/main/java/com/android/tools/r8/utils/InternalOptions.java
+++ b/src/main/java/com/android/tools/r8/utils/InternalOptions.java
@@ -251,15 +251,18 @@ public class InternalOptions {
 
     public void ensureValid(boolean isMinifying) {
       if (innerClasses && !enclosingMethod) {
-        throw new CompilationError("Attribute InnerClasses implies EnclosingMethod attribute. " +
-            "Check -keepattributes directive.");
+        throw new CompilationError("Attribute InnerClasses requires EnclosingMethod attribute. "
+            + "Check -keepattributes directive.");
       } else if (!innerClasses && enclosingMethod) {
-        throw new CompilationError("Attribute EnclosingMethod requires InnerClasses attribute. " +
-            "Check -keepattributes directive.");
+        throw new CompilationError("Attribute EnclosingMethod requires InnerClasses attribute. "
+            + "Check -keepattributes directive.");
+      } else if (signature && !innerClasses) {
+        throw new CompilationError("Attribute Signature requires InnerClasses attribute. Check "
+            + "-keepattributes directive.");
       } else if (signature && isMinifying) {
         // TODO(38188583): Allow this once we can minify signatures.
-        throw new CompilationError("Attribute Signature cannot be kept when minifying. " +
-            "Check -keepattributes directive.");
+        throw new CompilationError("Attribute Signature cannot be kept when minifying. "
+            + "Check -keepattributes directive.");
       }
     }
   }