Change how recursive calls to dataflow are detected by msridhar · Pull Request #1374 · uber/NullAway

msridhar · 2025-12-10T23:57:36Z

Expose a new API to determine if dataflow analysis is currently running for a particular method / lambda / initializer (based on Checker Framework APIs), and call that new API to avoid recursively calling into dataflow. This doesn't change functionality but better prepares us for the fix to #1373.

We retain the calledFromDataflow parameter to some methods to still ensure that we do not cache inference results when generic method inference is called from the dataflow analysis. But, in some places the parameter is no longer needed and has been removed.

Summary by CodeRabbit

New Features
- Added ability to query whether dataflow analysis is currently running for a given code path.
Refactor
- Simplified generic type inference by removing threaded boolean flags and centralizing dataflow awareness into a dedicated analysis interface, with corresponding method signature cleanups and reduced plumbing.

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

msridhar · 2025-12-10T23:59:57Z

nullaway/src/main/java/com/uber/nullaway/generics/GenericsChecks.java

        // the generic invocation is either a regular parameter to the parent call, or the
        // receiver expression
-        AtomicReference<Type> formalParamTypeRef = new AtomicReference<>();
+        AtomicReference<@Nullable Type> formalParamTypeRef = new AtomicReference<>();


this is a drive-by unrelated fix

coderabbitai · 2025-12-11T00:00:22Z

Walkthrough

Adds public isRunning(TreePath, Context) to AccessPathNullnessAnalysis and isRunning(TreePath, Context, AccessPathNullnessPropagation) to DataFlow to allow querying whether a dataflow analysis run is active for a given path and context. Refactors GenericsChecks to remove the boolean calledFromDataflow parameter from several methods and to use AccessPathNullnessAnalysis for nullness refinement; updates call sites and minor type annotations accordingly.

Possibly related PRs

Use refined types from dataflow analysis in generic method inference #1309: Also adds an API to query "running" dataflow state and adjusts GenericsChecks to use that information — strong overlap in dataflow↔generics changes.
Improve inference for generic method with void-returning lambda argument #1312: Changes how generics inference and dataflow nullness interact, including plumbing that touches GenericsChecks and AccessPathNullnessAnalysis usages.
Better fix for dataflow analysis caching #1353: Modifies analysis caching and retrieval (RunOnceForwardAnalysisImpl/analysisCache) that the new DataFlow.isRunning implementation calls into.

Suggested labels

run-benchmarks

Suggested reviewers

lazaroclapp
yuxincs

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title accurately describes the main change: introducing a new API to detect when dataflow analysis is recursively running, replacing the prior approach of threading a boolean flag through multiple methods.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

✨ 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 rework-recursive-dataflow-detection

📜 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 7084cab and 7223742.

📒 Files selected for processing (1)

nullaway/src/main/java/com/uber/nullaway/generics/GenericsChecks.java (10 hunks)

🧰 Additional context used

🧠 Learnings (3)

📓 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.

📚 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/generics/GenericsChecks.java

📚 Learning: 2025-08-29T18:41:43.584Z

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.

Applied to files:

nullaway/src/main/java/com/uber/nullaway/generics/GenericsChecks.java

🧬 Code graph analysis (1)

nullaway/src/main/java/com/uber/nullaway/generics/GenericsChecks.java (1)

nullaway/src/main/java/com/uber/nullaway/dataflow/AccessPathNullnessAnalysis.java (1)

AccessPathNullnessAnalysis (57-410)

⏰ 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). (4)

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

🔇 Additional comments (4)

nullaway/src/main/java/com/uber/nullaway/generics/GenericsChecks.java (4)

49-49: LGTM: Import added for new dataflow analysis integration.

The import is necessary to support the new isRunning() API usage in refineArgumentTypeWithDataflow.

668-668: LGTM: Systematic removal of calledFromDataflow parameter.

The removal from constraint generation methods (generateConstraintsForCall, generateConstraintsForPseudoAssignment, handleLambdaInGenericMethodInference) is correct. These methods no longer need the flag since dataflow running detection is now centralized in refineArgumentTypeWithDataflow using the isRunning() API. All call sites have been updated consistently.

Also applies to: 777-777, 799-799, 808-808, 819-819, 858-858, 864-864

1589-1589: LGTM: Type annotation improves nullability clarity.

The change from AtomicReference<Type> to AtomicReference<@Nullable Type> correctly reflects that formalParamType can be null, as confirmed by the null check at line 1599. This is a minor type safety improvement.

948-982: Refactoring correctly replaces threaded parameter with dynamic detection.

The method properly uses AccessPathNullnessAnalysis.isRunning() to determine whether dataflow analysis is active for the enclosing context, eliminating the need to thread a calledFromDataflow parameter. The key calls to expressionDataflow() are functionally equivalent:

getNullnessFromRunning() → expressionDataflow(..., true) (analysis running)

getNullness() → expressionDataflow(..., false) (analysis not running)

The implementation correctly handles null checks and properly constructs the argument path. This is a cleaner approach than parameter threading.

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-11T00:07:13Z

Codecov Report

❌ Patch coverage is 88.88889% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 88.47%. Comparing base (938a897) to head (7223742).
⚠️ Report is 1 commits behind head on master.

Files with missing lines	Patch %	Lines
.../nullaway/dataflow/AccessPathNullnessAnalysis.java	75.00%	0 Missing and 1 partial ⚠️
...ava/com/uber/nullaway/generics/GenericsChecks.java	88.88%	0 Missing and 1 partial ⚠️

Additional details and impacted files

@@             Coverage Diff              @@
##             master    #1374      +/-   ##
============================================
- Coverage     88.47%   88.47%   -0.01%     
- Complexity     2616     2619       +3     
============================================
  Files            98       98              
  Lines          8766     8774       +8     
  Branches       1749     1750       +1     
============================================
+ Hits           7756     7763       +7     
  Misses          504      504              
- Partials        506      507       +1

☔ 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 4 commits December 10, 2025 14:08

WIP

cc35ae5

tests passing

a3e872a

docs

7084cab

remove todo

7223742

msridhar commented Dec 10, 2025

View reviewed changes

msridhar marked this pull request as draft December 11, 2025 00:15

msridhar marked this pull request as ready for review December 13, 2025 01:46

msridhar mentioned this pull request Dec 13, 2025

Run dataflow analysis on bodies of lambdas passed to generic methods #1375

Merged

msridhar requested a review from yuxincs December 13, 2025 01:48

yuxincs approved these changes Dec 14, 2025

View reviewed changes

msridhar merged commit 9c41467 into master Dec 14, 2025
10 of 11 checks passed

msridhar deleted the rework-recursive-dataflow-detection branch December 14, 2025 18:43

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Change how recursive calls to dataflow are detected#1374

Change how recursive calls to dataflow are detected#1374
msridhar merged 4 commits intomasterfrom
rework-recursive-dataflow-detection

msridhar commented Dec 10, 2025 •

edited by coderabbitai bot

Loading

Uh oh!

msridhar Dec 10, 2025 •

edited

Loading

Uh oh!

coderabbitai bot commented Dec 11, 2025 •

edited

Loading

Uh oh!

codecov bot commented Dec 11, 2025 •

edited

Loading

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

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

Uh oh!

Summary by CodeRabbit

Uh oh!

msridhar Dec 10, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

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

Uh oh!

Walkthrough

Possibly related PRs

Suggested labels

Suggested reviewers

Pre-merge checks and finishing touches

Uh oh!

codecov bot commented Dec 11, 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 10, 2025 •

edited by coderabbitai bot

Loading

msridhar Dec 10, 2025 •

edited

Loading

coderabbitai bot commented Dec 11, 2025 •

edited

Loading

codecov bot commented Dec 11, 2025 •

edited

Loading