Fixes for processing return array types and multi-dimensional array types in AstubxGenerator - #1689
Conversation
|
This change is part of the following stack:
Change managed by git-spice. |
WalkthroughAstubxGenerator now handles nullable annotations across return and argument type signatures, including arrays, varargs, and qualified or simple Possibly related issues
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
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. Comment |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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
`@jdk-annotations/astubx-generator/src/main/java/com/uber/nullaway/jdkannotations/AstubxGenerator.java`:
- Around line 392-397: Update the array detection logic in AstubxGenerator to
inspect only the annotation immediately before the final [] dimension, so
inner-dimension annotations in multidimensional types are not classified as
top-level nullness. Adjust ARRAY_NULLNESS_ANNOTATION_PATTERN or its matching
logic accordingly, and add an AstubxTest regression covering String `@Nullable`
[][].
In
`@jdk-annotations/jdk-integration-test/src/test/java/com/uber/nullaway/jdkannotations/JDKIntegrationTest.java`:
- Around line 336-349: Update JDKIntegrationTest around the nullable array and
nullable element assignment checks to avoid annotating local variables with
`@Nullable`. Pass each ReturnAnnotation result directly to matching annotated
parameters, using ParameterAnnotation.takesNullableArray() for nullable array
values and takesNullableElements() for nullable-element values, while preserving
the existing incompatible-type diagnostics.
🪄 Autofix
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 Plus
Run ID: 9cac7963-c109-419c-ab4c-daf829bb71bd
📒 Files selected for processing (6)
jdk-annotations/astubx-generator/src/main/java/com/uber/nullaway/jdkannotations/AstubxGenerator.javajdk-annotations/astubx-generator/src/test/java/com/uber/nullaway/jdkannotations/AstubxTest.javajdk-annotations/jdk-integration-test/src/test/java/com/uber/nullaway/jdkannotations/JDKIntegrationTest.javajdk-annotations/test-annotated/src/main/java/com/uber/nullaway/jdkannotations/ReturnAnnotation.javajdk-annotations/test-unannotated/src/main/java/com/uber/nullaway/jdkannotations/ReturnAnnotation.javanullaway/src/test/java/com/uber/nullaway/JSpecifyJDKModelsTest.java
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #1689 +/- ##
============================================
- Coverage 87.87% 87.87% -0.01%
Complexity 3167 3167
============================================
Files 109 109
Lines 10742 10739 -3
Branches 2172 2171 -1
============================================
- Hits 9440 9437 -3
Misses 617 617
Partials 685 685 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
947908f to
00653d3
Compare
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
jdk-annotations/astubx-generator/src/main/java/com/uber/nullaway/jdkannotations/AstubxGenerator.java (1)
409-418: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winAdd Javadoc for both private helpers.
stripAnnotationsFromTypeSignatureandbuildTypeSignatureAnnotationPatterndefine the signature-key transformation. Document their behavior and the varargs case.As per coding guidelines, “Add Javadoc for every non-trivial method, including private methods.”
🤖 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 `@jdk-annotations/astubx-generator/src/main/java/com/uber/nullaway/jdkannotations/AstubxGenerator.java` around lines 409 - 418, Add Javadoc to both private helpers, stripAnnotationsFromTypeSignature and buildTypeSignatureAnnotationPattern, describing their signature-transformation behavior. Document that annotation removal handles annotations followed by whitespace and annotations applied directly to top-level varargs arrays before the ellipsis.Source: Coding guidelines
🤖 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.
Outside diff comments:
In
`@jdk-annotations/astubx-generator/src/main/java/com/uber/nullaway/jdkannotations/AstubxGenerator.java`:
- Around line 409-418: Add Javadoc to both private helpers,
stripAnnotationsFromTypeSignature and buildTypeSignatureAnnotationPattern,
describing their signature-transformation behavior. Document that annotation
removal handles annotations followed by whitespace and annotations applied
directly to top-level varargs arrays before the ellipsis.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 30c2dcc3-2ef8-4e8f-a19c-da489bd11e9f
📒 Files selected for processing (5)
jdk-annotations/astubx-generator/src/main/java/com/uber/nullaway/jdkannotations/AstubxGenerator.javajdk-annotations/astubx-generator/src/test/java/com/uber/nullaway/jdkannotations/AstubxTest.javajdk-annotations/jdk-integration-test/src/test/java/com/uber/nullaway/jdkannotations/JDKIntegrationTest.javajdk-annotations/test-annotated/src/main/java/com/uber/nullaway/jdkannotations/ReturnAnnotation.javajdk-annotations/test-unannotated/src/main/java/com/uber/nullaway/jdkannotations/ReturnAnnotation.java
Apply special logic for parameter types to return types also, and add fixes for multi-dimensional arrays. Most of the changes are in tests.
Summary by CodeRabbit
Bug Fixes
Tests