Skip to content

Enable test for issue 1500 and add comment#1560

Merged
msridhar merged 4 commits into
masterfrom
issue-1500
May 25, 2026
Merged

Enable test for issue 1500 and add comment#1560
msridhar merged 4 commits into
masterfrom
issue-1500

Conversation

@msridhar
Copy link
Copy Markdown
Collaborator

@msridhar msridhar commented May 1, 2026

Fixes #1500

NullAway behaves correctly for this case. Test-only change.

Summary by CodeRabbit

Tests

  • Re-enabled previously skipped test for wildcard type handling in generic declarations. Expanded test coverage with additional scenarios to validate null-safety analysis behavior across different generic type parameter patterns.

Review Change Stack

@msridhar msridhar marked this pull request as draft May 1, 2026 20:36
@codecov
Copy link
Copy Markdown

codecov Bot commented May 1, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 88.30%. Comparing base (1184b98) to head (5cbbfc3).
⚠️ Report is 1 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff            @@
##             master    #1560   +/-   ##
=========================================
  Coverage     88.30%   88.30%           
  Complexity     2926     2926           
=========================================
  Files           104      104           
  Lines          9776     9776           
  Branches       1967     1967           
=========================================
  Hits           8633     8633           
  Misses          543      543           
  Partials        600      600           

☔ 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
Copy link
Copy Markdown
Collaborator Author

msridhar commented May 2, 2026

Tinkering with this test revealed a deeper issue, unrelated to wildcards, that needs to be fixed separately. Will leave this in a draft state and then come back to it later.

@msridhar msridhar force-pushed the wildcard-inference-3 branch from d7f480a to 3207404 Compare May 3, 2026 01:18
@msridhar msridhar force-pushed the wildcard-inference-3 branch from 3207404 to 60a4bc1 Compare May 3, 2026 01:25
@msridhar msridhar force-pushed the wildcard-inference-3 branch from 60a4bc1 to 0803ac7 Compare May 3, 2026 19:11
@msridhar msridhar force-pushed the wildcard-inference-3 branch from 0803ac7 to c7a52fd Compare May 3, 2026 19:27
Base automatically changed from wildcard-inference-3 to master May 3, 2026 19:40
@msridhar msridhar force-pushed the issue-1500 branch 6 times, most recently from bb9203f to a0bbd9d Compare May 9, 2026 14:58
@msridhar msridhar changed the base branch from master to bug-from-caffeine May 13, 2026 01:19
@msridhar msridhar marked this pull request as ready for review May 13, 2026 01:23
@msridhar msridhar requested a review from lazaroclapp May 13, 2026 01:26
@msridhar msridhar force-pushed the bug-from-caffeine branch from fe19f94 to 4eaa8df Compare May 16, 2026 23:28
@msridhar msridhar force-pushed the bug-from-caffeine branch from 4eaa8df to 8242507 Compare May 17, 2026 18:48
@msridhar msridhar force-pushed the bug-from-caffeine branch from 8242507 to b53dead Compare May 18, 2026 20:36
@msridhar msridhar force-pushed the bug-from-caffeine branch from b53dead to 6310b10 Compare May 18, 2026 20:55
@msridhar msridhar force-pushed the bug-from-caffeine branch from 6310b10 to 94a08ab Compare May 20, 2026 13:49
@msridhar msridhar force-pushed the bug-from-caffeine branch from 94a08ab to 70b2120 Compare May 22, 2026 02:08
@msridhar msridhar force-pushed the bug-from-caffeine branch from 70b2120 to c2c3046 Compare May 23, 2026 20:14
@msridhar msridhar force-pushed the issue-1500 branch 3 times, most recently from 826210f to e659711 Compare May 23, 2026 23:56
@msridhar msridhar force-pushed the bug-from-caffeine branch from 80edec5 to df5baa3 Compare May 25, 2026 16:59
Base automatically changed from bug-from-caffeine to master May 25, 2026 17:31
@msridhar msridhar enabled auto-merge (squash) May 25, 2026 17:33
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented May 25, 2026

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: 9a35f390-8800-4a53-a2b9-f8ab210a300c

📥 Commits

Reviewing files that changed from the base of the PR and between 1184b98 and 5cbbfc3.

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

Walkthrough

This pull request re-enables the issue1500 test in WildcardTests that was previously ignored due to a tracked false positive with JSpecify wildcard inference. The test method annotation is updated to remove the @Ignore marker, and the embedded Java test source is expanded to include a non-wildcard factory method (Foo.ofNoWildcard) alongside the existing wildcard-based method, with additional test fields (FOO2 and FOO3) to demonstrate both the inference limitation case and the working alternative case.

Possibly related PRs

  • uber/NullAway#1548: Updates wildcard subtyping and containment logic in CheckIdenticalNullabilityVisitor that aligns with the corrected behavior validated by the re-enabled issue1500 test.
  • uber/NullAway#1549: Introduces wildcard generics inference changes that the updated issue1500 test directly exercises and validates.
  • uber/NullAway#1558: Modifies GenericsChecks wildcard nullability logic and adds wildcard-capture test coverage related to the same inference behavior validated here.

Suggested reviewers

  • lazaroclapp
  • yuxincs
🚥 Pre-merge checks | ✅ 4 | ❌ 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 (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately reflects the main change—enabling a previously ignored test for issue 1500 and adding a comment to document the related work.
Linked Issues check ✅ Passed The PR successfully addresses issue #1500 by enabling the test case and adding clarification via comments, directly meeting the objective to assert correct NullAway behavior for the reported JSpecify false positive with wildcard super types.
Out of Scope Changes check ✅ Passed All changes are scoped to the test file (WildcardTests.java) and directly support the objective of addressing issue #1500; no production code modifications or unrelated changes are present.

✏️ 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-1500

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.

@msridhar msridhar merged commit c315bd5 into master May 25, 2026
10 of 12 checks passed
@msridhar msridhar deleted the issue-1500 branch May 25, 2026 17:46
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.

JSpecify: false positive when ? super T is on the both sides of call and T is nullable

2 participants