JSpecify mode: `CastToNonNullMethod` reports `castNonNull(t)` as passing a known `@NonNull` value when the type variable of `t` admits null

Open
  • Is something wrong?
#1,961 0 comments 0 reactions 0 assignees View on GitHub

Maintainers usually reply within 1 day

Nobody has claimed this yet.

Assessment

Difficulty
4/5
Estimated time
3-5 days
Newbie friendliness
55/100
Issue type
Bug
Clarity
Clearly specified
Activity status
Active
Tech stack
java
Domain
compilers, devtools

Research direction

Start with checkCastToNonNullTakesNullable and mayBeNullExpr in NullAway's main NullAway.java, since the issue names both and says mayBeNullExpr treats a type-variable-typed expression as non-null. Reproduce it with the test sample in nullaway/src/test/java/com/uber/nullaway/jspecify/ using the listed flags. Done means no diagnostic on the cast line, while the bare return t case keeps its current behaviour.

Written by the indexing model from the issue text.

Description

false positive jspecify

With JSpecify mode on and a CastToNonNullMethod configured, castNonNull(t) on a value whose type is a type variable declared T extends @Nullable Object is reported as "passing known @NonNull parameter 't'". The value is not known to be non-null: a caller may instantiate T as @Nullable String. The diagnostic asks the author to remove a cast that states a real assumption, and once it is removed nothing reports the bare return t that remains, since checking generic class internals is a documented outstanding feature.

Versions: NullAway master at bc2fee556 (0.14.3-SNAPSHOT) and the 0.14.2 release. Error Prone 2.50.0, JDK 21.0.9. Flags: -XepOpt:NullAway:JSpecifyMode=true -XDaddTypeAnnotationsToSymbol=true -XepOpt:NullAway:OnlyNullMarked=true -XepOpt:NullAway:CastToNonNullMethod=Util.castNonNull.

Code sample. I ran it as a test in nullaway/src/test/java/com/uber/nullaway/jspecify/, with makeTestHelperWithArgs(JSpecifyJavacConfig.withJSpecifyModeArgs(...)) and the flags above.

// Util.java
import org.jspecify.annotations.NullMarked;
import org.jspecify.annotations.Nullable;
@NullMarked
class Util {
  @SuppressWarnings("NullAway")
  static <T extends @Nullable Object> T castNonNull(@Nullable T t) {
    return t;
  }
}

// Test.java
import org.jspecify.annotations.NullMarked;
import org.jspecify.annotations.Nullable;
@NullMarked
class Test<T extends @Nullable Object> {
  Object cast(T t) {
    return Util.castNonNull(t);
  }
  Object plain(T t) {
    return t;
  }
}

Expected: no diagnostic on Test.java line 6. The message's own text states the rule: the argument "should only take values that NullAway considers @Nullable at the invocation site, but which are known not to be null at runtime". Under JSpecify a value of a type variable whose bound is @Nullable Object may be null (new Test<@Nullable String>().cast(null) is a legal call), so t is the kind of value the cast exists for. What plain does on line 9 is out of scope here: it is the "checking of generic class implementations" item the wiki lists as not yet supported. The requirement is only that the cast on line 6 is not reported as passing a known @NonNull value; whether the check stays silent or reports something else is the project's choice.

Actual: on master and on 0.14.2:

Saw unexpected error on line 6. All errors:
/Test.java:6: warning: [NullAway] passing known @NonNull parameter 't' to CastToNonNullMethod (Util.castNonNull) at position 0. This method argument should only take values that NullAway considers @Nullable at the invocation site, but which are known not to be null at runtime.
    return Util.castNonNull(t);
                           ^
    (see http://t.uber.com/nullaway )

Line 9 compiles with no diagnostic, so removing the cast as the message suggests leaves Object plain(T t) { return t; }, which declares a non-null result for a value that may be null. This happened in apache/calcite#5213: following the diagnostic, the migration dropped castNonNull from FlatLists.Flat1List…Flat6List.toArray(), whose elements are of a T extends @Nullable Object, and the method ended up declared Object[] toArray() instead of @Nullable Object[], with no diagnostic anywhere.

Analysis, a guess; the report stands without it. The check in NullAway.checkCastToNonNullTakesNullable reports when !mayBeNullExpr(state, actual), and mayBeNullExpr treats an expression whose type is a type variable as non-null whatever the variable's bound, which is the same reason line 9 is accepted. Excluding from this one check an argument whose type is a type variable with a @Nullable upper bound (or whose bound otherwise admits null) would stop the report without touching the wider limitation.

Related: #1945 and #1946 (other places where a type variable whose bound admits null is treated as excluding null); the array-initializer gap (#1960) hides the same value when it is written into new Object[] {t}.

Assisted-by: Claude Code (claude-opus-5-5)

Dominant language
Java
Stars
4.1k
Forks
381
Avg merge
1d 2h
Merged PRs (30d)
106

Getting set up

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

More from uber/NullAway

All issues in uber/NullAway

Similar issues

More Java issues

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.