aboutsummaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorcpovirk <cpovirk@google.com>2021-08-19 09:41:17 -0700
committerGoogle Java Core Libraries <java-core-libraries-team+copybara@google.com>2021-08-19 09:44:03 -0700
commitfb109b051c9ec48d5e77bf73b33a228ab5d57462 (patch)
tree5bf73cd03fa4f922b68dd832057621807c5bffca
parentc7d9fef73b9314581bf4acd0d97b12178935eb11 (diff)
downloadguava-fb109b051c9ec48d5e77bf73b33a228ab5d57462.tar.gz
Remove `ClassValue` implementation of `Futures.getChecked` from the Android flavor.
It doesn't work there, even under new versions of Android, so it always triggers fallback, and the process of falling back burns resources and produces noise. (JRE users of guava-android could benefit from the `ClassValue` implementation, but now that we're dropping support for Java 7 from guava-android, there's no reason for them to use it. OK, probably some users run _Robolectric_ tests against guava-android. But presumably they aren't _too_ sensitive to the performance of `getChecked` in the failure case.) Removing `ClassValue` support also lets us remove a bunch of Animal-Sniffer cruft. That includes upgrading it to 1.19, which in turn forces us to remove some usages of new APIs from our _tests_ -- not nearly as important as usages in _prod_ but still a good idea so that any future Android test failures are reported to us correctly instead of hidden by `NoSuchMethodError` or similar. Or actually, Animal-Sniffer 1.20 [turned checking of test sources back off](https://github.com/mojohaus/animal-sniffer/commit/ac40ac46b3e02f0edf986bb43be7a45cc5382937), but let's opt in anyway, at least to see how it goes, at least for the tests of guava-testlib. (guava-tests itself is more of a pain, so I left a TODO.) RELNOTES=n/a PiperOrigin-RevId: 391779058
-rw-r--r--android/guava-testlib/test/com/google/common/collect/testing/MapTestSuiteBuilderTests.java10
-rw-r--r--android/guava-tests/benchmark/com/google/common/util/concurrent/FuturesGetCheckedBenchmark.java3
-rw-r--r--android/guava-tests/pom.xml3
-rw-r--r--android/guava-tests/test/com/google/common/util/concurrent/FuturesGetCheckedTest.java3
-rw-r--r--android/guava/src/com/google/common/util/concurrent/FuturesGetChecked.java42
-rw-r--r--android/guava/src/com/google/common/util/concurrent/IgnoreJRERequirement.java25
-rw-r--r--android/pom.xml20
-rw-r--r--guava-testlib/test/com/google/common/collect/testing/MapTestSuiteBuilderTests.java10
-rw-r--r--guava-tests/benchmark/com/google/common/util/concurrent/FuturesGetCheckedBenchmark.java3
-rw-r--r--guava-tests/pom.xml3
-rw-r--r--guava-tests/test/com/google/common/util/concurrent/FuturesGetCheckedTest.java3
-rw-r--r--guava/src/com/google/common/util/concurrent/FuturesGetChecked.java1
-rw-r--r--guava/src/com/google/common/util/concurrent/IgnoreJRERequirement.java25
-rw-r--r--pom.xml20
14 files changed, 36 insertions, 135 deletions
diff --git a/android/guava-testlib/test/com/google/common/collect/testing/MapTestSuiteBuilderTests.java b/android/guava-testlib/test/com/google/common/collect/testing/MapTestSuiteBuilderTests.java
index c23d0524f..a4d216d15 100644
--- a/android/guava-testlib/test/com/google/common/collect/testing/MapTestSuiteBuilderTests.java
+++ b/android/guava-testlib/test/com/google/common/collect/testing/MapTestSuiteBuilderTests.java
@@ -302,8 +302,8 @@ public final class MapTestSuiteBuilderTests extends TestCase {
return method.invoke(map, args);
} catch (InvocationTargetException e) {
throw e.getCause();
- } catch (ReflectiveOperationException e) {
- throw new LinkageError(e.getMessage(), e);
+ } catch (IllegalAccessException e) {
+ throw newLinkageError(e);
}
}
}
@@ -338,4 +338,10 @@ public final class MapTestSuiteBuilderTests extends TestCase {
.withTearDown(tearDown)
.createTestSuite();
}
+
+ private static LinkageError newLinkageError(Throwable cause) {
+ LinkageError error = new LinkageError(cause.toString());
+ error.initCause(cause);
+ return error;
+ }
}
diff --git a/android/guava-tests/benchmark/com/google/common/util/concurrent/FuturesGetCheckedBenchmark.java b/android/guava-tests/benchmark/com/google/common/util/concurrent/FuturesGetCheckedBenchmark.java
index dd1883bcb..bb764203f 100644
--- a/android/guava-tests/benchmark/com/google/common/util/concurrent/FuturesGetCheckedBenchmark.java
+++ b/android/guava-tests/benchmark/com/google/common/util/concurrent/FuturesGetCheckedBenchmark.java
@@ -21,7 +21,6 @@ import static com.google.common.collect.Lists.newArrayList;
import static com.google.common.util.concurrent.Futures.immediateFailedFuture;
import static com.google.common.util.concurrent.Futures.immediateFuture;
import static com.google.common.util.concurrent.FuturesGetChecked.checkExceptionClassValidity;
-import static com.google.common.util.concurrent.FuturesGetChecked.classValueValidator;
import static com.google.common.util.concurrent.FuturesGetChecked.getChecked;
import static com.google.common.util.concurrent.FuturesGetChecked.isCheckedException;
import static com.google.common.util.concurrent.FuturesGetChecked.weakSetValidator;
@@ -52,7 +51,7 @@ public class FuturesGetCheckedBenchmark {
NON_CACHING_WITH_CONSTRUCTOR_CHECK(nonCachingWithConstructorCheckValidator()),
NON_CACHING_WITHOUT_CONSTRUCTOR_CHECK(nonCachingWithoutConstructorCheckValidator()),
WEAK_SET(weakSetValidator()),
- CLASS_VALUE(classValueValidator());
+ ;
final GetCheckedTypeValidator validator;
diff --git a/android/guava-tests/pom.xml b/android/guava-tests/pom.xml
index 5f56f4b3a..b5923d73c 100644
--- a/android/guava-tests/pom.xml
+++ b/android/guava-tests/pom.xml
@@ -102,6 +102,9 @@
<plugin>
<groupId>org.codehaus.mojo</groupId>
<artifactId>animal-sniffer-maven-plugin</artifactId>
+ <configuration>
+ <checkTestClasses>false</checkTestClasses> <!-- TODO(cpovirk): Consider checking them. -->
+ </configuration>
</plugin>
<plugin>
<groupId>org.codehaus.mojo</groupId>
diff --git a/android/guava-tests/test/com/google/common/util/concurrent/FuturesGetCheckedTest.java b/android/guava-tests/test/com/google/common/util/concurrent/FuturesGetCheckedTest.java
index 72b5a4656..3bc69bd32 100644
--- a/android/guava-tests/test/com/google/common/util/concurrent/FuturesGetCheckedTest.java
+++ b/android/guava-tests/test/com/google/common/util/concurrent/FuturesGetCheckedTest.java
@@ -381,5 +381,8 @@ public class FuturesGetCheckedTest extends TestCase {
* environment that forces Futures.getChecked to its fallback WeakSetValidator. One awful way of
* doing so would be to derive a separate test library by using remove_from_jar to strip out
* ClassValueValidator.
+ *
+ * Fortunately, we get pretty good coverage "by accident": We run all these tests against the
+ * *backport*, where ClassValueValidator is not present.
*/
}
diff --git a/android/guava/src/com/google/common/util/concurrent/FuturesGetChecked.java b/android/guava/src/com/google/common/util/concurrent/FuturesGetChecked.java
index d40a1cc72..04564f3a0 100644
--- a/android/guava/src/com/google/common/util/concurrent/FuturesGetChecked.java
+++ b/android/guava/src/com/google/common/util/concurrent/FuturesGetChecked.java
@@ -23,7 +23,6 @@ import com.google.common.annotations.VisibleForTesting;
import com.google.common.base.Function;
import com.google.common.collect.Ordering;
import com.google.errorprone.annotations.CanIgnoreReturnValue;
-import com.google.j2objc.annotations.J2ObjCIncompatible;
import java.lang.ref.WeakReference;
import java.lang.reflect.Constructor;
import java.lang.reflect.InvocationTargetException;
@@ -101,12 +100,6 @@ final class FuturesGetChecked {
return GetCheckedTypeValidatorHolder.WeakSetValidator.INSTANCE;
}
- @J2ObjCIncompatible // ClassValue
- @VisibleForTesting
- static GetCheckedTypeValidator classValueValidator() {
- return GetCheckedTypeValidatorHolder.ClassValueValidator.INSTANCE;
- }
-
/**
* Provides a check of whether an exception type is valid for use with {@link
* FuturesGetChecked#getChecked(Future, Class)}, possibly using caching.
@@ -115,35 +108,8 @@ final class FuturesGetChecked {
*/
@VisibleForTesting
static class GetCheckedTypeValidatorHolder {
- static final String CLASS_VALUE_VALIDATOR_NAME =
- GetCheckedTypeValidatorHolder.class.getName() + "$ClassValueValidator";
-
static final GetCheckedTypeValidator BEST_VALIDATOR = getBestValidator();
- @IgnoreJRERequirement // getChecked falls back to another implementation if necessary
- @J2ObjCIncompatible // ClassValue
- enum ClassValueValidator implements GetCheckedTypeValidator {
- INSTANCE;
-
- /*
- * Static final fields are presumed to be fastest, based on our experience with
- * UnsignedBytesBenchmark. TODO(cpovirk): benchmark this
- */
- private static final ClassValue<Boolean> isValidClass =
- new ClassValue<Boolean>() {
- @Override
- protected Boolean computeValue(Class<?> type) {
- checkExceptionClassValidity(type.asSubclass(Exception.class));
- return true;
- }
- };
-
- @Override
- public void validateClass(Class<? extends Exception> exceptionClass) {
- isValidClass.get(exceptionClass); // throws if invalid; returns safely (and caches) if valid
- }
- }
-
enum WeakSetValidator implements GetCheckedTypeValidator {
INSTANCE;
@@ -190,13 +156,7 @@ final class FuturesGetChecked {
* unable to do so.
*/
static GetCheckedTypeValidator getBestValidator() {
- try {
- Class<? extends Enum> theClass =
- Class.forName(CLASS_VALUE_VALIDATOR_NAME).asSubclass(Enum.class);
- return (GetCheckedTypeValidator) theClass.getEnumConstants()[0];
- } catch (Throwable t) { // ensure we really catch *everything*
- return weakSetValidator();
- }
+ return weakSetValidator();
}
}
diff --git a/android/guava/src/com/google/common/util/concurrent/IgnoreJRERequirement.java b/android/guava/src/com/google/common/util/concurrent/IgnoreJRERequirement.java
deleted file mode 100644
index 49531b421..000000000
--- a/android/guava/src/com/google/common/util/concurrent/IgnoreJRERequirement.java
+++ /dev/null
@@ -1,25 +0,0 @@
-/*
- * Copyright 2019 The Guava Authors
- *
- * 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.common.util.concurrent;
-
-import static java.lang.annotation.ElementType.CONSTRUCTOR;
-import static java.lang.annotation.ElementType.METHOD;
-import static java.lang.annotation.ElementType.TYPE;
-
-import java.lang.annotation.Target;
-
-@Target({METHOD, CONSTRUCTOR, TYPE})
-@ElementTypesAreNonnullByDefault
-@interface IgnoreJRERequirement {}
diff --git a/android/pom.xml b/android/pom.xml
index b6e712e4c..2bf293988 100644
--- a/android/pom.xml
+++ b/android/pom.xml
@@ -16,23 +16,7 @@
<test.include>%regex[.*.class]</test.include>
<truth.version>1.1.2</truth.version>
<checker-framework.version>3.12.0</checker-framework.version>
- <!--
- Upgrading to 1.19 breaks things: Animal Sniffer reports a problem with the
- use of ClassValue in FuturesGetChecked, even though we've added a
- suppression annotation.
-
- The problem might be
- https://github.com/mojohaus/animal-sniffer/issues/131.
-
- We could probably work around this with a different suppression strategy,
- perhaps (untested):
-
- <ignores><ignore>com.google.common.util.concurrent.FuturesGetChecked$GetCheckedTypeValidatorHolder$ClassValueValidator</ignore></ignores>
-
- But upgrading doesn't really buy us much, as far as I know. (1.19 does
- check test code in addition to prod code, but I don't think we care.)
- -->
- <animal.sniffer.version>1.18</animal.sniffer.version>
+ <animal.sniffer.version>1.20</animal.sniffer.version>
<maven-javadoc-plugin.version>3.1.0</maven-javadoc-plugin.version>
<maven-source-plugin.version>3.2.1</maven-source-plugin.version>
<project.build.sourceEncoding>UTF-8</project.build.sourceEncoding>
@@ -167,7 +151,7 @@
<artifactId>animal-sniffer-maven-plugin</artifactId>
<version>${animal.sniffer.version}</version>
<configuration>
- <annotations>com.google.common.util.concurrent.IgnoreJRERequirement</annotations>
+ <checkTestClasses>true</checkTestClasses>
<signature>
<groupId>org.codehaus.mojo.signature</groupId>
<artifactId>java16-sun</artifactId>
diff --git a/guava-testlib/test/com/google/common/collect/testing/MapTestSuiteBuilderTests.java b/guava-testlib/test/com/google/common/collect/testing/MapTestSuiteBuilderTests.java
index bcacec4ac..5fd0d0368 100644
--- a/guava-testlib/test/com/google/common/collect/testing/MapTestSuiteBuilderTests.java
+++ b/guava-testlib/test/com/google/common/collect/testing/MapTestSuiteBuilderTests.java
@@ -313,8 +313,8 @@ public final class MapTestSuiteBuilderTests extends TestCase {
return method.invoke(map, args);
} catch (InvocationTargetException e) {
throw e.getCause();
- } catch (ReflectiveOperationException e) {
- throw new LinkageError(e.getMessage(), e);
+ } catch (IllegalAccessException e) {
+ throw newLinkageError(e);
}
}
}
@@ -349,4 +349,10 @@ public final class MapTestSuiteBuilderTests extends TestCase {
.withTearDown(tearDown)
.createTestSuite();
}
+
+ private static LinkageError newLinkageError(Throwable cause) {
+ LinkageError error = new LinkageError(cause.toString());
+ error.initCause(cause);
+ return error;
+ }
}
diff --git a/guava-tests/benchmark/com/google/common/util/concurrent/FuturesGetCheckedBenchmark.java b/guava-tests/benchmark/com/google/common/util/concurrent/FuturesGetCheckedBenchmark.java
index dd1883bcb..c25a8ff31 100644
--- a/guava-tests/benchmark/com/google/common/util/concurrent/FuturesGetCheckedBenchmark.java
+++ b/guava-tests/benchmark/com/google/common/util/concurrent/FuturesGetCheckedBenchmark.java
@@ -52,7 +52,8 @@ public class FuturesGetCheckedBenchmark {
NON_CACHING_WITH_CONSTRUCTOR_CHECK(nonCachingWithConstructorCheckValidator()),
NON_CACHING_WITHOUT_CONSTRUCTOR_CHECK(nonCachingWithoutConstructorCheckValidator()),
WEAK_SET(weakSetValidator()),
- CLASS_VALUE(classValueValidator());
+ CLASS_VALUE(classValueValidator()),
+ ;
final GetCheckedTypeValidator validator;
diff --git a/guava-tests/pom.xml b/guava-tests/pom.xml
index add78b401..a03dff23d 100644
--- a/guava-tests/pom.xml
+++ b/guava-tests/pom.xml
@@ -106,6 +106,9 @@
<plugin>
<groupId>org.codehaus.mojo</groupId>
<artifactId>animal-sniffer-maven-plugin</artifactId>
+ <configuration>
+ <checkTestClasses>false</checkTestClasses> <!-- TODO(cpovirk): Consider checking them. -->
+ </configuration>
</plugin>
<plugin>
<groupId>org.codehaus.mojo</groupId>
diff --git a/guava-tests/test/com/google/common/util/concurrent/FuturesGetCheckedTest.java b/guava-tests/test/com/google/common/util/concurrent/FuturesGetCheckedTest.java
index 72b5a4656..3bc69bd32 100644
--- a/guava-tests/test/com/google/common/util/concurrent/FuturesGetCheckedTest.java
+++ b/guava-tests/test/com/google/common/util/concurrent/FuturesGetCheckedTest.java
@@ -381,5 +381,8 @@ public class FuturesGetCheckedTest extends TestCase {
* environment that forces Futures.getChecked to its fallback WeakSetValidator. One awful way of
* doing so would be to derive a separate test library by using remove_from_jar to strip out
* ClassValueValidator.
+ *
+ * Fortunately, we get pretty good coverage "by accident": We run all these tests against the
+ * *backport*, where ClassValueValidator is not present.
*/
}
diff --git a/guava/src/com/google/common/util/concurrent/FuturesGetChecked.java b/guava/src/com/google/common/util/concurrent/FuturesGetChecked.java
index d40a1cc72..6f09b8066 100644
--- a/guava/src/com/google/common/util/concurrent/FuturesGetChecked.java
+++ b/guava/src/com/google/common/util/concurrent/FuturesGetChecked.java
@@ -120,7 +120,6 @@ final class FuturesGetChecked {
static final GetCheckedTypeValidator BEST_VALIDATOR = getBestValidator();
- @IgnoreJRERequirement // getChecked falls back to another implementation if necessary
@J2ObjCIncompatible // ClassValue
enum ClassValueValidator implements GetCheckedTypeValidator {
INSTANCE;
diff --git a/guava/src/com/google/common/util/concurrent/IgnoreJRERequirement.java b/guava/src/com/google/common/util/concurrent/IgnoreJRERequirement.java
deleted file mode 100644
index 49531b421..000000000
--- a/guava/src/com/google/common/util/concurrent/IgnoreJRERequirement.java
+++ /dev/null
@@ -1,25 +0,0 @@
-/*
- * Copyright 2019 The Guava Authors
- *
- * 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.common.util.concurrent;
-
-import static java.lang.annotation.ElementType.CONSTRUCTOR;
-import static java.lang.annotation.ElementType.METHOD;
-import static java.lang.annotation.ElementType.TYPE;
-
-import java.lang.annotation.Target;
-
-@Target({METHOD, CONSTRUCTOR, TYPE})
-@ElementTypesAreNonnullByDefault
-@interface IgnoreJRERequirement {}
diff --git a/pom.xml b/pom.xml
index 7bcaebbd8..87dae7e9b 100644
--- a/pom.xml
+++ b/pom.xml
@@ -16,23 +16,7 @@
<test.include>%regex[.*.class]</test.include>
<truth.version>1.1.2</truth.version>
<checker-framework.version>3.12.0</checker-framework.version>
- <!--
- Upgrading to 1.19 breaks things: Animal Sniffer reports a problem with the
- use of ClassValue in FuturesGetChecked, even though we've added a
- suppression annotation.
-
- The problem might be
- https://github.com/mojohaus/animal-sniffer/issues/131.
-
- We could probably work around this with a different suppression strategy,
- perhaps (untested):
-
- <ignores><ignore>com.google.common.util.concurrent.FuturesGetChecked$GetCheckedTypeValidatorHolder$ClassValueValidator</ignore></ignores>
-
- But upgrading doesn't really buy us much, as far as I know. (1.19 does
- check test code in addition to prod code, but I don't think we care.)
- -->
- <animal.sniffer.version>1.18</animal.sniffer.version>
+ <animal.sniffer.version>1.20</animal.sniffer.version>
<maven-javadoc-plugin.version>3.1.0</maven-javadoc-plugin.version>
<maven-source-plugin.version>3.2.1</maven-source-plugin.version>
<project.build.sourceEncoding>UTF-8</project.build.sourceEncoding>
@@ -168,7 +152,7 @@
<artifactId>animal-sniffer-maven-plugin</artifactId>
<version>${animal.sniffer.version}</version>
<configuration>
- <annotations>com.google.common.util.concurrent.IgnoreJRERequirement</annotations>
+ <checkTestClasses>true</checkTestClasses>
<signature>
<groupId>org.codehaus.mojo.signature</groupId>
<artifactId>java18</artifactId>