Skip to content

test case for issue 1493 - #1496

Merged
msridhar merged 1 commit into
masterfrom
issue-1493
Mar 23, 2026
Merged

msridhar merged 1 commit into
masterfrom
issue-1493

Conversation

@msridhar

@msridhar msridhar commented Mar 21, 2026 •

Copy link
Copy Markdown
Collaborator

We should test that this passes once we get the JSpecify JDK models in (#950)

Summary by CodeRabbit

  • Tests
    • Added test case to validate proper null-safety handling for nullable parameters with default values.

@coderabbitai

coderabbitai Bot commented Mar 21, 2026

Copy link
Copy Markdown
Contributor

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: 7df3529e-ce44-41dc-b344-4a42b0ddba16

📥 Commits

Reviewing files that changed from the base of the PR and between c85b66e and b96862e.

📒 Files selected for processing (1)
  • nullaway/src/test/java/com/uber/nullaway/jspecify/GenericMethodTests.java

Walkthrough

A new test method issue1493 was added to the GenericMethodTests class. The test is currently disabled via @Ignore annotation and verifies behavior related to Objects.requireNonNullElse invocation. The test code includes a @NullMarked class that passes a @Nullable string parameter alongside a non-annotated default value to the utility method, then executes the test via .doTest().

Possibly related PRs

  • PR #1471: Addresses generic-method nullability inference specifically for the Objects.requireNonNullElse case that this test exercises
  • PR #1289: Adds similarly-structured JSpecify inference test methods to GenericMethodTests using the same makeHelperWithInferenceFailureWarning pattern

Suggested reviewers

  • lazaroclapp
  • yuxincs
🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'test case for issue 1493' directly and clearly describes the main change: adding a test case for a specific issue.

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

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch issue-1493

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.

Tip

CodeRabbit can approve the review once all CodeRabbit's comments are resolved.

Enable the reviews.request_changes_workflow setting to automatically approve the review once all CodeRabbit's comments are resolved.

@codecov

codecov Bot commented Mar 21, 2026 •

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 88.43%. Comparing base (c85b66e) to head (b96862e).
⚠️ Report is 1 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff            @@
##             master    #1496   +/-   ##
=========================================
  Coverage     88.43%   88.43%           
  Complexity     2790     2790           
=========================================
  Files            99       99           
  Lines          9279     9279           
  Branches       1872     1872           
=========================================
  Hits           8206     8206           
  Misses          524      524           
  Partials        549      549           

☔ 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
msridhar merged commit 85af2f0 into master Mar 23, 2026
12 checks passed
@msridhar
msridhar deleted the issue-1493 branch March 23, 2026 00:22
mernst pushed a commit to mernst/NullAway that referenced this pull request Oct 23, 2026
…odels (uber#2033)

With `WarnOnGenericInferenceFailure` on and no JDK models, returning
`Objects.requireNonNullElse(nullable, nonNull)` from a method with a
non-null `String` return reports `inference failure: type variable T
constrained to be both @nonnull and @Nullable` (uber#1493). The JSpecify JDK
models (uber#1641, NullAway 0.14.0) declare `<T> T
requireNonNullElse(@nullable T obj, @nullable T defaultObj)`, so `T` is
inferred as non-null `String` and nothing is reported. The models are
enabled by `-XepOpt:NullAway:JSpecifyJDKModels=true`, or by
`-XepOpt:NullAway:JSpecifyExperimental=true`, whose value that flag
defaults to.

`GenericMethodTests.issue1493` was added with `@Ignore` in uber#1496 and
passes now. This change enables it as
`issue1493RequireNonNullElseWithJdkModels` and adds
`issue1493RequireNonNullElseWithoutJdkModels`, which compiles the same
source with `JSpecifyJDKModels=false` and expects the inference failure.
Both tests set `WarnOnGenericInferenceFailure=true` and
`JSpecifyJDKModels` explicitly instead of taking them from the
`JSpecifyExperimental` default, so the pair differs only in the models
flag; with that flag set to `true`, the second test fails with "There
were no errors".

Refs uber#1493. Merging does not close it: the reporter's configuration
without the models (`JSpecifyMode` plus
`WarnOnGenericInferenceFailure=true`) still reports the warning, and
whether the issue can close before the models are on by default is for
the maintainers to decide.

🤖 Generated with [Claude Code](https://claude.com/claude-code)


<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->

<!-- coderabbit-ai-generated:v1 -->

## Summary by CodeRabbit

* **Tests**
* Added enabled coverage for `Objects.requireNonNullElse` generic
inference with JSpecify JDK models enabled and disabled.

<!-- end of auto-generated comment: release notes by coderabbit.ai -->
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants