Remove duplicate NestedAnnotationInfo source from NullAway module - #1659
Conversation
|
This change is part of the following stack: Change managed by git-spice. |
ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (10)
💤 Files with no reviewable changes (1)
WalkthroughMoves Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 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 |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #1659 +/- ##
=========================================
Coverage 88.06% 88.07%
+ Complexity 3076 3075 -1
=========================================
Files 105 104 -1
Lines 10356 10344 -12
Branches 2095 2094 -1
=========================================
- Hits 9120 9110 -10
+ Misses 581 580 -1
+ Partials 655 654 -1 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
35d901e to
eb38b11
Compare
a202275 to
acab92d
Compare
There was a problem hiding this comment.
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
`@library-model/library-model-generator/src/shared/java/com/uber/nullaway/libmodel/NestedAnnotationInfo.java`:
- Around line 17-22: Update StubxCacheUtil.parseStubStream() to validate
typePathLength before constructing NestedAnnotationInfo, treating zero or
negative lengths as malformed stubx input and reporting them through the
parser’s established format/error handling rather than allowing
IllegalArgumentException to escape. Keep NestedAnnotationInfo’s nonempty-path
invariant intact and preserve normal parsing for positive lengths.
🪄 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 Plus
Run ID: f812a512-03ea-4654-a2a2-6747a732f1f8
📒 Files selected for processing (10)
library-model/library-model-generator/build.gradlelibrary-model/library-model-generator/src/shared/java/com/uber/nullaway/libmodel/NestedAnnotationInfo.javalibrary-model/library-model-generator/src/test/java/com/uber/nullaway/libmodel/NestedAnnotationInfoTest.javanullaway/build.gradlenullaway/src/main/java/com/uber/nullaway/LibraryModels.javanullaway/src/main/java/com/uber/nullaway/handlers/LibraryModelsHandler.javanullaway/src/main/java/com/uber/nullaway/handlers/StubxCacheUtil.javanullaway/src/main/java/com/uber/nullaway/librarymodel/AddAnnotationToNestedTypeVisitor.javanullaway/src/main/java/com/uber/nullaway/librarymodel/NestedAnnotationInfo.javatest-library-models/src/main/java/com/uber/nullaway/testlibrarymodels/TestLibraryModels.java
💤 Files with no reviewable changes (1)
- nullaway/src/main/java/com/uber/nullaway/librarymodel/NestedAnnotationInfo.java
There was a problem hiding this comment.
Caution
Inline review comments failed to post. This is likely due to GitHub's internal server error or limits when posting large numbers of comments. If you are seeing this consistently it is likely a permissions issue. Please check "Moderation" -> "Code review limits" under your organization settings.
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
`@library-model/library-model-generator/src/shared/java/com/uber/nullaway/libmodel/NestedAnnotationInfo.java`:
- Around line 17-22: Update StubxCacheUtil.parseStubStream() to validate
typePathLength before constructing NestedAnnotationInfo, treating zero or
negative lengths as malformed stubx input and reporting them through the
parser’s established format/error handling rather than allowing
IllegalArgumentException to escape. Keep NestedAnnotationInfo’s nonempty-path
invariant intact and preserve normal parsing for positive lengths.
🪄 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 Plus
Run ID: f812a512-03ea-4654-a2a2-6747a732f1f8
📒 Files selected for processing (10)
library-model/library-model-generator/build.gradlelibrary-model/library-model-generator/src/shared/java/com/uber/nullaway/libmodel/NestedAnnotationInfo.javalibrary-model/library-model-generator/src/test/java/com/uber/nullaway/libmodel/NestedAnnotationInfoTest.javanullaway/build.gradlenullaway/src/main/java/com/uber/nullaway/LibraryModels.javanullaway/src/main/java/com/uber/nullaway/handlers/LibraryModelsHandler.javanullaway/src/main/java/com/uber/nullaway/handlers/StubxCacheUtil.javanullaway/src/main/java/com/uber/nullaway/librarymodel/AddAnnotationToNestedTypeVisitor.javanullaway/src/main/java/com/uber/nullaway/librarymodel/NestedAnnotationInfo.javatest-library-models/src/main/java/com/uber/nullaway/testlibrarymodels/TestLibraryModels.java
💤 Files with no reviewable changes (1)
- nullaway/src/main/java/com/uber/nullaway/librarymodel/NestedAnnotationInfo.java
🛑 Comments failed to post (1)
library-model/library-model-generator/src/shared/java/com/uber/nullaway/libmodel/NestedAnnotationInfo.java (1)
17-22: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash set -eu rg -n -C 4 'typePathLength|methodNestedAnnotSize|NestedAnnotationInfo' --glob '*.java' .Repository: uber/NullAway
Length of output: 50370
🏁 Script executed:
#!/bin/bash set -eu echo "== candidate files ==" fd -a 'StubxCacheUtil|NestedAnnotationInfo|Stubx' library-model . | sed 's#^\./##' echo echo "== NestedAnnotationInfo.java ==" cat -n library-model/library-model-generator/src/shared/java/com/uber/nullaway/libmodel/NestedAnnotationInfo.java echo echo "== nested model tests ==" cat -n library-model/library-model-generator/src/test/java/com/uber/nullaway/libmodel/NestedAnnotationInfoTest.java echo echo "== targeted stubx references ==" rg -n -C 3 'typePathLength|typePath|nestedAnnot|NestedAnnotation|IllegalArgumentException|IOException' library-model/library-model-generator src --glob '*.java'Repository: uber/NullAway
Length of output: 21228
🏁 Script executed:
#!/bin/bash set -eu echo "== StubxCacheUtil outline and relevant lines ==" rg -n -C 5 'tryRead|methodNestedAnnotSize| NestedAnnotationInfo|typePath|IOException|parse|read' nullaway/src/main/java/com/uber/nullaway/handlers/StubxCacheUtil.java sed -n '1,260p' nullaway/src/main/java/com/uber/nullaway/handlers/StubxCacheUtil.java echo echo "== CreateNestedAnnotationInfoVisitor relevant lines ==" sed -n '1,260p' jdk-javac-plugin/src/main/java/com/uber/nullaway/javacplugin/CreateNestedAnnotationInfoVisitor.java echo echo "== other NestedAnnotationInfo construction sites ==" rg -n 'new NestedAnnotationInfo|NestedAnnotationInfo\(' --glob '*.java' .Repository: uber/NullAway
Length of output: 31711
Handle malformed nested stubx paths during parsing.
StubxCacheUtil.parseStubStream()readstypePathLengthand then constructsNestedAnnotationInfo; a zero or negative length makesparseStubStream()throwIllegalArgumentExceptionfrom athrows IOExceptionmethod and can turn an invalid.astubxinto an unexpected runtime error. Translate invalid lengths into the parser’s normal format/Error behavior instead of keeping the invariant only in the model record.🤖 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 `@library-model/library-model-generator/src/shared/java/com/uber/nullaway/libmodel/NestedAnnotationInfo.java` around lines 17 - 22, Update StubxCacheUtil.parseStubStream() to validate typePathLength before constructing NestedAnnotationInfo, treating zero or negative lengths as malformed stubx input and reporting them through the parser’s established format/error handling rather than allowing IllegalArgumentException to escape. Keep NestedAnnotationInfo’s nonempty-path invariant intact and preserve normal parsing for positive lengths.
fe86b32 to
b01c318
Compare
acab92d to
7c3c333
Compare
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
`@library-model/library-model-generator/src/shared/java/com/uber/nullaway/libmodel/NestedAnnotationInfo.java`:
- Around line 17-22: Add Javadoc to the compact constructor of
NestedAnnotationInfo documenting that typePath must be non-empty and that an
IllegalArgumentException is thrown when it is empty. Keep the existing
validation behavior unchanged.
In `@nullaway/src/main/java/com/uber/nullaway/LibraryModels.java`:
- Line 33: Document the package relocation of NestedAnnotationInfo as an
intentional breaking API change, noting that
LibraryModels.nestedAnnotationsForMethods() and ServiceLoader-based external
model providers must be rebuilt, including sample-library-model consumers.
🪄 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 Plus
Run ID: 48edefd3-8c4f-4d32-91c6-e85e5d0801b8
📒 Files selected for processing (10)
library-model/library-model-generator/build.gradlelibrary-model/library-model-generator/src/shared/java/com/uber/nullaway/libmodel/NestedAnnotationInfo.javalibrary-model/library-model-generator/src/test/java/com/uber/nullaway/libmodel/NestedAnnotationInfoTest.javanullaway/build.gradlenullaway/src/main/java/com/uber/nullaway/LibraryModels.javanullaway/src/main/java/com/uber/nullaway/handlers/LibraryModelsHandler.javanullaway/src/main/java/com/uber/nullaway/handlers/StubxCacheUtil.javanullaway/src/main/java/com/uber/nullaway/librarymodel/AddAnnotationToNestedTypeVisitor.javanullaway/src/main/java/com/uber/nullaway/librarymodel/NestedAnnotationInfo.javatest-library-models/src/main/java/com/uber/nullaway/testlibrarymodels/TestLibraryModels.java
💤 Files with no reviewable changes (1)
- nullaway/src/main/java/com/uber/nullaway/librarymodel/NestedAnnotationInfo.java
There was a problem hiding this comment.
Caution
Inline review comments failed to post. This is likely due to GitHub's internal server error or limits when posting large numbers of comments. If you are seeing this consistently it is likely a permissions issue. Please check "Moderation" -> "Code review limits" under your organization settings.
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
`@library-model/library-model-generator/src/shared/java/com/uber/nullaway/libmodel/NestedAnnotationInfo.java`:
- Around line 17-22: Add Javadoc to the compact constructor of
NestedAnnotationInfo documenting that typePath must be non-empty and that an
IllegalArgumentException is thrown when it is empty. Keep the existing
validation behavior unchanged.
In `@nullaway/src/main/java/com/uber/nullaway/LibraryModels.java`:
- Line 33: Document the package relocation of NestedAnnotationInfo as an
intentional breaking API change, noting that
LibraryModels.nestedAnnotationsForMethods() and ServiceLoader-based external
model providers must be rebuilt, including sample-library-model consumers.
🪄 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 Plus
Run ID: 48edefd3-8c4f-4d32-91c6-e85e5d0801b8
📒 Files selected for processing (10)
library-model/library-model-generator/build.gradlelibrary-model/library-model-generator/src/shared/java/com/uber/nullaway/libmodel/NestedAnnotationInfo.javalibrary-model/library-model-generator/src/test/java/com/uber/nullaway/libmodel/NestedAnnotationInfoTest.javanullaway/build.gradlenullaway/src/main/java/com/uber/nullaway/LibraryModels.javanullaway/src/main/java/com/uber/nullaway/handlers/LibraryModelsHandler.javanullaway/src/main/java/com/uber/nullaway/handlers/StubxCacheUtil.javanullaway/src/main/java/com/uber/nullaway/librarymodel/AddAnnotationToNestedTypeVisitor.javanullaway/src/main/java/com/uber/nullaway/librarymodel/NestedAnnotationInfo.javatest-library-models/src/main/java/com/uber/nullaway/testlibrarymodels/TestLibraryModels.java
💤 Files with no reviewable changes (1)
- nullaway/src/main/java/com/uber/nullaway/librarymodel/NestedAnnotationInfo.java
🛑 Comments failed to post (2)
library-model/library-model-generator/src/shared/java/com/uber/nullaway/libmodel/NestedAnnotationInfo.java (1)
17-22: 📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
Document the compact constructor.
This constructor enforces a public invariant and is non-trivial, but it has no Javadoc. Document the non-empty requirement and
IllegalArgumentException.As per coding guidelines, every non-trivial method, including private methods, must have Javadoc.
Proposed documentation
+ /** + * Validates that the nested annotation has a non-empty type path. + * + * `@throws` IllegalArgumentException if {`@code` typePath} is empty + */ public NestedAnnotationInfo {📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements./** * Validates that the nested annotation has a non-empty type path. * * `@throws` IllegalArgumentException if {`@code` typePath} is empty */ public NestedAnnotationInfo { if (typePath.isEmpty()) { throw new IllegalArgumentException( "Nested annotation type paths must be nonempty; use a top-level parameter or return model"); } }🤖 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 `@library-model/library-model-generator/src/shared/java/com/uber/nullaway/libmodel/NestedAnnotationInfo.java` around lines 17 - 22, Add Javadoc to the compact constructor of NestedAnnotationInfo documenting that typePath must be non-empty and that an IllegalArgumentException is thrown when it is empty. Keep the existing validation behavior unchanged.Source: Coding guidelines
nullaway/src/main/java/com/uber/nullaway/LibraryModels.java (1)
33-33: 🗄️ Data Integrity & Integration | 🔵 Trivial
🧩 Analysis chain
🏁 Script executed:
rg -n --glob '*.java' \ -e 'implements[[:space:]]+LibraryModels' \ -e 'nestedAnnotationsForMethods[[:space:]]*\(' . rg -n --glob '*.java' \ 'com\.uber\.nullaway\.librarymodel\.NestedAnnotationInfo' .Repository: uber/NullAway
Length of output: 2495
🏁 Script executed:
#!/bin/bash set -euo pipefail printf '--- files ---\n' git ls-files | rg '(^nullaway/src/main/java/com/uber/nullaway/LibraryModels.java$|nestedAnnotation|NestedAnnotationInfo|LibraryModelsHandler|TestLibraryModels|ExampleLibraryModels|gradle.build|pom.xml|build.gradle|settings.gradle)' printf '\n--- LibraryModels outline around nestedAnnotationsForMethods ---\n' ast-grep outline nullaway/src/main/java/com/uber/nullaway/LibraryModels.java --view expanded | sed -n '1,220p' || true printf '\n--- LibraryModels.java relevant lines ---\n' sed -n '1,80p;110,135p' nullaway/src/main/java/com/uber/nullaway/LibraryModels.java | cat -n printf '\n--- NestedAnnotationInfo definitions/imports/usages ---\n' rg -n --glob '*.java' 'class NestedAnnotationInfo|enum NestedAnnotationInfo|import .*NestedAnnotationInfo|NestedAnnotationInfo' nullaway test-library-models sample-library-model jdk-javac-plugin || true printf '\n--- ServiceLoader/LibraryModelsHandler integration ---\n' sed -n '1,70p;160,185p;440,470p;550,575p' nullaway/src/main/java/com/uber/nullaway/handlers/LibraryModelsHandler.java | cat -nRepository: uber/NullAway
Length of output: 242
🏁 Script executed:
#!/bin/bash set -euo pipefail echo '--- files ---' git ls-files | rg '(^nullaway/src/main/java/com/uber/nullaway/LibraryModels.java$|nestedAnnotation|NestedAnnotationInfo|LibraryModelsHandler|TestLibraryModels|ExampleLibraryModels|gradle.build|pom.xml|build.gradle|settings.gradle)' echo echo '--- LibraryModels.java relevant lines ---' sed -n '1,80p;110,135p' nullaway/src/main/java/com/uber/nullaway/LibraryModels.java | cat -n echo echo '--- NestedAnnotationInfo definitions/imports/usages ---' rg -n --glob '*.java' 'class NestedAnnotationInfo|enum NestedAnnotationInfo|import .*NestedAnnotationInfo|NestedAnnotationInfo' nullaway test-library-models sample-library-model jdk-javac-plugin || true echo echo '--- LibraryModelsHandler integration ---' sed -n '1,70p;160,185p;440,470p;550,575p' nullaway/src/main/java/com/uber/nullaway/handlers/LibraryModelsHandler.java | cat -n echo echo '--- LibraryModels implementations ---' sed -n '20,35p;90,115p' test-library-models/src/main/java/com/uber/nullaway/testlibrarymodels/TestLibraryModels.java | cat -n || true sed -n '185,205p' test-library-models/src/main/java/com/uber/nullaway/testlibrarymodels/TestLibraryModels.java | cat -n || true sed -n '20,35p;90,115p' sample-library-model/src/main/java/com/uber/modelexample/ExampleLibraryModels.java | cat -n || true sed -n '120,140p' nullaway/src/main/java/com/uber/nullaway/handlers/LibraryModelsHandler.java | cat -n || true sed -n '1220,1255p' nullaway/src/main/java/com/uber/nullaway/handlers/LibraryModelsHandler.java | cat -n || true sed -n '1355,1380p' nullaway/src/main/java/com/uber/nullaway/handlers/LibraryModelsHandler.java | cat -n || true sed -n '1468,1490p' nullaway/src/main/java/com/uber/nullaway/handlers/LibraryModelsHandler.java | cat -n || true sed -n '1648,1675p;1730,1760p' nullaway/src/main/java/com/uber/nullaway/handlers/LibraryModelsHandler.java | cat -n || trueRepository: uber/NullAway
Length of output: 47827
🏁 Script executed:
#!/bin/bash set -euo pipefail echo '--- NestedAnnotationInfo definition ---' sed -n '1,220p' library-model/library-model-generator/src/shared/java/com/uber/nullaway/libmodel/NestedAnnotationInfo.java | cat -n echo echo '--- LibraryModels method reference ---' sed -n '90,105p' nullaway/src/main/java/com/uber/nullaway/LibraryModels.java | cat -n echo echo '--- sample-library-model build metadata ---' cat sample-library-model/build.gradle | cat -n || true cat test-library-models/build.gradle | cat -n || trueRepository: uber/NullAway
Length of output: 6222
Document the
NestedAnnotationInfopackage relocation as an API breakage.
LibraryModels.nestedAnnotationsForMethods()returnsNestedAnnotationInfothroughServiceLoader, andsample-library-modelimports it directly. Movingcom.uber.nullaway.librarymodel.NestedAnnotationInfochanges the return-type contract for external model providers, so providers should be rebuilt; if this is intentional, document the breaking change for downstream users.🤖 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/LibraryModels.java` at line 33, Document the package relocation of NestedAnnotationInfo as an intentional breaking API change, noting that LibraryModels.nestedAnnotationsForMethods() and ServiceLoader-based external model providers must be rebuilt, including sample-library-model consumers.
Fixes #1657
NestedAnnotationInfowas duplicated between thenullawayandlibrary-model-generatormodules. This PR replaces the two copies with one shared source file owned bylibrary-model-generatorand compiled into both modules.Specifically, it:
library-model-generator.com.uber.nullaway.libmodel.NestedAnnotationInfo.NestedAnnotationInfoTesttolibrary-model-generator.NestedAnnotationInfo, rather than unrelated generator implementation classes, is included in the NullAway JAR.This changes the type exposed by
LibraryModels.nestedAnnotationsForMethods()fromcom.uber.nullaway.librarymodel.NestedAnnotationInfotocom.uber.nullaway.libmodel.NestedAnnotationInfo.Summary by CodeRabbit