diff options
19 files changed, 708 insertions, 81 deletions
diff --git a/src/test/java/com/google/devtools/build/android/desugar/BUILD b/src/test/java/com/google/devtools/build/android/desugar/BUILD index cb0c5ba9b5..4cbed0b37d 100644 --- a/src/test/java/com/google/devtools/build/android/desugar/BUILD +++ b/src/test/java/com/google/devtools/build/android/desugar/BUILD @@ -159,8 +159,7 @@ java_test( ], tags = ["no_windows"], deps = [ - ":libseparate.jar", - ":testdata_desugared_java8_like_in_android_studio.jar", # Make tests run against desugared library + ":testdata_desugared_java8_like_in_android_studio", # Make tests run against desugared library "//src/test/java/com/google/devtools/build/lib:testutil", "//third_party:guava", "//third_party:jsr305", @@ -307,8 +306,8 @@ java_test( "//third_party:jsr305", "//third_party:junit4", "//third_party:truth", - # Depend on Jacoco runtime in case :testdata was instrumented with - # --collect_code_coverage or by running "blaze coverage" + # Depend on Jacoco runtime in case testdata was built with coverage + # instrumentation "//third_party/java/jacoco:blaze-agent", ], ) @@ -326,8 +325,8 @@ java_test( "//third_party:guava", "//third_party:junit4", "//third_party:truth", - # Depend on Jacoco runtime in case :testdata was instrumented with - # --collect_code_coverage or by running "blaze coverage" + # Depend on Jacoco runtime in case testdata was built with coverage + # instrumentation "//third_party/java/jacoco:blaze-agent", ], ) @@ -350,6 +349,7 @@ java_test( tags = ["no_windows"], deps = [ "//src/tools/android/java/com/google/devtools/build/android/desugar", + "//src/tools/android/java/com/google/devtools/build/android/desugar:deps_collector_api", "//third_party:asm", "//third_party:asm-tree", "//third_party:guava", @@ -670,6 +670,7 @@ java_library( resources = ["testdata/testresource.txt"], deps = [ ":separate", + ":separate_java8", "//third_party:guava", ], ) @@ -692,6 +693,11 @@ java_library( ) java_library( + name = "separate_java8", + srcs = glob(["testdata/separate8/*.java"]), +) + +java_library( name = "testdata_like_in_android_studio", srcs = glob(["testdata/*.java"]), resources = ["testdata/testresource.txt"], @@ -710,6 +716,7 @@ java_library( resources = ["testdata/testresource.txt"], deps = [ ":libseparate.jar", # puts full Jar instead of header Jar on the classpath + ":libseparate_java8.jar", # puts full Jar instead of header Jar on the classpath "//third_party:guava", ], ) @@ -733,7 +740,7 @@ sh_test( ) # The following genrules run the code tested here as a build tool. While that's -# very similar to how Blaze will invoke it natively, the downside is that +# very similar to how Bazel will invoke it natively, the downside is that # running the tested tool in a genrule doesn't contribute code coverage # information for the tested tool. Note that the code in :testdata doesn't # appear in coverage reports when depending on this target regardless because @@ -744,8 +751,8 @@ genrule( ":guava_base_classpath.jar", ":separate", ":testdata", - # Depend on Jacoco runtime in case :testdata was instrumented with - # --collect_code_coverage or by running "blaze coverage" + # Depend on Jacoco runtime in case testdata was built with coverage + # instrumentation "//third_party/java/jacoco:blaze-agent", "//tools/defaults:android_jar", ], @@ -780,8 +787,8 @@ genrule( ":guava_base_classpath.jar", ":separate", ":generate_synthetic_methods_with_lambda_names_in_test_data", - # Depend on Jacoco runtime in case :testdata was instrumented with - # --collect_code_coverage or by running "blaze coverage" + # Depend on Jacoco runtime in case testdata was built with coverage + # instrumentation "//third_party/java/jacoco:blaze-agent", "//tools/defaults:android_jar", ], @@ -802,8 +809,8 @@ genrule( ":guava_base_classpath.jar", ":separate", ":testdata_desugared.jar", - # Depend on Jacoco runtime in case :testdata was instrumented with - # --collect_code_coverage or by running "blaze coverage" + # Depend on Jacoco runtime in case testdata was built with coverage + # instrumentation "//third_party/java/jacoco:blaze-agent", "//tools/defaults:android_jar", ], @@ -827,8 +834,8 @@ genrule( ":guava_base_classpath.jar", ":separate", ":testdata", - # Depend on Jacoco runtime in case :testdata was instrumented with - # --collect_code_coverage or by running "blaze coverage" + # Depend on Jacoco runtime in case testdata was built with coverage + # instrumentation "//third_party/java/jacoco:blaze-agent", "//tools/defaults:android_jar", ], @@ -995,16 +1002,19 @@ genrule( srcs = [ ":guava_base_classpath.jar", ":separate", + ":separate_java8", ":testdata_java8", - # Depend on Jacoco runtime in case :testdata_java8 was instrumented with - # --collect_code_coverage or by running "blaze coverage" + # Depend on Jacoco runtime in case testdata was built with coverage + # instrumentation "//third_party/java/jacoco:blaze-agent", "//tools/defaults:android_jar", ], outs = ["testdata_desugared_java8.jar"], cmd = "$(location //src/tools/android/java/com/google/devtools/build/android/desugar:Desugar) " + - "--min_sdk_version 24 -i $(location :testdata_java8) -o $@ " + + "--min_sdk_version 24 " + + "-i $(location :testdata_java8) -o $@ " + "--classpath_entry $(location :separate) " + + "--classpath_entry $(location :separate_java8) " + "--classpath_entry $(location :guava_base_classpath.jar) " + "--classpath_entry $(location //third_party/java/jacoco:blaze-agent) " + "--bootclasspath_entry $(location //tools/defaults:android_jar)", @@ -1017,9 +1027,10 @@ genrule( srcs = [ ":guava_base_classpath.jar", ":separate", + ":separate_java8", ":testdata_java8_like_in_android_studio", - # Depend on Jacoco runtime in case :testdata_java8 was instrumented with - # --collect_code_coverage or by running "blaze coverage" + # Depend on Jacoco runtime in case testdata was built with coverage + # instrumentation "//third_party/java/jacoco:blaze-agent", "//tools/defaults:android_jar", ], @@ -1030,6 +1041,7 @@ genrule( "--min_sdk_version 24 " + "-i $(location :testdata_java8_like_in_android_studio) -o $@ " + "--classpath_entry $(location :separate) " + + "--classpath_entry $(location :separate_java8) " + "--classpath_entry $(location :guava_base_classpath.jar) " + "--classpath_entry $(location //third_party/java/jacoco:blaze-agent) " + "--bootclasspath_entry $(location //tools/defaults:android_jar)", @@ -1042,16 +1054,19 @@ genrule( srcs = [ ":guava_base_classpath.jar", ":separate", + ":separate_java8", ":testdata_java8", - # Depend on Jacoco runtime in case :testdata_java8 was instrumented with - # --collect_code_coverage or by running "blaze coverage" + # Depend on Jacoco runtime in case testdata was built with coverage + # instrumentation "//third_party/java/jacoco:blaze-agent", "//tools/defaults:android_jar", ], outs = ["testdata_desugared_default_methods.jar"], cmd = "$(location //src/tools/android/java/com/google/devtools/build/android/desugar:Desugar) " + - "--desugar_interface_method_bodies_if_needed -i $(location :testdata_java8) -o $@ " + + "-i $(location :testdata_java8) -o $@ " + + "--emit_dependency_metadata_as_needed " + "--classpath_entry $(location :separate) " + + "--classpath_entry $(location :separate_java8) " + "--classpath_entry $(location :guava_base_classpath.jar) " + "--classpath_entry $(location //third_party/java/jacoco:blaze-agent) " + "--bootclasspath_entry $(location //tools/defaults:android_jar)", @@ -1060,13 +1075,33 @@ genrule( ) genrule( + name = "desugar_separate_java8_with_default_methods", + srcs = [ + ":separate_java8", + # Depend on Jacoco runtime in case testdata was built with coverage + # instrumentation + "//third_party/java/jacoco:blaze-agent", + "//tools/defaults:android_jar", + ], + outs = ["separate_java8_desugared_default_methods.jar"], + cmd = "$(location //src/tools/android/java/com/google/devtools/build/android/desugar:Desugar) " + + "-i $(location :separate_java8) -o $@ " + + "--emit_dependency_metadata_as_needed " + + "--classpath_entry $(location //third_party/java/jacoco:blaze-agent) " + + "--bootclasspath_entry $(location //tools/defaults:android_jar)", + tags = ["no_windows"], + tools = ["//src/tools/android/java/com/google/devtools/build/android/desugar:Desugar"], +) + +genrule( name = "desugar_testdata_with_default_methods_for_static_initializer_test", srcs = [ ":guava_base_classpath.jar", ":separate", + ":separate_java8", ":testdata_java8", - # Depend on Jacoco runtime in case :testdata_java8 was instrumented with - # --collect_code_coverage or by running "blaze coverage" + # Depend on Jacoco runtime in case testdata was built with coverage + # instrumentation "//third_party/java/jacoco:blaze-agent", "//tools/defaults:android_jar", ], @@ -1075,6 +1110,7 @@ genrule( "--desugar_interface_method_bodies_if_needed -i $(location :testdata_java8) " + "-o desugar_testdata_with_default_methods_for_static_initializer_test.jar " + "--classpath_entry $(location :separate) " + + "--classpath_entry $(location :separate_java8) " + "--classpath_entry $(location :guava_base_classpath.jar) " + "--classpath_entry $(location //third_party/java/jacoco:blaze-agent) " + "--bootclasspath_entry $(location //tools/defaults:android_jar) " + @@ -1088,9 +1124,10 @@ genrule( srcs = [ ":guava_base_classpath.jar", ":separate", + ":separate_java8", ":testdata_desugared_default_methods.jar", - # Depend on Jacoco runtime in case :testdata_java8 was instrumented with - # --collect_code_coverage or by running "blaze coverage" + # Depend on Jacoco runtime in case testdata was built with coverage + # instrumentation "//third_party/java/jacoco:blaze-agent", "//tools/defaults:android_jar", ], @@ -1098,6 +1135,7 @@ genrule( cmd = "$(location //src/tools/android/java/com/google/devtools/build/android/desugar:Desugar) " + "--desugar_interface_method_bodies_if_needed -i $(location :testdata_desugared_default_methods.jar) -o $@ " + "--classpath_entry $(location :separate) " + + "--classpath_entry $(location :separate_java8) " + "--classpath_entry $(location :guava_base_classpath.jar) " + "--classpath_entry $(location //third_party/java/jacoco:blaze-agent) " + "--bootclasspath_entry $(location //tools/defaults:android_jar)", @@ -1109,8 +1147,8 @@ genrule( name = "desugar_testdata_core_library", srcs = [ ":testdata_core_library", - # Depend on Jacoco runtime in case :testdata was instrumented with - # --collect_code_coverage or by running "blaze coverage" + # Depend on Jacoco runtime in case testdata was built with coverage + # instrumentation "//third_party/java/jacoco:blaze-agent", "//tools/defaults:android_jar", ], @@ -1129,8 +1167,8 @@ genrule( ":guava_base_classpath.jar", ":separate", ":testdata_like_in_android_studio", - # Depend on Jacoco runtime in case :testdata_java8 was instrumented with - # --collect_code_coverage or by running "blaze coverage" + # Depend on Jacoco runtime in case testdata was built with coverage + # instrumentation "//third_party/java/jacoco:blaze-agent", "//tools/defaults:android_jar", ], @@ -1155,8 +1193,8 @@ genrule( ":guava_base_classpath.jar", ":separate", ":testdata_like_in_android_studio", - # Depend on Jacoco runtime in case :testdata_java8 was instrumented with - # --collect_code_coverage or by running "blaze coverage" + # Depend on Jacoco runtime in case testdata was built with coverage + # instrumentation "//third_party/java/jacoco:blaze-agent", "//tools/defaults:android_jar", ], @@ -1183,8 +1221,8 @@ genrule( ":guava_base_classpath.jar", ":separate", ":testdata_like_in_android_studio", - # Depend on Jacoco runtime in case :testdata_java8 was instrumented with - # --collect_code_coverage or by running "blaze coverage" + # Depend on Jacoco runtime in case testdata was built with coverage + # instrumentation "//third_party/java/jacoco:blaze-agent", "//tools/defaults:android_jar", ], @@ -1219,8 +1257,8 @@ genrule( ":guava_base_classpath.jar", ":separate", ":testdata_like_in_android_studio", - # Depend on Jacoco runtime in case :testdata_java8 was instrumented with - # --collect_code_coverage or by running "blaze coverage" + # Depend on Jacoco runtime in case testdata was built with coverage + # instrumentation "//third_party/java/jacoco:blaze-agent", "//tools/defaults:android_jar", ], @@ -1260,8 +1298,8 @@ genrule( ":guava_base_classpath.jar", ":separate", ":testdata_like_in_android_studio", - # Depend on Jacoco runtime in case :testdata_java8 was instrumented with - # --collect_code_coverage or by running "blaze coverage" + # Depend on Jacoco runtime in case testdata was built with coverage + # instrumentation "//third_party/java/jacoco:blaze-agent", "//tools/defaults:android_jar", ], @@ -1298,8 +1336,8 @@ genrule( ":guava_base_classpath.jar", ":separate", ":testdata", - # Depend on Jacoco runtime in case :testdata was instrumented with - # --collect_code_coverage or by running "blaze coverage" + # Depend on Jacoco runtime in case testdata was built with coverage + # instrumentation "//third_party/java/jacoco:blaze-agent", "//tools/defaults:android_jar", ], @@ -1322,8 +1360,6 @@ genrule( name = "desugar_default_method_with_legacy_coverage", srcs = [ "jacoco_0_7_5_default_method.jar", - # Depend on Jacoco runtime in case :testdata_java8 was instrumented with - # --collect_code_coverage or by running "blaze coverage" "//third_party/java/jacoco:blaze-agent", "//tools/defaults:android_jar", ], @@ -1346,8 +1382,8 @@ genrule( ":guava_base_classpath.jar", ":separate", ":testdata", - # Depend on Jacoco runtime in case :testdata was instrumented with - # --collect_code_coverage or by running "blaze coverage" + # Depend on Jacoco runtime in case testdata was built with coverage + # instrumentation "//third_party/java/jacoco:blaze-agent", "//tools/defaults:android_jar", ], @@ -1369,8 +1405,8 @@ genrule( ":guava_base_classpath.jar", ":separate", ":desugar_testdata_by_desugaring_try_with_resources", - # Depend on Jacoco runtime in case :testdata was instrumented with - # --collect_code_coverage or by running "blaze coverage" + # Depend on Jacoco runtime in case testdata was built with coverage + # instrumentation "//third_party/java/jacoco:blaze-agent", "//tools/defaults:android_jar", ], @@ -1402,25 +1438,42 @@ java_import( java_import( name = "testdata_desugared_java8", jars = ["testdata_desugared_java8.jar"], - deps = [":separate"], + deps = [ + ":separate", + ":separate_java8", # use un-desugared since we're not desugaring default methods here + ], ) java_import( name = "testdata_desugared_java8_like_in_android_studio", jars = ["testdata_desugared_java8_like_in_android_studio.jar"], - deps = [":separate"], + deps = [ + ":separate", + ":separate_java8", # use un-desugared since we're not desugaring default methods here + ], ) java_import( name = "testdata_desugared_default_methods", jars = ["testdata_desugared_default_methods.jar"], - deps = [":separate"], + deps = [ + ":separate", + ":separate_java8_desugared_default_methods", + ], ) java_import( name = "testdata_desugared_default_methods_twice", jars = ["testdata_desugared_default_methods_twice.jar"], - deps = [":separate"], + deps = [ + ":separate", + ":separate_java8_desugared_default_methods", + ], +) + +java_import( + name = "separate_java8_desugared_default_methods", + jars = ["separate_java8_desugared_default_methods.jar"], ) java_import( @@ -1653,8 +1706,8 @@ genrule( name = "desugar_capture_lambda", srcs = [ ":capture_lambda", - # Depend on Jacoco runtime in case :testdata was instrumented with - # --collect_code_coverage or by running "blaze coverage" + # Depend on Jacoco runtime in case testdata was built with coverage + # instrumentation "//third_party/java/jacoco:blaze-agent", "//tools/defaults:android_jar", ], @@ -1671,8 +1724,8 @@ genrule( name = "desugar_capture_lambda_again", srcs = [ ":capture_lambda_desugared.jar", - # Depend on Jacoco runtime in case :testdata was instrumented with - # --collect_code_coverage or by running "blaze coverage" + # Depend on Jacoco runtime in case testdata was built with coverage + # instrumentation "//third_party/java/jacoco:blaze-agent", "//tools/defaults:android_jar", ], diff --git a/src/test/java/com/google/devtools/build/android/desugar/DefaultMethodClassFixerTest.java b/src/test/java/com/google/devtools/build/android/desugar/DefaultMethodClassFixerTest.java index cdc32639b3..c74febb184 100644 --- a/src/test/java/com/google/devtools/build/android/desugar/DefaultMethodClassFixerTest.java +++ b/src/test/java/com/google/devtools/build/android/desugar/DefaultMethodClassFixerTest.java @@ -98,7 +98,12 @@ public class DefaultMethodClassFixerTest { private byte[] desugar(ClassReader reader) { ClassWriter writer = new ClassWriter(ClassWriter.COMPUTE_MAXS); DefaultMethodClassFixer fixer = - new DefaultMethodClassFixer(writer, classpathReader, bootclassPath, classLoader); + new DefaultMethodClassFixer( + writer, + classpathReader, + DependencyCollector.NoWriteCollectors.FAIL_ON_MISSING, + bootclassPath, + classLoader); reader.accept(fixer, 0); return writer.toByteArray(); } diff --git a/src/test/java/com/google/devtools/build/android/desugar/DesugarJava8FunctionalTest.java b/src/test/java/com/google/devtools/build/android/desugar/DesugarJava8FunctionalTest.java index 8321d75350..63d8d27ac5 100644 --- a/src/test/java/com/google/devtools/build/android/desugar/DesugarJava8FunctionalTest.java +++ b/src/test/java/com/google/devtools/build/android/desugar/DesugarJava8FunctionalTest.java @@ -23,6 +23,9 @@ import com.google.devtools.build.android.desugar.testdata.java8.ConcreteDefaultI import com.google.devtools.build.android.desugar.testdata.java8.ConcreteOverridesDefaultWithLambda; import com.google.devtools.build.android.desugar.testdata.java8.DefaultInterfaceMethodWithStaticInitializer; import com.google.devtools.build.android.desugar.testdata.java8.DefaultInterfaceWithBridges; +import com.google.devtools.build.android.desugar.testdata.java8.DefaultMethodFromSeparateJava8Target; +import com.google.devtools.build.android.desugar.testdata.java8.DefaultMethodFromSeparateJava8TargetOverridden; +import com.google.devtools.build.android.desugar.testdata.java8.DefaultMethodTransitivelyFromSeparateJava8Target; import com.google.devtools.build.android.desugar.testdata.java8.FunctionWithDefaultMethod; import com.google.devtools.build.android.desugar.testdata.java8.FunctionalInterfaceWithInitializerAndDefaultMethods; import com.google.devtools.build.android.desugar.testdata.java8.GenericDefaultInterfaceWithLambda; @@ -394,4 +397,16 @@ public class DesugarJava8FunctionalTest extends DesugarFunctionalTest { .getExpectedInitializationOrder()); } } + + /** + * Tests that default methods on the classpath are correctly handled. We'll also verify the + * metadata that's emitted for this case to make sure the binary-wide double-check for correct + * desugaring of default and static interface methods keeps working (b/65645388). + */ + @Test + public void testDefaultMethodsInSeparateTarget() { + assertThat(new DefaultMethodFromSeparateJava8Target().dflt()).isEqualTo("dflt"); + assertThat(new DefaultMethodTransitivelyFromSeparateJava8Target().dflt()).isEqualTo("dflt"); + assertThat(new DefaultMethodFromSeparateJava8TargetOverridden().dflt()).isEqualTo("override"); + } } diff --git a/src/test/java/com/google/devtools/build/android/desugar/dependencies/MetadataCollectorTest.java b/src/test/java/com/google/devtools/build/android/desugar/dependencies/MetadataCollectorTest.java new file mode 100644 index 0000000000..e99abc49d9 --- /dev/null +++ b/src/test/java/com/google/devtools/build/android/desugar/dependencies/MetadataCollectorTest.java @@ -0,0 +1,113 @@ +// 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.dependencies; + +import static com.google.common.truth.Truth.assertThat; + +import com.google.common.collect.ImmutableList; +import com.google.devtools.build.android.desugar.proto.DesugarDeps.Dependency; +import com.google.devtools.build.android.desugar.proto.DesugarDeps.DesugarDepsInfo; +import com.google.devtools.build.android.desugar.proto.DesugarDeps.InterfaceDetails; +import com.google.devtools.build.android.desugar.proto.DesugarDeps.InterfaceWithCompanion; +import com.google.devtools.build.android.desugar.proto.DesugarDeps.Type; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.junit.runners.JUnit4; + +/** Unit tests for {@link MetadataCollector}. */ +@RunWith(JUnit4.class) +public class MetadataCollectorTest { + + @Test + public void testEmptyAvoidsOutput() { + assertThat(new MetadataCollector(false).toByteArray()).isNull(); + } + + @Test + public void testAssumeCompanionClass() throws Exception { + MetadataCollector collector = new MetadataCollector(false); + collector.assumeCompanionClass("a", "b$$CC"); + collector.assumeCompanionClass("b", "b$$CC"); + collector.assumeCompanionClass("a", "a$$CC"); + + DesugarDepsInfo info = extractProto(collector); + assertThat(info.getAssumePresentList()) + .containsExactly( + Dependency.newBuilder().setOrigin(wrapType("a")).setTarget(wrapType("b$$CC")).build(), + Dependency.newBuilder().setOrigin(wrapType("b")).setTarget(wrapType("b$$CC")).build(), + Dependency.newBuilder().setOrigin(wrapType("a")).setTarget(wrapType("a$$CC")).build()); + } + + @Test + public void testMissingImplementedInterface() throws Exception { + MetadataCollector collector = new MetadataCollector(true); + collector.missingImplementedInterface("a", "b"); + collector.missingImplementedInterface("a", "c"); + collector.missingImplementedInterface("c", "b"); + + DesugarDepsInfo info = extractProto(collector); + assertThat(info.getMissingInterfaceList()) + .containsExactly( + Dependency.newBuilder().setOrigin(wrapType("a")).setTarget(wrapType("b")).build(), + Dependency.newBuilder().setOrigin(wrapType("a")).setTarget(wrapType("c")).build(), + Dependency.newBuilder().setOrigin(wrapType("c")).setTarget(wrapType("b")).build()); + } + + @Test + public void testRecordExtendedInterfaces() throws Exception { + MetadataCollector collector = new MetadataCollector(false); + collector.recordExtendedInterfaces("a", "b", "c"); + collector.recordExtendedInterfaces("b"); + collector.recordExtendedInterfaces("c", "d"); + + DesugarDepsInfo info = extractProto(collector); + assertThat(info.getInterfaceWithSupertypesList()) + .containsExactly( + InterfaceDetails.newBuilder() + .setOrigin(wrapType("a")) + .addAllExtendedInterface(ImmutableList.of(wrapType("b"), wrapType("c"))) + .build(), + InterfaceDetails.newBuilder() + .setOrigin(wrapType("c")) + .addAllExtendedInterface(ImmutableList.of(wrapType("d"))) + .build()); + } + + @Test + public void testRecordDefaultMethods() throws Exception { + MetadataCollector collector = new MetadataCollector(false); + collector.recordDefaultMethods("a", 0); + collector.recordDefaultMethods("b", 1); + + DesugarDepsInfo info = extractProto(collector); + assertThat(info.getInterfaceWithCompanionList()) + .containsExactly( + InterfaceWithCompanion.newBuilder() + .setOrigin(wrapType("a")) + .setNumDefaultMethods(0) + .build(), + InterfaceWithCompanion.newBuilder() + .setOrigin(wrapType("b")) + .setNumDefaultMethods(1) + .build()); + } + + private static Type wrapType(String name) { + return Type.newBuilder().setBinaryName(name).build(); + } + + private DesugarDepsInfo extractProto(MetadataCollector collector) throws Exception { + return DesugarDepsInfo.parseFrom(collector.toByteArray()); + } +} diff --git a/src/test/java/com/google/devtools/build/android/desugar/testdata/java8/DefaultMethodFromSeparateJava8Target.java b/src/test/java/com/google/devtools/build/android/desugar/testdata/java8/DefaultMethodFromSeparateJava8Target.java new file mode 100644 index 0000000000..e0e870330b --- /dev/null +++ b/src/test/java/com/google/devtools/build/android/desugar/testdata/java8/DefaultMethodFromSeparateJava8Target.java @@ -0,0 +1,20 @@ +// 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.testdata.java8; + +import com.google.devtools.build.android.desugar.testdata.separate8.SeparateInterfaceWithDefaultMethod; + +/** Test class that inherits default method defined in separate target for testing b/65645388. */ +public class DefaultMethodFromSeparateJava8Target + implements SeparateInterfaceWithDefaultMethod {} diff --git a/src/test/java/com/google/devtools/build/android/desugar/testdata/java8/DefaultMethodFromSeparateJava8TargetOverridden.java b/src/test/java/com/google/devtools/build/android/desugar/testdata/java8/DefaultMethodFromSeparateJava8TargetOverridden.java new file mode 100644 index 0000000000..1613f8e64a --- /dev/null +++ b/src/test/java/com/google/devtools/build/android/desugar/testdata/java8/DefaultMethodFromSeparateJava8TargetOverridden.java @@ -0,0 +1,25 @@ +// 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.testdata.java8; + +import com.google.devtools.build.android.desugar.testdata.separate8.SeparateInterfaceThatInheritsDefaultMethod; + +/** Test class that overrides default method defined in separate target for testing b/65645388. */ +public class DefaultMethodFromSeparateJava8TargetOverridden + implements SeparateInterfaceThatInheritsDefaultMethod { + @Override + public String dflt() { + return "override"; + } +} diff --git a/src/test/java/com/google/devtools/build/android/desugar/testdata/java8/DefaultMethodTransitivelyFromSeparateJava8Target.java b/src/test/java/com/google/devtools/build/android/desugar/testdata/java8/DefaultMethodTransitivelyFromSeparateJava8Target.java new file mode 100644 index 0000000000..693eaf741f --- /dev/null +++ b/src/test/java/com/google/devtools/build/android/desugar/testdata/java8/DefaultMethodTransitivelyFromSeparateJava8Target.java @@ -0,0 +1,23 @@ +// 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.testdata.java8; + +import com.google.devtools.build.android.desugar.testdata.separate8.SeparateInterfaceThatInheritsDefaultMethod; + +/** + * Test class that transitively inherits default method defined in separate target for testing + * b/65645388. + */ +public class DefaultMethodTransitivelyFromSeparateJava8Target + implements SeparateInterfaceThatInheritsDefaultMethod {} diff --git a/src/test/java/com/google/devtools/build/android/desugar/testdata/separate8/SeparateInterfaceThatInheritsDefaultMethod.java b/src/test/java/com/google/devtools/build/android/desugar/testdata/separate8/SeparateInterfaceThatInheritsDefaultMethod.java new file mode 100644 index 0000000000..400a6f19dd --- /dev/null +++ b/src/test/java/com/google/devtools/build/android/desugar/testdata/separate8/SeparateInterfaceThatInheritsDefaultMethod.java @@ -0,0 +1,18 @@ +// 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.testdata.separate8; + +/** Interface that inherits default method in separate compilation target for testing b/65645388. */ +public interface SeparateInterfaceThatInheritsDefaultMethod + extends SeparateInterfaceWithDefaultMethod {} diff --git a/src/test/java/com/google/devtools/build/android/desugar/testdata/separate8/SeparateInterfaceWithDefaultMethod.java b/src/test/java/com/google/devtools/build/android/desugar/testdata/separate8/SeparateInterfaceWithDefaultMethod.java new file mode 100644 index 0000000000..b3c04c8c2d --- /dev/null +++ b/src/test/java/com/google/devtools/build/android/desugar/testdata/separate8/SeparateInterfaceWithDefaultMethod.java @@ -0,0 +1,21 @@ +// 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.testdata.separate8; + +/** Interface with default method in separate compilation target for testing b/65645388. */ +public interface SeparateInterfaceWithDefaultMethod { + default String dflt() { + return "dflt"; + } +} diff --git a/src/test/java/com/google/devtools/build/android/desugar/testdata_desugared_java8_jar_toc_golden.txt b/src/test/java/com/google/devtools/build/android/desugar/testdata_desugared_java8_jar_toc_golden.txt index 8664932324..907edd0a0a 100644 --- a/src/test/java/com/google/devtools/build/android/desugar/testdata_desugared_java8_jar_toc_golden.txt +++ b/src/test/java/com/google/devtools/build/android/desugar/testdata_desugared_java8_jar_toc_golden.txt @@ -56,6 +56,9 @@ com/google/devtools/build/android/desugar/testdata/java8/DefaultInterfaceMethodW com/google/devtools/build/android/desugar/testdata/java8/DefaultInterfaceMethodWithStaticInitializer.class com/google/devtools/build/android/desugar/testdata/java8/DefaultInterfaceWithBridges.class com/google/devtools/build/android/desugar/testdata/java8/DefaultInterfaceWithLambda.class +com/google/devtools/build/android/desugar/testdata/java8/DefaultMethodFromSeparateJava8Target.class +com/google/devtools/build/android/desugar/testdata/java8/DefaultMethodFromSeparateJava8TargetOverridden.class +com/google/devtools/build/android/desugar/testdata/java8/DefaultMethodTransitivelyFromSeparateJava8Target.class com/google/devtools/build/android/desugar/testdata/java8/FunctionWithDefaultMethod$DoubleInts.class com/google/devtools/build/android/desugar/testdata/java8/FunctionWithDefaultMethod$DoubleInts2.class com/google/devtools/build/android/desugar/testdata/java8/FunctionWithDefaultMethod.class diff --git a/src/tools/android/java/com/google/devtools/build/android/desugar/BUILD b/src/tools/android/java/com/google/devtools/build/android/desugar/BUILD index eacb620843..a75afb2f04 100644 --- a/src/tools/android/java/com/google/devtools/build/android/desugar/BUILD +++ b/src/tools/android/java/com/google/devtools/build/android/desugar/BUILD @@ -10,16 +10,31 @@ filegroup( ) java_library( + name = "deps_collector_api", + srcs = ["DependencyCollector.java"], + visibility = [ + "//src/test/java/com/google/devtools/build/android/desugar:__subpackages__", + "//src/tools/android/java/com/google/devtools/build/android:__subpackages__", + ], + deps = [ + "//third_party:jsr305", + ], +) + +java_library( name = "desugar", srcs = glob(["*.java"]), visibility = [ "//src/test/java/com/google/devtools/build/android/desugar:__pkg__", "//src/tools/android/java/com/google/devtools/build/android:__pkg__", ], - runtime_deps = ["//src/tools/android/java/com/google/devtools/build/android/desugar/runtime:throwable_extension"], + runtime_deps = [ + "//src/tools/android/java/com/google/devtools/build/android/desugar/dependencies", + "//src/tools/android/java/com/google/devtools/build/android/desugar/runtime:throwable_extension", + ], deps = [ + ":deps_collector_api", "//src/main/java/com/google/devtools/common/options", - "//src/main/protobuf:worker_protocol_java_proto", "//src/tools/android/java/com/google/devtools/build/android:android_builder_lib", "//third_party:asm", "//third_party:asm-commons", @@ -42,6 +57,7 @@ java_binary( filegroup( name = "srcs", srcs = glob(["**"]) + [ + "//src/tools/android/java/com/google/devtools/build/android/desugar/dependencies:srcs", "//src/tools/android/java/com/google/devtools/build/android/desugar/runtime:srcs", ], visibility = ["//src/tools/android/java/com/google/devtools/build/android:__pkg__"], diff --git a/src/tools/android/java/com/google/devtools/build/android/desugar/DefaultMethodClassFixer.java b/src/tools/android/java/com/google/devtools/build/android/desugar/DefaultMethodClassFixer.java index d7d46a1919..2d89e8b2b1 100644 --- a/src/tools/android/java/com/google/devtools/build/android/desugar/DefaultMethodClassFixer.java +++ b/src/tools/android/java/com/google/devtools/build/android/desugar/DefaultMethodClassFixer.java @@ -43,6 +43,7 @@ public class DefaultMethodClassFixer extends ClassVisitor { private final ClassReaderFactory classpath; private final ClassReaderFactory bootclasspath; private final ClassLoader targetLoader; + private final DependencyCollector depsCollector; private final HashSet<String> instanceMethods = new HashSet<>(); private boolean isInterface; @@ -55,12 +56,14 @@ public class DefaultMethodClassFixer extends ClassVisitor { public DefaultMethodClassFixer( ClassVisitor dest, ClassReaderFactory classpath, + DependencyCollector depsCollector, ClassReaderFactory bootclasspath, ClassLoader targetLoader) { super(Opcodes.ASM5, dest); this.classpath = classpath; this.bootclasspath = bootclasspath; this.targetLoader = targetLoader; + this.depsCollector = depsCollector; } @Override @@ -310,8 +313,17 @@ public class DefaultMethodClassFixer extends ClassVisitor { */ private boolean defaultMethodsDefined(ImmutableList<String> interfaces) { for (String implemented : interfaces) { + if (bootclasspath.isKnown(implemented)) { + continue; + } ClassReader bytecode = classpath.readIfKnown(implemented); - if (bytecode != null && !bootclasspath.isKnown(implemented)) { + if (bytecode == null) { + // Interface isn't on the classpath, which indicates incomplete classpaths. Record missing + // dependency so we can check it later. If we don't check then we may get runtime failures + // or wrong behavior from default methods that should've been stubbed in. + // TODO(kmb): Print a warning so people can start fixing their deps? + depsCollector.missingImplementedInterface(internalName, implemented); + } else { // Class in classpath and bootclasspath is a bad idea but in any event, assume the // bootclasspath will take precedence like in a classloader. // We can skip code attributes as we just need to find default methods to stub. @@ -321,10 +333,6 @@ public class DefaultMethodClassFixer extends ClassVisitor { return true; } } - // Else interface isn't on the classpath, which indicates incomplete classpaths. For now - // we'll just assume the missing interfaces don't declare default methods but if they do - // we'll end up with concrete classes that don't implement an abstract method, which can - // cause runtime failures. The classpath needs to be fixed in this case. } return false; } @@ -405,6 +413,8 @@ public class DefaultMethodClassFixer extends ClassVisitor { // definitions conflict, but see stubMissingDefaultMethods() for how we deal with default // methods redefined in interfaces extending another. recordIfInstanceMethod(access, name, desc); + depsCollector.assumeCompanionClass( + internalName, InterfaceDesugaring.getCompanionClassName(interfaceName)); // Add this method to the class we're desugaring and stub in a body to call the default // implementation in the interface's companion class. ijar omits these methods when setting @@ -423,7 +433,7 @@ public class DefaultMethodClassFixer extends ClassVisitor { } stubMethod.visitMethodInsn( Opcodes.INVOKESTATIC, - interfaceName + InterfaceDesugaring.COMPANION_SUFFIX, + InterfaceDesugaring.getCompanionClassName(interfaceName), name, InterfaceDesugaring.companionDefaultMethodDescriptor(interfaceName, desc), /*itf*/ false); @@ -434,6 +444,8 @@ public class DefaultMethodClassFixer extends ClassVisitor { return null; } else if (shouldStubAsBridgeDefaultMethod(access, name, desc)) { recordIfInstanceMethod(access, name, desc); + depsCollector.assumeCompanionClass( + internalName, InterfaceDesugaring.getCompanionClassName(interfaceName)); // For bridges we just copy their bodies instead of going through the companion class. // Meanwhile, we also need to desugar the copied method bodies, so that any calls to // interface methods are correctly handled. diff --git a/src/tools/android/java/com/google/devtools/build/android/desugar/DependencyCollector.java b/src/tools/android/java/com/google/devtools/build/android/desugar/DependencyCollector.java new file mode 100644 index 0000000000..272a273598 --- /dev/null +++ b/src/tools/android/java/com/google/devtools/build/android/desugar/DependencyCollector.java @@ -0,0 +1,98 @@ +// 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 javax.annotation.Nullable; + +/** + * Interface for collecting desugaring metadata that we can use to double-check correct desugaring + * at the binary level by looking at the metadata written for all Jars on the runtime classpath + * (b/65645388). Use {@link NoWriteCollectors} for "no-op" collectors and {@link + * com.google.devtools.build.android.desugar.dependencies.MetadataCollector} for writing out + * metadata files. + */ +// TODO(kmb): There could conceivably be a "self-contained" version where we check at the end that +// we actually saw all the companion classes (in recordDefaultMethods) that we "assumed"; useful +// for one-shot runs over an entire binary. +@SuppressWarnings("unused") // default implementations consist of empty method stubs +public interface DependencyCollector { + + /** Class name suffix used for interface companion classes. */ + public String INTERFACE_COMPANION_SUFFIX = "$$CC"; + + /** + * Records that {@code origin} depends on companion class {@code target}. For the resulting + * binary to be valid, {@code target} needs to exist, which isn't the case if the corresponding + * interface is only available as a compile-time ("neverlink") dependency. + */ + default void assumeCompanionClass(String origin, String target) {} + + /** + * Records that {@code origin} transitively implements {@code target} but {@code target} isn't + * in the classpath. This can lead to wrong desugarings if {@code target} or an interface it + * extends defines default methods. + */ + default void missingImplementedInterface(String origin, String target) {} + + /** + * Records that the given interface extends the given interfaces. + * + * <p>This information is useful reference to double-check {@link #missingImplementedInterface}s + * without reading and parsing .class files, specifically if default methods are defined in + * interfaces that a missing interface transitively extends. + */ + default void recordExtendedInterfaces(String origin, String... targets) {} + + /** + * Records that the given interface has a companion class that includes the given number of + * default methods (0 if there were only static methods). This method should not be called for + * purely abstract interfaces, to allow verifying available companion classes against this. + * + * <p>This information is useful reference to double-check {@link #missingImplementedInterface}s + * without reading and parsing .class files with better precision than just looking for + * companion classes on the runtime classpath (which may only contain static methods). + */ + default void recordDefaultMethods(String origin, int count) {} + + /** + * Returns metadata to include into the desugaring output or {@code null} if none. Returning + * anything but {@code null} will cause an extra file to be written into the output, including + * an empty array. + */ + @Nullable public byte[] toByteArray(); + + /** Simple collectors that don't collect any information. */ + public enum NoWriteCollectors implements DependencyCollector { + /** Singleton instance that does nothing. */ + NOOP, + /** + * Singleton instance that does nothing besides throwing if {@link #missingImplementedInterface} + * is called. + */ + FAIL_ON_MISSING { + @Override + public void missingImplementedInterface(String origin, String target) { + throw new IllegalStateException( + String.format( + "Couldn't find interface %s on the classpath for desugaring %s", target, origin)); + } + }; + + @Override + @Nullable + public final byte[] toByteArray() { + return null; + } + } +} 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 31c362e67a..07702fed06 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 @@ -176,6 +176,26 @@ class Desugar { public int minSdkVersion; @Option( + name = "emit_dependency_metadata_as_needed", + defaultValue = "false", + documentationCategory = OptionDocumentationCategory.UNCATEGORIZED, + effectTags = {OptionEffectTag.UNKNOWN}, + help = "Whether to emit META-INF/desugar_deps as needed for later consistency checking." + ) + public boolean emitDependencyMetadata; + + @Option( + name = "best_effort_tolerate_missing_deps", + defaultValue = "true", + category = "misc", + documentationCategory = OptionDocumentationCategory.UNCATEGORIZED, + effectTags = {OptionEffectTag.UNKNOWN}, + help = "Whether to tolerate missing dependencies on the classpath in some cases. You should " + + "strive to set this flag to false." + ) + public boolean tolerateMissingDependencies; + + @Option( name = "desugar_interface_method_bodies_if_needed", defaultValue = "true", category = "misc", @@ -316,6 +336,7 @@ class Desugar { try (OutputFileProvider outputFileProvider = toOutputFileProvider(outputPath); InputFileProvider inputFiles = toInputFileProvider(inputPath)) { + DependencyCollector depsCollector = createDepsCollector(); IndexedInputs indexedInputFiles = new IndexedInputs(ImmutableList.of(inputFiles)); // Prepend classpath with input file itself so LambdaDesugaring can load classes with // lambdas. @@ -344,6 +365,7 @@ class Desugar { outputFileProvider, loader, classpathReader, + depsCollector, bootclasspathReader, interfaceLambdaMethodCollector); @@ -351,12 +373,18 @@ class Desugar { outputFileProvider, loader, classpathReader, + depsCollector, bootclasspathReader, interfaceLambdaMethodCollector.build(), bridgeMethodReader); desugarAndWriteGeneratedClasses(outputFileProvider); copyThrowableExtensionClass(outputFileProvider); + + byte[] depsInfo = depsCollector.toByteArray(); + if (depsInfo != null) { + outputFileProvider.write(OutputFileProvider.DESUGAR_DEPS_FILENAME, depsInfo); + } } ImmutableMap<Path, LambdaInfo> lambdasLeftBehind = lambdas.drain(); @@ -365,6 +393,32 @@ class Desugar { checkState(generatedLeftBehind.isEmpty(), "Didn't process %s", generatedLeftBehind.keySet()); } + /** + * Returns a dependency collector for use with a single input Jar. If + * {@link DesugarOptions#emitDependencyMetadata} is set, this method instantiates the collector + * reflectively to allow compiling and using the desugar tool without this mechanism. + */ + private DependencyCollector createDepsCollector() { + if (options.emitDependencyMetadata) { + try { + return (DependencyCollector) + Thread.currentThread() + .getContextClassLoader() + .loadClass( + "com.google.devtools.build.android.desugar.dependencies.MetadataCollector") + .getConstructor(Boolean.TYPE) + .newInstance(options.tolerateMissingDependencies); + } catch (ReflectiveOperationException + | SecurityException e) { + throw new IllegalStateException("Can't emit desugaring metadata as requested"); + } + } else if (options.tolerateMissingDependencies) { + return DependencyCollector.NoWriteCollectors.NOOP; + } else { + return DependencyCollector.NoWriteCollectors.FAIL_ON_MISSING; + } + } + private void copyThrowableExtensionClass(OutputFileProvider outputFileProvider) { if (allowTryWithResources || options.desugarTryWithResourcesOmitRuntimeClasses) { // try-with-resources statements are okay in the output jar. @@ -390,10 +444,15 @@ class Desugar { OutputFileProvider outputFileProvider, ClassLoader loader, @Nullable ClassReaderFactory classpathReader, + DependencyCollector depsCollector, ClassReaderFactory bootclasspathReader, Builder<String> interfaceLambdaMethodCollector) throws IOException { for (String filename : inputFiles) { + if (OutputFileProvider.DESUGAR_DEPS_FILENAME.equals(filename)) { + // TODO(kmb): rule out that this happens or merge input file with what's in depsCollector + continue; // skip as we're writing a new file like this at the end or don't want it + } try (InputStream content = inputFiles.getInputStream(filename)) { // We can write classes uncompressed since they need to be converted to .dex format // for Android anyways. Resources are written as they were in the input jar to avoid @@ -405,6 +464,7 @@ class Desugar { createClassVisitorsForClassesInInputs( loader, classpathReader, + depsCollector, bootclasspathReader, interfaceLambdaMethodCollector, writer, @@ -431,6 +491,7 @@ class Desugar { OutputFileProvider outputFileProvider, ClassLoader loader, @Nullable ClassReaderFactory classpathReader, + DependencyCollector depsCollector, ClassReaderFactory bootclasspathReader, ImmutableSet<String> interfaceLambdaMethods, @Nullable ClassReaderFactory bridgeMethodReader) @@ -460,6 +521,7 @@ class Desugar { createClassVisitorsForDumpedLambdaClasses( loader, classpathReader, + depsCollector, bootclasspathReader, interfaceLambdaMethods, bridgeMethodReader, @@ -499,6 +561,7 @@ class Desugar { private ClassVisitor createClassVisitorsForDumpedLambdaClasses( ClassLoader loader, @Nullable ClassReaderFactory classpathReader, + DependencyCollector depsCollector, ClassReaderFactory bootclasspathReader, ImmutableSet<String> interfaceLambdaMethods, @Nullable ClassReaderFactory bridgeMethodReader, @@ -523,9 +586,11 @@ class Desugar { visitor = new Java7Compatibility(visitor, (ClassReaderFactory) null); if (options.desugarInterfaceMethodBodiesIfNeeded) { visitor = - new DefaultMethodClassFixer(visitor, classpathReader, bootclasspathReader, loader); + new DefaultMethodClassFixer( + visitor, classpathReader, depsCollector, bootclasspathReader, loader); visitor = - new InterfaceDesugaring(visitor, bootclasspathReader, store, options.legacyJacocoFix); + new InterfaceDesugaring( + visitor, depsCollector, bootclasspathReader, store, options.legacyJacocoFix); } } visitor = @@ -553,6 +618,7 @@ class Desugar { private ClassVisitor createClassVisitorsForClassesInInputs( ClassLoader loader, @Nullable ClassReaderFactory classpathReader, + DependencyCollector depsCollector, ClassReaderFactory bootclasspathReader, Builder<String> interfaceLambdaMethodCollector, UnprefixingClassWriter writer, @@ -574,9 +640,11 @@ class Desugar { visitor = new Java7Compatibility(visitor, classpathReader); if (options.desugarInterfaceMethodBodiesIfNeeded) { visitor = - new DefaultMethodClassFixer(visitor, classpathReader, bootclasspathReader, loader); + new DefaultMethodClassFixer( + visitor, classpathReader, depsCollector, bootclasspathReader, loader); visitor = - new InterfaceDesugaring(visitor, bootclasspathReader, store, options.legacyJacocoFix); + new InterfaceDesugaring( + visitor, 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 6bac9edb26..0f03236e72 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 @@ -42,9 +42,9 @@ class InterfaceDesugaring extends ClassVisitor { static final String COMPANION_METHOD_TO_TRIGGER_INTERFACE_CLINIT_NAME = "$$triggerInterfaceInit"; static final String COMPANION_METHOD_TO_TRIGGER_INTERFACE_CLINIT_DESC = "()V"; - static final String COMPANION_SUFFIX = "$$CC"; static final String INTERFACE_STATIC_COMPANION_METHOD_SUFFIX = "$$STATIC$$"; + private final DependencyCollector depsCollector; private final ClassReaderFactory bootclasspath; private final GeneratedClassStore store; private final boolean legacyJaCoCo; @@ -52,13 +52,18 @@ class InterfaceDesugaring extends ClassVisitor { private String internalName; private int bytecodeVersion; private int accessFlags; + private int numberOfDefaultMethods; @Nullable private ClassVisitor companion; @Nullable private FieldInfo interfaceFieldToAccessInCompanionMethodToTriggerInterfaceClinit; public InterfaceDesugaring( - ClassVisitor dest, ClassReaderFactory bootclasspath, GeneratedClassStore store, + ClassVisitor dest, + DependencyCollector depsCollector, + ClassReaderFactory bootclasspath, + GeneratedClassStore store, boolean legacyJaCoCo) { super(Opcodes.ASM5, dest); + this.depsCollector = depsCollector; this.bootclasspath = bootclasspath; this.store = store; this.legacyJaCoCo = legacyJaCoCo; @@ -73,15 +78,26 @@ class InterfaceDesugaring extends ClassVisitor { String superName, String[] interfaces) { companion = null; + numberOfDefaultMethods = 0; internalName = name; bytecodeVersion = version; accessFlags = access; + if (isInterface()) { + // Record interface hierarchy. This helps avoid parsing .class files when double-checking + // desugaring results later using collected dependency information. + depsCollector.recordExtendedInterfaces(name, interfaces); + } super.visit(version, access, name, signature, superName, interfaces); } @Override public void visitEnd() { if (companion != null) { + // Record classes with default methods. This increases precision when double-checking + // desugaring results later, without parsing .class files again, compared to just looking + // for companion classes in a given desugared Jar which may only contain static methods. + depsCollector.recordDefaultMethods(internalName, numberOfDefaultMethods); + // Emit a method to access the fields of the interfaces that need initialization. emitInterfaceFieldAccessInCompanionMethodToTriggerInterfaceClinit(); companion.visitEnd(); @@ -185,6 +201,7 @@ class InterfaceDesugaring extends ClassVisitor { name, internalName, desc); + ++numberOfDefaultMethods; abstractDest = super.visitMethod(access | Opcodes.ACC_ABSTRACT, name, desc, signature, exceptions); } @@ -228,7 +245,7 @@ class InterfaceDesugaring extends ClassVisitor { // 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 = COMPANION_SUFFIX; + suffix = DependencyCollector.INTERFACE_COMPANION_SUFFIX; } else if (isStatic) { suffix = INTERFACE_STATIC_COMPANION_METHOD_SUFFIX; } else { @@ -238,7 +255,7 @@ class InterfaceDesugaring extends ClassVisitor { } static String getCompanionClassName(String interfaceName) { - return interfaceName + COMPANION_SUFFIX; + return interfaceName + DependencyCollector.INTERFACE_COMPANION_SUFFIX; } /** @@ -329,8 +346,8 @@ class InterfaceDesugaring extends ClassVisitor { name = normalizeInterfaceMethodName(name, isLambda, opcode == Opcodes.INVOKESTATIC); if (isLambda) { // Redirect lambda invocations to completely remove all lambda methods from interfaces. - checkArgument( - !owner.endsWith(COMPANION_SUFFIX), "shouldn't consider %s an interface", owner); + checkArgument(!owner.endsWith(DependencyCollector.INTERFACE_COMPANION_SUFFIX), + "shouldn't consider %s an interface", owner); checkArgument(!bootclasspath.isKnown(owner)); // must be in current input if (opcode == Opcodes.INVOKEINTERFACE) { opcode = Opcodes.INVOKESTATIC; @@ -344,7 +361,7 @@ class InterfaceDesugaring extends ClassVisitor { name); } // Reflect that InterfaceDesugaring moves and renames the lambda body method - owner += COMPANION_SUFFIX; + owner += DependencyCollector.INTERFACE_COMPANION_SUFFIX; String expectedLambdaMethodName = LambdaDesugaring.uniqueInPackage(owner, name); checkState( name.equals(expectedLambdaMethodName), @@ -355,14 +372,14 @@ class InterfaceDesugaring extends ClassVisitor { itf = false; } else if ((opcode == Opcodes.INVOKESTATIC || opcode == Opcodes.INVOKESPECIAL) && !bootclasspath.isKnown(owner)) { - checkArgument( - !owner.endsWith(COMPANION_SUFFIX), "shouldn't consider %s an interface", owner); + checkArgument(!owner.endsWith(DependencyCollector.INTERFACE_COMPANION_SUFFIX), + "shouldn't consider %s an interface", owner); if (opcode == Opcodes.INVOKESPECIAL) { // Turn Interface.super.m() into Interface$$CC.m(receiver) opcode = Opcodes.INVOKESTATIC; desc = companionDefaultMethodDescriptor(owner, desc); } - owner += COMPANION_SUFFIX; + owner += DependencyCollector.INTERFACE_COMPANION_SUFFIX; itf = false; } } @@ -384,7 +401,8 @@ class InterfaceDesugaring extends ClassVisitor { @Override public void visitFieldInsn(int opcode, String owner, String name, String desc) { if ("$jacocoData".equals(name)) { - checkState(!owner.endsWith(COMPANION_SUFFIX), "Expected interface: %s", owner); + checkState(!owner.endsWith(DependencyCollector.INTERFACE_COMPANION_SUFFIX), + "Expected interface: %s", owner); owner = getCompanionClassName(owner); } super.visitFieldInsn(opcode, owner, name, desc); diff --git a/src/tools/android/java/com/google/devtools/build/android/desugar/OutputFileProvider.java b/src/tools/android/java/com/google/devtools/build/android/desugar/OutputFileProvider.java index bf3b710746..7a590ef2ba 100644 --- a/src/tools/android/java/com/google/devtools/build/android/desugar/OutputFileProvider.java +++ b/src/tools/android/java/com/google/devtools/build/android/desugar/OutputFileProvider.java @@ -18,6 +18,9 @@ import java.io.IOException; /** Output file provider allows to write files in directory or jar files. */ interface OutputFileProvider extends AutoCloseable { + /** Filename to use to write out dependency metadata for later consistency checking. */ + public static final String DESUGAR_DEPS_FILENAME = "META-INF/desugar_deps"; + /** * Copy {@code filename} from {@code inputFileProvider} to this output. If input file provider is * a {@link ZipInputFileProvider} then the metadata of the zip entry are kept. diff --git a/src/tools/android/java/com/google/devtools/build/android/desugar/ZipOutputFileProvider.java b/src/tools/android/java/com/google/devtools/build/android/desugar/ZipOutputFileProvider.java index 3f4f344954..8d6501d198 100644 --- a/src/tools/android/java/com/google/devtools/build/android/desugar/ZipOutputFileProvider.java +++ b/src/tools/android/java/com/google/devtools/build/android/desugar/ZipOutputFileProvider.java @@ -13,7 +13,8 @@ // limitations under the License. package com.google.devtools.build.android.desugar; -import com.google.common.base.Preconditions; +import static com.google.common.base.Preconditions.checkArgument; + import com.google.common.io.ByteStreams; import java.io.BufferedOutputStream; import java.io.IOException; @@ -45,7 +46,8 @@ public class ZipOutputFileProvider implements OutputFileProvider { @Override public void write(String filename, byte[] content) throws IOException { - Preconditions.checkArgument(filename.endsWith(".class")); + checkArgument(filename.equals(DESUGAR_DEPS_FILENAME) || filename.endsWith(".class"), + "Expect file to be copied: %s", filename); writeStoredEntry(out, filename, content); } diff --git a/src/tools/android/java/com/google/devtools/build/android/desugar/dependencies/BUILD b/src/tools/android/java/com/google/devtools/build/android/desugar/dependencies/BUILD new file mode 100644 index 0000000000..a34488d0d4 --- /dev/null +++ b/src/tools/android/java/com/google/devtools/build/android/desugar/dependencies/BUILD @@ -0,0 +1,26 @@ +# Description: +# Dependency tracking helper library for desugar that we package separately +# for the benefit of desugar users who want to compile the tool from source. + +package( + default_visibility = [ + "//src/test/java/com/google/devtools/build/android/desugar:__subpackages__", + "//src/tools/android/java/com/google/devtools/build/android/desugar:__pkg__", + ], +) + +java_library( + name = "dependencies", + srcs = glob(["*.java"]), + deps = [ + "//src/main/protobuf:desugar_deps_java_proto", + "//src/tools/android/java/com/google/devtools/build/android/desugar:deps_collector_api", + "//third_party:guava", + "//third_party:jsr305", + ], +) + +filegroup( + name = "srcs", + srcs = glob(["**"]), +) diff --git a/src/tools/android/java/com/google/devtools/build/android/desugar/dependencies/MetadataCollector.java b/src/tools/android/java/com/google/devtools/build/android/desugar/dependencies/MetadataCollector.java new file mode 100644 index 0000000000..732ef50721 --- /dev/null +++ b/src/tools/android/java/com/google/devtools/build/android/desugar/dependencies/MetadataCollector.java @@ -0,0 +1,88 @@ +// 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.dependencies; + +import static com.google.common.base.Preconditions.checkArgument; +import static com.google.common.base.Preconditions.checkState; + +import com.google.devtools.build.android.desugar.DependencyCollector; +import com.google.devtools.build.android.desugar.proto.DesugarDeps; +import com.google.devtools.build.android.desugar.proto.DesugarDeps.Dependency; +import com.google.devtools.build.android.desugar.proto.DesugarDeps.DesugarDepsInfo; +import com.google.devtools.build.android.desugar.proto.DesugarDeps.InterfaceDetails; +import com.google.devtools.build.android.desugar.proto.DesugarDeps.InterfaceWithCompanion; +import com.google.devtools.build.android.desugar.proto.DesugarDeps.Type; +import javax.annotation.Nullable; + +/** Dependency collector that emits collected metadata as a {@link DesugarDepsInfo} proto. */ +public final class MetadataCollector implements DependencyCollector { + + private final boolean tolerateMissingDeps; + private final DesugarDepsInfo.Builder info = DesugarDeps.DesugarDepsInfo.newBuilder(); + + public MetadataCollector(boolean tolerateMissingDeps) { + this.tolerateMissingDeps = tolerateMissingDeps; + } + + @Override + public void assumeCompanionClass(String origin, String target) { + checkArgument(target.endsWith(INTERFACE_COMPANION_SUFFIX), + "target not a companion: %s -> %s", origin, target); + info.addAssumePresent( + Dependency.newBuilder().setOrigin(wrapType(origin)).setTarget(wrapType(target))); + } + + @Override + public void missingImplementedInterface(String origin, String target) { + checkArgument(!target.endsWith(INTERFACE_COMPANION_SUFFIX), + "target seems to be a companion: %s -> %s", origin, target); + checkState(tolerateMissingDeps, + "Couldn't find interface %s on the classpath for desugaring %s", target, origin); + info.addMissingInterface( + Dependency.newBuilder().setOrigin(wrapType(origin)).setTarget(wrapType(target))); + } + + @Override + public void recordExtendedInterfaces(String origin, String... targets) { + if (targets.length > 0) { + InterfaceDetails.Builder details = + InterfaceDetails.newBuilder().setOrigin(wrapType(origin)); + for (String target : targets) { + details.addExtendedInterface(wrapType(target)); + } + info.addInterfaceWithSupertypes(details); + } + } + + @Override + public void recordDefaultMethods(String origin, int count) { + checkArgument(!origin.endsWith(INTERFACE_COMPANION_SUFFIX), + "seems to be a companion: %s", origin); + info.addInterfaceWithCompanion( + InterfaceWithCompanion.newBuilder() + .setOrigin(wrapType(origin)) + .setNumDefaultMethods(count)); + } + + @Override + @Nullable + public byte[] toByteArray() { + DesugarDepsInfo result = info.build(); + return DesugarDepsInfo.getDefaultInstance().equals(result) ? null : result.toByteArray(); + } + + private static Type wrapType(String internalName) { + return Type.newBuilder().setBinaryName(internalName).build(); + } +}
\ No newline at end of file |