Refactoring: clarify docs and naming for RestoreNullnessAnnotationsVisitor

[msridhar]

No behavior changes

Summary by CodeRabbit

• Refactor
  • Renamed parameter/field related to extra type-variable nullability annotations across public signatures, constructors and internal usages; updated related documentation. No behavioral or API-semantic changes—purely naming and clarity improvements.

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

[coderabbitai]

Walkthrough

This pull request renames the parameter and related field from annotsOnTypeVarsFromSubtypes to extraTypeVariableAnnotations in TypeSubstitutionUtils.java. The public API restoreExplicitNullabilityAnnotations, the private RestoreNullnessAnnotationsVisitor constructor and field, and internal references (notably in updateNullabilityAnnotationsForType) were updated to use the new name. Javadoc/comments were updated to describe extraTypeVariableAnnotations. No behavioral changes were made.

Possibly related PRs

• uber/NullAway PR 1371: Touches the same TypeSubstitutionUtils.restoreExplicitNullabilityAnnotations API and adds call sites that invoke it to restore annotations on method returns and field reads (strong code-level overlap).

Suggested reviewers

• yuxincs

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%.	You can run @coderabbitai generate docstrings to improve docstring coverage.

✅ 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 accurately describes the main change: refactoring to improve documentation and naming for RestoreNullnessAnnotationsVisitor, which directly aligns with the PR's renaming of parameters, fields, and expanded Javadoc.

✨ 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 improve-type-subst-docs-and-naming

---

📜 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 b8fa1a0 and d6b58f5.

📒 Files selected for processing (1)

• nullaway/src/main/java/com/uber/nullaway/generics/TypeSubstitutionUtils.java

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

Learnt from: msridhar
Repo: uber/NullAway PR: 1316
File: jdk-javac-plugin/src/main/java/com/uber/nullaway/javacplugin/NullnessAnnotationSerializer.java:261-293
Timestamp: 2025-10-29T23:56:18.236Z
Learning: In NullAway's jdk-javac-plugin NullnessAnnotationSerializer, type variable bounds with annotations (e.g., `T extends Nullable Object`) are checked at their declaration sites by the typeParamHasAnnotation method for both class-level and method-level type parameters. The hasJSpecifyAnnotationDeep method is designed to check type uses (return types, parameters, etc.) and does not need a TYPEVAR case because type variable declaration bounds are already handled separately.

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

📚 Learning: 2025-10-29T23:56:18.236Z

Learnt from: msridhar
Repo: uber/NullAway PR: 1316
File: jdk-javac-plugin/src/main/java/com/uber/nullaway/javacplugin/NullnessAnnotationSerializer.java:261-293
Timestamp: 2025-10-29T23:56:18.236Z
Learning: In NullAway's jdk-javac-plugin NullnessAnnotationSerializer, type variable bounds with annotations (e.g., `T extends Nullable Object`) are checked at their declaration sites by the typeParamHasAnnotation method for both class-level and method-level type parameters. The hasJSpecifyAnnotationDeep method is designed to check type uses (return types, parameters, etc.) and does not need a TYPEVAR case because type variable declaration bounds are already handled separately.

Applied to files:

• nullaway/src/main/java/com/uber/nullaway/generics/TypeSubstitutionUtils.java

---

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: 2

📜 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 47e85c5 and 4f4f327.

📒 Files selected for processing (1)

• nullaway/src/main/java/com/uber/nullaway/generics/TypeSubstitutionUtils.java

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

Learnt from: msridhar
Repo: uber/NullAway PR: 1316
File: jdk-javac-plugin/src/main/java/com/uber/nullaway/javacplugin/NullnessAnnotationSerializer.java:261-293
Timestamp: 2025-10-29T23:56:18.236Z
Learning: In NullAway's jdk-javac-plugin NullnessAnnotationSerializer, type variable bounds with annotations (e.g., `T extends Nullable Object`) are checked at their declaration sites by the typeParamHasAnnotation method for both class-level and method-level type parameters. The hasJSpecifyAnnotationDeep method is designed to check type uses (return types, parameters, etc.) and does not need a TYPEVAR case because type variable declaration bounds are already handled separately.

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

📚 Learning: 2025-10-29T23:56:18.236Z

Learnt from: msridhar
Repo: uber/NullAway PR: 1316
File: jdk-javac-plugin/src/main/java/com/uber/nullaway/javacplugin/NullnessAnnotationSerializer.java:261-293
Timestamp: 2025-10-29T23:56:18.236Z
Learning: In NullAway's jdk-javac-plugin NullnessAnnotationSerializer, type variable bounds with annotations (e.g., `T extends Nullable Object`) are checked at their declaration sites by the typeParamHasAnnotation method for both class-level and method-level type parameters. The hasJSpecifyAnnotationDeep method is designed to check type uses (return types, parameters, etc.) and does not need a TYPEVAR case because type variable declaration bounds are already handled separately.

Applied to files:

• nullaway/src/main/java/com/uber/nullaway/generics/TypeSubstitutionUtils.java

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

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

🔇 Additional comments (3)

nullaway/src/main/java/com/uber/nullaway/generics/TypeSubstitutionUtils.java (3)

90-96: LGTM! Improved parameter naming.

The renamed parameter extraTypeVariableAnnotations is more descriptive and clearer than the previous annotsOnTypeVarsFromSubtypes. The parameter is correctly passed through to the visitor constructor.

---

117-122: LGTM! Constructor consistently updated.

The constructor parameter and field assignment have been correctly updated to use the new name extraTypeVariableAnnotations.

---

217-219: LGTM! Field usage correctly updated.

The field reference has been correctly updated to use the new name, and the comment has been appropriately clarified.

[codecov]

Codecov Report

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

Additional details and impacted files

@@            Coverage Diff            @@
##             master    #1406   +/-   ##
=========================================
  Coverage     88.21%   88.21%           
  Complexity     2615     2615           
=========================================
  Files            95       95           
  Lines          8671     8671           
  Branches       1741     1741           
=========================================
  Hits           7649     7649           
  Misses          509      509           
  Partials        513      513           

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