From 4f68e2ecebee00ab4fe882c691bb750ce6dab64b Mon Sep 17 00:00:00 2001 From: kmb Date: Wed, 21 Feb 2018 21:34:01 -0800 Subject: add ability to move individual core library methods RELNOTES: None. PiperOrigin-RevId: 186565673 GitOrigin-RevId: deb99ccfb4e6b236c21e6d425281870aa598804a Change-Id: I56030d75aa6b3666299aa98ec961ef7078917975 --- .../desugar/CoreLibraryInvocationRewriter.java | 12 ++++- .../build/android/desugar/CoreLibrarySupport.java | 30 +++++++++++- .../devtools/build/android/desugar/Desugar.java | 15 +++++- .../android/desugar/CoreLibrarySupportTest.java | 55 ++++++++++++++++++---- .../android/desugar/CorePackageRenamerTest.java | 20 ++++++-- 5 files changed, 115 insertions(+), 17 deletions(-) diff --git a/java/com/google/devtools/build/android/desugar/CoreLibraryInvocationRewriter.java b/java/com/google/devtools/build/android/desugar/CoreLibraryInvocationRewriter.java index e83ae41..b4bc98b 100644 --- a/java/com/google/devtools/build/android/desugar/CoreLibraryInvocationRewriter.java +++ b/java/com/google/devtools/build/android/desugar/CoreLibraryInvocationRewriter.java @@ -52,8 +52,18 @@ public class CoreLibraryInvocationRewriter extends ClassVisitor { public void visitMethodInsn(int opcode, String owner, String name, String desc, boolean itf) { Class coreInterface = support.getCoreInterfaceRewritingTarget(opcode, owner, name, desc, itf); + String newOwner = support.getMoveTarget(owner, name); - if (coreInterface != null) { + if (newOwner != null) { + checkState(coreInterface == null, + "Can't move and use companion: %s.%s : %s", owner, name, desc); + if (opcode != Opcodes.INVOKESTATIC) { + // assuming a static method + desc = InterfaceDesugaring.companionDefaultMethodDescriptor(owner, desc); + opcode = Opcodes.INVOKESTATIC; + } + itf = false; // assuming a class + } else if (coreInterface != null) { String coreInterfaceName = coreInterface.getName().replace('.', '/'); name = InterfaceDesugaring.normalizeInterfaceMethodName( diff --git a/java/com/google/devtools/build/android/desugar/CoreLibrarySupport.java b/java/com/google/devtools/build/android/desugar/CoreLibrarySupport.java index 9f01638..76eb346 100644 --- a/java/com/google/devtools/build/android/desugar/CoreLibrarySupport.java +++ b/java/com/google/devtools/build/android/desugar/CoreLibrarySupport.java @@ -16,9 +16,12 @@ package com.google.devtools.build.android.desugar; import static com.google.common.base.Preconditions.checkArgument; import static com.google.common.base.Preconditions.checkState; +import com.google.common.base.Splitter; import com.google.common.collect.ImmutableList; +import com.google.common.collect.ImmutableMap; import java.lang.reflect.Method; import java.util.LinkedHashSet; +import java.util.List; import java.util.Objects; import java.util.Set; import javax.annotation.Nullable; @@ -36,9 +39,12 @@ class CoreLibrarySupport { private final ImmutableList renamedPrefixes; /** Internal names of interfaces whose default and static interface methods we'll emulate. */ private final ImmutableList> emulatedInterfaces; + /** Map from {@code owner#name} core library members to their new owners. */ + private final ImmutableMap memberMoves; public CoreLibrarySupport(CoreLibraryRewriter rewriter, ClassLoader targetLoader, - ImmutableList renamedPrefixes, ImmutableList emulatedInterfaces) { + ImmutableList renamedPrefixes, ImmutableList emulatedInterfaces, + List memberMoves) { this.rewriter = rewriter; this.targetLoader = targetLoader; checkArgument( @@ -52,6 +58,23 @@ class CoreLibrarySupport { classBuilder.add(clazz); } this.emulatedInterfaces = classBuilder.build(); + + // We can call isRenamed and rename below b/c we initialized the necessary fields above + ImmutableMap.Builder movesBuilder = ImmutableMap.builder(); + Splitter splitter = Splitter.on("->").trimResults().omitEmptyStrings(); + for (String move : memberMoves) { + List pair = splitter.splitToList(move); + checkArgument(pair.size() == 2, "Doesn't split as expected: %s", move); + checkArgument(pair.get(0).startsWith("java/"), "Unexpected member: %s", move); + int sep = pair.get(0).indexOf('#'); + checkArgument(sep > 0 && sep == pair.get(0).lastIndexOf('#'), "invalid member: %s", move); + checkArgument(!isRenamedCoreLibrary(pair.get(0).substring(0, sep)), + "Original renamed, no need to move it: %s", move); + checkArgument(isRenamedCoreLibrary(pair.get(1)), "Target not renamed: %s", move); + + movesBuilder.put(pair.get(0), renameCoreLibrary(pair.get(1))); + } + this.memberMoves = movesBuilder.build(); } public boolean isRenamedCoreLibrary(String internalName) { @@ -73,6 +96,11 @@ class CoreLibrarySupport { : internalName; } + @Nullable + public String getMoveTarget(String owner, String name) { + return memberMoves.get(rewriter.unprefix(owner) + '#' + name); + } + /** * Returns {@code true} for java.* classes or interfaces that are subtypes of emulated interfaces. * Note that implies that this method always returns {@code false} for user-written classes. diff --git a/java/com/google/devtools/build/android/desugar/Desugar.java b/java/com/google/devtools/build/android/desugar/Desugar.java index 58b9b88..cd55655 100644 --- a/java/com/google/devtools/build/android/desugar/Desugar.java +++ b/java/com/google/devtools/build/android/desugar/Desugar.java @@ -271,6 +271,18 @@ class Desugar { ) public List emulateCoreLibraryInterfaces; + /** Members that we will retarget to the given new owner. */ + @Option( + name = "retarget_core_library_member", + defaultValue = "", // ignored + allowMultiple = true, + documentationCategory = OptionDocumentationCategory.UNDOCUMENTED, + effectTags = {OptionEffectTag.UNKNOWN}, + help = "Method invocations to retarget, given as \"class/Name#member->new/class/Name\". " + + "The new owner is blindly assumed to exist." + ) + public List retargetCoreLibraryMembers; + /** Set to work around b/62623509 with JaCoCo versions prior to 0.7.9. */ // TODO(kmb): Remove when Android Studio doesn't need it anymore (see b/37116789) @Option( @@ -388,7 +400,8 @@ class Desugar { rewriter, loader, ImmutableList.copyOf(options.rewriteCoreLibraryPrefixes), - ImmutableList.copyOf(options.emulateCoreLibraryInterfaces)); + ImmutableList.copyOf(options.emulateCoreLibraryInterfaces), + options.retargetCoreLibraryMembers); desugarClassesInInput( inputFiles, diff --git a/test/java/com/google/devtools/build/android/desugar/CoreLibrarySupportTest.java b/test/java/com/google/devtools/build/android/desugar/CoreLibrarySupportTest.java index 853dbbe..cc17748 100644 --- a/test/java/com/google/devtools/build/android/desugar/CoreLibrarySupportTest.java +++ b/test/java/com/google/devtools/build/android/desugar/CoreLibrarySupportTest.java @@ -32,7 +32,11 @@ public class CoreLibrarySupportTest { public void testIsRenamedCoreLibrary() throws Exception { CoreLibrarySupport support = new CoreLibrarySupport( - new CoreLibraryRewriter(""), null, ImmutableList.of("java/time/"), ImmutableList.of()); + new CoreLibraryRewriter(""), + null, + ImmutableList.of("java/time/"), + ImmutableList.of(), + ImmutableList.of()); assertThat(support.isRenamedCoreLibrary("java/time/X")).isTrue(); assertThat(support.isRenamedCoreLibrary("java/time/y/X")).isTrue(); assertThat(support.isRenamedCoreLibrary("java/io/X")).isFalse(); @@ -48,6 +52,7 @@ public class CoreLibrarySupportTest { new CoreLibraryRewriter("__/"), null, ImmutableList.of("java/time/"), + ImmutableList.of(), ImmutableList.of()); assertThat(support.isRenamedCoreLibrary("__/java/time/X")).isTrue(); assertThat(support.isRenamedCoreLibrary("__/java/time/y/X")).isTrue(); @@ -60,7 +65,11 @@ public class CoreLibrarySupportTest { public void testRenameCoreLibrary() throws Exception { CoreLibrarySupport support = new CoreLibrarySupport( - new CoreLibraryRewriter(""), null, ImmutableList.of(), ImmutableList.of()); + new CoreLibraryRewriter(""), + null, + ImmutableList.of(), + ImmutableList.of(), + ImmutableList.of()); assertThat(support.renameCoreLibrary("java/time/X")).isEqualTo("j$/time/X"); assertThat(support.renameCoreLibrary("com/google/X")).isEqualTo("com/google/X"); } @@ -69,11 +78,30 @@ public class CoreLibrarySupportTest { public void testRenameCoreLibrary_prefixedLoader() throws Exception { CoreLibrarySupport support = new CoreLibrarySupport( - new CoreLibraryRewriter("__/"), null, ImmutableList.of(), ImmutableList.of()); + new CoreLibraryRewriter("__/"), + null, + ImmutableList.of(), + ImmutableList.of(), + ImmutableList.of()); assertThat(support.renameCoreLibrary("__/java/time/X")).isEqualTo("j$/time/X"); assertThat(support.renameCoreLibrary("com/google/X")).isEqualTo("com/google/X"); } + @Test + public void testMoveTarget() throws Exception { + CoreLibrarySupport support = + new CoreLibrarySupport( + new CoreLibraryRewriter("__/"), + null, + ImmutableList.of("java/util/Helper"), + ImmutableList.of(), + ImmutableList.of("java/util/Existing#match -> java/util/Helper")); + assertThat(support.getMoveTarget("__/java/util/Existing", "match")).isEqualTo("j$/util/Helper"); + assertThat(support.getMoveTarget("java/util/Existing", "match")).isEqualTo("j$/util/Helper"); + assertThat(support.getMoveTarget("__/java/util/Existing", "matchesnot")).isNull(); + assertThat(support.getMoveTarget("__/java/util/ExistingOther", "match")).isNull(); + } + @Test public void testIsEmulatedCoreClassOrInterface() throws Exception { CoreLibrarySupport support = @@ -81,7 +109,8 @@ public class CoreLibrarySupportTest { new CoreLibraryRewriter(""), Thread.currentThread().getContextClassLoader(), ImmutableList.of("java/util/concurrent/"), - ImmutableList.of("java/util/Map")); + ImmutableList.of("java/util/Map"), + ImmutableList.of()); assertThat(support.isEmulatedCoreClassOrInterface("java/util/Map")).isTrue(); assertThat(support.isEmulatedCoreClassOrInterface("java/util/Map$$Lambda$17")).isFalse(); assertThat(support.isEmulatedCoreClassOrInterface("java/util/Map$$CC")).isFalse(); @@ -98,7 +127,8 @@ public class CoreLibrarySupportTest { new CoreLibraryRewriter(""), Thread.currentThread().getContextClassLoader(), ImmutableList.of(), - ImmutableList.of("java/util/Collection")); + ImmutableList.of("java/util/Collection"), + ImmutableList.of()); assertThat( support.getCoreInterfaceRewritingTarget( Opcodes.INVOKEINTERFACE, @@ -132,7 +162,8 @@ public class CoreLibrarySupportTest { new CoreLibraryRewriter(""), Thread.currentThread().getContextClassLoader(), ImmutableList.of(), - ImmutableList.of("java/util/Collection")); + ImmutableList.of("java/util/Collection"), + ImmutableList.of()); assertThat( support.getCoreInterfaceRewritingTarget( Opcodes.INVOKEINTERFACE, @@ -158,7 +189,8 @@ public class CoreLibrarySupportTest { new CoreLibraryRewriter(""), Thread.currentThread().getContextClassLoader(), ImmutableList.of(), - ImmutableList.of("java/util/Map")); + ImmutableList.of("java/util/Map"), + ImmutableList.of()); assertThat( support.getCoreInterfaceRewritingTarget( Opcodes.INVOKEINTERFACE, @@ -192,7 +224,8 @@ public class CoreLibrarySupportTest { new CoreLibraryRewriter(""), Thread.currentThread().getContextClassLoader(), ImmutableList.of(), - ImmutableList.of("java/util/Comparator")); + ImmutableList.of("java/util/Comparator"), + ImmutableList.of()); assertThat( support.getCoreInterfaceRewritingTarget( Opcodes.INVOKESTATIC, @@ -208,12 +241,13 @@ public class CoreLibrarySupportTest { * {@link InterfaceDesugaring}. */ @Test - public void testGetEmulatedCoreLibraryInvocationTarget_renamed() throws Exception { + public void testGetCoreInterfaceRewritingTarget_renamed() throws Exception { CoreLibrarySupport support = new CoreLibrarySupport( new CoreLibraryRewriter(""), Thread.currentThread().getContextClassLoader(), ImmutableList.of("java/util/"), + ImmutableList.of(), ImmutableList.of()); // regular invocations of default methods: ignored @@ -281,7 +315,8 @@ public class CoreLibrarySupportTest { new CoreLibraryRewriter(""), Thread.currentThread().getContextClassLoader(), ImmutableList.of("java/util/concurrent/"), // should return null for these - ImmutableList.of("java/util/Map")); + ImmutableList.of("java/util/Map"), + ImmutableList.of()); assertThat( support.getCoreInterfaceRewritingTarget( Opcodes.INVOKEINTERFACE, diff --git a/test/java/com/google/devtools/build/android/desugar/CorePackageRenamerTest.java b/test/java/com/google/devtools/build/android/desugar/CorePackageRenamerTest.java index 0626b46..0ff0f25 100644 --- a/test/java/com/google/devtools/build/android/desugar/CorePackageRenamerTest.java +++ b/test/java/com/google/devtools/build/android/desugar/CorePackageRenamerTest.java @@ -29,10 +29,15 @@ public class CorePackageRenamerTest { @Test public void testSymbolRewrite() throws Exception { MockClassVisitor out = new MockClassVisitor(); - CorePackageRenamer renamer = new CorePackageRenamer( - out, - new CoreLibrarySupport( - new CoreLibraryRewriter(""), null, ImmutableList.of("java/time/"), ImmutableList.of())); + CorePackageRenamer renamer = + new CorePackageRenamer( + out, + new CoreLibrarySupport( + new CoreLibraryRewriter(""), + null, + ImmutableList.of("java/time/"), + ImmutableList.of(), + ImmutableList.of("java/util/A#m->java/time/B"))); MethodVisitor mv = renamer.visitMethod(0, "test", "()V", null, null); mv.visitMethodInsn( @@ -40,6 +45,13 @@ public class CorePackageRenamerTest { assertThat(out.mv.owner).isEqualTo("j$/time/Instant"); assertThat(out.mv.desc).isEqualTo("()Lj$/time/Instant;"); + // Ignore moved methods but not their descriptors + mv.visitMethodInsn( + Opcodes.INVOKESTATIC, "java/util/A", "m", "()Ljava/time/Instant;", false); + assertThat(out.mv.owner).isEqualTo("java/util/A"); + assertThat(out.mv.desc).isEqualTo("()Lj$/time/Instant;"); + + // Ignore arbitrary other methods but not their descriptors mv.visitMethodInsn( Opcodes.INVOKESTATIC, "other/time/Instant", "now", "()Ljava/time/Instant;", false); assertThat(out.mv.owner).isEqualTo("other/time/Instant"); -- cgit v1.2.3