Fix EnclosingMethod attribute when moving interface methods to companion class

RELNOTES: None.

PiperOrigin-RevId: 175613518

4 files changed, 132 insertions, 16 deletions

diff --git a/src/tools/android/java/com/google/devtools/build/android/desugar/BitFlags.java b/src/tools/android/java/com/google/devtools/build/android/desugar/BitFlags.java
index 8542719c87..bb32c452e2 100644
--- a/src/tools/android/java/com/google/devtools/build/android/desugar/BitFlags.java
+++ b/src/tools/android/java/com/google/devtools/build/android/desugar/BitFlags.java
@@ -13,6 +13,8 @@
 // limitations under the License.
 package com.google.devtools.build.android.desugar;
 
+import org.objectweb.asm.Opcodes;
+
 /**
  * Convenience method for working with {@code int} bitwise flags.
  */
@@ -34,6 +36,18 @@ class BitFlags {
     return (flags & bitmask) == 0;
   }
 
+  public static boolean isInterface(int access) {
+    return isSet(access, Opcodes.ACC_INTERFACE);
+  }
+
+  public static boolean isStatic(int access) {
+    return isSet(access, Opcodes.ACC_STATIC);
+  }
+
+  public static boolean isSynthetic(int access) {
+    return isSet(access, Opcodes.ACC_SYNTHETIC);
+  }
+
   // Static methods only
   private BitFlags() {}
 }
diff --git a/src/tools/android/java/com/google/devtools/build/android/desugar/ClassVsInterface.java b/src/tools/android/java/com/google/devtools/build/android/desugar/ClassVsInterface.java
new file mode 100644
index 0000000000..cb62deb4b0
--- /dev/null
+++ b/src/tools/android/java/com/google/devtools/build/android/desugar/ClassVsInterface.java
@@ -0,0 +1,63 @@
+// Copyright 2017 The Bazel Authors. All rights reserved.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//    http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+package com.google.devtools.build.android.desugar;
+
+import static com.google.common.base.Preconditions.checkNotNull;
+import static com.google.common.base.Preconditions.checkState;
+
+import java.util.HashMap;
+import javax.annotation.Nullable;
+import org.objectweb.asm.ClassReader;
+
+/**
+ * Simple memoizer for whether types are classes or interfaces.
+ */
+class ClassVsInterface {
+  /** Map from internal names to whether they are an interface ({@code false} thus means class). */
+  private final HashMap<String, Boolean> known = new HashMap<>();
+  private final ClassReaderFactory classpath;
+
+  public ClassVsInterface(ClassReaderFactory classpath) {
+    this.classpath = classpath;
+  }
+
+  public ClassVsInterface addKnownClass(@Nullable String internalName) {
+    if (internalName != null) {
+      Boolean previous = known.put(internalName, false);
+      checkState(previous == null || !previous, "Already recorded as interface: %s", internalName);
+    }
+    return this;
+  }
+
+  public ClassVsInterface addKnownInterfaces(String... internalNames) {
+    for (String internalName : internalNames) {
+      Boolean previous = known.put(internalName, true);
+      checkState(previous == null || previous, "Already recorded as class: %s", internalName);
+    }
+    return this;
+  }
+
+  public boolean isOuterInterface(String outerName, String innerName) {
+    Boolean result = known.get(outerName);
+    if (result == null) {
+      // We could just load the outer class here, but this tolerates incomplete classpaths better.
+      // Note the outer class should be in the Jar we're desugaring, so it should always be there.
+      ClassReader outerClass = checkNotNull(classpath.readIfKnown(outerName),
+          "Couldn't find outer class %s of %s", outerName, innerName);
+      result = BitFlags.isInterface(outerClass.getAccess());
+      known.put(outerName, result);
+    }
+    return result;
+  }
+}
diff --git a/src/tools/android/java/com/google/devtools/build/android/desugar/Desugar.java b/src/tools/android/java/com/google/devtools/build/android/desugar/Desugar.java
index 87aa0d7d34..2b02386856 100644
--- a/src/tools/android/java/com/google/devtools/build/android/desugar/Desugar.java
+++ b/src/tools/android/java/com/google/devtools/build/android/desugar/Desugar.java
@@ -24,7 +24,6 @@ import com.google.common.annotations.VisibleForTesting;
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableMap;
 import com.google.common.collect.ImmutableSet;
-import com.google.common.collect.ImmutableSet.Builder;
 import com.google.common.io.ByteStreams;
 import com.google.common.io.Closer;
 import com.google.devtools.build.android.Converters.ExistingPathConverter;
@@ -358,7 +357,7 @@ class Desugar {
       }
 
       ImmutableSet.Builder<String> interfaceLambdaMethodCollector = ImmutableSet.builder();
-
+      ClassVsInterface interfaceCache = new ClassVsInterface(classpathReader);
       desugarClassesInInput(
           inputFiles,
           outputFileProvider,
@@ -366,6 +365,7 @@ class Desugar {
           classpathReader,
           depsCollector,
           bootclasspathReader,
+          interfaceCache,
           interfaceLambdaMethodCollector);
 
       desugarAndWriteDumpedLambdaClassesToOutput(
@@ -374,6 +374,7 @@ class Desugar {
           classpathReader,
           depsCollector,
           bootclasspathReader,
+          interfaceCache,
           interfaceLambdaMethodCollector.build(),
           bridgeMethodReader);
 
@@ -445,7 +446,8 @@ class Desugar {
       @Nullable ClassReaderFactory classpathReader,
       DependencyCollector depsCollector,
       ClassReaderFactory bootclasspathReader,
-      Builder<String> interfaceLambdaMethodCollector)
+      ClassVsInterface interfaceCache,
+      ImmutableSet.Builder<String> interfaceLambdaMethodCollector)
       throws IOException {
     for (String filename : inputFiles) {
       if (OutputFileProvider.DESUGAR_DEPS_FILENAME.equals(filename)) {
@@ -465,6 +467,7 @@ class Desugar {
                   classpathReader,
                   depsCollector,
                   bootclasspathReader,
+                  interfaceCache,
                   interfaceLambdaMethodCollector,
                   writer,
                   reader);
@@ -492,6 +495,7 @@ class Desugar {
       @Nullable ClassReaderFactory classpathReader,
       DependencyCollector depsCollector,
       ClassReaderFactory bootclasspathReader,
+      ClassVsInterface interfaceCache,
       ImmutableSet<String> interfaceLambdaMethods,
       @Nullable ClassReaderFactory bridgeMethodReader)
       throws IOException {
@@ -522,6 +526,7 @@ class Desugar {
                 classpathReader,
                 depsCollector,
                 bootclasspathReader,
+                interfaceCache,
                 interfaceLambdaMethods,
                 bridgeMethodReader,
                 lambdaClass.getValue(),
@@ -564,6 +569,7 @@ class Desugar {
       @Nullable ClassReaderFactory classpathReader,
       DependencyCollector depsCollector,
       ClassReaderFactory bootclasspathReader,
+      ClassVsInterface interfaceCache,
       ImmutableSet<String> interfaceLambdaMethods,
       @Nullable ClassReaderFactory bridgeMethodReader,
       LambdaInfo lambdaClass,
@@ -591,7 +597,12 @@ class Desugar {
                 visitor, classpathReader, depsCollector, bootclasspathReader, loader);
         visitor =
             new InterfaceDesugaring(
-                visitor, depsCollector, bootclasspathReader, store, options.legacyJacocoFix);
+                visitor,
+                interfaceCache,
+                depsCollector,
+                bootclasspathReader,
+                store,
+                options.legacyJacocoFix);
       }
     }
     visitor =
@@ -621,7 +632,8 @@ class Desugar {
       @Nullable ClassReaderFactory classpathReader,
       DependencyCollector depsCollector,
       ClassReaderFactory bootclasspathReader,
-      Builder<String> interfaceLambdaMethodCollector,
+      ClassVsInterface interfaceCache,
+      ImmutableSet.Builder<String> interfaceLambdaMethodCollector,
       UnprefixingClassWriter writer,
       ClassReader input) {
     ClassVisitor visitor = checkNotNull(writer);
@@ -645,7 +657,12 @@ class Desugar {
                   visitor, classpathReader, depsCollector, bootclasspathReader, loader);
           visitor =
               new InterfaceDesugaring(
-                  visitor, depsCollector, bootclasspathReader, store, options.legacyJacocoFix);
+                  visitor,
+                  interfaceCache,
+                  depsCollector,
+                  bootclasspathReader,
+                  store,
+                  options.legacyJacocoFix);
         }
       }
       // LambdaDesugaring is relatively expensive, so check first whether we need it.  Additionally,
diff --git a/src/tools/android/java/com/google/devtools/build/android/desugar/InterfaceDesugaring.java b/src/tools/android/java/com/google/devtools/build/android/desugar/InterfaceDesugaring.java
index d7e3886641..9f24646cee 100644
--- a/src/tools/android/java/com/google/devtools/build/android/desugar/InterfaceDesugaring.java
+++ b/src/tools/android/java/com/google/devtools/build/android/desugar/InterfaceDesugaring.java
@@ -44,6 +44,7 @@ class InterfaceDesugaring extends ClassVisitor {
 
   static final String INTERFACE_STATIC_COMPANION_METHOD_SUFFIX = "$$STATIC$$";
 
+  private final ClassVsInterface interfaceCache;
   private final DependencyCollector depsCollector;
   private final ClassReaderFactory bootclasspath;
   private final GeneratedClassStore store;
@@ -58,11 +59,13 @@ class InterfaceDesugaring extends ClassVisitor {
 
   public InterfaceDesugaring(
       ClassVisitor dest,
+      ClassVsInterface interfaceCache,
       DependencyCollector depsCollector,
       ClassReaderFactory bootclasspath,
       GeneratedClassStore store,
       boolean legacyJaCoCo) {
     super(Opcodes.ASM5, dest);
+    this.interfaceCache = interfaceCache;
     this.depsCollector = depsCollector;
     this.bootclasspath = bootclasspath;
     this.store = store;
@@ -83,10 +86,14 @@ class InterfaceDesugaring extends ClassVisitor {
     bytecodeVersion = version;
     accessFlags = access;
     if (isInterface()) {
+      interfaceCache.addKnownInterfaces(name);
       // Record interface hierarchy.  This helps avoid parsing .class files when double-checking
       // desugaring results later using collected dependency information.
       depsCollector.recordExtendedInterfaces(name, interfaces);
+    } else {
+      interfaceCache.addKnownClass(name);
     }
+    interfaceCache.addKnownClass(superName).addKnownInterfaces(interfaces);
     super.visit(version, access, name, signature, superName, interfaces);
   }
 
@@ -174,16 +181,14 @@ class InterfaceDesugaring extends ClassVisitor {
       checkArgument(BitFlags.noneSet(access, Opcodes.ACC_NATIVE), "Forbidden per JLS ch 9.4");
 
       boolean isLambdaBody =
-          name.startsWith("lambda$") && BitFlags.isSet(access, Opcodes.ACC_SYNTHETIC);
+          name.startsWith("lambda$") && BitFlags.isSynthetic(access);
       if (isLambdaBody) {
         access &= ~Opcodes.ACC_PUBLIC; // undo visibility change from LambdaDesugaring
       }
-      name =
-          normalizeInterfaceMethodName(
-              name, isLambdaBody, BitFlags.isSet(access, Opcodes.ACC_STATIC));
+      name = normalizeInterfaceMethodName(name, isLambdaBody, BitFlags.isStatic(access));
       codeOwner = getCompanionClassName(internalName);
 
-      if (BitFlags.isSet(access, Opcodes.ACC_STATIC)) {
+      if (BitFlags.isStatic(access)) {
         // Completely move static interface methods, which requires rewriting call sites
         result =
             companion()
@@ -233,8 +238,27 @@ class InterfaceDesugaring extends ClassVisitor {
         : null;
   }
 
+  @Override
+  public void visitOuterClass(String owner, String name, String desc) {
+    // Proguard gets grumpy if an outer method doesn't exist, which can be the result of moving
+    // interface methods to companion classes (b/68260836).  In that case (for which we need to
+    // figure out if "owner" is an interface) need to adjust the outer method information.
+    if (name != null && interfaceCache.isOuterInterface(owner, internalName)) {
+      // Just drop outer method info.  That's unfortunate, but the only alternative would be to
+      // change the outer method to point to the companion class, which would mean the
+      // reflection methods that use this information would return a companion ($$CC) class name
+      // as well as a possibly-modified method name and signature, so it seems better to return
+      // the correct original interface name and no method information.  Doing this also saves
+      // us from doing even more work to figure out whether the method is static and a lambda
+      // method, which we'd need to known to adjust name and descriptor correctly.
+      name = null;
+      desc = null;
+    } // otherwise there's no enclosing method that could've been moved, or owner is a class
+    super.visitOuterClass(owner, name, desc);
+  }
+
   private boolean isInterface() {
-    return BitFlags.isSet(accessFlags, Opcodes.ACC_INTERFACE);
+    return BitFlags.isInterface(accessFlags);
   }
 
   private static boolean isStaticInitializer(String methodName) {
@@ -243,18 +267,16 @@ class InterfaceDesugaring extends ClassVisitor {
 
   private static String normalizeInterfaceMethodName(
       String name, boolean isLambda, boolean isStatic) {
-    String suffix;
     if (isLambda) {
       // Rename lambda method to reflect the new owner.  Not doing so confuses LambdaDesugaring
       // if it's run over this class again. LambdaDesugaring has already renamed the method from
       // its original name to include the interface name at this point.
-      suffix = DependencyCollector.INTERFACE_COMPANION_SUFFIX;
+      return name + DependencyCollector.INTERFACE_COMPANION_SUFFIX;
     } else if (isStatic) {
-      suffix = INTERFACE_STATIC_COMPANION_METHOD_SUFFIX;
+      return name + INTERFACE_STATIC_COMPANION_METHOD_SUFFIX;
     } else {
       return name;
     }
-    return name + suffix;
   }
 
   static String getCompanionClassName(String interfaceName) {