Repository navigation
Remove nullness annotations on local variable root types - #2011
Conversation
There was a problem hiding this comment.
🟢 Approval recommended
The changes are mechanical nullness-annotation removals aligned with JSpecify/NullAway behavior and do not alter runtime logic.
Pull request overview
This PR removes @Nullable annotations applied to the root type of local variables, which JSpecify treats as a no-op and which NullAway will soon warn on (JSpecifyUnrecognizedAnnotationLocation) under the project’s -Werror build configuration.
Changes:
- Removed root-type
@Nullableannotations from local variables across caffeine, jcache, and simulator code, relying on NullAway’s local nullness inference from assignments instead. - Dropped an unused
org.jspecify.annotations.Nullableimport inCacheusPolicy. - Updated
.claude/rules/errorprone.mdguidance to discouragevarand root local@Nullablein this scenario.
File summaries
| File | Description |
|---|---|
| simulator/src/main/java/com/github/benmanes/caffeine/cache/simulator/policy/two_queue/TwoQueuePolicy.java | Removes root local @Nullable on a data.get result. |
| simulator/src/main/java/com/github/benmanes/caffeine/cache/simulator/policy/two_queue/TuQueuePolicy.java | Removes root local @Nullable on a data.get result. |
| simulator/src/main/java/com/github/benmanes/caffeine/cache/simulator/policy/two_queue/S3FifoPolicy.java | Removes root local @Nullable on a reassigned local used in null checks. |
| simulator/src/main/java/com/github/benmanes/caffeine/cache/simulator/policy/sketch/WindowTinyLfuPolicy.java | Removes root local @Nullable on locals inferred nullable via assignments/null initializers. |
| simulator/src/main/java/com/github/benmanes/caffeine/cache/simulator/policy/sketch/segment/S4WindowTinyLfuPolicy.java | Removes root local @Nullable on a data.get result. |
| simulator/src/main/java/com/github/benmanes/caffeine/cache/simulator/policy/sketch/segment/RandomWindowTinyLfuPolicy.java | Removes root local @Nullable on a mutable local assigned from data.get. |
| simulator/src/main/java/com/github/benmanes/caffeine/cache/simulator/policy/sketch/segment/LruWindowTinyLfuPolicy.java | Removes root local @Nullable on a mutable local assigned from data.get. |
| simulator/src/main/java/com/github/benmanes/caffeine/cache/simulator/policy/sketch/segment/FullySegmentedWindowTinyLfuPolicy.java | Removes root local @Nullable on a data.get result. |
| simulator/src/main/java/com/github/benmanes/caffeine/cache/simulator/policy/sketch/feedback/FeedbackWindowTinyLfuPolicy.java | Removes root local @Nullable on a data.get result. |
| simulator/src/main/java/com/github/benmanes/caffeine/cache/simulator/policy/sketch/feedback/FeedbackTinyLfuPolicy.java | Removes root local @Nullable on a data.get result. |
| simulator/src/main/java/com/github/benmanes/caffeine/cache/simulator/policy/sketch/climbing/HillClimberWindowTinyLfuPolicy.java | Removes root local @Nullable on a data.get result. |
| simulator/src/main/java/com/github/benmanes/caffeine/cache/simulator/policy/linked/SievePolicy.java | Removes root local @Nullable on a data.get result. |
| simulator/src/main/java/com/github/benmanes/caffeine/cache/simulator/policy/linked/SegmentedLruPolicy.java | Removes root local @Nullable on a data.get result. |
| simulator/src/main/java/com/github/benmanes/caffeine/cache/simulator/policy/linked/S4LruPolicy.java | Removes root local @Nullable on a data.get result. |
| simulator/src/main/java/com/github/benmanes/caffeine/cache/simulator/policy/linked/MultiQueuePolicy.java | Removes root local @Nullable on a mutable local assigned from data.get. |
| simulator/src/main/java/com/github/benmanes/caffeine/cache/simulator/policy/linked/LinkedPolicy.java | Removes root local @Nullable on a data.get result. |
| simulator/src/main/java/com/github/benmanes/caffeine/cache/simulator/policy/linked/FrequentlyUsedPolicy.java | Removes root local @Nullable on a data.get result. |
| simulator/src/main/java/com/github/benmanes/caffeine/cache/simulator/policy/irr/LirsPolicy.java | Removes root local @Nullable on a mutable local assigned from data.get. |
| simulator/src/main/java/com/github/benmanes/caffeine/cache/simulator/policy/irr/Lirs2Policy.java | Removes root local @Nullable on a mutable local assigned from data.get. |
| simulator/src/main/java/com/github/benmanes/caffeine/cache/simulator/policy/irr/IndicatorFrdPolicy.java | Removes root local @Nullable on a mutable local assigned from data.get. |
| simulator/src/main/java/com/github/benmanes/caffeine/cache/simulator/policy/irr/HillClimberFrdPolicy.java | Removes root local @Nullable on a mutable local assigned from data.get. |
| simulator/src/main/java/com/github/benmanes/caffeine/cache/simulator/policy/irr/FrdPolicy.java | Removes root local @Nullable on a mutable local assigned from data.get. |
| simulator/src/main/java/com/github/benmanes/caffeine/cache/simulator/policy/irr/DClockPolicy.java | Removes root local @Nullable on a data.get result. |
| simulator/src/main/java/com/github/benmanes/caffeine/cache/simulator/policy/irr/ClockProSimplePolicy.java | Removes root local @Nullable on a data.get result. |
| simulator/src/main/java/com/github/benmanes/caffeine/cache/simulator/policy/irr/ClockProPolicy.java | Removes root local @Nullable on locals inferred nullable via assignments/null checks. |
| simulator/src/main/java/com/github/benmanes/caffeine/cache/simulator/policy/irr/ClockProPlusPolicy.java | Removes root local @Nullable on locals inferred nullable via assignments/null checks. |
| simulator/src/main/java/com/github/benmanes/caffeine/cache/simulator/policy/greedy_dual/GDWheelPolicy.java | Removes root local @Nullable on a mutable local assigned from data.get. |
| simulator/src/main/java/com/github/benmanes/caffeine/cache/simulator/policy/greedy_dual/GdsfPolicy.java | Removes root local @Nullable on a data.get result. |
| simulator/src/main/java/com/github/benmanes/caffeine/cache/simulator/policy/greedy_dual/CampPolicy.java | Removes root local @Nullable on a data.get result. |
| simulator/src/main/java/com/github/benmanes/caffeine/cache/simulator/policy/adaptive/CacheusPolicy.java | Removes root local @Nullable and drops the now-unused @Nullable import. |
| simulator/src/main/java/com/github/benmanes/caffeine/cache/simulator/policy/adaptive/ArcPolicy.java | Removes root local @Nullable on a data.get result. |
| simulator/src/main/java/com/github/benmanes/caffeine/cache/simulator/BasicSettings.java | Removes root local @Nullable on a parsed value that is explicitly null-checked. |
| jcache/src/main/java/com/github/benmanes/caffeine/jcache/LoadingCacheProxy.java | Removes root local @Nullable on a local assigned from @Nullable getOrLoad. |
| jcache/src/main/java/com/github/benmanes/caffeine/jcache/CacheProxy.java | Removes root local @Nullable on locals inferred nullable through assignments/branches. |
| caffeine/src/main/java/com/github/benmanes/caffeine/cache/MpscGrowableArrayQueue.java | Removes root local @Nullable on locals guarded by null checks / requireNonNull. |
| caffeine/src/main/java/com/github/benmanes/caffeine/cache/LocalLoadingCache.java | Removes root local @Nullable on a local assigned from compute(...). |
| caffeine/src/main/java/com/github/benmanes/caffeine/cache/Interner.java | Removes root local @Nullable on putIfAbsent result checked for null. |
| caffeine/src/main/java/com/github/benmanes/caffeine/cache/BoundedLocalCache.java | Removes root local @Nullable on locals inferred nullable via assignments and explicit null checks. |
| .claude/rules/errorprone.md | Updates NullAway guidance: avoid var here and avoid root local @Nullable under JSpecify. |
Review details
- Files reviewed: 39/39 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.
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.
efadf52 to
b9f199a
Compare
There was a problem hiding this comment.
🟢 Approval recommended
The changes are mechanical, preserve existing null checks/logic, and are aligned with the stated goal of preventing upcoming NullAway JSpecify-local warnings from breaking builds.
Review details
- Files reviewed: 39/39 changed files
- Comments generated: 0 new
- Review effort level: Lite
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 newJSpecifyUnrecognizedAnnotationLocationcheck reports each of them: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 theBuild caffeine with snapshotjob of uber/NullAway#1787 is red on the rest today.Drop the annotation, and let
varcarry 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@Varstay where they are. Ten declarations keep an explicit type for reasons that have nothing to do with nullness: five have no initializer,WindowTinyLfuPolicy.evictFromWindowinitializes fromnull, which javac cannot infer a type from, and the four reads oflvElementinMpscGrowableArrayQueueare declaredObjecton purpose, sincevarthere infersEand makes the(E) ecasts that follow redundant..claude/rules/errorprone.mdwarned thatvarwidens an inferred@Nullabletype 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 withvarand 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
@CacheSpecmatrix is left to CI.