Skip to content

Commit

Permalink
Recognize that <T extends @nullable Object> implies nullability.
Browse files Browse the repository at this point in the history
If you have a method with a parameter of type `T`, we should not expect it to
throw an exception when called with null.

RELNOTES=In NullPointerTester, a parameter of type `<T extends @nullable Object>` is allowed to be null.
PiperOrigin-RevId: 367051816
  • Loading branch information
eamonnmcmanus authored and Google Java Core Libraries committed Apr 6, 2021
1 parent a6489b6 commit e856722
Show file tree
Hide file tree
Showing 3 changed files with 85 additions and 3 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@
/**
* 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}, {@lcode
* parameter that isn't annotated with an annotation with the simple name {@code Nullable}, {@code
* CheckForNull}, {@link NullableType}, or {@link NullableDecl}.
*
* <p>The tested methods and constructors are invoked -- each time with one parameter being null and
Expand Down
43 changes: 41 additions & 2 deletions guava-testlib/src/com/google/common/testing/NullPointerTester.java
Original file line number Diff line number Diff line change
Expand Up @@ -34,16 +34,19 @@
import com.google.common.reflect.Reflection;
import com.google.common.reflect.TypeToken;
import java.lang.annotation.Annotation;
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;
import java.util.function.Function;
import junit.framework.Assert;
import junit.framework.AssertionFailedError;
import org.checkerframework.checker.nullness.qual.Nullable;
Expand All @@ -52,7 +55,7 @@
* A test utility that verifies that your methods and constructors throw {@link
* NullPointerException} or {@link UnsupportedOperationException} whenever null is passed to a
* parameter whose declaration or type isn't annotated with an annotation with the simple name
* {@code Nullable}, {@lcode CheckForNull}, {@link NullableType}, or {@link NullableDecl}.
* {@code Nullable}, {@code CheckForNull}, {@link NullableType}, or {@link 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 @@ -483,7 +486,22 @@ static boolean isNullable(Invokable<?, ?> invokable) {

static boolean isNullable(Parameter param) {
return isNullable(param.getAnnotatedType().getAnnotations())
|| isNullable(param.getAnnotations());
|| isNullable(param.getAnnotations())
|| isNullableTypeVariable(param.getAnnotatedType().getType());
}

private static boolean isNullableTypeVariable(Type type) {
if (!(type instanceof TypeVariable)) {
return false;
}
TypeVariable<?> var = (TypeVariable<?>) type;
AnnotatedType[] bounds = GET_ANNOTATED_BOUNDS.apply(var);
for (AnnotatedType bound : bounds) {
if (isNullable(bound.getAnnotations()) || isNullableTypeVariable(bound.getType())) {
return true;
}
}
return false;
}

private static boolean isNullable(Annotation[] annotations) {
Expand All @@ -495,6 +513,27 @@ private static boolean isNullable(Annotation[] annotations) {
return false;
}

// This is currently required because of j2objc restrictions.
private static final Function<TypeVariable<?>, AnnotatedType[]> GET_ANNOTATED_BOUNDS =
initGetAnnotatedBounds();

private static Function<TypeVariable<?>, AnnotatedType[]> initGetAnnotatedBounds() {
AnnotatedType[] noBounds = new AnnotatedType[0];
Method getAnnotatedBounds;
try {
getAnnotatedBounds = TypeVariable.class.getMethod("getAnnotatedBounds");
} catch (ReflectiveOperationException e) {
return v -> noBounds;
}
return v -> {
try {
return (AnnotatedType[]) getAnnotatedBounds.invoke(v);
} catch (ReflectiveOperationException e) {
return noBounds;
}
};
}

private boolean isIgnored(Member member) {
return member.isSynthetic() || ignoredMembers.contains(member) || isEquals(member);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1484,4 +1484,47 @@ public void testConstructor_ShouldFail() throws Exception {
}
fail("Should detect problem in " + FailOnOneOfTwoConstructors.class.getSimpleName());
}

public static class NullBounds<T extends @Nullable Object, U extends T, X> {
boolean xWasCalled;

@SuppressWarnings("unused") // Called by reflection
public void x(X x) {
xWasCalled = true;
checkNotNull(x);
}

@SuppressWarnings("unused") // Called by reflection
public void t(T t) {
fail("Method with parameter <T extends @Nullable Object> should not be called");
}

@SuppressWarnings("unused") // Called by reflection
public void u(U u) {
fail(
"Method with parameter <U extends T> where <T extends @Nullable Object> should not be"
+ " called");
}

@SuppressWarnings("unused") // Called by reflection
public <A extends @Nullable Object> void a(A a) {
fail("Method with parameter <A extends @Nullable Object> should not be called");
}

@SuppressWarnings("unused") // Called by reflection
public <A extends B, B extends @Nullable Object> void b(A a) {
fail(
"Method with parameter <A extends B> where <B extends @Nullable Object> should not be"
+ " called");
}
}

public void testNullBounds() {
// NullBounds has methods whose parameters are type variables that have
// "extends @Nullable Object" as a bound. This test ensures that NullPointerTester considers
// those parameters to be @Nullable, so it won't call the methods.
NullBounds<?, ?, ?> nullBounds = new NullBounds<>();
new NullPointerTester().testAllPublicInstanceMethods(nullBounds);
assertThat(nullBounds.xWasCalled).isTrue();
}
}

0 comments on commit e856722

Please sign in to comment.