Merged
Conversation
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 55 minutes and 10 seconds. ⌛ How to resolve this issue?After the wait time has elapsed, 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 have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (4)
📝 WalkthroughWalkthroughThis change updates Checker Framework effect and purity annotations across many source files. Several String, StringBuilder, StringBuffer, and AbstractStringBuilder methods gained 🚥 Pre-merge checks | ❌ 1❌ Failed checks (1 warning)
✏️ 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
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)
src/java.base/share/classes/java/lang/StringBuffer.java (1)
306-312:⚠️ Potential issue | 🟠 MajorRemove
@SideEffectFreefromgetChars(...)because the method mutates the caller-provideddstarray.The Checker Framework's purity analysis explicitly forbids array element assignment (e.g.,
a[i] = x) under@SideEffectFree. Writing into thedstparameter at line 308 violates this contract.Suggested fix
- `@SideEffectFree` `@Override` public synchronized void getChars(int srcBegin, int srcEnd, char[] dst, `@IndexOrHigh`({"#3"}) int dstBegin)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/java.base/share/classes/java/lang/StringBuffer.java` around lines 306 - 312, The `@SideEffectFree` annotation must be removed from the synchronized overridden method getChars(int srcBegin, int srcEnd, char[] dst, int dstBegin) because it mutates the caller-provided dst array (via the super.getChars call); edit the method declaration to delete the `@SideEffectFree` annotation while keeping `@Override` and synchronized intact so the method signature remains the same.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@src/java.base/share/classes/java/lang/StringBuffer.java`:
- Around line 306-312: The `@SideEffectFree` annotation must be removed from the
synchronized overridden method getChars(int srcBegin, int srcEnd, char[] dst,
int dstBegin) because it mutates the caller-provided dst array (via the
super.getChars call); edit the method declaration to delete the `@SideEffectFree`
annotation while keeping `@Override` and synchronized intact so the method
signature remains the same.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 2132a3f0-13e2-4000-a10a-d89cf68fa3de
📒 Files selected for processing (3)
src/java.base/share/classes/java/lang/String.javasrc/java.base/share/classes/java/lang/StringBuffer.javasrc/java.base/share/classes/java/lang/StringBuilder.java
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/java.base/share/classes/java/lang/StringBuffer.java (1)
763-770:⚠️ Potential issue | 🟠 MajorChange
@SideEffectFreeto@SideEffectsOnly("this")—the annotation forbids instance field writes.The method mutates
toStringCacheat line 768, which violates the@SideEffectFreecontract. Per Checker Framework documentation,@SideEffectFreeexplicitly forbids assignments to instance fields, including caching operations. Use@SideEffectsOnly("this")instead to accurately declare receiver-only mutation.Proposed fix
- `@SideEffectFree` + `@SideEffectsOnly`("this") `@Override` `@IntrinsicCandidate` public synchronized String toString(`@GuardSatisfied` StringBuffer this) {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/java.base/share/classes/java/lang/StringBuffer.java` around lines 763 - 770, The `@SideEffectFree` annotation on StringBuffer.toString is incorrect because the method writes the instance field toStringCache; update the annotation to `@SideEffectsOnly`("this") on the synchronized String toString(`@GuardSatisfied` StringBuffer this) method in class StringBuffer so the receiver-only mutation is correctly declared (refer to method toString and field toStringCache).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/java.base/share/classes/java/lang/StringLatin1.java`:
- Around line 82-83: The file uses the `@SideEffectsOnly` annotation on
StringLatin1.getChars but lacks the import; add an import for the annotation
(import jdk.internal.vm.annotation.SideEffectsOnly;) at the top of the file so
`@SideEffectsOnly`("#3") on getChars(byte[] value, int srcBegin, int srcEnd,
char[] dst, int dstBegin) resolves correctly and leave the parameter "#3" as-is
targeting the mutable dst array.
In `@src/java.base/share/classes/jdk/internal/icu/text/Replaceable.java`:
- Line 120: The `@SideEffectsOnly` annotation in Replaceable.java is referenced
without an import; add the Checker Framework import for that annotation (import
org.checkerframework.dataflow.qual.SideEffectsOnly;) near the other imports so
the annotation resolves for the Replaceable interface and any methods annotated
with `@SideEffectsOnly`.
In `@src/java.base/share/classes/jdk/internal/icu/text/ReplaceableString.java`:
- Line 113: The file uses the annotation `@SideEffectsOnly` (in ReplaceableString)
but has no import, causing a compile error; add the Checker Framework import
org.checkerframework.framework.qual.SideEffectsOnly to the top of the file
(alongside other imports if added later) so the annotation resolves correctly.
---
Outside diff comments:
In `@src/java.base/share/classes/java/lang/StringBuffer.java`:
- Around line 763-770: The `@SideEffectFree` annotation on StringBuffer.toString
is incorrect because the method writes the instance field toStringCache; update
the annotation to `@SideEffectsOnly`("this") on the synchronized String
toString(`@GuardSatisfied` StringBuffer this) method in class StringBuffer so the
receiver-only mutation is correctly declared (refer to method toString and field
toStringCache).
🪄 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
Run ID: 07eb44eb-2661-4ce1-bbd8-b4a6e1c6a7a6
📒 Files selected for processing (7)
src/java.base/share/classes/java/lang/AbstractStringBuilder.javasrc/java.base/share/classes/java/lang/String.javasrc/java.base/share/classes/java/lang/StringBuffer.javasrc/java.base/share/classes/java/lang/StringLatin1.javasrc/java.base/share/classes/java/lang/StringUTF16.javasrc/java.base/share/classes/jdk/internal/icu/text/Replaceable.javasrc/java.base/share/classes/jdk/internal/icu/text/ReplaceableString.java
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@src/java.xml/share/classes/com/sun/org/apache/xerces/internal/impl/xs/util/XSObjectListImpl.java`:
- Line 24: Restore the Checker Framework import for SideEffectsOnly (the import
removed at the top) and add the `@SideEffectsOnly`("this") annotation to both
overriding next() methods: EmptyIterator.next() and XSObjectListIterator.next()
so they match the effect contract of ListIterator.next(); ensure the import name
matches the annotation and place the annotation immediately above each next()
method declaration.
🪄 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
Run ID: ff3b7641-0881-4d40-b529-f30e44424dca
📒 Files selected for processing (32)
src/java.base/share/classes/java/lang/StringLatin1.javasrc/java.base/share/classes/java/lang/StringUTF16.javasrc/java.base/share/classes/java/util/EnumMap.javasrc/java.management/share/classes/sun/management/counter/perf/PerfInstrumentation.javasrc/java.net.http/share/classes/jdk/internal/net/http/HeaderParser.javasrc/java.net.http/share/classes/jdk/internal/net/http/RequestPublishers.javasrc/java.sql/share/classes/java/sql/SQLException.javasrc/java.xml.crypto/share/classes/com/sun/org/apache/xml/internal/security/keys/keyresolver/KeyResolver.javasrc/java.xml.crypto/share/classes/com/sun/org/apache/xml/internal/security/keys/storage/StorageResolver.javasrc/java.xml.crypto/share/classes/com/sun/org/apache/xml/internal/security/keys/storage/implementations/KeyStoreResolver.javasrc/java.xml.crypto/share/classes/com/sun/org/apache/xml/internal/security/keys/storage/implementations/SingleCertificateResolver.javasrc/java.xml.crypto/share/classes/com/sun/org/apache/xml/internal/security/signature/Reference.javasrc/java.xml.crypto/share/classes/com/sun/org/apache/xml/internal/security/signature/reference/ReferenceSubTreeData.javasrc/java.xml.crypto/share/classes/org/jcp/xml/dsig/internal/dom/DOMSubTreeData.javasrc/java.xml/share/classes/com/sun/org/apache/bcel/internal/generic/InstructionList.javasrc/java.xml/share/classes/com/sun/org/apache/xerces/internal/impl/XMLStreamFilterImpl.javasrc/java.xml/share/classes/com/sun/org/apache/xerces/internal/impl/XMLStreamReaderImpl.javasrc/java.xml/share/classes/com/sun/org/apache/xerces/internal/impl/xs/XSModelImpl.javasrc/java.xml/share/classes/com/sun/org/apache/xerces/internal/impl/xs/util/XSNamedMapImpl.javasrc/java.xml/share/classes/com/sun/org/apache/xerces/internal/impl/xs/util/XSObjectListImpl.javasrc/java.xml/share/classes/com/sun/org/apache/xerces/internal/util/NamespaceSupport.javasrc/java.xml/share/classes/com/sun/org/apache/xerces/internal/util/XMLAttributesIteratorImpl.javasrc/java.xml/share/classes/com/sun/org/apache/xpath/internal/jaxp/XPathNodesImpl.javasrc/java.xml/share/classes/com/sun/xml/internal/stream/EventFilterSupport.javasrc/java.xml/share/classes/com/sun/xml/internal/stream/XMLEventReaderImpl.javasrc/java.xml/share/classes/com/sun/xml/internal/stream/util/ReadOnlyIterator.javasrc/java.xml/share/classes/javax/xml/catalog/CatalogImpl.javasrc/java.xml/share/classes/javax/xml/stream/XMLEventReader.javasrc/java.xml/share/classes/javax/xml/stream/XMLStreamReader.javasrc/java.xml/share/classes/javax/xml/stream/util/EventReaderDelegate.javasrc/java.xml/share/classes/javax/xml/stream/util/StreamReaderDelegate.javasrc/jdk.charsets/share/classes/sun/nio/cs/ext/AbstractCharsetProvider.java
💤 Files with no reviewable changes (22)
- src/java.xml/share/classes/com/sun/org/apache/xerces/internal/impl/xs/util/XSNamedMapImpl.java
- src/java.sql/share/classes/java/sql/SQLException.java
- src/java.xml/share/classes/com/sun/xml/internal/stream/XMLEventReaderImpl.java
- src/java.management/share/classes/sun/management/counter/perf/PerfInstrumentation.java
- src/java.xml/share/classes/javax/xml/stream/util/StreamReaderDelegate.java
- src/java.net.http/share/classes/jdk/internal/net/http/RequestPublishers.java
- src/jdk.charsets/share/classes/sun/nio/cs/ext/AbstractCharsetProvider.java
- src/java.xml/share/classes/com/sun/org/apache/xerces/internal/impl/XMLStreamReaderImpl.java
- src/java.xml.crypto/share/classes/com/sun/org/apache/xml/internal/security/keys/storage/implementations/KeyStoreResolver.java
- src/java.xml/share/classes/com/sun/org/apache/xerces/internal/impl/xs/XSModelImpl.java
- src/java.net.http/share/classes/jdk/internal/net/http/HeaderParser.java
- src/java.xml/share/classes/com/sun/xml/internal/stream/EventFilterSupport.java
- src/java.xml/share/classes/com/sun/xml/internal/stream/util/ReadOnlyIterator.java
- src/java.xml/share/classes/javax/xml/stream/util/EventReaderDelegate.java
- src/java.xml/share/classes/javax/xml/stream/XMLEventReader.java
- src/java.xml/share/classes/com/sun/org/apache/xerces/internal/impl/XMLStreamFilterImpl.java
- src/java.xml/share/classes/com/sun/org/apache/bcel/internal/generic/InstructionList.java
- src/java.xml/share/classes/javax/xml/stream/XMLStreamReader.java
- src/java.xml.crypto/share/classes/com/sun/org/apache/xml/internal/security/keys/storage/implementations/SingleCertificateResolver.java
- src/java.xml/share/classes/com/sun/org/apache/xerces/internal/util/XMLAttributesIteratorImpl.java
- src/java.base/share/classes/java/util/EnumMap.java
- src/java.xml.crypto/share/classes/com/sun/org/apache/xml/internal/security/keys/storage/StorageResolver.java
There was a problem hiding this comment.
♻️ Duplicate comments (2)
src/java.base/share/classes/jdk/internal/icu/text/Replaceable.java (1)
40-40:⚠️ Potential issue | 🔴 CriticalFix unresolved
@SideEffectsOnlyimport mismatch.Line 40 imports
SideEffectFree, but Line 122 uses@SideEffectsOnly. Without importingSideEffectsOnly, this can fail to compile.Proposed fix
-import org.checkerframework.dataflow.qual.SideEffectFree; +import org.checkerframework.dataflow.qual.SideEffectsOnly;#!/bin/bash set -euo pipefail f="src/java.base/share/classes/jdk/internal/icu/text/Replaceable.java" echo "Verifying imports/annotation usage in $f" rg -n '@SideEffectsOnly|import org\.checkerframework\.dataflow\.qual\.(SideEffectFree|SideEffectsOnly)' "$f"Also applies to: 122-122
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/java.base/share/classes/jdk/internal/icu/text/Replaceable.java` at line 40, The import and annotation mismatch in class Replaceable: the file imports org.checkerframework.dataflow.qual.SideEffectFree but the code uses `@SideEffectsOnly`; fix by making the import and annotation consistent—either change the import to import org.checkerframework.dataflow.qual.SideEffectsOnly or change the annotation on Replaceable to `@SideEffectFree` so the referenced symbol matches; update the import statement near the top and the annotation usage on the Replaceable class/method declaration to the chosen, matching identifier.src/java.base/share/classes/jdk/internal/icu/text/ReplaceableString.java (1)
35-35:⚠️ Potential issue | 🔴 CriticalImport
SideEffectsOnlyfor the new annotation usage.Line 115 adds
@SideEffectsOnly("#3"), but Line 35 importsSideEffectFreeinstead. This leavesSideEffectsOnlyunresolved.Proposed fix
-import org.checkerframework.dataflow.qual.SideEffectFree; +import org.checkerframework.dataflow.qual.SideEffectsOnly;#!/bin/bash set -euo pipefail f="src/java.base/share/classes/jdk/internal/icu/text/ReplaceableString.java" echo "Verifying imports/annotation usage in $f" rg -n '@SideEffectsOnly|import org\.checkerframework\.dataflow\.qual\.(SideEffectFree|SideEffectsOnly)' "$f"Also applies to: 115-115
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/java.base/share/classes/jdk/internal/icu/text/ReplaceableString.java` at line 35, The code uses the `@SideEffectsOnly`("#3") annotation but imports org.checkerframework.dataflow.qual.SideEffectFree, causing an unresolved annotation; update the imports in ReplaceableString to import org.checkerframework.dataflow.qual.SideEffectsOnly (and remove SideEffectFree if unused) so the `@SideEffectsOnly` annotation is resolved, ensuring the import and annotation names match exactly.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@src/java.base/share/classes/jdk/internal/icu/text/Replaceable.java`:
- Line 40: The import and annotation mismatch in class Replaceable: the file
imports org.checkerframework.dataflow.qual.SideEffectFree but the code uses
`@SideEffectsOnly`; fix by making the import and annotation consistent—either
change the import to import org.checkerframework.dataflow.qual.SideEffectsOnly
or change the annotation on Replaceable to `@SideEffectFree` so the referenced
symbol matches; update the import statement near the top and the annotation
usage on the Replaceable class/method declaration to the chosen, matching
identifier.
In `@src/java.base/share/classes/jdk/internal/icu/text/ReplaceableString.java`:
- Line 35: The code uses the `@SideEffectsOnly`("#3") annotation but imports
org.checkerframework.dataflow.qual.SideEffectFree, causing an unresolved
annotation; update the imports in ReplaceableString to import
org.checkerframework.dataflow.qual.SideEffectsOnly (and remove SideEffectFree if
unused) so the `@SideEffectsOnly` annotation is resolved, ensuring the import and
annotation names match exactly.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 847e9e42-d177-46a9-b6f4-24750a85ce5c
📒 Files selected for processing (2)
src/java.base/share/classes/jdk/internal/icu/text/Replaceable.javasrc/java.base/share/classes/jdk/internal/icu/text/ReplaceableString.java
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/java.base/share/classes/jdk/internal/icu/text/Replaceable.java`:
- Around line 40-42: The import list in Replaceable.java contains an unused
SideEffectFree import; remove the unused import declaration for
org.checkerframework.dataflow.qual.SideEffectFree and keep only the used
org.checkerframework.dataflow.qual.SideEffectsOnly so that the Replaceable
interface uses the single required annotation (`@SideEffectsOnly`) without the
redundant import.
In `@src/java.base/share/classes/jdk/internal/icu/text/ReplaceableString.java`:
- Around line 35-37: Remove the unused import SideEffectFree from
ReplaceableString to eliminate the dead import; keep the existing import and
usages of SideEffectsOnly intact (look for the import line "import
org.checkerframework.dataflow.qual.SideEffectFree;" and delete it, leaving
"import org.checkerframework.dataflow.qual.SideEffectsOnly;" and any annotations
that reference SideEffectsOnly unchanged).
🪄 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
Run ID: fb59e62c-4c9a-42b7-8712-0914c48ad98f
📒 Files selected for processing (2)
src/java.base/share/classes/jdk/internal/icu/text/Replaceable.javasrc/java.base/share/classes/jdk/internal/icu/text/ReplaceableString.java
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.