From 45383a982f296015a233fdb5ffa46fad9e0ae245 Mon Sep 17 00:00:00 2001 From: cnsun Date: Fri, 5 Jan 2018 11:10:25 -0800 Subject: Relax the assertion on the inferred resource type. Now we only require that the resource type should have a (public) close() method. The old version requires the resource type implements AutoCloseable. When the classpath provided to Desugar has some problems, the resource type may not implement AutoCloseable, though it has the close() method. RELNOTES:n/a. PiperOrigin-RevId: 180950815 GitOrigin-RevId: 7bde688a21b781caa666fe2bebe4482cf987270b Change-Id: Id0a03911e12f903ce62fec72317a7dbc8d311287 --- .../android/desugar/TryWithResourcesRewriter.java | 46 ++++++++++++++++------ .../DesugarTryWithResourcesFunctionalTest.java | 30 ++++++++++++++ .../testdata/ClassUsingTryWithResources.java | 14 +++++++ ...twr_with_large_minsdkversion_jar_toc_golden.txt | 1 + ...gared_for_try_with_resources_jar_toc_golden.txt | 1 + .../desugar/testdata_desugared_jar_toc_golden.txt | 1 + .../testdata_desugared_java8_jar_toc_golden.txt | 1 + ...red_without_lambda_desugared_jar_toc_golden.txt | 1 + 8 files changed, 83 insertions(+), 12 deletions(-) diff --git a/java/com/google/devtools/build/android/desugar/TryWithResourcesRewriter.java b/java/com/google/devtools/build/android/desugar/TryWithResourcesRewriter.java index 8e6d6d5..e8509e7 100644 --- a/java/com/google/devtools/build/android/desugar/TryWithResourcesRewriter.java +++ b/java/com/google/devtools/build/android/desugar/TryWithResourcesRewriter.java @@ -311,18 +311,20 @@ public class TryWithResourcesRewriter extends ClassVisitor { InferredType resourceType = typeInference.getTypeOfOperandFromTop(0); Optional resourceClassInternalName = resourceType.getInternalName(); - checkState( - resourceClassInternalName.isPresent(), - "The resource class %s is not a reference type in %s.%s", - resourceType, - internalName, - methodSignature); - checkState( - isAssignableFrom( - "java.lang.AutoCloseable", resourceClassInternalName.get().replace('/', '.')), - "The resource type should be a subclass of java.lang.AutoCloseable: %s", - resourceClassInternalName); - + { + // Check the resource type. + checkState( + resourceClassInternalName.isPresent(), + "The resource class %s is not a reference type in %s.%s", + resourceType, + internalName, + methodSignature); + String resourceClassName = resourceClassInternalName.get().replace('/', '.'); + checkState( + hasCloseMethod(resourceClassName), + "The resource class %s should have a close() method.", + resourceClassName); + } resourceTypeInternalNames.add(resourceClassInternalName.get()); super.visitMethodInsn( opcode, @@ -356,6 +358,26 @@ public class TryWithResourcesRewriter extends ClassVisitor { return isAssignableFrom("java.lang.Throwable", owner.replace('/', '.')); } + private boolean hasCloseMethod(String resourceClassName) { + try { + Class klass = classLoader.loadClass(resourceClassName); + klass.getMethod("close"); + return true; + } catch (ClassNotFoundException e) { + throw new AssertionError( + "Failed to load class " + + resourceClassName + + " when desugaring method " + + internalName + + "." + + methodSignature, + e); + } catch (NoSuchMethodException e) { + // There is no close() method in the class, so return false. + return false; + } + } + private boolean isAssignableFrom(String baseClassName, String subClassName) { try { Class baseClass = classLoader.loadClass(baseClassName); diff --git a/test/java/com/google/devtools/build/android/desugar/DesugarTryWithResourcesFunctionalTest.java b/test/java/com/google/devtools/build/android/desugar/DesugarTryWithResourcesFunctionalTest.java index 38df9e3..2d567d3 100644 --- a/test/java/com/google/devtools/build/android/desugar/DesugarTryWithResourcesFunctionalTest.java +++ b/test/java/com/google/devtools/build/android/desugar/DesugarTryWithResourcesFunctionalTest.java @@ -96,4 +96,34 @@ public class DesugarTryWithResourcesFunctionalTest { } } } + + @Test + public void testInheritanceTryWithResources() { + + try { + ClassUsingTryWithResources.inheritanceTryWithResources(); + fail("Expected RuntimeException"); + } catch (Exception expected) { + assertThat(expected.getClass()).isEqualTo(RuntimeException.class); + + String expectedStrategyName = getTwrStrategyClassNameSpecifiedInSystemProperty(); + assertThat(getStrategyClassName()).isEqualTo(expectedStrategyName); + if (isMimicStrategy()) { + assertThat(expected.getSuppressed()).isEmpty(); + assertThat(ThrowableExtension.getSuppressed(expected)).hasLength(1); + assertThat(ThrowableExtension.getSuppressed(expected)[0].getClass()) + .isEqualTo(IOException.class); + } else if (isReuseStrategy()) { + assertThat(expected.getSuppressed()).hasLength(1); + assertThat(expected.getSuppressed()[0].getClass()).isEqualTo(IOException.class); + assertThat(ThrowableExtension.getSuppressed(expected)[0].getClass()) + .isEqualTo(IOException.class); + } else if (isNullStrategy()) { + assertThat(expected.getSuppressed()).isEmpty(); + assertThat(ThrowableExtension.getSuppressed(expected)).isEmpty(); + } else { + fail("unexpected desugaring strategy " + getStrategyClassName()); + } + } + } } diff --git a/test/java/com/google/devtools/build/android/desugar/testdata/ClassUsingTryWithResources.java b/test/java/com/google/devtools/build/android/desugar/testdata/ClassUsingTryWithResources.java index c340c84..e4f7f18 100644 --- a/test/java/com/google/devtools/build/android/desugar/testdata/ClassUsingTryWithResources.java +++ b/test/java/com/google/devtools/build/android/desugar/testdata/ClassUsingTryWithResources.java @@ -49,6 +49,9 @@ public class ClassUsingTryWithResources { } } + /** A resource inheriting the close() method from its parent. */ + public static class InheritanceResource extends SimpleResource {} + /** This method will always throw {@link java.lang.Exception}. */ public static void simpleTryWithResources() throws Exception { // Throwable.addSuppressed(Throwable) should be called in the following block. @@ -57,6 +60,17 @@ public class ClassUsingTryWithResources { } } + /** + * This method useds {@link InheritanceResource}, which inherits all methods from {@link + * SimpleResource}. + */ + public static void inheritanceTryWithResources() throws Exception { + // Throwable.addSuppressed(Throwable) should be called in the following block. + try (InheritanceResource resource = new InheritanceResource()) { + resource.call(true); + } + } + public static Throwable[] checkSuppressedExceptions(boolean throwException) { // Throwable.addSuppressed(Throwable) should be called in the following block. try (SimpleResource resource = new SimpleResource()) { diff --git a/test/java/com/google/devtools/build/android/desugar/testdata_desugared_for_disabling_twr_with_large_minsdkversion_jar_toc_golden.txt b/test/java/com/google/devtools/build/android/desugar/testdata_desugared_for_disabling_twr_with_large_minsdkversion_jar_toc_golden.txt index b7c3c25..8e396f5 100644 --- a/test/java/com/google/devtools/build/android/desugar/testdata_desugared_for_disabling_twr_with_large_minsdkversion_jar_toc_golden.txt +++ b/test/java/com/google/devtools/build/android/desugar/testdata_desugared_for_disabling_twr_with_large_minsdkversion_jar_toc_golden.txt @@ -12,6 +12,7 @@ com/google/devtools/build/android/desugar/testdata/CaptureLambda.class com/google/devtools/build/android/desugar/testdata/ClassCallingLongCompare$LongCmpFunc.class com/google/devtools/build/android/desugar/testdata/ClassCallingLongCompare.class com/google/devtools/build/android/desugar/testdata/ClassCallingRequireNonNull.class +com/google/devtools/build/android/desugar/testdata/ClassUsingTryWithResources$InheritanceResource.class com/google/devtools/build/android/desugar/testdata/ClassUsingTryWithResources$SimpleResource.class com/google/devtools/build/android/desugar/testdata/ClassUsingTryWithResources.class com/google/devtools/build/android/desugar/testdata/ConcreteFunction$Parser.class diff --git a/test/java/com/google/devtools/build/android/desugar/testdata_desugared_for_try_with_resources_jar_toc_golden.txt b/test/java/com/google/devtools/build/android/desugar/testdata_desugared_for_try_with_resources_jar_toc_golden.txt index d03b121..41e8d1c 100644 --- a/test/java/com/google/devtools/build/android/desugar/testdata_desugared_for_try_with_resources_jar_toc_golden.txt +++ b/test/java/com/google/devtools/build/android/desugar/testdata_desugared_for_try_with_resources_jar_toc_golden.txt @@ -12,6 +12,7 @@ com/google/devtools/build/android/desugar/testdata/CaptureLambda.class com/google/devtools/build/android/desugar/testdata/ClassCallingLongCompare$LongCmpFunc.class com/google/devtools/build/android/desugar/testdata/ClassCallingLongCompare.class com/google/devtools/build/android/desugar/testdata/ClassCallingRequireNonNull.class +com/google/devtools/build/android/desugar/testdata/ClassUsingTryWithResources$InheritanceResource.class com/google/devtools/build/android/desugar/testdata/ClassUsingTryWithResources$SimpleResource.class com/google/devtools/build/android/desugar/testdata/ClassUsingTryWithResources.class com/google/devtools/build/android/desugar/testdata/ConcreteFunction$Parser.class diff --git a/test/java/com/google/devtools/build/android/desugar/testdata_desugared_jar_toc_golden.txt b/test/java/com/google/devtools/build/android/desugar/testdata_desugared_jar_toc_golden.txt index 91fc415..b79961e 100644 --- a/test/java/com/google/devtools/build/android/desugar/testdata_desugared_jar_toc_golden.txt +++ b/test/java/com/google/devtools/build/android/desugar/testdata_desugared_jar_toc_golden.txt @@ -12,6 +12,7 @@ com/google/devtools/build/android/desugar/testdata/CaptureLambda.class com/google/devtools/build/android/desugar/testdata/ClassCallingLongCompare$LongCmpFunc.class com/google/devtools/build/android/desugar/testdata/ClassCallingLongCompare.class com/google/devtools/build/android/desugar/testdata/ClassCallingRequireNonNull.class +com/google/devtools/build/android/desugar/testdata/ClassUsingTryWithResources$InheritanceResource.class com/google/devtools/build/android/desugar/testdata/ClassUsingTryWithResources$SimpleResource.class com/google/devtools/build/android/desugar/testdata/ClassUsingTryWithResources.class com/google/devtools/build/android/desugar/testdata/ConcreteFunction$Parser.class diff --git a/test/java/com/google/devtools/build/android/desugar/testdata_desugared_java8_jar_toc_golden.txt b/test/java/com/google/devtools/build/android/desugar/testdata_desugared_java8_jar_toc_golden.txt index 907edd0..67a46c3 100644 --- a/test/java/com/google/devtools/build/android/desugar/testdata_desugared_java8_jar_toc_golden.txt +++ b/test/java/com/google/devtools/build/android/desugar/testdata_desugared_java8_jar_toc_golden.txt @@ -12,6 +12,7 @@ com/google/devtools/build/android/desugar/testdata/CaptureLambda.class com/google/devtools/build/android/desugar/testdata/ClassCallingLongCompare$LongCmpFunc.class com/google/devtools/build/android/desugar/testdata/ClassCallingLongCompare.class com/google/devtools/build/android/desugar/testdata/ClassCallingRequireNonNull.class +com/google/devtools/build/android/desugar/testdata/ClassUsingTryWithResources$InheritanceResource.class com/google/devtools/build/android/desugar/testdata/ClassUsingTryWithResources$SimpleResource.class com/google/devtools/build/android/desugar/testdata/ClassUsingTryWithResources.class com/google/devtools/build/android/desugar/testdata/ConcreteFunction$Parser.class diff --git a/test/java/com/google/devtools/build/android/desugar/testdata_desugared_without_lambda_desugared_jar_toc_golden.txt b/test/java/com/google/devtools/build/android/desugar/testdata_desugared_without_lambda_desugared_jar_toc_golden.txt index 256760b..4fdc023 100644 --- a/test/java/com/google/devtools/build/android/desugar/testdata_desugared_without_lambda_desugared_jar_toc_golden.txt +++ b/test/java/com/google/devtools/build/android/desugar/testdata_desugared_without_lambda_desugared_jar_toc_golden.txt @@ -11,6 +11,7 @@ com/google/devtools/build/android/desugar/testdata/CaptureLambda.class com/google/devtools/build/android/desugar/testdata/ClassCallingLongCompare$LongCmpFunc.class com/google/devtools/build/android/desugar/testdata/ClassCallingLongCompare.class com/google/devtools/build/android/desugar/testdata/ClassCallingRequireNonNull.class +com/google/devtools/build/android/desugar/testdata/ClassUsingTryWithResources$InheritanceResource.class com/google/devtools/build/android/desugar/testdata/ClassUsingTryWithResources$SimpleResource.class com/google/devtools/build/android/desugar/testdata/ClassUsingTryWithResources.class com/google/devtools/build/android/desugar/testdata/ConcreteFunction$Parser.class -- cgit v1.2.3