Fix subtype checking for nested captured types

AGENTS.md

@@ -24,7 +24,7 @@ Our `CHANGELOG.md` file should be formatted as follows:
 Do not add top-level `@Nullable` annotations on local variables.  NullAway infers the nullability of local variables and
 ignores these explicit annotations.
 
-Whenever you add a non-trivial method, add Javadoc, even if it's a private method.
+Whenever you add a non-trivial method, add Javadoc, even if it's a private method.  JUnit test methods do not require Javadoc.
 
 You do _not_ need to run `./gradlew spotlessJavaCheck` to check formatting.  We have a pre-commit hook that
 automatically formats code before it is committed.

nullaway/src/main/java/com/uber/nullaway/generics/CheckIdenticalNullabilityVisitor.java

@@ -11,6 +11,7 @@
 import com.uber.nullaway.handlers.Handler;
 import java.util.List;
 import javax.lang.model.type.NullType;
+import javax.lang.model.type.TypeKind;
 
 /**
  * Visitor that checks for identical nullability annotations at all nesting levels within two types.
@@ -31,14 +32,36 @@ public class CheckIdenticalNullabilityVisitor extends Types.DefaultTypeVisitor<B
     this.handler = handler;
   }
 
+  /**
+   * Checks whether the nested nullability of an RHS type is compatible with an LHS class type.
+   *
+   * <p>The RHS is aligned with the LHS's base type before comparing type arguments. When {@link
+   * Config#handleWildcardGenerics()} is enabled, a direct or captured RHS wildcard is first
+   * replaced with its effective upper bound.
+   *
+   * @param lhsType class type on the left side of the comparison
+   * @param rhsType type on the right side of the comparison
+   * @return {@code true} if the types have compatible nested nullability, or if the comparison is
+   *     intentionally skipped
+   */
   @Override
   public Boolean visitClassType(Type.ClassType lhsType, Type rhsType) {
     if (rhsType instanceof NullType || rhsType.isPrimitive()) {
       return true;
     }
-    if (GenericsUtils.asWildcard(rhsType) != null) {
-      // TODO Handle wildcard types
-      return true;
+    if (!config.handleWildcardGenerics()) {
+      // skip checking of wildcards
+      if (rhsType.getKind().equals(TypeKind.WILDCARD)) {
+        return true;
+      }
+    } else if (GenericsUtils.asWildcard(rhsType) != null) {
+      Type rhsUpperBound =
+          GenericsUtils.effectiveWildcardUpperBound(rhsType, state, config, handler);
+      if (GenericsUtils.asWildcard(rhsUpperBound) != null) {
+        // Bail out if resolving the upper bound did not produce a usable concrete bound.
+        return true;
+      }
+      rhsType = rhsUpperBound;
     }
     if (lhsType.isIntersection()) {
       return handleIntersectionType((Type.IntersectionClassType) lhsType, rhsType);
@@ -98,12 +121,7 @@ public Boolean visitArrayType(Type.ArrayType lhsType, Type rhsType) {
       return true;
     }
     Type rhsComponentType = rhsArrayType.getComponentType();
-    boolean isLHSNullableAnnotated = genericsChecks.isNullableAnnotated(lhsComponentType);
-    boolean isRHSNullableAnnotated = genericsChecks.isNullableAnnotated(rhsComponentType);
-    if (isRHSNullableAnnotated != isLHSNullableAnnotated) {
-      return false;
-    }
-    return lhsComponentType.accept(this, rhsComponentType);
+    return haveIdenticalNullability(lhsComponentType, rhsComponentType);
   }
 
   @Override
@@ -119,35 +137,47 @@ public Boolean visitType(Type t, Type type) {
    * matching nested type arguments. Wildcard formals are delegated to {@link #wildcardContains}.
    */
   private boolean typeArgumentContainedBy(Type lhsTypeArgument, Type rhsTypeArgument) {
-    // Do not use GenericsUtils.asWildcard() for the LHS. A captured LHS is a type variable, not a
-    // wildcard formal; unwrapping it can repeatedly expand recursive upper bounds (for example,
-    // F-bounded type parameters) as containment delegates back into subtype checking.  See test
-    // com.uber.nullaway.jspecify.WildcardTests.capturedLhsWithFBoundedTypeParametersDoesNotRecurse
-    Type.WildcardType lhsWildcard =
-        lhsTypeArgument instanceof Type.WildcardType wildcardType ? wildcardType : null;
-    Type.WildcardType rhsWildcard = GenericsUtils.asWildcard(rhsTypeArgument);
-    if (!config.handleWildcardGenerics() && (lhsWildcard != null || rhsWildcard != null)) {
-      // Preserve the pre-flag behavior of skipping wildcard-aware checks entirely.
-      return true;
-    }
-    if (lhsWildcard != null) {
-      return wildcardContains(lhsWildcard, rhsTypeArgument);
-    }
-    if (rhsWildcard != null) {
-      // This case should only arise when generic method invocation inference / capture conversion
-      // lets a wildcard actual argument flow into a non-wildcard formal type argument, e.g.,
-      // passing Foo<? extends T> to <U> void m(Foo<U>). We do not yet support wildcard inference.
-      // For non-inference assignment / return / parameter checks, javac rejects these conversions
-      // before NullAway runs.
-      // TODO: Add proper support when inference for wildcards is implemented.
-      return true;
+    if (!config.handleWildcardGenerics()) {
+      if (lhsTypeArgument.getKind().equals(TypeKind.WILDCARD)
+          || rhsTypeArgument.getKind().equals(TypeKind.WILDCARD)) {
+        // Preserve the pre-flag behavior of skipping wildcard-aware checks entirely.
+        return true;
+      }
+    } else {
+      // Do not use GenericsUtils.asWildcard() for the LHS. A captured LHS is a type variable, not a
+      // wildcard formal; unwrapping it can repeatedly expand recursive upper bounds (for example,
+      // F-bounded type parameters) as containment delegates back into subtype checking.  See test
+      // com.uber.nullaway.jspecify.WildcardTests.capturedLhsWithFBoundedTypeParametersDoesNotRecurse
+      Type.WildcardType lhsWildcard =
+          lhsTypeArgument instanceof Type.WildcardType wildcardType ? wildcardType : null;
+      Type.WildcardType rhsWildcard = GenericsUtils.asWildcard(rhsTypeArgument);
+      if (lhsWildcard != null) {
+        return wildcardContains(lhsWildcard, rhsTypeArgument);
+      }
+      if (rhsWildcard != null) {
+        // This case should only arise when generic method invocation inference / capture conversion
+        // lets a wildcard actual argument flow into a non-wildcard formal type argument, e.g.,
+        // passing Foo<? extends T> to <U> void m(Foo<U>). We do not yet support wildcard inference.
+        // For non-inference assignment / return / parameter checks, javac rejects these conversions
+        // before NullAway runs.
+        // TODO: Add proper support when inference for wildcards is implemented.
+        return true;
+      }
     }
-    boolean isLHSNullableAnnotated = genericsChecks.isNullableAnnotated(lhsTypeArgument);
-    boolean isRHSNullableAnnotated = genericsChecks.isNullableAnnotated(rhsTypeArgument);
+    return haveIdenticalNullability(lhsTypeArgument, rhsTypeArgument);
+  }
+
+  /**
+   * Returns whether two types have identical top-level nullability and compatible nested
+   * nullability.
+   */
+  private boolean haveIdenticalNullability(Type lhsType, Type rhsType) {
+    boolean isLHSNullableAnnotated = genericsChecks.isNullableAnnotated(lhsType);
+    boolean isRHSNullableAnnotated = genericsChecks.isNullableAnnotated(rhsType);
     if (isLHSNullableAnnotated != isRHSNullableAnnotated) {
       return false;
     }
-    return lhsTypeArgument.accept(this, rhsTypeArgument);
+    return lhsType.accept(this, rhsType);
   }
 
   /**

nullaway/src/test/java/com/uber/nullaway/jspecify/WildcardTests.java

@@ -4,6 +4,7 @@
 import com.uber.nullaway.NullAwayTestsBase;
 import com.uber.nullaway.generics.JSpecifyJavacConfig;
 import java.util.Arrays;
+import java.util.List;
 import org.junit.Test;
 
 public class WildcardTests extends NullAwayTestsBase {
@@ -406,6 +407,31 @@ static void testNonNullSuperBound(Foo<? super String> f) {
         .doTest();
   }
 
+  @Test
+  public void wildcardCaptureReturnPreservesNestedNullability() {
+    makeHelper()
+        .addSourceLines(
+            "Test.java",
+            """
+            import org.jspecify.annotations.NullMarked;
+            import org.jspecify.annotations.Nullable;
+            @NullMarked
+            class Test {
+              static class Box<T extends @Nullable Object> {}
+              static class Holder<T extends @Nullable Object> {
+                T get() {
+                  throw new RuntimeException();
+                }
+              }
+              static void test(Holder<? extends Box<@Nullable String>> holder) {
+                // BUG: Diagnostic contains: incompatible types
+                Box<String> box = holder.get();
+              }
+            }
+            """)
+        .doTest();
+  }
+
   @Test
   public void wildcardCaptureReturnWithTypeVariableUpperBound() {
     makeHelper()
@@ -979,6 +1005,57 @@ static class Analysis<
         .doTest();
   }
 
+  /** ensures we avoid a crash related to wildcards when wildcard handling is disabled */
+  @Test
+  public void methodRefParameterSuperWildcardWithHandlingDisabled() {
+    makeTestHelperWithArgs(
+            List.of(
+                "-XepOpt:NullAway:OnlyNullMarked=true",
+                JSpecifyJavacConfig.JSPECIFY_MODE_FLAG,
+                JSpecifyJavacConfig.ADD_TYPE_ANNOTATIONS_FLAG))
+        .addSourceLines(
+            "Test.java",
+            """
+            import java.util.function.Function;
+            import org.jspecify.annotations.NullMarked;
+            @NullMarked
+            final class Test {
+                static void reproduce() {
+                    getOrThrow(Test::throwAsUncheckedException);
+                }
+                private static void getOrThrow(Function<? super Exception, RuntimeException> exceptionTransformer) {
+                }
+                private static RuntimeException throwAsUncheckedException(Throwable throwable) {
+                    return new RuntimeException(throwable);
+                }
+            }
+            """)
+        .doTest();
+  }
+
+  /** reduced from a crasher found when checking junit */
+  @Test
+  public void methodRefReturnUnboundedWildcard() {
+    makeHelper()
+        .addSourceLines(
+            "Test.java",
+            """
+            package repro;
+            import java.util.concurrent.FutureTask;
+            import java.util.function.Supplier;
+            import org.jspecify.annotations.NullMarked;
+            import org.jspecify.annotations.Nullable;
+            @NullMarked
+            final class Test {
+                private final FutureTask<@Nullable Object> task;
+                Test(Supplier<?> delegate) {
+                    this.task = new FutureTask<>(delegate::get);
+                }
+            }
+            """)
+        .doTest();
+  }
+
   @Test
   public void nullableOnWildcard() {
     makeHelper()