Skip to content

[TEXT-241] TextStringBuilder.lastIndexOf("") and StrBuilder.lastIndexOf("") return incorrect index for empty string (size - 1 instead of size) - #763

Merged
garydgregory merged 3 commits into
apache:masterfrom
kzhunmax:TEXT-241-lastindexof-empty-search-strings
Jul 30, 2026
Merged

[TEXT-241] TextStringBuilder.lastIndexOf("") and StrBuilder.lastIndexOf("") return incorrect index for empty string (size - 1 instead of size)#763
garydgregory merged 3 commits into
apache:masterfrom
kzhunmax:TEXT-241-lastindexof-empty-search-strings

Conversation

@kzhunmax

Copy link
Copy Markdown
Contributor

StrBuilder.lastIndexOf(String) and extStringBuilder.lastIndexOf(String)
do not match java.lang.String / StringBuilder for the empty string.

For a builder of length n, lastIndexOf("") should return n (empty match
at the end). Both implementations currently return n - 1.

Cause:
lastIndexOf(String, int) clamps startIndex with:
startIndex = startIndex >= size ? size - 1 : startIndex
before the empty-string case, which returns that clamped index. So
lastIndexOf("") can never return size.

Example:

new StrBuilder("abab").lastIndexOf("") // actual 3, expected 4
"abab".lastIndexOf("") // 4
new StringBuilder("abab").lastIndexOf("") // 4
Same issue in TextStringBuilder.

Fix:
In lastIndexOf(String, int), for an empty search string allow the start
index up to size and return that, keep the existing size - 1 clamp for non-empty strings.

Affects:

  • org.apache.commons.text.StrBuilder
  • org.apache.commons.text.TextStringBuilder

@kzhunmax

Copy link
Copy Markdown
Contributor Author

Jira ticket TEXT-241

@kzhunmax

Copy link
Copy Markdown
Contributor Author

Additionally, I have question on what code is preferred startIndex = startIndex >= size ? size : startIndex; or Math.min(startIndex, size)?

@garydgregory

Copy link
Copy Markdown
Member

@kzhunmax
I prefer Math.min(startIndex, size).

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Fixes lastIndexOf("") behavior in StrBuilder and TextStringBuilder to match java.lang.String / StringBuilder semantics (for a builder of length n, lastIndexOf("") returns n).

Changes:

  • Update lastIndexOf(String) to delegate using startIndex = size so empty-search can return size.
  • Adjust lastIndexOf(String, int) clamping so empty search strings allow startIndex up to size, while non-empty searches still clamp to size - 1.
  • Add regression tests for lastIndexOf("") on a non-empty builder.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.

File Description
src/main/java/org/apache/commons/text/TextStringBuilder.java Fixes empty-string lastIndexOf clamping to allow returning size.
src/main/java/org/apache/commons/text/StrBuilder.java Same fix as TextStringBuilder for parity with JDK behavior.
src/test/java/org/apache/commons/text/TextStringBuilderTest.java Adds regression assertion for lastIndexOf("") returning the builder length.
src/test/java/org/apache/commons/text/StrBuilderTest.java Adds regression assertion for lastIndexOf("") and adds @Deprecated annotation to match the existing Javadoc deprecation tag.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread src/test/java/org/apache/commons/text/StrBuilderTest.java
Comment thread src/test/java/org/apache/commons/text/TextStringBuilderTest.java
@garydgregory
garydgregory merged commit a0b0181 into apache:master Jul 30, 2026
10 checks passed
@garydgregory

Copy link
Copy Markdown
Member

Merged 🚀 , thank you @kzhunmax

garydgregory added a commit that referenced this pull request Jul 30, 2026
StrBuilder.lastIndexOf("") return incorrect index for empty string (size
- 1 instead of size) (#763).
@kzhunmax
kzhunmax deleted the TEXT-241-lastindexof-empty-search-strings branch July 30, 2026 11:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants