An inference failure is reported together with the errors that follow from it

Open
  • Is something wrong?
#2,132 1 comment 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
38/100
Issue type
Bug
Clarity
Mostly clear
Activity status
Active
Tech stack
java
Domain
backend-api-design, testing

Research direction

Start by reading the inference-failure plan in #1551, the experimental warning work in #1649, and the related note-line discussion in #1828. Reproduce the reported cases through NullAwayTestsBase#compileAndReportDiagnostics and inspect the diagnostic reporting paths and generic inference tests named in the issue. Done means choosing how to suppress diagnostics subsumed by an inference failure and adding checks that catch extra diagnostics on affected lines.

Written by the indexing model from the issue text.

Description

With -XepOpt:NullAway:JSpecifyExperimental=true, which turns on WarnOnGenericInferenceFailure, a call whose type-argument inference fails gets the inference failure warning and, on the same line, one or two more diagnostics caused by the same mismatch.

import org.jspecify.annotations.NullMarked;
import org.jspecify.annotations.Nullable;
@NullMarked
class Test {
  static class Box<T extends @Nullable Object> {}
  static <U> Box<U> boxOf(U u) { return new Box<>(); }
  static <U> void take(U u) {}
  static void takeNullableBox(Box<@Nullable Object> b) {}
  static @Nullable String maybe() { return null; }
  void test() {
    take(maybe());
    takeNullableBox(boxOf(new Object()));
    takeNullableBox(boxOf(null));
  }
}

Flags: -XepOpt:NullAway:OnlyNullMarked=true -XepOpt:NullAway:JSpecifyMode=true -XDaddTypeAnnotationsToSymbol=true -XepOpt:NullAway:JSpecifyExperimental=true

Test.java:11: [NullAway] inference failure: type variable U is constrained to be @Nullable, but its upper bound requires it to be @NonNull
Test.java:11: [NullAway] passing @Nullable parameter 'maybe()' where @NonNull is required
Test.java:12: [NullAway] inference failure: type variable U is constrained to be @Nullable, but its upper bound requires it to be @NonNull
Test.java:12: [NullAway] incompatible types: Box<Object> cannot be converted to Box<@Nullable Object>
Test.java:13: [NullAway] inference failure: type variable U is constrained to be @Nullable, but its upper bound requires it to be @NonNull
Test.java:13: [NullAway] incompatible types: Box<Object> cannot be converted to Box<@Nullable Object>
Test.java:13: [NullAway] passing @Nullable parameter 'null' where @NonNull is required

Reproduced on master (6dbd9a004), Error Prone 2.52.0, JDK 21.0.9, through NullAwayTestsBase#compileAndReportDiagnostics.

Expected behavior

Lines 11 and 12 each get one diagnostic. Which one to keep, the inference failure or the concrete mismatch, is the project's choice. On line 13 both the null argument and the Box<@Nullable Object> target constrain U, so I cannot tell whether passing @Nullable parameter 'null' is a separate error there or is subsumed too.

This follows the plan in #1551: "report inference failures as errors by default and to suppress warnings that are subsumed by the inference failure as best we can". #1649 enabled the warning in experimental mode; the diagnostics above still appear alongside it.

How common it is

I wrapped every state.reportMatch call to log the file, line, and message, and ran ./gradlew :nullaway:test on 6dbd9a004. 130 source lines in 69 test methods carry an inference failure diagnostic plus at least one more NullAway diagnostic on the same line, mostly in GenericMethodTests (71 lines), GenericMethodLambdaOrMethodRefArgTests (14), ConditionalExprTests (14), and GenericDiamondTests (10). I have not checked that the second diagnostic follows from the failure on every one of these lines; a line can hold two unrelated calls.

The tests do not catch this because a // BUG: Diagnostic contains: marker passes when the marked line has more diagnostics than the marker matches. The review of #2080 found the same pattern by hand: on the branch, acceptsNullableInput(Test::nonNullVarargs) with @SafeVarargs static <V> Object nonNullVarargs(V... values) gets both inference failure: type variable V ... and parameter values of referenced method is @NonNull ....

Questions
  1. Should a failed inference produce one diagnostic per call? I see three ways to get there:

    • (a) Keep the inference failure warning and drop the diagnostics it causes, as #1551 plans.

    • (b) Keep the concrete mismatch and drop the inference failure. #1551 points out that the concrete message alone misleads, since NullAway did not actually infer @NonNull.

    • (c) Keep the concrete mismatch, drop the inference failure, and move its cause into a note: line of the remaining diagnostic. The first line stays as it is today. A sketch for line 11, wording open:

      passing @Nullable parameter 'maybe()' where @NonNull is required
    note: parameter u of take(U) has type U, whose upper bound Object excludes null

      #1828, still open, adds a note: line to incompatible nullability messages to say which type parameter a wildcard bound comes from. Option (c) would use the same line for the bound of an inferred type variable.

  2. If yes, should the tests pin the count? CompilationTestHelper cannot (it is final, and a marker does not limit the diagnostics on its line), so this would be an opt-in check built on compileAndReportDiagnostics, for example "no other NullAway diagnostic on a line that has an inference failure". A blanket "one diagnostic per line" rule does not work: the same run found 26 other lines with two legitimate diagnostics, such as i + j with both operands @Nullable, or both branches of a conditional.
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.