Repository navigation
Remove unrecognized JSpecify local annotations - #2008
Merged
ben-manes merged 1 commit intoSep 3, 2026
Merged
Conversation
JSpecify does not recognize nullness annotations on the root type of a local variable. NullAway reports the two annotations in LocalAsyncCache as warnings, and the build treats warnings as errors. Remove the root annotations while retaining the existing null checks and nested type-use annotations.
vlsi
requested review from
ben-manes,
cpovirk,
gilga1983,
kluever and
ohadeytan
as code owners
September 3, 2026 14:20
|
|
Contributor
There was a problem hiding this comment.
🟢 Approval recommended
The change is narrowly scoped to removing problematic local annotations and preserves the existing runtime null-handling behavior while addressing the build-breaking warnings.
Pull request overview
This PR resolves NullAway’s JSpecify checker warnings (treated as errors under -Werror) by removing root local-variable nullness annotations in LocalAsyncCache, relying on existing null-handling logic instead.
Changes:
- Removed the local
@Nullableannotation from thevalueretrieved viafuture.getNow(null)incomposeResult. - Removed the local
@Nullableannotation from thefuturereturned bycomputeIfAbsentinLocalAsyncCache.
File summaries
| File | Description |
|---|---|
| caffeine/src/main/java/com/github/benmanes/caffeine/cache/LocalAsyncCache.java | Removes two root local-variable nullness annotations to avoid NullAway JSpecify “unrecognized location” warnings while preserving existing null checks. |
Review details
- Files reviewed: 1/1 changed files
- Comments generated: 0
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
ben-manes
approved these changes
Sep 3, 2026
This was referenced Sep 3, 2026
Merged
vlsi
added a commit
to vlsi/caffeine
that referenced
this pull request
Sep 4, 2026
JSpecify does not recognize a nullness annotation on the root type of a local variable, so `@Nullable V value = cache().compute(...)` makes no claim about nullness even though it reads like one. NullAway's new `JSpecifyUnrecognizedAnnotationLocation` check reports each of them: ```text A nullness annotation on the root type of a local variable has no meaning under JSpecify. ``` The build promotes every Error Prone suggestion to a warning (`allSuggestionsAsWarnings`) and CI compiles with `-Werror`, so the 53 annotations left in caffeine, jcache and simulator fail the build as soon as the check is released. ben-manes#2008 removed the first two, and the `Build caffeine with snapshot` job of uber/NullAway#1787 is red on the rest today. Drop the annotation, and let `var` carry the declaration wherever the local allows it, which is 43 of the 53. The null checks, the nested type-use annotations such as `(K k, @nullable V currentValue) ->`, and `@Var` stay where they are. Ten declarations keep an explicit type for reasons that have nothing to do with nullness: five have no initializer, `WindowTinyLfuPolicy.evictFromWindow` initializes from `null`, which javac cannot infer a type from, and the four reads of `lvElement` in `MpscGrowableArrayQueue` are declared `Object` on purpose, since `var` there infers `E` and makes the `(E) e` casts that follow redundant. `.claude/rules/errorprone.md` warned that `var` widens an inferred `@Nullable` type to non-null, and told the reader to give the local an explicit `@Nullable`-typed declaration, which is the annotation this check reports. The bullet was written against NullAway 0.13.8 and no longer holds: its own example, `values.stream().reduce(...).orElse(null)` assigned to a local and returned as `@Nullable T`, compiles clean under 0.14.2-SNAPSHOT both with `var` and with an explicit type, as do the 43 locals converted here. Remove it. Every Java compile task in the four modules, tests and the specialized suites included, builds against a NullAway snapshot carrying the check and reports nothing. The jcache and simulator suites pass; the caffeine `@CacheSpec` matrix is left to CI.
ben-manes
pushed a commit
that referenced
this pull request
Sep 4, 2026
JSpecify does not recognize a nullness annotation on the root type of a local variable, so `@Nullable V value = cache().compute(...)` makes no claim about nullness even though it reads like one. NullAway's new `JSpecifyUnrecognizedAnnotationLocation` check reports each of them: ```text A nullness annotation on the root type of a local variable has no meaning under JSpecify. ``` The build promotes every Error Prone suggestion to a warning (`allSuggestionsAsWarnings`) and CI compiles with `-Werror`, so the 53 annotations left in caffeine, jcache and simulator fail the build as soon as the check is released. #2008 removed the first two, and the `Build caffeine with snapshot` job of uber/NullAway#1787 is red on the rest today. Drop the annotation, and let `var` carry the declaration wherever the local allows it, which is 43 of the 53. The null checks, the nested type-use annotations such as `(K k, @nullable V currentValue) ->`, and `@Var` stay where they are. Ten declarations keep an explicit type for reasons that have nothing to do with nullness: five have no initializer, `WindowTinyLfuPolicy.evictFromWindow` initializes from `null`, which javac cannot infer a type from, and the four reads of `lvElement` in `MpscGrowableArrayQueue` are declared `Object` on purpose, since `var` there infers `E` and makes the `(E) e` casts that follow redundant. `.claude/rules/errorprone.md` warned that `var` widens an inferred `@Nullable` type to non-null, and told the reader to give the local an explicit `@Nullable`-typed declaration, which is the annotation this check reports. The bullet was written against NullAway 0.13.8 and no longer holds: its own example, `values.stream().reduce(...).orElse(null)` assigned to a local and returned as `@Nullable T`, compiles clean under 0.14.2-SNAPSHOT both with `var` and with an explicit type, as do the 43 locals converted here. Remove it. Every Java compile task in the four modules, tests and the specialized suites included, builds against a NullAway snapshot carrying the check and reports nothing. The jcache and simulator suites pass; the caffeine `@CacheSpec` matrix is left to CI.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Why
NullAway’s JSpecify checker reports root-type nullness annotations on local variables as unrecognized locations. With -Werror, the two warnings in LocalAsyncCache fail compilation.
What
Remove the two root local-variable annotations. Their nullability is still handled by the existing null checks, and no nested type-use annotation changes.
Verification
./gradlew --quiet :caffeine:compileJava
Related: uber/NullAway#1787