Skip to content

Fix lookaround regexes in String.lastIndexOf and replaceLast - #1908

Open
kwy404 wants to merge 1 commit into
apple:mainfrom
kwy404:fix-findlast-lookaround
Open

kwy404 wants to merge 1 commit into
apple:mainfrom
kwy404:fix-findlast-lookaround

Conversation

@kwy404

@kwy404 kwy404 commented Oct 11, 2026

Copy link
Copy Markdown
Contributor

Root cause

findLast in StringNodes finds the last regex match, then repositions the matcher on it with m.region(r.start(), r.end()).lookingAt(). A region uses opaque bounds by default, so lookahead and lookbehind cannot see the text outside the old match and the second match attempt fails. findLast then returns false even though a match exists:

"abab".lastIndexOf(Regex("a(?=b)"))      // error: does not contain a match, expected 2
"xab".replaceLast(Regex("(?<=x)ab"), "-") // "xab", expected "x-"

This affects the regex overloads of lastIndexOf, lastIndexOfOrNull, replaceLast and replaceLastMapped.

Fix

Reposition with m.find(r.start()) instead. It resets the matcher and searches the whole input from the start of the last match, so lookaround sees the full string and the same match is found again.

Test

Added a lookahead case to lastIndexOf() and a lookbehind case to replaceLast() in the api/string language snippet test. Before the fix, LanguageSnippetTests fails on api/string.pkl (lastIndexOf throws and replaceLast returns the string unchanged). After the fix it passes, and ./gradlew :pkl-core:spotlessCheck is clean.

`findLast` repositioned the matcher on the last match with
`region(start, end).lookingAt()`. Regions have opaque bounds by default,
so lookahead and lookbehind could not see the text around the match and
the second attempt failed. `"abab".lastIndexOf(Regex("a(?=b)"))` threw
"does not contain a match" and `replaceLast` left the string unchanged.

Reposition with `find(start)` instead, which searches the whole input
from the start of the last match.

This branch has not been deployed

No deployments
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.

1 participant