Fix for unbounded wildcard passed to @NullUnmarked type variable

[msridhar]

See the new test case unboundWildcardTypeVarUnmarked. Previously, for an unbound wildcard, we neglected to check whether the corresponding type variable was in @NullUnmarked code. If the type variable is in a @NullUnmarked scope, we treat its upper bound as @Nullable. Most of the code in this PR is passing around additional parameters so we can move the methods for reasoning about upper bounds to GenericsUtils and share the reasoning.

Summary by CodeRabbit

• Bug Fixes
  • Improved nullability checking for generic wildcards and their effective upper bounds
  • Enhanced constraint solving for generic type parameters to better handle nullable type-variable bounds
  • Better support for type variables originating from unannotated code in generic contexts
  • Fixed edge cases with unmarked type variables in generic classes and interfaces

[msridhar]

This change is part of the following stack:

• Run inference for generic method calls nested inside receivers #1571
  • Improve handling of var-declared local variables #1573
    • Compensate for more annotations inserted by javac #1574
      • Enable test for issue 1500 and add comment #1560
        • Use ground target types when handling lambdas and method refs passed to generic methods #1575
          • Fix for unbounded wildcard passed to @NullUnmarked type variable #1577 ◀
            • Detect and warn on annotations directly on wildcard types #1579

_{Change managed by git-spice.}

[codecov]

Codecov Report

❌ Patch coverage is 80.35714% with 11 lines in your changes missing coverage. Please review.
✅ Project coverage is 88.20%. Comparing base (9e7cfb2) to head (fda7118).

Files with missing lines	Patch %	Lines
...java/com/uber/nullaway/generics/GenericsUtils.java	74.28%	3 Missing and 6 partials ⚠️
...m/uber/nullaway/generics/ConstraintSolverImpl.java	77.77%	2 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff            @@
##             master    #1577   +/-   ##
=========================================
  Coverage     88.20%   88.20%           
- Complexity     2934     2938    +4     
=========================================
  Files           104      104           
  Lines          9807     9819   +12     
  Branches       1974     1977    +3     
=========================================
+ Hits           8650     8661   +11     
- Misses          557      558    +1     
  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.

[coderabbitai]

Walkthrough

This PR enhances NullAway's generics handling by threading Handler and Config parameters into wildcard upper-bound nullability computations. GenericsUtils gains new overloads of effectiveWildcardUpperBound and wildcardUpperBound that incorporate library model overrides and unannotated-code contexts. CheckIdenticalNullabilityVisitor now accepts a Handler and uses it in wildcard containment checks. ConstraintSolverImpl is refactored to delegate upper-bound nullability logic to GenericsUtils instead of duplicating it locally. GenericsChecks threads the handler through all generic-nullability inference call sites. A new test verifies correct handling of unbound wildcards in unmarked type-variable contexts.

Possibly related PRs

• uber/NullAway#1548: Modifies CheckIdenticalNullabilityVisitor's wildcard containment/effective-upper-bound handling for wildcard dispatch and extendsBoundContains logic.
• uber/NullAway#1520: Adjusts wildcard type-argument comparison flow in the same visitor and GenericsUtils wildcard-handling code path.
• uber/NullAway#1549: Reworks wildcard-nullability containment in CheckIdenticalNullabilityVisitor and GenericsUtils effective/wildcard upper-bound logic.

Suggested reviewers

• lazaroclapp
• yuxincs

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 64.29% 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 describes the main fix: addressing unbounded wildcard handling for @NullUnmarked type variables.
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 docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch wildcard-upper-bound-type-var-unmarked

---

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 and usage tips.}

[coderabbitai]

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@nullaway/src/main/java/com/uber/nullaway/generics/GenericsUtils.java`:
- Around line 109-117: The code silently treats
methodSymbol.getTypeParameters().indexOf(typeVariableSymbol) == -1 as "no
override" and skips calling handler.onOverrideMethodTypeVariableUpperBound;
change this to perform a fallback lookup when indexOf returns -1: in
GenericsUtils (the branch where enclosingElement instanceof Symbol.MethodSymbol
methodSymbol and typeVarElement instanceof Symbol.TypeVariableSymbol
typeVariableSymbol), if indexOf yields -1, scan methodSymbol.getTypeParameters()
manually to find a matching TypeVariableSymbol (compare by symbol
identity/equality and, if needed, by simple name via
typeVariableSymbol.getSimpleName()) to derive a valid typeVarIndex, then use
that index to call handler.onOverrideMethodTypeVariableUpperBound(methodSymbol,
typeVarIndex, state) as before so method-type-variable overrides are not
dropped.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 6d81b54f-d990-4615-b478-06b84e81f2ee

📥 Commits

Reviewing files that changed from the base of the PR and between 9e7cfb2 and fda7118.

📒 Files selected for processing (5)

• nullaway/src/main/java/com/uber/nullaway/generics/CheckIdenticalNullabilityVisitor.java
• nullaway/src/main/java/com/uber/nullaway/generics/ConstraintSolverImpl.java
• nullaway/src/main/java/com/uber/nullaway/generics/GenericsChecks.java
• nullaway/src/main/java/com/uber/nullaway/generics/GenericsUtils.java
• nullaway/src/test/java/com/uber/nullaway/jspecify/WildcardTests.java

[coderabbitai]

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Don't silently drop method-type-variable handler overrides when the index lookup fails.

If getTypeParameters().indexOf(typeVariableSymbol) returns -1, this helper skips onOverrideMethodTypeVariableUpperBound(...) and falls back to annotation-only logic. Since upperBoundIsNullable(...) now feeds both wildcard upper-bound computation here and ConstraintSolverImpl.getState(...), the modeled nullable upper bound is still lost for the method-type-variable cases already called out by the TODO. Please add a fallback lookup instead of treating -1 as "no override".

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@nullaway/src/main/java/com/uber/nullaway/generics/GenericsUtils.java` around
lines 109 - 117, The code silently treats
methodSymbol.getTypeParameters().indexOf(typeVariableSymbol) == -1 as "no
override" and skips calling handler.onOverrideMethodTypeVariableUpperBound;
change this to perform a fallback lookup when indexOf returns -1: in
GenericsUtils (the branch where enclosingElement instanceof Symbol.MethodSymbol
methodSymbol and typeVarElement instanceof Symbol.TypeVariableSymbol
typeVariableSymbol), if indexOf yields -1, scan methodSymbol.getTypeParameters()
manually to find a matching TypeVariableSymbol (compare by symbol
identity/equality and, if needed, by simple name via
typeVariableSymbol.getSimpleName()) to derive a valid typeVarIndex, then use
that index to call handler.onOverrideMethodTypeVariableUpperBound(methodSymbol,
typeVarIndex, state) as before so method-type-variable overrides are not
dropped.

[msridhar]

What kind of fallback loop?

[msridhar]

I think this is a weird corner case that doesn't need to be handled now

[coderabbitai]

@msridhar, understood — the existing TODO already documents the known -1 case, so this can be deferred. Would you like me to open a follow-up GitHub issue to track it, or shall I just resolve this review comment?

---

🧠 Learnings used

Learnt from: msridhar
Repo: uber/NullAway PR: 1485
File: nullaway/src/main/java/com/uber/nullaway/NullAway.java:2119-2128
Timestamp: 2026-03-20T15:34:10.657Z
Learning: In this repo (uber/NullAway), don’t annotate local variables with `Nullable` or `NonNull`. NullAway infers nullability for local variables automatically, so adding these annotations to locals is incorrect style for this codebase.

[coderabbitai]

@msridhar — the loop I had in mind was a linear name-based scan as a fallback:

if (typeVarIndex < 0) {
  List<Symbol.TypeVariableSymbol> typeParams = methodSymbol.getTypeParameters();
  for (int i = 0; i < typeParams.size(); i++) {
    if (typeParams.get(i).getSimpleName()
        .equals(typeVariableSymbol.getSimpleName())) {
      typeVarIndex = i;
      break;
    }
  }
}

That said, I realise the root cause of why indexOf returns -1 in the first place hasn't been investigated yet (per the existing TODO pointing at GenericMethodTests.instanceGenericMethodWithMethodRefArgument). If the typeVariableSymbol reaching this point is a specialised/substituted copy of the original method's type variable (which javac sometimes produces during inference), then a name-based fallback could accidentally map to the wrong index or mask a deeper problem.

Given that uncertainty, it's probably more correct to first understand why indexOf fails — whether it's an identity vs. equality issue with javac's Symbol, a different MethodSymbol being used as enclosingElement, or something else — before deciding what the right fallback is.

Would you like to open a follow-up issue to track that investigation?

---

🧠 Learnings used

Learnt from: msridhar
Repo: uber/NullAway PR: 1485
File: nullaway/src/main/java/com/uber/nullaway/NullAway.java:2119-2128
Timestamp: 2026-03-20T15:34:10.657Z
Learning: In this repo (uber/NullAway), don’t annotate local variables with `Nullable` or `NonNull`. NullAway infers nullability for local variables automatically, so adding these annotations to locals is incorrect style for this codebase.