Skip to content

Commit

Permalink
Make the guava-android copy of NullPointerTester read type-use anno…
Browse files Browse the repository at this point in the history
…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: 522214387
  • Loading branch information
cpovirk authored and Google Java Core Libraries committed Apr 6, 2023
1 parent 33d9ff7 commit b30e73c
Show file tree
Hide file tree
Showing 14 changed files with 481 additions and 67 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
/*
* 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.testing;

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;

/**
* Disables Animal Sniffer's checking of compatibility with older versions of Java/Android.
*
* <p>Each package's copy of this annotation needs to be listed in our {@code pom.xml}.
*/
@Target({METHOD, CONSTRUCTOR, TYPE})
@interface IgnoreJRERequirement {}
Original file line number Diff line number Diff line change
Expand Up @@ -34,14 +34,15 @@
import com.google.common.reflect.TypeToken;
import com.google.errorprone.annotations.CanIgnoreReturnValue;
import java.lang.annotation.Annotation;
import java.lang.reflect.AnnotatedElement;
import java.lang.reflect.AnnotatedType;
import java.lang.reflect.Constructor;
import java.lang.reflect.InvocationTargetException;
import java.lang.reflect.Member;
import java.lang.reflect.Method;
import java.lang.reflect.Modifier;
import java.lang.reflect.ParameterizedType;
import java.lang.reflect.Type;
import java.lang.reflect.TypeVariable;
import java.util.Arrays;
import java.util.List;
import java.util.concurrent.ConcurrentMap;
Expand All @@ -52,8 +53,8 @@
/**
* A test utility that verifies that your methods and constructors throw {@link
* NullPointerException} or {@link UnsupportedOperationException} whenever null is passed to a
* parameter that isn't annotated with an annotation with the simple name {@code Nullable}, {@code
* CheckForNull}, {@code NullableType}, or {@code NullableDecl}.
* parameter whose declaration or type isn't annotated with an annotation with the simple name
* {@code Nullable}, {@code CheckForNull}, {@code NullableType}, or {@code NullableDecl}.
*
* <p>The tested methods and constructors are invoked -- each time with one parameter being null and
* the rest not null -- and the test fails if no expected exception is thrown. {@code
Expand Down Expand Up @@ -495,8 +496,16 @@ static boolean isPrimitiveOrNullable(Parameter param) {
ImmutableSet.of(
"CheckForNull", "Nullable", "NullableDecl", "NullableType", "ParametricNullness");

static boolean isNullable(AnnotatedElement e) {
for (Annotation annotation : e.getAnnotations()) {
static boolean isNullable(Invokable<?, ?> invokable) {
return NULLNESS_ANNOTATION_READER.isNullable(invokable);
}

static boolean isNullable(Parameter param) {
return NULLNESS_ANNOTATION_READER.isNullable(param);
}

private static boolean containsNullable(Annotation[] annotations) {
for (Annotation annotation : annotations) {
if (NULLABLE_ANNOTATION_SIMPLE_NAMES.contains(annotation.annotationType().getSimpleName())) {
return true;
}
Expand Down Expand Up @@ -567,4 +576,82 @@ public boolean isExpectedType(Throwable cause) {

public abstract boolean isExpectedType(Throwable cause);
}

private static boolean annotatedTypeExists() {
try {
Class.forName("java.lang.reflect.AnnotatedType");
} catch (ClassNotFoundException e) {
return false;
}
return true;
}

private static final NullnessAnnotationReader NULLNESS_ANNOTATION_READER =
annotatedTypeExists()
? NullnessAnnotationReader.FROM_DECLARATION_AND_TYPE_USE_ANNOTATIONS
: NullnessAnnotationReader.FROM_DECLARATION_ANNOTATIONS_ONLY;

/**
* Looks for declaration nullness annotations and, if supported, type-use nullness annotations.
*
* <p>Under Android VMs, the methods for retrieving type-use annotations don't exist. This means
* that {@link NullPointerException} may misbehave under Android when used on classes that rely on
* type-use annotations.
*
* <p>Under j2objc, the necessary APIs exist, but some (perhaps all) return stub values, like
* empty arrays. Presumably {@link NullPointerException} could likewise misbehave under j2objc,
* but I don't know that anyone uses it there, anyway.
*/
private enum NullnessAnnotationReader {
// Usages (which are unsafe only for Android) are guarded by the annotatedTypeExists() check.
@SuppressWarnings({"Java7ApiChecker", "AndroidApiChecker", "DoNotCall", "deprecation"})
FROM_DECLARATION_AND_TYPE_USE_ANNOTATIONS {
@Override
@IgnoreJRERequirement
boolean isNullable(Invokable<?, ?> invokable) {
return FROM_DECLARATION_ANNOTATIONS_ONLY.isNullable(invokable)
|| containsNullable(invokable.getAnnotatedReturnType().getAnnotations());
// TODO(cpovirk): Should we also check isNullableTypeVariable?
}

@Override
@IgnoreJRERequirement
boolean isNullable(Parameter param) {
return FROM_DECLARATION_ANNOTATIONS_ONLY.isNullable(param)
|| containsNullable(param.getAnnotatedType().getAnnotations())
|| isNullableTypeVariable(param.getAnnotatedType().getType());
}

@IgnoreJRERequirement
boolean isNullableTypeVariable(Type type) {
if (!(type instanceof TypeVariable)) {
return false;
}
TypeVariable<?> typeVar = (TypeVariable<?>) type;
for (AnnotatedType bound : typeVar.getAnnotatedBounds()) {
// Until Java 15, the isNullableTypeVariable case here won't help:
// https://bugs.openjdk.java.net/browse/JDK-8202469
if (containsNullable(bound.getAnnotations()) || isNullableTypeVariable(bound.getType())) {
return true;
}
}
return false;
}
},
FROM_DECLARATION_ANNOTATIONS_ONLY {
@Override
boolean isNullable(Invokable<?, ?> invokable) {
return containsNullable(invokable.getAnnotations());
}

@Override
boolean isNullable(Parameter param) {
return containsNullable(param.getAnnotations());
}
};

abstract boolean isNullable(Invokable<?, ?> invokable);

abstract boolean isNullable(Parameter param);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,16 @@
public class ParameterTest extends TestCase {

public void testNulls() {
try {
Class.forName("java.lang.reflect.AnnotatedType");
} catch (ClassNotFoundException runningInAndroidVm) {
/*
* Parameter declares a method that returns AnnotatedType, which isn't available on Android.
* This would cause NullPointerTester, which calls Class.getDeclaredMethods, to throw
* NoClassDefFoundError.
*/
return;
}
for (Method method : ParameterTest.class.getDeclaredMethods()) {
for (Parameter param : Invokable.from(method).getParameters()) {
new NullPointerTester().testAllPublicInstanceMethods(param);
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
/*
* 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.reflect;

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;

/**
* Disables Animal Sniffer's checking of compatibility with older versions of Java/Android.
*
* <p>Each package's copy of this annotation needs to be listed in our {@code pom.xml}.
*/
@Target({METHOD, CONSTRUCTOR, TYPE})
@ElementTypesAreNonnullByDefault
@interface IgnoreJRERequirement {}
69 changes: 68 additions & 1 deletion android/guava/src/com/google/common/reflect/Invokable.java
Original file line number Diff line number Diff line change
Expand Up @@ -19,9 +19,11 @@
import com.google.common.annotations.Beta;
import com.google.common.collect.ImmutableList;
import com.google.errorprone.annotations.CanIgnoreReturnValue;
import com.google.errorprone.annotations.DoNotCall;
import java.lang.annotation.Annotation;
import java.lang.reflect.AccessibleObject;
import java.lang.reflect.AnnotatedElement;
import java.lang.reflect.AnnotatedType;
import java.lang.reflect.Constructor;
import java.lang.reflect.InvocationTargetException;
import java.lang.reflect.Member;
Expand Down Expand Up @@ -272,12 +274,17 @@ public final TypeToken<? extends R> getReturnType() {
* of a non-static inner class, unlike {@link Constructor#getParameterTypes}, the hidden {@code
* this} parameter of the enclosing class is excluded from the returned parameters.
*/
@IgnoreJRERequirement
public final ImmutableList<Parameter> getParameters() {
Type[] parameterTypes = getGenericParameterTypes();
Annotation[][] annotations = getParameterAnnotations();
@Nullable Object[] annotatedTypes =
ANNOTATED_TYPE_EXISTS ? getAnnotatedParameterTypes() : new Object[parameterTypes.length];
ImmutableList.Builder<Parameter> builder = ImmutableList.builder();
for (int i = 0; i < parameterTypes.length; i++) {
builder.add(new Parameter(this, i, TypeToken.of(parameterTypes[i]), annotations[i]));
builder.add(
new Parameter(
this, i, TypeToken.of(parameterTypes[i]), annotations[i], annotatedTypes[i]));
}
return builder.build();
}
Expand Down Expand Up @@ -337,13 +344,32 @@ abstract Object invokeInternal(@CheckForNull Object receiver, @Nullable Object[]

abstract Type[] getGenericParameterTypes();

@SuppressWarnings({"Java7ApiChecker", "AndroidJdkLibsChecker"})
@IgnoreJRERequirement
abstract AnnotatedType[] getAnnotatedParameterTypes();

/** This should never return a type that's not a subtype of Throwable. */
abstract Type[] getGenericExceptionTypes();

abstract Annotation[][] getParameterAnnotations();

abstract Type getGenericReturnType();

/**
* Returns the {@link AnnotatedType} for the return type.
*
* <p>This method will fail if run under an Android VM.
*
* @since NEXT for guava-android (available since 14.0 in guava-jre)
* @deprecated This method does not work under Android VMs. It is safe to use from guava-jre, but
* this copy in guava-android is not safe to use.
*/
@SuppressWarnings({"Java7ApiChecker", "AndroidJdkLibsChecker"})
@DoNotCall("fails under Android VMs; do not use from guava-android")
@Deprecated
@IgnoreJRERequirement
public abstract AnnotatedType getAnnotatedReturnType();

static class MethodInvokable<T> extends Invokable<T, Object> {

final Method method;
Expand All @@ -370,6 +396,21 @@ Type[] getGenericParameterTypes() {
return method.getGenericParameterTypes();
}

@Override
@SuppressWarnings({"Java7ApiChecker", "AndroidJdkLibsChecker"})
@IgnoreJRERequirement
AnnotatedType[] getAnnotatedParameterTypes() {
return method.getAnnotatedParameterTypes();
}

@Override
@SuppressWarnings({"Java7ApiChecker", "AndroidJdkLibsChecker", "DoNotCall"})
@DoNotCall
@IgnoreJRERequirement
public AnnotatedType getAnnotatedReturnType() {
return method.getAnnotatedReturnType();
}

@Override
Type[] getGenericExceptionTypes() {
return method.getGenericExceptionTypes();
Expand Down Expand Up @@ -447,6 +488,21 @@ Type[] getGenericParameterTypes() {
return types;
}

@Override
@SuppressWarnings({"Java7ApiChecker", "AndroidJdkLibsChecker"})
@IgnoreJRERequirement
AnnotatedType[] getAnnotatedParameterTypes() {
return constructor.getAnnotatedParameterTypes();
}

@Override
@SuppressWarnings({"Java7ApiChecker", "AndroidJdkLibsChecker", "DoNotCall"})
@DoNotCall
@IgnoreJRERequirement
public AnnotatedType getAnnotatedReturnType() {
return constructor.getAnnotatedReturnType();
}

@Override
Type[] getGenericExceptionTypes() {
return constructor.getGenericExceptionTypes();
Expand Down Expand Up @@ -510,4 +566,15 @@ private boolean mayNeedHiddenThis() {
}
}
}

private static final boolean ANNOTATED_TYPE_EXISTS = initAnnotatedTypeExists();

private static boolean initAnnotatedTypeExists() {
try {
Class.forName("java.lang.reflect.AnnotatedType");
} catch (ClassNotFoundException e) {
return false;
}
return true;
}
}
Loading

0 comments on commit b30e73c

Please sign in to comment.