Add JSpecify null annotations#379
Conversation
|
Warning Review limit reached
More reviews will be available in 28 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughAdds JSpecify: dependency and static module requirement, package-level ChangesJSpecify Nullability Annotations
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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.
Pull request overview
This pull request introduces JSpecify nullness annotations across the main ThreeTen-Extra codebase by applying package-level non-null defaults (@NullMarked) and adding targeted @Nullable annotations where null is permitted (notably equals(Object) parameters, selected isSupported(...) parameters that explicitly allow null, and resolver methods that may return null).
Changes:
- Add
org.jspecifyas an optional/static annotation dependency (Maven + JPMS) to support JSpecify usage. - Apply
@NullMarkedat package level fororg.threeten.extra,org.threeten.extra.chrono, andorg.threeten.extra.scale. - Add
@Nullableannotations to align method signatures/fields with existing “null returns false” and “may return null” behavior.
Reviewed changes
Copilot reviewed 39 out of 39 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| src/main/java/org/threeten/extra/YearWeek.java | Annotates isSupported(...) parameters and equals(...) parameter as nullable. |
| src/main/java/org/threeten/extra/Years.java | Annotates equals(...) parameter as nullable. |
| src/main/java/org/threeten/extra/YearQuarter.java | Annotates isSupported(...) parameters and equals(...) parameter as nullable. |
| src/main/java/org/threeten/extra/YearHalf.java | Annotates isSupported(...) parameters and equals(...) parameter as nullable. |
| src/main/java/org/threeten/extra/Weeks.java | Annotates equals(...) parameter as nullable. |
| src/main/java/org/threeten/extra/TemporalFields.java | Marks resolve(...) return type as nullable. |
| src/main/java/org/threeten/extra/Seconds.java | Annotates equals(...) parameter as nullable. |
| src/main/java/org/threeten/extra/scale/UtcInstant.java | Marks cached toString field and equals(...) parameter as nullable. |
| src/main/java/org/threeten/extra/scale/TaiInstant.java | Annotates equals(...) parameter as nullable. |
| src/main/java/org/threeten/extra/scale/package-info.java | Applies package-level @NullMarked for scale package. |
| src/main/java/org/threeten/extra/Quarter.java | Annotates isSupported(...) parameter as nullable. |
| src/main/java/org/threeten/extra/PeriodDuration.java | Annotates equals(...) parameter as nullable. |
| src/main/java/org/threeten/extra/PackedFields.java | Marks resolve(...) return types as nullable. |
| src/main/java/org/threeten/extra/package-info.java | Applies package-level @NullMarked for main package. |
| src/main/java/org/threeten/extra/OffsetDate.java | Annotates isSupported(...) parameters and equals(...) parameter as nullable. |
| src/main/java/org/threeten/extra/MutableClock.java | Annotates equals(...) parameter as nullable. |
| src/main/java/org/threeten/extra/Months.java | Annotates equals(...) parameter as nullable. |
| src/main/java/org/threeten/extra/Minutes.java | Annotates equals(...) parameter as nullable. |
| src/main/java/org/threeten/extra/LocalDateRange.java | Marks spliterator state and comparator return as nullable; annotates equals(...) parameter. |
| src/main/java/org/threeten/extra/Interval.java | Annotates equals(...) parameter as nullable. |
| src/main/java/org/threeten/extra/Hours.java | Annotates equals(...) parameter as nullable. |
| src/main/java/org/threeten/extra/HourMinute.java | Annotates isSupported(...) parameters and equals(...) parameter as nullable. |
| src/main/java/org/threeten/extra/Half.java | Annotates isSupported(...) parameter as nullable. |
| src/main/java/org/threeten/extra/Days.java | Annotates equals(...) parameter as nullable. |
| src/main/java/org/threeten/extra/DayOfYear.java | Annotates isSupported(...) and equals(...) parameters as nullable. |
| src/main/java/org/threeten/extra/DayOfMonth.java | Annotates isSupported(...), isValidYearMonth(...), and equals(...) parameters as nullable. |
| src/main/java/org/threeten/extra/chrono/Symmetry454Chronology.java | Marks getCalendarType() return as nullable. |
| src/main/java/org/threeten/extra/chrono/Symmetry010Chronology.java | Marks getCalendarType() return as nullable. |
| src/main/java/org/threeten/extra/chrono/package-info.java | Applies package-level @NullMarked for chrono package. |
| src/main/java/org/threeten/extra/chrono/InternationalFixedChronology.java | Marks getCalendarType() return as nullable. |
| src/main/java/org/threeten/extra/chrono/BritishCutoverDate.java | Annotates equals(...) parameter as nullable. |
| src/main/java/org/threeten/extra/chrono/BritishCutoverChronology.java | Marks getCalendarType() return as nullable. |
| src/main/java/org/threeten/extra/chrono/AccountingDate.java | Annotates equals(...) parameter as nullable. |
| src/main/java/org/threeten/extra/chrono/AccountingChronologyBuilder.java | Marks builder fields as nullable where unset until configured. |
| src/main/java/org/threeten/extra/chrono/AccountingChronology.java | Marks getCalendarType() return and equals(...) parameter as nullable. |
| src/main/java/org/threeten/extra/chrono/AbstractDate.java | Annotates equals(...) parameter as nullable. |
| src/main/java/org/threeten/extra/AmPm.java | Annotates isSupported(...) parameter as nullable. |
| src/main/java/module-info.java | Adds requires static org.jspecify; for JPMS builds. |
| pom.xml | Adds optional JSpecify dependency and version property. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
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 `@pom.xml`:
- Around line 508-514: Remove the optional=true attribute and the copied
"mandatory in Scala" comment from the JSpecify dependency entry so the
org.jspecify:jspecify artifact is not marked optional; update the dependency
block that contains groupId org.jspecify and artifactId jspecify to omit the
<optional> element (and delete the inline comment) so downstream consumers can
access the annotations.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: c458efe7-9b78-4a7e-abb8-c44fb52c9bd9
📒 Files selected for processing (39)
pom.xmlsrc/main/java/module-info.javasrc/main/java/org/threeten/extra/AmPm.javasrc/main/java/org/threeten/extra/DayOfMonth.javasrc/main/java/org/threeten/extra/DayOfYear.javasrc/main/java/org/threeten/extra/Days.javasrc/main/java/org/threeten/extra/Half.javasrc/main/java/org/threeten/extra/HourMinute.javasrc/main/java/org/threeten/extra/Hours.javasrc/main/java/org/threeten/extra/Interval.javasrc/main/java/org/threeten/extra/LocalDateRange.javasrc/main/java/org/threeten/extra/Minutes.javasrc/main/java/org/threeten/extra/Months.javasrc/main/java/org/threeten/extra/MutableClock.javasrc/main/java/org/threeten/extra/OffsetDate.javasrc/main/java/org/threeten/extra/PackedFields.javasrc/main/java/org/threeten/extra/PeriodDuration.javasrc/main/java/org/threeten/extra/Quarter.javasrc/main/java/org/threeten/extra/Seconds.javasrc/main/java/org/threeten/extra/TemporalFields.javasrc/main/java/org/threeten/extra/Weeks.javasrc/main/java/org/threeten/extra/YearHalf.javasrc/main/java/org/threeten/extra/YearQuarter.javasrc/main/java/org/threeten/extra/YearWeek.javasrc/main/java/org/threeten/extra/Years.javasrc/main/java/org/threeten/extra/chrono/AbstractDate.javasrc/main/java/org/threeten/extra/chrono/AccountingChronology.javasrc/main/java/org/threeten/extra/chrono/AccountingChronologyBuilder.javasrc/main/java/org/threeten/extra/chrono/AccountingDate.javasrc/main/java/org/threeten/extra/chrono/BritishCutoverChronology.javasrc/main/java/org/threeten/extra/chrono/BritishCutoverDate.javasrc/main/java/org/threeten/extra/chrono/InternationalFixedChronology.javasrc/main/java/org/threeten/extra/chrono/Symmetry010Chronology.javasrc/main/java/org/threeten/extra/chrono/Symmetry454Chronology.javasrc/main/java/org/threeten/extra/chrono/package-info.javasrc/main/java/org/threeten/extra/package-info.javasrc/main/java/org/threeten/extra/scale/TaiInstant.javasrc/main/java/org/threeten/extra/scale/UtcInstant.javasrc/main/java/org/threeten/extra/scale/package-info.java
|
Unfortunately, JSpecify has no firm conclusion as to what library authors are supposed to do with the My choice is to use |
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 `@src/main/java/org/threeten/extra/chrono/AccountingChronologyBuilder.java`:
- Around line 193-196: The builder's toChronology() currently uses
Objects.requireNonNull on endsOn, end, and division which throws
NullPointerException, violating the method's documented DateTimeException
contract; update toChronology() so it checks the same fields (endsOn, end,
division, and any other required state like
inLastWeek/leapWeekInMonth/yearOffset if applicable) and throws
java.time.DateTimeException with clear messages when a required field is unset,
then call AccountingChronology.create(...) only after all validations pass to
preserve the documented exception behavior.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 72898300-eb70-418b-8563-cac201320160
📒 Files selected for processing (8)
src/main/java/org/threeten/extra/OffsetDate.javasrc/main/java/org/threeten/extra/PeriodDuration.javasrc/main/java/org/threeten/extra/YearHalf.javasrc/main/java/org/threeten/extra/YearQuarter.javasrc/main/java/org/threeten/extra/chrono/AccountingChronology.javasrc/main/java/org/threeten/extra/chrono/AccountingChronologyBuilder.javasrc/main/java/org/threeten/extra/chrono/AccountingDate.javasrc/main/java/org/threeten/extra/chrono/BritishCutoverDate.java
🚧 Files skipped from review as they are similar to previous changes (1)
- src/main/java/org/threeten/extra/chrono/AccountingChronology.java
Introduce JSpecify nullness annotations to ThreeTen-Extra.
JSpecify is becoming the de facto standard for nullness annotations in Java. It allows methods and fields to be marked as accepting null, with a non-null default.
This change introduces the annotations to the main code, not the tests.
See #170
Summary by CodeRabbit