Add library model for AtomicReferenceFieldUpdater by msridhar · Pull Request #1409 · uber/NullAway

msridhar · 2025-12-26T00:47:17Z

Leverages recent new functionality in LibraryModels (#1407, #1345) to properly support inference for calls to the AtomicReferenceFieldUpdater::newUpdater generic method.

Summary by CodeRabbit

Release Notes

Improvements
- Enhanced null-safety analysis for AtomicReferenceFieldUpdater to properly support nullable type variable upper bounds and nested annotations for generic method parameters.
Tests
- Added test coverage for AtomicReferenceFieldUpdater functionality within JDK library integration and null-marked code scenarios.

_{✏️ Tip: You can customize this high-level summary in your review settings.}

coderabbitai · 2025-12-26T00:50:29Z

Walkthrough

This PR introduces support for nullable upper bounds on type variables and nested annotations specifically for the AtomicReferenceFieldUpdater.newUpdater method in NullAway's library models. The changes include: adding a new NULLABLE_METHOD_TYPE_VARIABLE_UPPER_BOUNDS multimap marking the type parameter index as nullable, adding a NESTED_ANNOTATIONS_FOR_METHODS map to apply @NonNull annotations on type arguments, updating NULLMARKED_CLASSES to include AtomicReferenceFieldUpdater, updating NULLABLE_VARIABLE_TYPE_UPPER_BOUNDS with an entry for the same class, and exposing two new public override methods in the DefaultLibraryModels inner class. A test method is added to verify AtomicReferenceFieldUpdater behavior in a @NullMarked context.

Possibly related PRs

PR #1407: Directly adds nested-annotation library-model entries for AtomicReferenceFieldUpdater using the nested-annotation machinery.
PR #1388: Modifies onOverrideMethodTypeVariableUpperBound to use the newly exposed methodTypeVariablesWithNullableUpperBounds accessor.
PR #1370: Adds comprehensive method type-parameter nullable-upper-bound support across LibraryModelsHandler and related files.

Suggested reviewers

yuxincs
lazaroclapp

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 25.00% which is insufficient. The required threshold is 80.00%.	You can run `@coderabbitai generate docstrings` to improve docstring coverage.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	Title clearly and concisely summarizes the main change: adding a library model for AtomicReferenceFieldUpdater, which matches the changeset.
Linked Issues check	✅ Passed	Changes implement proper nullability handling for AtomicReferenceFieldUpdater.newUpdater by adding library models with nullable upper bounds and nested annotations, directly addressing issue #1157's requirement.
Out of Scope Changes check	✅ Passed	All changes are scoped to supporting AtomicReferenceFieldUpdater library models; test additions verify the implementation without introducing unrelated modifications.

✨ Finishing touches

📝 Generate docstrings

🧪 Generate unit tests (beta)

Create PR with unit tests
Post copyable unit tests in a comment
Commit unit tests in branch fix-1157

📜 Recent review details

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 17d3462 and 526f063.

📒 Files selected for processing (2)

nullaway/src/main/java/com/uber/nullaway/handlers/LibraryModelsHandler.java
nullaway/src/test/java/com/uber/nullaway/jspecify/GenericMethodTests.java

🧰 Additional context used

🧠 Learnings (4)

📓 Common learnings

Learnt from: msridhar
Repo: uber/NullAway PR: 1248
File: nullaway/src/main/java/com/uber/nullaway/generics/GenericsChecks.java:847-857
Timestamp: 2025-08-28T04:54:20.953Z
Learning: In NullAway's GenericsChecks.java, NewClassTree support for explicit type argument substitution requires more extensive changes beyond just modifying the conditional in compareGenericTypeParameterNullabilityForCall. The maintainers prefer to handle NewClassTree support in a separate follow-up rather than expanding the scope of PRs focused on specific issues like super constructor calls.

Learnt from: msridhar
Repo: uber/NullAway PR: 1316
File: jdk-javac-plugin/src/main/java/com/uber/nullaway/javacplugin/NullnessAnnotationSerializer.java:261-293
Timestamp: 2025-10-29T23:56:18.236Z
Learning: In NullAway's jdk-javac-plugin NullnessAnnotationSerializer, type variable bounds with annotations (e.g., `T extends Nullable Object`) are checked at their declaration sites by the typeParamHasAnnotation method for both class-level and method-level type parameters. The hasJSpecifyAnnotationDeep method is designed to check type uses (return types, parameters, etc.) and does not need a TYPEVAR case because type variable declaration bounds are already handled separately.

Learnt from: msridhar
Repo: uber/NullAway PR: 1245
File: guava-recent-unit-tests/src/test/java/com/uber/nullaway/guava/NullAwayGuavaParametricNullnessTests.java:101-102
Timestamp: 2025-08-14T18:50:06.159Z
Learning: In NullAway JSpecify tests, when JDK version requirements exist due to bytecode annotation reading capabilities, prefer failing tests over skipping them on unsupported versions to ensure CI catches regressions and enforces proper JDK version usage for developers.

Learnt from: msridhar
Repo: uber/NullAway PR: 1259
File: jdk-recent-unit-tests/src/test/java/com/uber/nullaway/jdk17/SwitchTests.java:318-321
Timestamp: 2025-08-29T18:41:43.584Z
Learning: Classes annotated with NullMarked are analyzed by NullAway even if they are not in packages specified by the AnnotatedPackages configuration. The NullMarked annotation guarantees NullAway analysis.

📚 Learning: 2025-08-28T04:54:20.953Z

Learnt from: msridhar
Repo: uber/NullAway PR: 1248
File: nullaway/src/main/java/com/uber/nullaway/generics/GenericsChecks.java:847-857
Timestamp: 2025-08-28T04:54:20.953Z
Learning: In NullAway's GenericsChecks.java, NewClassTree support for explicit type argument substitution requires more extensive changes beyond just modifying the conditional in compareGenericTypeParameterNullabilityForCall. The maintainers prefer to handle NewClassTree support in a separate follow-up rather than expanding the scope of PRs focused on specific issues like super constructor calls.

Applied to files:

nullaway/src/main/java/com/uber/nullaway/handlers/LibraryModelsHandler.java
nullaway/src/test/java/com/uber/nullaway/jspecify/GenericMethodTests.java

📚 Learning: 2025-10-29T23:56:18.236Z

Learnt from: msridhar
Repo: uber/NullAway PR: 1316
File: jdk-javac-plugin/src/main/java/com/uber/nullaway/javacplugin/NullnessAnnotationSerializer.java:261-293
Timestamp: 2025-10-29T23:56:18.236Z
Learning: In NullAway's jdk-javac-plugin NullnessAnnotationSerializer, type variable bounds with annotations (e.g., `T extends Nullable Object`) are checked at their declaration sites by the typeParamHasAnnotation method for both class-level and method-level type parameters. The hasJSpecifyAnnotationDeep method is designed to check type uses (return types, parameters, etc.) and does not need a TYPEVAR case because type variable declaration bounds are already handled separately.

Applied to files:

nullaway/src/main/java/com/uber/nullaway/handlers/LibraryModelsHandler.java

📚 Learning: 2025-08-14T18:50:06.159Z

Learnt from: msridhar
Repo: uber/NullAway PR: 1245
File: guava-recent-unit-tests/src/test/java/com/uber/nullaway/guava/NullAwayGuavaParametricNullnessTests.java:101-102
Timestamp: 2025-08-14T18:50:06.159Z
Learning: In NullAway JSpecify tests, when JDK version requirements exist due to bytecode annotation reading capabilities, prefer failing tests over skipping them on unsupported versions to ensure CI catches regressions and enforces proper JDK version usage for developers.

Applied to files:

nullaway/src/test/java/com/uber/nullaway/jspecify/GenericMethodTests.java

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)

GitHub Check: Build and test on ubuntu-latest
GitHub Check: Build and test on windows-latest
GitHub Check: Build spring-framework with snapshot
GitHub Check: Build and test on macos-latest
GitHub Check: Build caffeine with snapshot

🔇 Additional comments (8)

nullaway/src/test/java/com/uber/nullaway/jspecify/GenericMethodTests.java (2)

1260-1260: LGTM!

Good improvement converting the multi-line comment to a concise Javadoc linking to the issue.

1279-1292: LGTM!

The test correctly validates that the new library model allows assigning AtomicReferenceFieldUpdater.newUpdater(...) to a field with @Nullable Object type parameter, which is the fix for issue #1157. The test mirrors the self-contained pattern while exercising the actual JDK class.

nullaway/src/main/java/com/uber/nullaway/handlers/LibraryModelsHandler.java (6)

29-29: LGTM!

The static import for TYPE_ARGUMENT is needed for constructing the TypePathEntry in the nested annotation info below.

989-989: LGTM!

Correctly marks type parameter V (at index 1) of AtomicReferenceFieldUpdater<T, V> as having a nullable upper bound, enabling field types like @Nullable Object.

992-1000: LGTM!

Correctly configures the newUpdater method's type variable W (at index 1) to have a nullable upper bound, enabling proper type inference when the result is assigned to fields with nullable type parameters.

1002-1015: LGTM!

This is the key piece that makes the fix work correctly. By annotating the type argument of Class<W> as @NonNull, the method signature becomes newUpdater(Class<U>, Class<@NonNull W>, String). This correctly models that:

The passed Object.class is Class<Object> (non-null)

But the inferred type for W can still be @Nullable Object based on the assignment context

This resolves the mismatch reported in issue #1157.

1021-1021: LGTM!

Adding AtomicReferenceFieldUpdater to NULLMARKED_CLASSES ensures NullAway applies proper generics nullability checking for this JDK class.

1086-1095: LGTM!

The new override methods correctly expose the library model data through the LibraryModels interface, following the established pattern in DefaultLibraryModels.

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

codecov · 2025-12-26T00:53:51Z

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 88.36%. Comparing base (17d3462) to head (526f063).
⚠️ Report is 1 commits behind head on master.

Additional details and impacted files

@@            Coverage Diff            @@
##             master    #1409   +/-   ##
=========================================
  Coverage     88.35%   88.36%           
+ Complexity     2662     2661    -1     
=========================================
  Files            98       98           
  Lines          8897     8912   +15     
  Branches       1765     1765           
=========================================
+ Hits           7861     7875   +14     
- Misses          514      515    +1     
  Partials        522      522

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

msridhar added 2 commits December 25, 2025 15:46

WIP

6482c65

finish work

526f063

msridhar enabled auto-merge (squash) December 26, 2025 00:53

msridhar requested a review from yuxincs December 26, 2025 00:53

yuxincs approved these changes Dec 26, 2025

View reviewed changes

msridhar merged commit 50ff582 into master Dec 26, 2025
10 of 11 checks passed

msridhar deleted the fix-1157 branch December 26, 2025 00:57

msridhar mentioned this pull request Dec 26, 2025

Add support for nested annotations to JDK Javac plugin #1410

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add library model for AtomicReferenceFieldUpdater#1409

Add library model for AtomicReferenceFieldUpdater#1409
msridhar merged 2 commits intomasterfrom
fix-1157

msridhar commented Dec 26, 2025 •

edited by coderabbitai bot

Loading

Uh oh!

coderabbitai bot commented Dec 26, 2025

Uh oh!

codecov bot commented Dec 26, 2025 •

edited

Loading

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

msridhar commented Dec 26, 2025 • edited by coderabbitai bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary by CodeRabbit

Release Notes

Uh oh!

coderabbitai bot commented Dec 26, 2025

Walkthrough

Possibly related PRs

Suggested reviewers

Pre-merge checks and finishing touches

Uh oh!

codecov bot commented Dec 26, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

msridhar commented Dec 26, 2025 •

edited by coderabbitai bot

Loading

codecov bot commented Dec 26, 2025 •

edited

Loading