Skip to content

JSpecify: false positive when a method returns ? extends T and T has a @Nullable upper bound (support NO_CHANGE nullness operator better) #1727

Open

Description

@vlsi

Version

NullAway 0.14.0, and current master (21f02dd). Error Prone 2.50.0, JDK 21.

Flags: -XepOpt:NullAway:JSpecifyMode=true -XepOpt:NullAway:HandleWildcardGenerics=true -XDaddTypeAnnotationsToSymbol=true. HandleWildcardGenerics defaults to JSpecifyExperimental, so JSpecifyExperimental=true triggers it too. With wildcard handling off, nothing is reported.

Summary

A method whose return type is a type variable R instantiated to ? extends T produces a @Nullable result whenever T is declared T extends @Nullable Object, even though the wildcard's bound is T itself. Returning that result as T is then reported, and so is passing it to anything that expects T.

The wildcard's bound is what gets lost: Box<? extends String> and Box<T> are both handled correctly, and ? extends T misbehaves only when T has a @Nullable upper bound. That suggests NullAway falls back to the type parameter's declared bound (@Nullable Object on Box's R) instead of reading the wildcard's own bound.

Reproducer

import org.jspecify.annotations.NullMarked;
import org.jspecify.annotations.Nullable;

@NullMarked
public class Test {
  interface Box<R extends @Nullable Object> {
    R get();
  }

  static <T extends @Nullable Object> T unwrap(Box<? extends T> b) {
    // BUG: Diagnostic contains: returning @Nullable expression from method with @NonNull return type
    return b.get();
  }
}
Test.java:11: warning: [NullAway] returning @Nullable expression from method with @NonNull return type
    return b.get();
    ^

Note that the message calls T a @NonNull return type although T is bounded by @Nullable Object.

What does and does not report

Same Box, same flags:

Signature Reported
<T extends @Nullable Object> T unwrap(Box<T> b) no
<T> T unwrap(Box<? extends T> b) no
String unwrap(Box<? extends String> b) no
<T extends @Nullable Object> Box<T> unwrap(Box<? extends Box<T>> b) no
<T extends @Nullable Object> T unwrap(Box<? extends T> b) yes

The result also poisons a local. Storing it in a T local reports nothing on its own, but every later use of that local that requires non-null is reported:

static <T extends @Nullable Object> void pass(Box<? extends T> b) {
  T t = b.get();
  // BUG: Diagnostic contains: passing @Nullable parameter 't' where @NonNull is required
  Test.<T>sink(t);
}

static <T extends @Nullable Object> void sink(T t) {}

Why the wildcard is the right declaration

The wildcard is not decoration. In a reduce whose accumulator starts at null:

static <S, A extends @Nullable Object> A aggregate(
    Iterable<S> source, A seed, Fn2<A, S, ? extends A> func) { ... }

callers pass reducers that accept a null accumulator but never produce one, so their type is Fn2<@Nullable Integer, Integer, Integer>. Without the wildcard, generic invariance rejects them against Fn2<A, S, A> where A is @Nullable Integer.

Workaround

@SuppressWarnings("NullAway") on the enclosing method.

Where we hit it

EnumerableDefaults.aggregate, while replacing the Checker Framework with NullAway in Apache Calcite (apache/calcite#5213).

Activity

  1. vlsi commented on Aug 24, 2026

    @vlsi
    CollaboratorAuthor

    Another sink for the same wrong nullness: assigning the result to a field of type T. Verified on master (21f02dd) with the same flags.

    import java.util.Iterator;
    import org.jspecify.annotations.NullMarked;
    import org.jspecify.annotations.Nullable;
    
    @NullMarked
    public class Test {
      interface Box<R extends @Nullable Object> {
        R get();
      }
    
      static class Cursor<T extends @Nullable Object> {
        private final Box<? extends T> box;
        T current;
    
        // BUG: Diagnostic contains: assigning @Nullable expression to @NonNull field
        Cursor(Box<? extends T> box) {
          this.box = box;
          this.current = box.get();
        }
    
        // BUG: Diagnostic contains: assigning @Nullable expression to @NonNull field
        void advance() {
          current = box.get();
        }
      }
    }

    The constructor assignment also drags a second, cascading report along with it:

    Test.java:17: warning: [NullAway] initializer method does not guarantee @NonNull field 'current' (line 14) is initialized along all control-flow paths (remember to check for exceptions or early returns).
    

    The field variant splits the same way as the return variant: Box<T> is clean, <T> ... Box<? extends T> with a non-null bound is clean, and only <T extends @Nullable Object> ... Box<? extends T> reports.

    A JDK type behaves identically, so library models are not involved:

    static class Cursor<T extends @Nullable Object> {
      private final Iterator<? extends T> iterator;
      T current;
    
      Cursor(Iterator<? extends T> iterator) {
        this.iterator = iterator;
        // BUG: Diagnostic contains: assigning @Nullable expression to @NonNull field
        this.current = iterator.next();
      }
    }

    Found in the same Calcite migration (apache/calcite#5213).

  2. msridhar commented on Aug 24, 2026

    @msridhar
    Collaborator

    @vlsi I just wanted to thank you quickly for all these great reports and for testing JSpecifyExperimental! It'll take us some time to go through the reports, particularly as I'm juggling a few other things. But, we'll get to them as soon as we can, and try to fix as much as possible for the next release.

  3. added
    jspecifyRelated to support for jspecify standard (see jspecify.dev)
    and removed on Aug 26, 2026
  4. changed the title [-]JSpecify: false positive when a method returns `? extends T` and `T` has a `@Nullable` upper bound[/-] [+]JSpecify: false positive when a method returns `? extends T` and `T` has a `@Nullable` upper bound (support parametric nullness)[/+] on Aug 26, 2026
  5. msridhar commented on Aug 26, 2026

    @msridhar
    Collaborator

    This one is going to require some work, since it requires better modeling the NO_CHANGE nullness operator from the JSpecify spec, rather than just treating expressions as being either @Nullable or @NonNull. When we have code like:

    static <T extends @Nullable Object> T unwrap(Box<? extends T> b) {
      // BUG: Diagnostic contains: returning @Nullable expression from method with @NonNull return type
      return b.get();
    }

    We want to reason that the nullability of b.get() will always be compatible with the nullness of T, so the code is safe. But, if we have code like:

    static class Foo<T extends @Nullable Object> {
      T get() { throw new RuntimeException(); }
    }
    static class NullableBound<U extends @Nullable Object> {
      void test(Foo<? extends U> f) {
        // BUG: Diagnostic contains: dereferenced expression 'f.get()' is @Nullable
        f.get().hashCode();
      }
    }

    We want to reason that f.get() could be @Nullable, so we should warn on a dereference (relevant spec section).

    @vlsi how critical is this one for you all? For now I plan to de-prioritize to see if there are easier issues we can quickly fix first.

  6. 11 remaining items

  7. vlsi commented on Oct 22, 2026

    @vlsi
    CollaboratorAuthor

    Status check on master at 9555b20: every case in this issue still behaves as reported, and #1767 and #1806 track them all as @Ignored tests in NullnessOperatorTests.

    • The false positives (the body's return and local-argument examples, the aggregate signature, and both field examples from the first comment) still reproduce with JSpecifyExperimental=true and share one cause, the missing NO_CHANGE operator. With JSpecifyMode=true alone, the return example is clean.
    • For aggregate, NullAway reports twice: passing @Nullable parameter 'acc' where @NonNull is required on acc = func.apply(acc, s);, as well as at return acc;.
    • The dereference through Foo<? extends U> from the 2026-08-26 comment is already reported, so that guard holds.
    • The t.hashCode() false negative from the 2026-08-28 comment involves no wildcard and is missed in plain JSpecify mode too. Since the plan is to get the non-wildcard cases working first, I've moved it to JSpecify: dereferencing a T value is not reported when T extends @Nullable Object #2027 so it can be fixed and closed on its own; issue1727NullableBoundDereferenceFalseNegative can point there.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions