From 8a1c90e7b34765d720b3711d4180526dde24c124 Mon Sep 17 00:00:00 2001 From: cpovirk Date: Thu, 17 Oct 2024 12:41:15 -0700 Subject: [PATCH] Prepare for usage of `ReflectionFreeAssertThrows` from more packages: - Make `AbstractPackageSanityTests` ignore the class for simplicity. (But also add null checks (which solve only _part_ of the problem) to `ReflectionFreeAssertThrows` because why not.) - Remove support for `util.concurrent` types so that we can also remove `util.concurrent` deps (mostly added during cl/675634517 and cl/686155720). I'll include support for those types in the packages that actually need it. RELNOTES=n/a PiperOrigin-RevId: 687009402 --- .../common/testing/AbstractPackageSanityTests.java | 9 +++++++++ .../google/common/base/ReflectionFreeAssertThrows.java | 8 ++++---- .../common/collect/ReflectionFreeAssertThrows.java | 8 ++++---- .../com/google/common/collect/ImmutableSortedMap.java | 6 +++++- .../common/testing/AbstractPackageSanityTests.java | 9 +++++++++ .../google/common/base/ReflectionFreeAssertThrows.java | 8 ++++---- .../common/collect/ReflectionFreeAssertThrows.java | 8 ++++---- .../com/google/common/collect/ImmutableSortedMap.java | 6 +++++- 8 files changed, 44 insertions(+), 18 deletions(-) diff --git a/android/guava-testlib/src/com/google/common/testing/AbstractPackageSanityTests.java b/android/guava-testlib/src/com/google/common/testing/AbstractPackageSanityTests.java index 127540ecf2bb..0f84e106e4e7 100644 --- a/android/guava-testlib/src/com/google/common/testing/AbstractPackageSanityTests.java +++ b/android/guava-testlib/src/com/google/common/testing/AbstractPackageSanityTests.java @@ -232,6 +232,15 @@ public void testSerializable() throws Exception { @Test public void testNulls() throws Exception { for (Class classToTest : findClassesToTest(loadClassesInPackage(), NULL_TEST_METHOD_NAMES)) { + if (classToTest.getSimpleName().equals("ReflectionFreeAssertThrows")) { + /* + * These classes handle null properly but throw IllegalArgumentException for the default + * Class argument that this test uses. Normally we'd fix that by declaring a + * ReflectionFreeAssertThrowsTest with a testNulls method, but that's annoying to have to do + * for a package-private utility class. So we skip the class entirely instead. + */ + continue; + } try { tester.doTestNulls(classToTest, visibility); } catch (Throwable e) { diff --git a/android/guava-tests/test/com/google/common/base/ReflectionFreeAssertThrows.java b/android/guava-tests/test/com/google/common/base/ReflectionFreeAssertThrows.java index 7996a8b26fdf..e8f98fdd542e 100644 --- a/android/guava-tests/test/com/google/common/base/ReflectionFreeAssertThrows.java +++ b/android/guava-tests/test/com/google/common/base/ReflectionFreeAssertThrows.java @@ -14,6 +14,8 @@ package com.google.common.base; +import static com.google.common.base.Preconditions.checkNotNull; + import com.google.common.annotations.GwtCompatible; import com.google.common.annotations.GwtIncompatible; import com.google.common.annotations.J2ktIncompatible; @@ -22,8 +24,6 @@ import com.google.common.base.TestExceptions.SomeOtherCheckedException; import com.google.common.base.TestExceptions.SomeUncheckedException; import com.google.common.collect.ImmutableMap; -import com.google.common.util.concurrent.ExecutionError; -import com.google.common.util.concurrent.UncheckedExecutionException; import com.google.errorprone.annotations.CanIgnoreReturnValue; import java.lang.reflect.InvocationTargetException; import java.nio.charset.UnsupportedCharsetException; @@ -67,6 +67,8 @@ static T assertThrows( private static T doAssertThrows( Class expectedThrowable, ThrowingSupplier supplier, boolean userPassedSupplier) { + checkNotNull(expectedThrowable); + checkNotNull(supplier); Predicate predicate = INSTANCE_OF.get(expectedThrowable); if (predicate == null) { throw new IllegalArgumentException( @@ -132,7 +134,6 @@ ImmutableMap, Predicate> exceptions() { .put( ConcurrentModificationException.class, e -> e instanceof ConcurrentModificationException) - .put(ExecutionError.class, e -> e instanceof ExecutionError) .put(ExecutionException.class, e -> e instanceof ExecutionException) .put(IllegalArgumentException.class, e -> e instanceof IllegalArgumentException) .put(IllegalStateException.class, e -> e instanceof IllegalStateException) @@ -146,7 +147,6 @@ ImmutableMap, Predicate> exceptions() { .put(SomeOtherCheckedException.class, e -> e instanceof SomeOtherCheckedException) .put(SomeUncheckedException.class, e -> e instanceof SomeUncheckedException) .put(TimeoutException.class, e -> e instanceof TimeoutException) - .put(UncheckedExecutionException.class, e -> e instanceof UncheckedExecutionException) .put(UnsupportedCharsetException.class, e -> e instanceof UnsupportedCharsetException) .put(UnsupportedOperationException.class, e -> e instanceof UnsupportedOperationException) .put(VerifyException.class, e -> e instanceof VerifyException) diff --git a/android/guava-tests/test/com/google/common/collect/ReflectionFreeAssertThrows.java b/android/guava-tests/test/com/google/common/collect/ReflectionFreeAssertThrows.java index e24b5b1027b9..72e31b0e0186 100644 --- a/android/guava-tests/test/com/google/common/collect/ReflectionFreeAssertThrows.java +++ b/android/guava-tests/test/com/google/common/collect/ReflectionFreeAssertThrows.java @@ -14,6 +14,8 @@ package com.google.common.collect; +import static com.google.common.base.Preconditions.checkNotNull; + import com.google.common.annotations.GwtCompatible; import com.google.common.annotations.GwtIncompatible; import com.google.common.annotations.J2ktIncompatible; @@ -24,8 +26,6 @@ import com.google.common.collect.TestExceptions.SomeError; import com.google.common.collect.TestExceptions.SomeOtherCheckedException; import com.google.common.collect.TestExceptions.SomeUncheckedException; -import com.google.common.util.concurrent.ExecutionError; -import com.google.common.util.concurrent.UncheckedExecutionException; import com.google.errorprone.annotations.CanIgnoreReturnValue; import java.lang.reflect.InvocationTargetException; import java.nio.charset.UnsupportedCharsetException; @@ -69,6 +69,8 @@ static T assertThrows( private static T doAssertThrows( Class expectedThrowable, ThrowingSupplier supplier, boolean userPassedSupplier) { + checkNotNull(expectedThrowable); + checkNotNull(supplier); Predicate predicate = INSTANCE_OF.get(expectedThrowable); if (predicate == null) { throw new IllegalArgumentException( @@ -134,7 +136,6 @@ ImmutableMap, Predicate> exceptions() { .put( ConcurrentModificationException.class, e -> e instanceof ConcurrentModificationException) - .put(ExecutionError.class, e -> e instanceof ExecutionError) .put(ExecutionException.class, e -> e instanceof ExecutionException) .put(IllegalArgumentException.class, e -> e instanceof IllegalArgumentException) .put(IllegalStateException.class, e -> e instanceof IllegalStateException) @@ -149,7 +150,6 @@ ImmutableMap, Predicate> exceptions() { .put(SomeOtherCheckedException.class, e -> e instanceof SomeOtherCheckedException) .put(SomeUncheckedException.class, e -> e instanceof SomeUncheckedException) .put(TimeoutException.class, e -> e instanceof TimeoutException) - .put(UncheckedExecutionException.class, e -> e instanceof UncheckedExecutionException) .put(UnsupportedCharsetException.class, e -> e instanceof UnsupportedCharsetException) .put(UnsupportedOperationException.class, e -> e instanceof UnsupportedOperationException) .put(VerifyException.class, e -> e instanceof VerifyException) diff --git a/android/guava/src/com/google/common/collect/ImmutableSortedMap.java b/android/guava/src/com/google/common/collect/ImmutableSortedMap.java index a5a85b42ab1e..5ebf140babfa 100644 --- a/android/guava/src/com/google/common/collect/ImmutableSortedMap.java +++ b/android/guava/src/com/google/common/collect/ImmutableSortedMap.java @@ -295,7 +295,11 @@ public static , V> ImmutableSortedMap of( V v8, K k9, V v9) { - return fromEntries( + /* + * This explicit type parameter works around what seems to be a javac bug in certain + * configurations: b/339186525#comment6 + */ + return ImmutableSortedMap.fromEntries( entryOf(k1, v1), entryOf(k2, v2), entryOf(k3, v3), diff --git a/guava-testlib/src/com/google/common/testing/AbstractPackageSanityTests.java b/guava-testlib/src/com/google/common/testing/AbstractPackageSanityTests.java index 858e581fa39b..249731a39a21 100644 --- a/guava-testlib/src/com/google/common/testing/AbstractPackageSanityTests.java +++ b/guava-testlib/src/com/google/common/testing/AbstractPackageSanityTests.java @@ -233,6 +233,15 @@ public void testSerializable() throws Exception { @Test public void testNulls() throws Exception { for (Class classToTest : findClassesToTest(loadClassesInPackage(), NULL_TEST_METHOD_NAMES)) { + if (classToTest.getSimpleName().equals("ReflectionFreeAssertThrows")) { + /* + * These classes handle null properly but throw IllegalArgumentException for the default + * Class argument that this test uses. Normally we'd fix that by declaring a + * ReflectionFreeAssertThrowsTest with a testNulls method, but that's annoying to have to do + * for a package-private utility class. So we skip the class entirely instead. + */ + continue; + } try { tester.doTestNulls(classToTest, visibility); } catch (Throwable e) { diff --git a/guava-tests/test/com/google/common/base/ReflectionFreeAssertThrows.java b/guava-tests/test/com/google/common/base/ReflectionFreeAssertThrows.java index 7996a8b26fdf..e8f98fdd542e 100644 --- a/guava-tests/test/com/google/common/base/ReflectionFreeAssertThrows.java +++ b/guava-tests/test/com/google/common/base/ReflectionFreeAssertThrows.java @@ -14,6 +14,8 @@ package com.google.common.base; +import static com.google.common.base.Preconditions.checkNotNull; + import com.google.common.annotations.GwtCompatible; import com.google.common.annotations.GwtIncompatible; import com.google.common.annotations.J2ktIncompatible; @@ -22,8 +24,6 @@ import com.google.common.base.TestExceptions.SomeOtherCheckedException; import com.google.common.base.TestExceptions.SomeUncheckedException; import com.google.common.collect.ImmutableMap; -import com.google.common.util.concurrent.ExecutionError; -import com.google.common.util.concurrent.UncheckedExecutionException; import com.google.errorprone.annotations.CanIgnoreReturnValue; import java.lang.reflect.InvocationTargetException; import java.nio.charset.UnsupportedCharsetException; @@ -67,6 +67,8 @@ static T assertThrows( private static T doAssertThrows( Class expectedThrowable, ThrowingSupplier supplier, boolean userPassedSupplier) { + checkNotNull(expectedThrowable); + checkNotNull(supplier); Predicate predicate = INSTANCE_OF.get(expectedThrowable); if (predicate == null) { throw new IllegalArgumentException( @@ -132,7 +134,6 @@ ImmutableMap, Predicate> exceptions() { .put( ConcurrentModificationException.class, e -> e instanceof ConcurrentModificationException) - .put(ExecutionError.class, e -> e instanceof ExecutionError) .put(ExecutionException.class, e -> e instanceof ExecutionException) .put(IllegalArgumentException.class, e -> e instanceof IllegalArgumentException) .put(IllegalStateException.class, e -> e instanceof IllegalStateException) @@ -146,7 +147,6 @@ ImmutableMap, Predicate> exceptions() { .put(SomeOtherCheckedException.class, e -> e instanceof SomeOtherCheckedException) .put(SomeUncheckedException.class, e -> e instanceof SomeUncheckedException) .put(TimeoutException.class, e -> e instanceof TimeoutException) - .put(UncheckedExecutionException.class, e -> e instanceof UncheckedExecutionException) .put(UnsupportedCharsetException.class, e -> e instanceof UnsupportedCharsetException) .put(UnsupportedOperationException.class, e -> e instanceof UnsupportedOperationException) .put(VerifyException.class, e -> e instanceof VerifyException) diff --git a/guava-tests/test/com/google/common/collect/ReflectionFreeAssertThrows.java b/guava-tests/test/com/google/common/collect/ReflectionFreeAssertThrows.java index e24b5b1027b9..72e31b0e0186 100644 --- a/guava-tests/test/com/google/common/collect/ReflectionFreeAssertThrows.java +++ b/guava-tests/test/com/google/common/collect/ReflectionFreeAssertThrows.java @@ -14,6 +14,8 @@ package com.google.common.collect; +import static com.google.common.base.Preconditions.checkNotNull; + import com.google.common.annotations.GwtCompatible; import com.google.common.annotations.GwtIncompatible; import com.google.common.annotations.J2ktIncompatible; @@ -24,8 +26,6 @@ import com.google.common.collect.TestExceptions.SomeError; import com.google.common.collect.TestExceptions.SomeOtherCheckedException; import com.google.common.collect.TestExceptions.SomeUncheckedException; -import com.google.common.util.concurrent.ExecutionError; -import com.google.common.util.concurrent.UncheckedExecutionException; import com.google.errorprone.annotations.CanIgnoreReturnValue; import java.lang.reflect.InvocationTargetException; import java.nio.charset.UnsupportedCharsetException; @@ -69,6 +69,8 @@ static T assertThrows( private static T doAssertThrows( Class expectedThrowable, ThrowingSupplier supplier, boolean userPassedSupplier) { + checkNotNull(expectedThrowable); + checkNotNull(supplier); Predicate predicate = INSTANCE_OF.get(expectedThrowable); if (predicate == null) { throw new IllegalArgumentException( @@ -134,7 +136,6 @@ ImmutableMap, Predicate> exceptions() { .put( ConcurrentModificationException.class, e -> e instanceof ConcurrentModificationException) - .put(ExecutionError.class, e -> e instanceof ExecutionError) .put(ExecutionException.class, e -> e instanceof ExecutionException) .put(IllegalArgumentException.class, e -> e instanceof IllegalArgumentException) .put(IllegalStateException.class, e -> e instanceof IllegalStateException) @@ -149,7 +150,6 @@ ImmutableMap, Predicate> exceptions() { .put(SomeOtherCheckedException.class, e -> e instanceof SomeOtherCheckedException) .put(SomeUncheckedException.class, e -> e instanceof SomeUncheckedException) .put(TimeoutException.class, e -> e instanceof TimeoutException) - .put(UncheckedExecutionException.class, e -> e instanceof UncheckedExecutionException) .put(UnsupportedCharsetException.class, e -> e instanceof UnsupportedCharsetException) .put(UnsupportedOperationException.class, e -> e instanceof UnsupportedOperationException) .put(VerifyException.class, e -> e instanceof VerifyException) diff --git a/guava/src/com/google/common/collect/ImmutableSortedMap.java b/guava/src/com/google/common/collect/ImmutableSortedMap.java index 495cb23dd5f1..ae8660080366 100644 --- a/guava/src/com/google/common/collect/ImmutableSortedMap.java +++ b/guava/src/com/google/common/collect/ImmutableSortedMap.java @@ -294,7 +294,11 @@ public static , V> ImmutableSortedMap of( V v8, K k9, V v9) { - return fromEntries( + /* + * This explicit type parameter works around what seems to be a javac bug in certain + * configurations: b/339186525#comment6 + */ + return ImmutableSortedMap.fromEntries( entryOf(k1, v1), entryOf(k2, v2), entryOf(k3, v3),