Test/increase test coverage

[Beluomini]

Fix false positives in SpringHandler and RequiresNonNullHandler; add tests for coverage gaps

What and why

This PR fixes two bugs found while increasing test coverage, and adds unit tests for previously
untested code paths in four handlers.

Bug fixes

1. SpringHandler: false positive for SpEL expressions with null in comparisons

containsNullSpELExpression used a single regex that matched null anywhere inside a #{...}
expression. This caused a false positive for conditional expressions like:

@Value("#{someBean != null ? someBean.value : 'default'}")
private String field;

NullAway incorrectly flagged field as potentially null, even though null here is only used in
a comparison, not as a possible return value. Fix: a second regex (NULL_COMPARISON_PATTERN) strips
null-in-comparison occurrences before re-checking whether null can actually be the value.

2. RequiresNonNullHandler: error message in validateOverridingRules listed the wrong fields

When a child method's @RequiresNonNull was stricter than its parent's (violating LSP), the error
message listed the parent's fields instead of the extra fields added by the child. Fix: use
the correct iterator (overridingFieldNames instead of overriddenFieldNames) when building the
message.

New tests

• JakartaPersistenceTests.jakartaPersistenceMixedAccessBothMappingsIsUnknown — covers the
  case where a JPA entity has both field-level and getter-level mapping annotations simultaneously,
  which resolves to UNKNOWN access type and causes NullAway to report uninitialized-field errors.

• FrameworkTests.springValueSpelWithNullInConditional,
  springValueSpelNullAsReturnValue, springValueSpelNullComparisonLeftSide — regression
  tests for the SpringHandler fix, covering null-as-comparison-operand (both sides) and
  null-as-return-value in SpEL expressions.

• SyncLambdasTests.forEachOnMapWithAnonymousClass — documents that anonymous classes passed
  to Map.forEach do not benefit from nullness-fact propagation (unlike lambdas), because the AST
  parent node is NewClassTree, not MethodInvocationTree.

• EnsuresNonNullTests.requiresNonNullOverridingErrorMessageListsExtraFields — regression test
  for the RequiresNonNullHandler fix, verifying that the error message lists [b, c] (the extra
  fields added by the child) rather than [a] (the parent's fields) when multiple extra fields
  cause the violation.

Unit tests

All four items above include unit tests. The two bug-fix items include regression tests that fail on
the unfixed code and pass after the fix.

Summary by CodeRabbit

• Bug Fixes

  • Improved null-handling for Spring-style expression values so comparison-only uses of null are treated more accurately.
  • Clarified error messages for precondition inheritance issues, including the specific fields that make an override stricter.
  • Improved detection for mixed Jakarta Persistence access patterns and anonymous callback null checks.

• Tests

  • Added coverage for Spring expression cases, persistence access edge cases, and overriding precondition diagnostics.

[CLAassistant]

Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
_{You have signed the CLA already but the status is still pending? Let us recheck it.}

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 17335875-faf1-43a8-ab62-bb126dcf315b

📥 Commits

Reviewing files that changed from the base of the PR and between a388d4f and 22ea0cf.

📒 Files selected for processing (6)

• nullaway/src/main/java/com/uber/nullaway/handlers/SpringHandler.java
• nullaway/src/main/java/com/uber/nullaway/handlers/contract/fieldcontract/RequiresNonNullHandler.java
• nullaway/src/test/java/com/uber/nullaway/EnsuresNonNullTests.java
• nullaway/src/test/java/com/uber/nullaway/FrameworkTests.java
• nullaway/src/test/java/com/uber/nullaway/JakartaPersistenceTests.java
• nullaway/src/test/java/com/uber/nullaway/SyncLambdasTests.java

---

Walkthrough

The PR makes two bug fixes with accompanying tests, plus two standalone test additions. In SpringHandler, a new NULL_COMPARISON_PATTERN regex is added and containsNullSpELExpression is updated to strip comparison-only null tokens (from == null / != null expressions) before applying the standalone-null check, preventing false positives. In RequiresNonNullHandler.validateOverridingRules, the error message construction now iterates over overridingFieldNames instead of overriddenFieldNames, correcting which fields are reported. Three FrameworkTests cover the SpEL null scenarios; two EnsuresNonNullTests cover the corrected error message. Additionally, a new JakartaPersistenceTests test covers mixed field/property JPA access type as UNKNOWN, and a new SyncLambdasTests test documents that nullness facts are not propagated into anonymous class methods used as Map.forEach callbacks.

Possibly related PRs

• uber/NullAway#1505: Introduced the original Spring @Value SpEL null-skipping heuristic in SpringHandler that this PR's NULL_COMPARISON_PATTERN change directly refines.
• uber/NullAway#1584: Modified SpringHandler's @Value/SpEL null-decision path in the same shouldSkipFieldInitializationCheck area that this PR's regex changes affect.
• uber/NullAway#1604: Refactored SpringHandler.shouldSkipFieldInitializationCheck's @Value/SpEL null handling, overlapping directly with this PR's tightened containsNullSpELExpression logic.

Suggested reviewers

• yuxincs
• msridhar

🚥 Pre-merge checks | ✅ 3 | ❌ 2

❌ Failed checks (1 warning, 1 inconclusive)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 7.14% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.
Title check	❓ Inconclusive	The title is too generic and does not describe the specific fixes or new tests in this PR.	Use a concise, specific title that names the main change, such as fixing SpringHandler null SpEL handling and RequiresNonNull error reporting.

✅ Passed checks (3 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

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

• X
• Mastodon
• Reddit
• LinkedIn

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

[msridhar]

@Beluomini thanks for these contributions! For cleanliness, and to ease a narrow revert in case of some unexpected future issue, I'd prefer this be split into separate PRs; one for the SpringHandler fix (with its tests), one for the RequiresNonNullHandler fix (with its tests), and one for the remaining tests. Could you possibly do that?