Skip to content

Commit

Permalink
Allow an AutoValue or AutoBuilder property to be null if its type is …
Browse files Browse the repository at this point in the history
…a type variable with a `@Nullable` bound, like `<T extends @nullable Object>`.

Fixes #1320.

RELNOTES=An AutoValue or AutoBuilder property is now allowed to be null if its type is a type variable with a `@Nullable` bound, like `<T extends @nullable Object>`.
PiperOrigin-RevId: 515104184
  • Loading branch information
eamonnmcmanus authored and Google Java Core Libraries committed Mar 8, 2023
1 parent 2e734f6 commit f781123
Show file tree
Hide file tree
Showing 4 changed files with 109 additions and 9 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,8 @@
import static com.google.common.base.Preconditions.checkNotNull;
import static com.google.common.truth.Truth.assertThat;
import static com.google.common.truth.Truth8.assertThat;
import static java.lang.annotation.ElementType.TYPE_USE;
import static org.junit.Assert.assertThrows;
import static org.junit.Assert.fail;

import com.google.common.base.MoreObjects;
Expand All @@ -27,6 +29,7 @@
import java.io.IOException;
import java.lang.annotation.Retention;
import java.lang.annotation.RetentionPolicy;
import java.lang.annotation.Target;
import java.math.BigInteger;
import java.time.LocalTime;
import java.util.AbstractSet;
Expand Down Expand Up @@ -751,4 +754,33 @@ public void builderAnnotationsCopiedIfRequested() {
.asList()
.contains(myAnnotationBuilder().value("thing").build());
}

@Target(TYPE_USE)
public @interface Nullable {}

public static <T extends @Nullable Object, U> T frob(T arg, U notNull) {
return arg;
}

@AutoBuilder(callMethod = "frob")
interface FrobCaller<T extends @Nullable Object, U> {
FrobCaller<T, U> arg(T arg);

FrobCaller<T, U> notNull(U notNull);

T call();

static <T extends @Nullable Object, U> FrobCaller<T, U> caller() {
return new AutoBuilder_AutoBuilderTest_FrobCaller<>();
}
}

@Test
public void builderTypeVariableWithNullableBound() {
assertThat(FrobCaller.<@Nullable String, String>caller().arg(null).notNull("foo").call())
.isNull();
assertThrows(
NullPointerException.class,
() -> FrobCaller.<@Nullable String, String>caller().arg(null).notNull(null).call());
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@
import com.google.testing.compile.Compilation;
import com.google.testing.compile.Compiler;
import com.google.testing.compile.JavaFileObjects;
import java.io.Serializable;
import java.lang.annotation.Annotation;
import java.lang.annotation.ElementType;
import java.lang.annotation.Retention;
Expand All @@ -39,8 +40,8 @@
import java.lang.reflect.Constructor;
import java.lang.reflect.Method;
import java.lang.reflect.TypeVariable;
import java.util.Arrays;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.List;
import java.util.Optional;
import java.util.OptionalDouble;
Expand Down Expand Up @@ -934,7 +935,7 @@ public void nestedOptionalGetter() {
public abstract static class PropertyBuilderWildcard<T> {
public abstract List<? extends T> list();

public static <T>PropertyBuilderWildcard.Builder<T> builder() {
public static <T> PropertyBuilderWildcard.Builder<T> builder() {
return new AutoValue_AutoValueJava8Test_PropertyBuilderWildcard.Builder<>();
}

Expand Down Expand Up @@ -964,4 +965,42 @@ public void propertyBuilderWildcard() {
builder.listBuilder().add("foo");
assertThat(builder.build().list()).containsExactly("foo");
}

@AutoValue
public abstract static class NullableBound<T extends @Nullable Object> {
public abstract T maybeNullable();

public static <T extends @Nullable Object> NullableBound<T> create(T maybeNullable) {
return new AutoValue_AutoValueJava8Test_NullableBound<>(maybeNullable);
}
}

@Test
public void propertyCanBeNullIfNullableBound() {
// The generated class doesn't know what the actual type argument is, so it can't know whether
// it is @Nullable. Because of the @Nullable bound, it omits an explicit null check, under the
// assumption that some static-checking framework is validating type uses.
NullableBound<@Nullable String> x = NullableBound.create(null);
assertThat(x.maybeNullable()).isNull();
}

@AutoValue
public abstract static class NullableIntersectionBound<
T extends @Nullable Object & Serializable> {
public abstract T maybeNullable();

public static <T extends @Nullable Object & Serializable> NullableIntersectionBound<T> create(
T maybeNullable) {
return new AutoValue_AutoValueJava8Test_NullableIntersectionBound<>(maybeNullable);
}
}

@Test
public void propertyCanBeNullIfNullableIntersectionBound() {
// The generated class doesn't know what the actual type argument is, so it can't know whether
// it is @Nullable. Because of the @Nullable bound, it omits an explicit null check, under the
// assumption that some static-checking framework is validating type uses.
NullableIntersectionBound<@Nullable String> x = NullableIntersectionBound.create(null);
assertThat(x.maybeNullable()).isNull();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -80,11 +80,13 @@ public static void setSourceRoot() {
private static final Predicate<File> JAVA_FILE =
f -> f.getName().endsWith(".java") && !IGNORED_TEST_FILES.contains(f.getName());

private static final Predicate<File> JAVA8_TEST =
f ->
f.getName().equals("AutoValueJava8Test.java")
|| f.getName().equals("AutoOneOfJava8Test.java")
|| f.getName().equals("EmptyExtension.java");
private static final ImmutableSet<String> JAVA8_TEST_FILES =
ImmutableSet.of(
"AutoBuilderTest.java",
"AutoOneOfJava8Test.java",
"AutoValueJava8Test.java",
"EmptyExtension.java");
private static final Predicate<File> JAVA8_TEST = f -> JAVA8_TEST_FILES.contains(f.getName());

@Test
public void compileWithEclipseJava7() throws Exception {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,7 @@
import javax.lang.model.type.DeclaredType;
import javax.lang.model.type.TypeKind;
import javax.lang.model.type.TypeMirror;
import javax.lang.model.type.TypeVariable;
import javax.lang.model.util.ElementFilter;
import javax.lang.model.util.Elements;
import javax.lang.model.util.SimpleAnnotationValueVisitor8;
Expand Down Expand Up @@ -715,8 +716,7 @@ private static boolean gettersAllPrefixed(Set<ExecutableElement> methods) {
* parameter type for a parameter.
*/
static Optional<String> nullableAnnotationFor(Element element, TypeMirror elementType) {
List<? extends AnnotationMirror> typeAnnotations = elementType.getAnnotationMirrors();
if (nullableAnnotationIndex(typeAnnotations).isPresent()) {
if (isNullable(elementType)) {
return Optional.of("");
}
List<? extends AnnotationMirror> elementAnnotations = element.getAnnotationMirrors();
Expand All @@ -736,6 +736,33 @@ private static OptionalInt nullableAnnotationIndex(List<? extends AnnotationMirr
.findFirst();
}

private static boolean isNullable(TypeMirror type) {
return isNullable(type, 0);
}

private static boolean isNullable(TypeMirror type, int depth) {
// Some versions of the Eclipse compiler can report that the upper bound of a type variable T
// is another T, and if you ask for the upper bound of that other T you'll get a third T, and so
// ad infinitum. To avoid StackOverflowError, we bottom out after 10 iterations.
if (depth > 10) {
return false;
}
List<? extends AnnotationMirror> typeAnnotations = type.getAnnotationMirrors();
if (nullableAnnotationIndex(typeAnnotations).isPresent()) {
return true;
}
if (type.getKind().equals(TypeKind.TYPEVAR)) {
TypeVariable typeVariable = MoreTypes.asTypeVariable(type);
TypeMirror bound = typeVariable.getUpperBound();
if (bound.getKind().equals(TypeKind.INTERSECTION)) {
return MoreTypes.asIntersection(bound).getBounds().stream()
.anyMatch(t -> isNullable(t, depth + 1));
}
return isNullable(bound, depth + 1);
}
return false;
}

private static boolean isNullable(AnnotationMirror annotation) {
return annotation.getAnnotationType().asElement().getSimpleName().contentEquals("Nullable");
}
Expand Down

0 comments on commit f781123

Please sign in to comment.