Skip to content
Merged
Show file tree
Hide file tree
Changes from 18 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
import static org.junit.Assert.assertTrue;

import java.io.IOException;
import org.junit.Ignore;
import org.junit.Test;

/** Tests that all our JMH benchmarks compile successfully */
Expand All @@ -13,6 +14,7 @@ public void testAutodispose() throws IOException {
assertTrue(new AutodisposeCompiler().compile());
}

@Ignore("Fails after wildcard nullability checking changes; needs benchmark-specific follow-up")
Comment thread
msridhar marked this conversation as resolved.
Outdated
@Test
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Outdated
public void testCaffeine() throws IOException {
assertTrue(new CaffeineCompiler().compile());
Expand Down
3 changes: 3 additions & 0 deletions nullaway/src/main/java/com/uber/nullaway/Config.java
Original file line number Diff line number Diff line change
Expand Up @@ -304,6 +304,9 @@ public interface Config {
/** Should new checks based on JSpecify (like checks for generic types) be enabled? */
boolean isJSpecifyMode();

/** Should wildcard-aware generic nullability checks be enabled? */
boolean handleWildcardGenerics();

/**
* Checks if legacy annotation locations are enabled.
*
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -219,6 +219,11 @@ public boolean isJSpecifyMode() {
throw new IllegalStateException(ERROR_MESSAGE);
}

@Override
public boolean handleWildcardGenerics() {
throw new IllegalStateException(ERROR_MESSAGE);
}

@Override
public boolean isLegacyAnnotationLocation() {
throw new IllegalStateException(ERROR_MESSAGE);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -104,6 +104,8 @@ final class ErrorProneCLIFlagsConfig implements Config {
static final String FL_WARN_ON_GENERIC_INFERENCE_FAILURE =
EP_FL_NAMESPACE + ":WarnOnGenericInferenceFailure";

static final String FL_HANDLE_WILDCARD_GENERICS = EP_FL_NAMESPACE + ":HandleWildcardGenerics";

static final String ANNOTATED_PACKAGES_ONLY_NULLMARKED_ERROR_MSG =
"DO NOT report an issue to Error Prone for this crash! NullAway configuration is "
+ "incorrect. "
Expand Down Expand Up @@ -231,6 +233,7 @@ final class ErrorProneCLIFlagsConfig implements Config {
private final boolean treatGeneratedAsUnannotated;
private final boolean acknowledgeAndroidRecent;
private final boolean jspecifyMode;
private final boolean handleWildcardGenerics;
private final boolean legacyAnnotationLocation;
private final boolean warnOnInferenceFailure;
private final ImmutableSet<MethodClassAndName> knownInitializers;
Expand Down Expand Up @@ -303,6 +306,7 @@ final class ErrorProneCLIFlagsConfig implements Config {
treatGeneratedAsUnannotated = flags.getBoolean(FL_GENERATED_UNANNOTATED).orElse(false);
acknowledgeAndroidRecent = flags.getBoolean(FL_ACKNOWLEDGE_ANDROID_RECENT).orElse(false);
jspecifyMode = flags.getBoolean(FL_JSPECIFY_MODE).orElse(false);
handleWildcardGenerics = flags.getBoolean(FL_HANDLE_WILDCARD_GENERICS).orElse(false);
assertsEnabled = flags.getBoolean(FL_ASSERTS_ENABLED).orElse(false);
fieldAnnotPattern =
getPackagePattern(
Expand Down Expand Up @@ -605,6 +609,11 @@ public boolean isJSpecifyMode() {
return jspecifyMode;
}

@Override
public boolean handleWildcardGenerics() {
return handleWildcardGenerics;
}

@Override
public boolean isLegacyAnnotationLocation() {
return legacyAnnotationLocation;
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package com.uber.nullaway.generics;

import com.google.errorprone.VisitorState;
import com.sun.tools.javac.code.BoundKind;
import com.sun.tools.javac.code.Symbol;
import com.sun.tools.javac.code.Type;
import com.sun.tools.javac.code.Types;
Expand Down Expand Up @@ -63,18 +64,7 @@ public Boolean visitClassType(Type.ClassType lhsType, Type rhsType) {
for (int i = 0; i < lhsTypeArguments.size(); i++) {
Type lhsTypeArgument = lhsTypeArguments.get(i);
Type rhsTypeArgument = rhsTypeArguments.get(i);
if (lhsTypeArgument.getKind().equals(TypeKind.WILDCARD)
|| rhsTypeArgument.getKind().equals(TypeKind.WILDCARD)) {
// TODO Handle wildcard types
continue;
}
boolean isLHSNullableAnnotated = genericsChecks.isNullableAnnotated(lhsTypeArgument);
boolean isRHSNullableAnnotated = genericsChecks.isNullableAnnotated(rhsTypeArgument);
if (isLHSNullableAnnotated != isRHSNullableAnnotated) {
return false;
}
// nested generics
if (!lhsTypeArgument.accept(this, rhsTypeArgument)) {
if (!typeArgumentContainedBy(lhsTypeArgument, rhsTypeArgument)) {
return false;
}
}
Expand Down Expand Up @@ -116,4 +106,87 @@ public Boolean visitArrayType(Type.ArrayType lhsType, Type rhsType) {
public Boolean visitType(Type t, Type type) {
return true;
}

/**
* Returns whether the actual type argument on the right is contained by the formal type argument
* on the left, following the JLS 4.5.1 notion of type-argument containment but interpreted with
* NullAway's nullability-aware subtype relation. Non-wildcard pairs require matching nullability
Comment thread
msridhar marked this conversation as resolved.
Outdated
* annotations and recursively matching nested type arguments. Wildcard formals are delegated to
* {@link #wildcardContains}.
*/
private boolean typeArgumentContainedBy(Type lhsTypeArgument, Type rhsTypeArgument) {
if (!config.handleWildcardGenerics()
&& (lhsTypeArgument.getKind().equals(TypeKind.WILDCARD)
|| rhsTypeArgument.getKind().equals(TypeKind.WILDCARD))) {
// Preserve the pre-flag behavior of skipping wildcard-aware checks entirely.
return true;
}
if (lhsTypeArgument.getKind().equals(TypeKind.WILDCARD)) {
return wildcardContains((Type.WildcardType) lhsTypeArgument, rhsTypeArgument);
}
if (rhsTypeArgument.getKind().equals(TypeKind.WILDCARD)) {
// TODO: Add proper support for the remaining case where the formal type argument is not a
// wildcard but the actual type argument is a wildcard.
Comment thread
msridhar marked this conversation as resolved.
return true;
}
Comment thread
coderabbitai[bot] marked this conversation as resolved.
boolean isLHSNullableAnnotated = genericsChecks.isNullableAnnotated(lhsTypeArgument);
boolean isRHSNullableAnnotated = genericsChecks.isNullableAnnotated(rhsTypeArgument);
if (isLHSNullableAnnotated != isRHSNullableAnnotated) {
return false;
}
return lhsTypeArgument.accept(this, rhsTypeArgument);
}

/**
* Handles a narrow slice of the JLS type-argument containment rules from JLS 4.5.1 for wildcard
* type arguments. In particular, for a formal argument {@code ? extends S}, we accept either a
* concrete actual argument {@code T} or a wildcard actual argument {@code ? extends T} whenever
* {@code T <: S}, using NullAway's nullability-aware subtype check in place of plain Java
* subtyping. This covers the JLS cases behind both {@code T <= ? extends S} and {@code ? extends
* T <= ? extends S}. For now, this method intentionally leaves {@code super} wildcards and other
* more complex cases to existing fallback behavior.
*/
private boolean wildcardContains(Type.WildcardType lhsWildcard, Type rhsTypeArgument) {
if (lhsWildcard.kind == BoundKind.UNBOUND) {
// TODO: For unbounded wildcards, we need to find the bound of the corresponding type
// variable rather than accepting outright; see
// https://jspecify.dev/docs/user-guide/#wildcard-bounds
return true;
}
if (lhsWildcard.kind != BoundKind.EXTENDS) {
// Treat non-extends wildcards as accepted here until we add more complete support.
return true;
Comment thread
msridhar marked this conversation as resolved.
}
Type lhsBound = lhsWildcard.getExtendsBound();
if (lhsBound == null) {
return true;
Comment thread
msridhar marked this conversation as resolved.
Outdated
}
if (rhsTypeArgument.getKind().equals(TypeKind.WILDCARD)) {
Type.WildcardType rhsWildcard = (Type.WildcardType) rhsTypeArgument;
if (rhsWildcard.kind != BoundKind.EXTENDS) {
// Treat non-extends wildcard actual arguments as accepted here until we add more complete
// support.
return true;
}
Type rhsBound = rhsWildcard.getExtendsBound();
return typeArgumentSubtype(lhsBound, rhsBound);
}
return typeArgumentSubtype(lhsBound, rhsTypeArgument);
}

/**
* Returns whether the actual type argument on the right is a nullability-aware subtype of the
* formal type argument on the left. This check first rejects flows from nullable to non-null at
* the top level of the type argument, then delegates to {@link
* GenericsChecks#subtypeParameterNullability(Type, Type, VisitorState)} for recursive nested
* checks.
*/
private boolean typeArgumentSubtype(Type lhsType, Type rhsType) {
boolean isLHSNullableAnnotated = genericsChecks.isNullableAnnotated(lhsType);
boolean isRHSNullableAnnotated = genericsChecks.isNullableAnnotated(rhsType);
if (isRHSNullableAnnotated && !isLHSNullableAnnotated) {
return false;
}
return genericsChecks.subtypeParameterNullability(lhsType, rhsType, state);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -1450,7 +1450,7 @@ private boolean identicalTypeParameterNullability(
* @param rhsType type for the rhs of the assignment
* @param state the visitor state
*/
private boolean subtypeParameterNullability(Type lhsType, Type rhsType, VisitorState state) {
boolean subtypeParameterNullability(Type lhsType, Type rhsType, VisitorState state) {
if (lhsType.isRaw()) {
return true;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,15 +11,18 @@
* configuring NullAway compilation tests that rely on JSpecify annotations.
*
* <p>For tests, this utility ensures that they always pass {@code
* -XepOpt:NullAway:JSpecifyMode=true} and {@code -XDaddTypeAnnotationsToSymbol=true} together.
* -XepOpt:NullAway:JSpecifyMode=true}, {@code -XDaddTypeAnnotationsToSymbol=true}, and the
* wildcard-support flag together.
*/
public final class JSpecifyJavacConfig {

public static final String JSPECIFY_MODE_FLAG = "-XepOpt:NullAway:JSpecifyMode=true";
public static final String ADD_TYPE_ANNOTATIONS_FLAG = "-XDaddTypeAnnotationsToSymbol=true";
public static final String HANDLE_WILDCARD_GENERICS_FLAG =
"-XepOpt:NullAway:HandleWildcardGenerics=true";

private static final List<String> JSPECIFY_MODE_ARGS =
List.of(JSPECIFY_MODE_FLAG, ADD_TYPE_ANNOTATIONS_FLAG);
List.of(JSPECIFY_MODE_FLAG, ADD_TYPE_ANNOTATIONS_FLAG, HANDLE_WILDCARD_GENERICS_FLAG);

private JSpecifyJavacConfig() {}

Expand Down
Loading
Loading