Unify method-reference return-nullness handling for contracts and library models

Open
  • Is something wrong?
#1,986 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
48/100
Issue type
Bug
Clarity
Clearly specified
Activity status
Active
Tech stack
java
Domain
backend

Research direction

Read ContractHandler.onMatchMethodReference, onOverrideMethodReferenceReturnNullability in LibraryModelsHandler, and ContractsTests.contractMethodReferenceReturnNullnessCurrentBehaviorJSpecify first. Trace how each path derives argument nullness for bound and unbound references and feeds compatibility checks and generic inference. Unify those paths while keeping stream-filter refinements separate, then update the regression test to verify the contract cases are accepted and nullable inputs remain rejected.

Written by the indexing model from the issue text.

Description

bug false positive jdk-models jspecify

Follow-up to #1968 and this review discussion.

Method-reference return nullness currently travels through two paths. ContractHandler.onMatchMethodReference uses setComputedNullness for override checks, while the new onMethodReferenceReturnNullnessHint hook is used by LibraryModelsHandler for both return compatibility and generic inference. Equivalent contracts and library models therefore behave differently. The divergence includes false positives for non-null inputs and a false negative for a type-variable input whose nullness is unresolved.

With #1968 in JSpecify mode, Class.cast references support bound and unbound references and generic stream inference. For a method annotated @Contract("!null -> !null") with a nullable declared return, the current behavior is:

Case Current result
Static or bound reference assigned to Function<String, String> Accepted
Unbound reference assigned to BiFunction<Test, String, String> Rejected as returning @Nullable
Stream<String>.map(Test::id) returned as Stream<String> Inference failure: R constrained to both @NonNull and @Nullable
Equivalent lambdas Accepted
<T extends @Nullable Object> Function<T, String> using Test::str Accepted despite unresolved input nullness (false negative)
Same type-variable target using String.class::cast Rejected as returning @Nullable

For example:

import java.util.function.BiFunction;
import java.util.function.Function;
import java.util.stream.Stream;
import org.jetbrains.annotations.Contract;
import org.jspecify.annotations.NullMarked;
import org.jspecify.annotations.Nullable;

@NullMarked
class Test {
  @Contract("!null -> !null")
  static @Nullable String id(@Nullable String s) { return s; }

  @Contract("!null -> !null")
  @Nullable String self(@Nullable String s) { return s; }

  @Contract("!null -> !null")
  static @Nullable String str(@Nullable Object o) {
    return o == null ? null : o.toString();
  }

  Function<String, String> staticReference() {
    return Test::id; // Accepted.
  }

  BiFunction<Test, String, String> unboundReference() {
    return Test::self; // False positive: referenced method returns @Nullable.
  }

  Stream<String> streamMethodReference(Stream<String> input) {
    return input.map(Test::id); // False positive: conflicting nullness constraints on R.
  }

  Stream<String> streamLambda(Stream<String> input) {
    return input.map(s -> id(s)); // Accepted.
  }

  <T extends @Nullable Object> Function<T, String> contractTypeVariable() {
    return Test::str; // False negative: accepted even though T may admit null.
  }

  <T extends @Nullable Object> Function<T, String> libraryModelTypeVariable() {
    return String.class::cast; // Rejected: referenced method returns @Nullable.
  }
}

The regression test ContractsTests.contractMethodReferenceReturnNullnessCurrentBehaviorJSpecify pins the unbound-reference and stream-inference behavior pending the follow-up.

For the type-variable case, ContractHandler.funcInterfaceParamIsNonNull treats the parameter of type T as non-null and sets the computed return nullness to non-null. The library-model path instead uses GenericsChecks.getTypeNullnessHint, which returns UNKNOWN for an unannotated type variable with a nullable upper bound. The contract only guarantees a non-null result for non-null inputs; it cannot guarantee a non-null result for every permitted instantiation of T. The contract reference should therefore be rejected just as the Class.cast reference is. This comparison was identified in the re-review of #1968.

A possible direction is to use one handler path for contract- and library-model-based method-reference return nullness in both compatibility checking and generic inference. Share argument-nullness derivation, including declaration annotations, substituted type arguments, unresolved type variables, and unbound receiver offsets. Keep flow-dependent stream-filter refinements separate.

The fix should accept the non-null contract cases above, reject the nullable-bound type-variable contract reference, retain rejection when inputs may be null, and remain conservative for unresolved nullness and unsupported reference forms. Update the pinned regression test when the two paths are unified.

Dominant language
Java
Stars
4.1k
Forks
379
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.