Skip to content

Commit b30e73c

Browse files
cpovirkGoogle Java Core Libraries
authored andcommitted
Make the guava-android copy of NullPointerTester read 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](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: 522214387
1 parent 33d9ff7 commit b30e73c

File tree

14 files changed

+481
-67
lines changed

14 files changed

+481
-67
lines changed
Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,29 @@
1+
/*
2+
* Copyright 2019 The Guava Authors
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except
5+
* in compliance with the License. You may obtain a copy of the License at
6+
*
7+
* http://www.apache.org/licenses/LICENSE-2.0
8+
*
9+
* Unless required by applicable law or agreed to in writing, software distributed under the License
10+
* is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express
11+
* or implied. See the License for the specific language governing permissions and limitations under
12+
* the License.
13+
*/
14+
15+
package com.google.common.testing;
16+
17+
import static java.lang.annotation.ElementType.CONSTRUCTOR;
18+
import static java.lang.annotation.ElementType.METHOD;
19+
import static java.lang.annotation.ElementType.TYPE;
20+
21+
import java.lang.annotation.Target;
22+
23+
/**
24+
* Disables Animal Sniffer's checking of compatibility with older versions of Java/Android.
25+
*
26+
* <p>Each package's copy of this annotation needs to be listed in our {@code pom.xml}.
27+
*/
28+
@Target({METHOD, CONSTRUCTOR, TYPE})
29+
@interface IgnoreJRERequirement {}

android/guava-testlib/src/com/google/common/testing/NullPointerTester.java

Lines changed: 92 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -34,14 +34,15 @@
3434
import com.google.common.reflect.TypeToken;
3535
import com.google.errorprone.annotations.CanIgnoreReturnValue;
3636
import java.lang.annotation.Annotation;
37-
import java.lang.reflect.AnnotatedElement;
37+
import java.lang.reflect.AnnotatedType;
3838
import java.lang.reflect.Constructor;
3939
import java.lang.reflect.InvocationTargetException;
4040
import java.lang.reflect.Member;
4141
import java.lang.reflect.Method;
4242
import java.lang.reflect.Modifier;
4343
import java.lang.reflect.ParameterizedType;
4444
import java.lang.reflect.Type;
45+
import java.lang.reflect.TypeVariable;
4546
import java.util.Arrays;
4647
import java.util.List;
4748
import java.util.concurrent.ConcurrentMap;
@@ -52,8 +53,8 @@
5253
/**
5354
* A test utility that verifies that your methods and constructors throw {@link
5455
* NullPointerException} or {@link UnsupportedOperationException} whenever null is passed to a
55-
* parameter that isn't annotated with an annotation with the simple name {@code Nullable}, {@code
56-
* CheckForNull}, {@code NullableType}, or {@code NullableDecl}.
56+
* parameter whose declaration or type isn't annotated with an annotation with the simple name
57+
* {@code Nullable}, {@code CheckForNull}, {@code NullableType}, or {@code NullableDecl}.
5758
*
5859
* <p>The tested methods and constructors are invoked -- each time with one parameter being null and
5960
* the rest not null -- and the test fails if no expected exception is thrown. {@code
@@ -495,8 +496,16 @@ static boolean isPrimitiveOrNullable(Parameter param) {
495496
ImmutableSet.of(
496497
"CheckForNull", "Nullable", "NullableDecl", "NullableType", "ParametricNullness");
497498

498-
static boolean isNullable(AnnotatedElement e) {
499-
for (Annotation annotation : e.getAnnotations()) {
499+
static boolean isNullable(Invokable<?, ?> invokable) {
500+
return NULLNESS_ANNOTATION_READER.isNullable(invokable);
501+
}
502+
503+
static boolean isNullable(Parameter param) {
504+
return NULLNESS_ANNOTATION_READER.isNullable(param);
505+
}
506+
507+
private static boolean containsNullable(Annotation[] annotations) {
508+
for (Annotation annotation : annotations) {
500509
if (NULLABLE_ANNOTATION_SIMPLE_NAMES.contains(annotation.annotationType().getSimpleName())) {
501510
return true;
502511
}
@@ -567,4 +576,82 @@ public boolean isExpectedType(Throwable cause) {
567576

568577
public abstract boolean isExpectedType(Throwable cause);
569578
}
579+
580+
private static boolean annotatedTypeExists() {
581+
try {
582+
Class.forName("java.lang.reflect.AnnotatedType");
583+
} catch (ClassNotFoundException e) {
584+
return false;
585+
}
586+
return true;
587+
}
588+
589+
private static final NullnessAnnotationReader NULLNESS_ANNOTATION_READER =
590+
annotatedTypeExists()
591+
? NullnessAnnotationReader.FROM_DECLARATION_AND_TYPE_USE_ANNOTATIONS
592+
: NullnessAnnotationReader.FROM_DECLARATION_ANNOTATIONS_ONLY;
593+
594+
/**
595+
* Looks for declaration nullness annotations and, if supported, type-use nullness annotations.
596+
*
597+
* <p>Under Android VMs, the methods for retrieving type-use annotations don't exist. This means
598+
* that {@link NullPointerException} may misbehave under Android when used on classes that rely on
599+
* type-use annotations.
600+
*
601+
* <p>Under j2objc, the necessary APIs exist, but some (perhaps all) return stub values, like
602+
* empty arrays. Presumably {@link NullPointerException} could likewise misbehave under j2objc,
603+
* but I don't know that anyone uses it there, anyway.
604+
*/
605+
private enum NullnessAnnotationReader {
606+
// Usages (which are unsafe only for Android) are guarded by the annotatedTypeExists() check.
607+
@SuppressWarnings({"Java7ApiChecker", "AndroidApiChecker", "DoNotCall", "deprecation"})
608+
FROM_DECLARATION_AND_TYPE_USE_ANNOTATIONS {
609+
@Override
610+
@IgnoreJRERequirement
611+
boolean isNullable(Invokable<?, ?> invokable) {
612+
return FROM_DECLARATION_ANNOTATIONS_ONLY.isNullable(invokable)
613+
|| containsNullable(invokable.getAnnotatedReturnType().getAnnotations());
614+
// TODO(cpovirk): Should we also check isNullableTypeVariable?
615+
}
616+
617+
@Override
618+
@IgnoreJRERequirement
619+
boolean isNullable(Parameter param) {
620+
return FROM_DECLARATION_ANNOTATIONS_ONLY.isNullable(param)
621+
|| containsNullable(param.getAnnotatedType().getAnnotations())
622+
|| isNullableTypeVariable(param.getAnnotatedType().getType());
623+
}
624+
625+
@IgnoreJRERequirement
626+
boolean isNullableTypeVariable(Type type) {
627+
if (!(type instanceof TypeVariable)) {
628+
return false;
629+
}
630+
TypeVariable<?> typeVar = (TypeVariable<?>) type;
631+
for (AnnotatedType bound : typeVar.getAnnotatedBounds()) {
632+
// Until Java 15, the isNullableTypeVariable case here won't help:
633+
// https://bugs.openjdk.java.net/browse/JDK-8202469
634+
if (containsNullable(bound.getAnnotations()) || isNullableTypeVariable(bound.getType())) {
635+
return true;
636+
}
637+
}
638+
return false;
639+
}
640+
},
641+
FROM_DECLARATION_ANNOTATIONS_ONLY {
642+
@Override
643+
boolean isNullable(Invokable<?, ?> invokable) {
644+
return containsNullable(invokable.getAnnotations());
645+
}
646+
647+
@Override
648+
boolean isNullable(Parameter param) {
649+
return containsNullable(param.getAnnotations());
650+
}
651+
};
652+
653+
abstract boolean isNullable(Invokable<?, ?> invokable);
654+
655+
abstract boolean isNullable(Parameter param);
656+
}
570657
}

android/guava-tests/test/com/google/common/reflect/ParameterTest.java

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,16 @@
2929
public class ParameterTest extends TestCase {
3030

3131
public void testNulls() {
32+
try {
33+
Class.forName("java.lang.reflect.AnnotatedType");
34+
} catch (ClassNotFoundException runningInAndroidVm) {
35+
/*
36+
* Parameter declares a method that returns AnnotatedType, which isn't available on Android.
37+
* This would cause NullPointerTester, which calls Class.getDeclaredMethods, to throw
38+
* NoClassDefFoundError.
39+
*/
40+
return;
41+
}
3242
for (Method method : ParameterTest.class.getDeclaredMethods()) {
3343
for (Parameter param : Invokable.from(method).getParameters()) {
3444
new NullPointerTester().testAllPublicInstanceMethods(param);
Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,30 @@
1+
/*
2+
* Copyright 2019 The Guava Authors
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except
5+
* in compliance with the License. You may obtain a copy of the License at
6+
*
7+
* http://www.apache.org/licenses/LICENSE-2.0
8+
*
9+
* Unless required by applicable law or agreed to in writing, software distributed under the License
10+
* is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express
11+
* or implied. See the License for the specific language governing permissions and limitations under
12+
* the License.
13+
*/
14+
15+
package com.google.common.reflect;
16+
17+
import static java.lang.annotation.ElementType.CONSTRUCTOR;
18+
import static java.lang.annotation.ElementType.METHOD;
19+
import static java.lang.annotation.ElementType.TYPE;
20+
21+
import java.lang.annotation.Target;
22+
23+
/**
24+
* Disables Animal Sniffer's checking of compatibility with older versions of Java/Android.
25+
*
26+
* <p>Each package's copy of this annotation needs to be listed in our {@code pom.xml}.
27+
*/
28+
@Target({METHOD, CONSTRUCTOR, TYPE})
29+
@ElementTypesAreNonnullByDefault
30+
@interface IgnoreJRERequirement {}

android/guava/src/com/google/common/reflect/Invokable.java

Lines changed: 68 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,9 +19,11 @@
1919
import com.google.common.annotations.Beta;
2020
import com.google.common.collect.ImmutableList;
2121
import com.google.errorprone.annotations.CanIgnoreReturnValue;
22+
import com.google.errorprone.annotations.DoNotCall;
2223
import java.lang.annotation.Annotation;
2324
import java.lang.reflect.AccessibleObject;
2425
import java.lang.reflect.AnnotatedElement;
26+
import java.lang.reflect.AnnotatedType;
2527
import java.lang.reflect.Constructor;
2628
import java.lang.reflect.InvocationTargetException;
2729
import java.lang.reflect.Member;
@@ -272,12 +274,17 @@ public final TypeToken<? extends R> getReturnType() {
272274
* of a non-static inner class, unlike {@link Constructor#getParameterTypes}, the hidden {@code
273275
* this} parameter of the enclosing class is excluded from the returned parameters.
274276
*/
277+
@IgnoreJRERequirement
275278
public final ImmutableList<Parameter> getParameters() {
276279
Type[] parameterTypes = getGenericParameterTypes();
277280
Annotation[][] annotations = getParameterAnnotations();
281+
@Nullable Object[] annotatedTypes =
282+
ANNOTATED_TYPE_EXISTS ? getAnnotatedParameterTypes() : new Object[parameterTypes.length];
278283
ImmutableList.Builder<Parameter> builder = ImmutableList.builder();
279284
for (int i = 0; i < parameterTypes.length; i++) {
280-
builder.add(new Parameter(this, i, TypeToken.of(parameterTypes[i]), annotations[i]));
285+
builder.add(
286+
new Parameter(
287+
this, i, TypeToken.of(parameterTypes[i]), annotations[i], annotatedTypes[i]));
281288
}
282289
return builder.build();
283290
}
@@ -337,13 +344,32 @@ abstract Object invokeInternal(@CheckForNull Object receiver, @Nullable Object[]
337344

338345
abstract Type[] getGenericParameterTypes();
339346

347+
@SuppressWarnings({"Java7ApiChecker", "AndroidJdkLibsChecker"})
348+
@IgnoreJRERequirement
349+
abstract AnnotatedType[] getAnnotatedParameterTypes();
350+
340351
/** This should never return a type that's not a subtype of Throwable. */
341352
abstract Type[] getGenericExceptionTypes();
342353

343354
abstract Annotation[][] getParameterAnnotations();
344355

345356
abstract Type getGenericReturnType();
346357

358+
/**
359+
* Returns the {@link AnnotatedType} for the return type.
360+
*
361+
* <p>This method will fail if run under an Android VM.
362+
*
363+
* @since NEXT for guava-android (available since 14.0 in guava-jre)
364+
* @deprecated This method does not work under Android VMs. It is safe to use from guava-jre, but
365+
* this copy in guava-android is not safe to use.
366+
*/
367+
@SuppressWarnings({"Java7ApiChecker", "AndroidJdkLibsChecker"})
368+
@DoNotCall("fails under Android VMs; do not use from guava-android")
369+
@Deprecated
370+
@IgnoreJRERequirement
371+
public abstract AnnotatedType getAnnotatedReturnType();
372+
347373
static class MethodInvokable<T> extends Invokable<T, Object> {
348374

349375
final Method method;
@@ -370,6 +396,21 @@ Type[] getGenericParameterTypes() {
370396
return method.getGenericParameterTypes();
371397
}
372398

399+
@Override
400+
@SuppressWarnings({"Java7ApiChecker", "AndroidJdkLibsChecker"})
401+
@IgnoreJRERequirement
402+
AnnotatedType[] getAnnotatedParameterTypes() {
403+
return method.getAnnotatedParameterTypes();
404+
}
405+
406+
@Override
407+
@SuppressWarnings({"Java7ApiChecker", "AndroidJdkLibsChecker", "DoNotCall"})
408+
@DoNotCall
409+
@IgnoreJRERequirement
410+
public AnnotatedType getAnnotatedReturnType() {
411+
return method.getAnnotatedReturnType();
412+
}
413+
373414
@Override
374415
Type[] getGenericExceptionTypes() {
375416
return method.getGenericExceptionTypes();
@@ -447,6 +488,21 @@ Type[] getGenericParameterTypes() {
447488
return types;
448489
}
449490

491+
@Override
492+
@SuppressWarnings({"Java7ApiChecker", "AndroidJdkLibsChecker"})
493+
@IgnoreJRERequirement
494+
AnnotatedType[] getAnnotatedParameterTypes() {
495+
return constructor.getAnnotatedParameterTypes();
496+
}
497+
498+
@Override
499+
@SuppressWarnings({"Java7ApiChecker", "AndroidJdkLibsChecker", "DoNotCall"})
500+
@DoNotCall
501+
@IgnoreJRERequirement
502+
public AnnotatedType getAnnotatedReturnType() {
503+
return constructor.getAnnotatedReturnType();
504+
}
505+
450506
@Override
451507
Type[] getGenericExceptionTypes() {
452508
return constructor.getGenericExceptionTypes();
@@ -510,4 +566,15 @@ private boolean mayNeedHiddenThis() {
510566
}
511567
}
512568
}
569+
570+
private static final boolean ANNOTATED_TYPE_EXISTS = initAnnotatedTypeExists();
571+
572+
private static boolean initAnnotatedTypeExists() {
573+
try {
574+
Class.forName("java.lang.reflect.AnnotatedType");
575+
} catch (ClassNotFoundException e) {
576+
return false;
577+
}
578+
return true;
579+
}
513580
}

0 commit comments

Comments
 (0)