Skip to content
Merged
Show file tree
Hide file tree
Changes from 6 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
@@ -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,80 @@ 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 (lhsTypeArgument.getKind().equals(TypeKind.WILDCARD)) {
return wildcardContains((Type.WildcardType) lhsTypeArgument, rhsTypeArgument);
}
if (rhsTypeArgument.getKind().equals(TypeKind.WILDCARD)) {
// Treat wildcard actual arguments as accepted here until we add more complete support.
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) {
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();
if (rhsBound == null) {
return true;
}
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
190 changes: 188 additions & 2 deletions nullaway/src/test/java/com/uber/nullaway/jspecify/WildcardTests.java
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,34 @@

public class WildcardTests extends NullAwayTestsBase {

@Ignore("https://github.com/uber/NullAway/issues/1360")
@Test
public void simpleWildcardNoInference() {
makeHelper()
.addSourceLines(
"Test.java",
"""
import org.jspecify.annotations.*;
@NullMarked
class Test {
class Foo<T extends @Nullable Object> {}
String nullableWildcard(Foo<? extends @Nullable String> foo) { throw new RuntimeException(); }
String nonnullWildcard(Foo<? extends String> foo) { throw new RuntimeException(); }
void testNegative(Foo<@Nullable String> f) {
// this is legal since the wildcard upper bound is @Nullable
String s = nullableWildcard(f);
Comment thread
msridhar marked this conversation as resolved.
s.hashCode();
}
void testPositive(Foo<@Nullable String> f) {
// not legal since the wildcard upper bound is non-null
// BUG: Diagnostic contains: incompatible types: Test.Foo<@Nullable String> cannot be converted to Test.Foo<? extends String>
String s = nonnullWildcard(f);
s.hashCode();
}
}
""")
.doTest();
}

@Test
public void simpleWildcard() {
makeHelper()
Expand All @@ -29,7 +56,70 @@ void testNegative(Foo<@Nullable String> f) {
}
void testPositive(Foo<@Nullable String> f) {
// not legal since the wildcard upper bound is non-null
// BUG: Diagnostic contains: something about how f cannot be passed here
Comment thread
msridhar marked this conversation as resolved.
// BUG: Diagnostic contains: incompatible types: Test.Foo<@Nullable String> cannot be converted to Test.Foo<? extends String>
String s = nonnullWildcard(f);
s.hashCode();
}
}
""")
.doTest();
}

@Test
public void nestedTypeArgsInWildcardBoundNoInference() {
makeHelper()
.addSourceLines(
"Test.java",
"""
import org.jspecify.annotations.*;
@NullMarked
class Test {
class Foo<T extends @Nullable Object> {}
class Bar<T extends @Nullable Object> {}
String nullableWildcard(Foo<? extends Bar<@Nullable String>> foo) {
throw new RuntimeException();
}
String nonnullWildcard(Foo<? extends Bar<String>> foo) {
throw new RuntimeException();
}
void testNegative(Foo<Bar<@Nullable String>> f) {
String s = nullableWildcard(f);
s.hashCode();
}
void testPositive(Foo<Bar<@Nullable String>> f) {
// BUG: Diagnostic contains: incompatible types: Test.Foo<Test.Bar<@Nullable String>> cannot be converted to Test.Foo<? extends Test.Bar<String>>
String s = nonnullWildcard(f);
s.hashCode();
}
}
""")
.doTest();
}

@Test
public void deeplyNestedTypeArgsInWildcardBoundNoInference() {
makeHelper()
.addSourceLines(
"Test.java",
"""
import org.jspecify.annotations.*;
@NullMarked
class Test {
class Foo<T extends @Nullable Object> {}
class Bar<T extends @Nullable Object> {}
class Baz<T extends @Nullable Object> {}
String nullableWildcard(Foo<? extends Bar<Baz<@Nullable String>>> foo) {
throw new RuntimeException();
}
String nonnullWildcard(Foo<? extends Bar<Baz<String>>> foo) {
throw new RuntimeException();
}
void testNegative(Foo<Bar<Baz<@Nullable String>>> f) {
String s = nullableWildcard(f);
s.hashCode();
}
void testPositive(Foo<Bar<Baz<@Nullable String>>> f) {
// BUG: Diagnostic contains: incompatible types: Test.Foo<Test.Bar<Test.Baz<@Nullable String>>> cannot be converted to Test.Foo<? extends Test.Bar<Test.Baz<String>>>
String s = nonnullWildcard(f);
s.hashCode();
}
Expand All @@ -38,6 +128,102 @@ void testPositive(Foo<@Nullable String> f) {
.doTest();
}

@Test
public void intermediateNestedTypeArgsInWildcardBoundNoInference() {
makeHelper()
.addSourceLines(
"Test.java",
"""
import org.jspecify.annotations.*;
@NullMarked
class Test {
class Foo<T extends @Nullable Object> {}
class Bar<T extends @Nullable Object> {}
class Baz<T extends @Nullable Object> {}
String nullableWildcard(Foo<? extends @Nullable Bar<Baz<String>>> foo) {
throw new RuntimeException();
}
String nonnullWildcard(Foo<? extends Bar<Baz<String>>> foo) {
throw new RuntimeException();
}
void testNegative(Foo<@Nullable Bar<Baz<String>>> f) {
String s = nullableWildcard(f);
s.hashCode();
}
void testPositive(Foo<@Nullable Bar<Baz<String>>> f) {
// BUG: Diagnostic contains: incompatible types: Test.Foo<Test.@Nullable Bar<Test.Baz<String>>> cannot be converted to Test.Foo<? extends Test.Bar<Test.Baz<String>>>
String s = nonnullWildcard(f);
s.hashCode();
}
}
""")
.doTest();
}

@Test
public void wildcardActualArgumentNoInference() {
makeHelper()
.addSourceLines(
"Test.java",
"""
import org.jspecify.annotations.*;
@NullMarked
class Test {
class Foo<T extends @Nullable Object> {}
String nullableWildcard(Foo<? extends @Nullable String> foo) {
throw new RuntimeException();
}
String nonnullWildcard(Foo<? extends String> foo) {
throw new RuntimeException();
}
void testNegative(Foo<? extends @Nullable String> f) {
String s = nullableWildcard(f);
s.hashCode();
}
void testPositive(Foo<? extends @Nullable String> f) {
// BUG: Diagnostic contains: incompatible types: Test.Foo<? extends @Nullable String> cannot be converted to Test.Foo<? extends String>
String s = nonnullWildcard(f);
s.hashCode();
}
}
""")
.doTest();
}

@Test
public void wildcardCheckingForReturnsAndAssignments() {
makeHelper()
.addSourceLines(
"Test.java",
"""
import org.jspecify.annotations.*;
@NullMarked
class Test {
class Foo<T extends @Nullable Object> {}
Foo<? extends String> nonnullField;
Foo<? extends @Nullable String> nullableField;
Test(Foo<? extends @Nullable String> f) {
nullableField = f;
// BUG: Diagnostic contains: incompatible types: Test.Foo<? extends @Nullable String> cannot be converted to Test.Foo<? extends String>
nonnullField = f;
}
Foo<? extends @Nullable String> nullableReturn(Foo<? extends @Nullable String> f) {
return f;
}
Foo<? extends String> nonnullReturn(Foo<? extends @Nullable String> f) {
// BUG: Diagnostic contains: incompatible types: Test.Foo<? extends @Nullable String> cannot be converted to Test.Foo<? extends String>
return f;
}
void testLocal(Foo<? extends @Nullable String> f) {
Foo<? extends @Nullable String> ok = f;
// BUG: Diagnostic contains: incompatible types: Test.Foo<? extends @Nullable String> cannot be converted to Test.Foo<? extends String>
Foo<? extends String> bad = f;
Comment thread
msridhar marked this conversation as resolved.
}
}
""")
.doTest();
}

@Ignore("https://github.com/uber/NullAway/issues/1350")
@Test
public void genericMethodLambdaArgWildCard() {
Expand Down
Loading