Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #1394 +/- ##
=========================================
Coverage 88.19% 88.19%
Complexity 2616 2616
=========================================
Files 95 95
Lines 8675 8675
Branches 1743 1743
=========================================
Hits 7651 7651
Misses 510 510
Partials 514 514 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
WalkthroughThis pull request raises Java compilation targets across the build from Java 11 to Java 17 in multiple build.gradle files and test tasks, updates the project version to 0.13.0-SNAPSHOT, and disables three error-prone checks in the main build configuration. The annotations module retains Java 11 compatibility for annotations on the runtime classpath. No public API signatures were changed. Possibly related PRs
Suggested reviewers
Pre-merge checks and finishing touches✅ Passed checks (5 passed)
✨ Finishing touches
🧪 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
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (7)
annotations/build.gradlebuild.gradlegradle.propertiesguava-recent-unit-tests/build.gradlejar-infer/test-android-lib-jarinfer/build.gradlenullaway/build.gradlesample-app/build.gradle
🧰 Additional context used
🧠 Learnings (4)
📓 Common learnings
Learnt from: msridhar
Repo: uber/NullAway PR: 1245
File: guava-recent-unit-tests/src/test/java/com/uber/nullaway/guava/NullAwayGuavaParametricNullnessTests.java:101-102
Timestamp: 2025-08-14T18:50:06.159Z
Learning: In NullAway JSpecify tests, when JDK version requirements exist due to bytecode annotation reading capabilities, prefer failing tests over skipping them on unsupported versions to ensure CI catches regressions and enforces proper JDK version usage for developers.
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.
📚 Learning: 2025-08-14T18:50:06.159Z
Learnt from: msridhar
Repo: uber/NullAway PR: 1245
File: guava-recent-unit-tests/src/test/java/com/uber/nullaway/guava/NullAwayGuavaParametricNullnessTests.java:101-102
Timestamp: 2025-08-14T18:50:06.159Z
Learning: In NullAway JSpecify tests, when JDK version requirements exist due to bytecode annotation reading capabilities, prefer failing tests over skipping them on unsupported versions to ensure CI catches regressions and enforces proper JDK version usage for developers.
Applied to files:
gradle.propertiesbuild.gradlenullaway/build.gradleguava-recent-unit-tests/build.gradle
📚 Learning: 2025-10-09T19:59:16.543Z
Learnt from: msridhar
Repo: uber/NullAway PR: 1243
File: jdk-annotations/astubx-generator/build.gradle:22-22
Timestamp: 2025-10-09T19:59:16.543Z
Learning: When disabling testJdk17 tasks for modules requiring JDK 21, use `onlyIf { false }` to skip the task:
```gradle
tasks.named("testJdk17").configure {
onlyIf { false }
}
```
Do not use `doFirst { throw new GradleException(...) }` as it will cause CI failures when the task is executed.
Applied to files:
build.gradlenullaway/build.gradleguava-recent-unit-tests/build.gradlejar-infer/test-android-lib-jarinfer/build.gradle
📚 Learning: 2025-11-25T22:43:06.446Z
Learnt from: CR
Repo: uber/NullAway PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-25T22:43:06.446Z
Learning: Run only the tests for the main NullAway module using `./gradlew :nullaway:test` unless specifically asked to run tests in a different module
Applied to files:
nullaway/build.gradle
⏰ 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 caffeine with snapshot
- GitHub Check: Build and test on macos-latest
- GitHub Check: Build and test on windows-latest
- GitHub Check: Build spring-framework with snapshot
- GitHub Check: Build and test on ubuntu-latest
🔇 Additional comments (6)
guava-recent-unit-tests/build.gradle (1)
50-50: LGTM! Test launcher correctly updated to Java 17.This change aligns with the PR objective to require JDK 17 as the minimum runtime for NullAway.
gradle.properties (1)
15-15: LGTM! Version bump appropriate for breaking change.The bump to 0.13.0 correctly reflects that requiring JDK 17 is a breaking change that warrants a minor version increment.
nullaway/build.gradle (1)
115-115: LGTM! Test launcher correctly updated to Java 17.This change is consistent with the PR objective and aligns with the same update in guava-recent-unit-tests.
build.gradle (2)
89-93: Verify issue #1398 documents the rationale for disabling these checks.The disabled checks (PatternMatchingInstanceof, StatementSwitchToExpressionSwitch, MisformattedTestData) appear related to Java 17+ features. The PR description mentions follow-up issues #1396-#1399, which is good practice.
Please confirm that issue #1398 exists and documents why these checks are disabled and when they might be re-enabled.
123-124: LGTM! Core change to require Java 17 compilation target.This is the primary change that enforces JDK 17 as the minimum runtime across the project (except for the annotations module, which correctly preserves Java 11 compatibility).
annotations/build.gradle (1)
26-30: LGTM! Critical backward compatibility preserved for annotations.This correctly implements the PR objective to keep the annotations artifact at Java 11 compatibility. The comment clearly explains the rationale: clients may include annotations on their runtime classpath, so maintaining Java 11 compatibility avoids breaking those clients even though NullAway itself now requires JDK 17.
See #1170
We leave the
annotationsartifact at targeting JDK 11, since some clients may include that artifact in their runtime classpath. Also, our next release will be 0.13.0 due to this change.To do in follow ups:
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.