Make the guava-android copy of NullPointerTester read type-use annotations when they're available.
#6390
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
![copybara-service[bot]](https://avatars.githubusercontent.com/in/44061?s=60&v=4){kind=small}
copybara-service
bot
force-pushed
the
test_521795608
branch
from
f793f87
to
b31c020
Compare
…tations when they're available. They're never available under an Android VM, but we also test guava-android under a JRE (which is always Java 8+ now, which makes our implementation job a bit easier), so the annotations _are_ available there. This is a step toward letting us [use type-use annotations more widely in Guava](jspecify/jspecify#239). In particular, it unblocks cl/521011962 (which in turn reduces the number of merge conflicts we'll see when using type-use annotations in our tests), and it helps our other testing of type-use annotations in Guava. In order to implement this, I copied two methods that were previously only available in guava-jre to make them also available in guava-android for `NullPointerTester` there: `Invokable.getAnnotatedReturnType()` and `Parameter.getAnnotatedType()`. Those methods do not work under and Android VM (where, again, type-use annotations aren't available), but they're useful for running our tests under a JRE. FYI: We did see one project that needed some new `-dontwarn` lines in [our Proguard config](#2117) as a result of this CL. But we expect this to be unlikely, especially since reflection is typically a bad idea under Android. (An alternative approach would be for us to expose two _new_ methods that are similar to `getAnnotatedReturnType()` and `getAnnotatedType()` but that declare their return type as plain `Object`. That would be safer, but I think there's actually more value in testing the more aggressive approach: We'd like to someday be more aggressive about adding APIs that use Java 8+ types in guava-android, so it's nice to test them on low-stakes `@Beta` APIs that shouldn't be used from Android to begin with.) On the plus side, we no longer need to perform reflection on account of j2objc: As planned in cl/367051816, j2objc was subsequently updated in cl/367072772 to contain stub implementations of the necessary `TypeVariable` methods. RELNOTES=n/a PiperOrigin-RevId: 521795608
copybara-service
bot
force-pushed
the
test_521795608
branch
2 times, most recently
from
99143ec
to
b30e73c
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Make the guava-android copy of
NullPointerTesterread type-use annotations when they're available.They're never available under an Android VM, but we also test guava-android under a JRE (which is always Java 8+ now, which makes our implementation job a bit easier), so the annotations are available there.
This is a step toward letting us use type-use annotations more widely in Guava. In particular, it unblocks cl/521011962 (which in turn reduces the number of merge conflicts we'll see when using type-use annotations in our tests), and it helps our other testing of type-use annotations in Guava.
In order to implement this, I copied two methods that were previously only available in guava-jre to make them also available in guava-android for
NullPointerTesterthere:Invokable.getAnnotatedReturnType()andParameter.getAnnotatedType(). Those methods do not work under and Android VM (where, again, type-use annotations aren't available), but they're useful for running our tests under a JRE.FYI: We did see one project that needed some new
-dontwarnlines in our Proguard config as a result of this CL. But we expect this to be unlikely, especially since reflection is typically a bad idea under Android. (An alternative approach would be for us to expose two new methods that are similar togetAnnotatedReturnType()andgetAnnotatedType()but that declare their return type as plainObject. That would be safer, but I think there's actually more value in testing the more aggressive approach: We'd like to someday be more aggressive about adding APIs that use Java 8+ types in guava-android, so it's nice to test them on low-stakes@BetaAPIs that shouldn't be used from Android to begin with.)On the plus side, we no longer need to perform reflection on account of j2objc: As planned in cl/367051816, j2objc was subsequently updated in cl/367072772 to contain stub implementations of the necessary
TypeVariablemethods.RELNOTES=n/a