fix(tools): expand unmatched-group $!N backreferences to the empty string (#2070)

* fix(tools): expand unmatched-group $!N backreferences to the empty string

A $!N backreference in a regex-mode replacement template refers to the Nth
group of the search expression. When the group exists but did not participate
in the match (e.g. it sits inside an optional construct that was skipped), the
expansion emitted the literal template text instead of the empty string.

Observed in practice when an agent replaced 15 occurrences in an MQL header
with a template containing EA_INPUT$!1(...): the group was optional and never
participated, so every site came out containing the literal EA_INPUT$!1(...).

A reference to a group the expression does not define at all now raises a
clear ValueError instead of a raw IndexError - which also crashed
literal-mode replacements whose template contained $!N, since literal mode
compiles an escaped pattern without any groups.

* chore(ci): re-trigger test workflow

The previous run failed in native (macos-latest) on
test_cpp_basic.py::test_get_document_symbols[cpp_ccls] ("Expected 'main'
in document symbols, got: []"), a ccls language-server startup/indexing
flake unrelated to this PR's changes (text_utils.py and its tests only).

* fix(tools): pass literal-mode replacement through verbatim (no $!N expansion)

ContentReplacer ran the $!N backreference expansion on the replacement
template in both modes, so a literal-mode replacement whose template
contained a $!N sequence (e.g. documenting the convention itself in a
memory) failed with a backreference error instead of writing the text.
This contradicted the tools' documented contract ('the replacement string
(verbatim)' in literal mode) and the behavior of the dry-run path, where
MultiFileContentReplacer.find_occurrences already gated the expansion on
regex mode.

The expansion is now gated on the mode, mirroring the dry-run path; the
ambiguity validation still applies in both modes. The literal-mode test
that asserted the crash is inverted accordingly and the changelog entry
is corrected.
This commit is contained in:
davalillo authored and GitHub committed 2026-09-21 20:46:05 +02:00
1 parent 7ccffb6ee3
commit d80da2cdc6
3 files changed
+90 -12

No files matched your search

+6
View File
@@ -55,6 +55,12 @@ Status of the `main` branch. Changes prior to the next official version change w
including its project configuration, are left untouched (#2029)
* Tools:
- Fix: `$!N` backreferences in regex-mode replacements expanded to the literal template text
(e.g. `EA_INPUT$!1(...)`) when the referenced group existed but did not participate in the
match (e.g. a group inside an optional construct that was skipped); unmatched groups now expand
to the empty string, and a reference to a group that the search expression does not define
raises a clear error instead of a raw `IndexError`. In literal mode, the replacement is now
used verbatim (`$!N` sequences need no escaping) instead of failing with a backreference error
- Fix: the file-editing tools saved the edited file with `open(path, "w")`, which truncates it
before the new content is complete, so a crash, an OOM kill or a full disk partway through the
write could leave a source file empty or half-written. Saves now go through the same atomic
+28 -11
View File
@@ -406,13 +406,18 @@ class ContentReplacer:
self.regex_multiline = regex_multiline
@staticmethod
def _create_replacement_function(regex_pattern: str, repl_template: str, regex_flags: int) -> Callable[[re.Match], str]:
def _create_replacement_function(
regex_pattern: str, repl_template: str, regex_flags: int, expand_backrefs: bool
) -> Callable[[re.Match], str]:
"""
Creates a replacement function that validates for ambiguity and handles backreferences.
:param regex_pattern: The regex pattern being used for matching
:param repl_template: The replacement template with $!1, $!2, etc. for backreferences
:param repl_template: The replacement template; in regex mode, it may contain $!1, $!2, etc. for
backreferences; in literal mode, it is used verbatim
:param regex_flags: The flags to use when searching (e.g., re.DOTALL | re.MULTILINE)
:param expand_backrefs: Whether $!N backreferences are expanded in the template; false in literal mode,
mirroring the mode gate in MultiFileContentReplacer.find_occurrences
:return: A function suitable for use with re.sub() or re.subn()
"""
@@ -434,11 +439,19 @@ class ContentReplacer:
"e.g. by matching specific context after the match, or try using the literal mode."
)
# Handle backreferences: replace $!1, $!2, etc. with actual matched groups
# in literal mode, the template is the final replacement; $!N sequences need no escaping
if not expand_backrefs:
return repl_template
# Handle backreferences: replace $!1, $!2, etc. with actual matched groups; groups that
# exist but did not participate in the match expand to the empty string
def expand_backreference(m: re.Match) -> str:
group_num = int(m.group(1))
group_value = match.group(group_num)
return group_value if group_value is not None else m.group(0)
try:
group_value = match.group(group_num)
except IndexError as e:
raise ValueError(f"Backreference $!{group_num} refers to a group that does not exist in the search expression") from e
return group_value if group_value is not None else ""
result = re.sub(r"\$!(\d+)", expand_backreference, repl_template)
return result
@@ -458,8 +471,8 @@ class ContentReplacer:
:param content: the content in which to perform the replacement
:param needle: the search expression, which is either a literal string or a regular expression, depending on the mode
:param repl: the replacement string, which, in regex mode, may contain backreferences in the form of $!1, $!2, etc. to
refer to matched groups in the search expression
:param repl: the replacement string; in regex mode, it may contain backreferences in the form of $!1, $!2, etc.
to refer to matched groups in the search expression; in literal mode, it is used verbatim
:return: the updated content after performing the replacement
"""
if self.mode == "literal":
@@ -471,8 +484,8 @@ class ContentReplacer:
regex_flags = (re.MULTILINE | re.DOTALL) if self.regex_multiline else 0
# create replacement function with validation and backreference handling
repl_fn = self._create_replacement_function(regex, repl, regex_flags=regex_flags)
# create replacement function with ambiguity validation and, in regex mode, backreference handling
repl_fn = self._create_replacement_function(regex, repl, regex_flags=regex_flags, expand_backrefs=self.mode == "regex")
# perform replacement
updated_content, n = re.subn(regex, repl_fn, content, flags=regex_flags)
@@ -548,8 +561,12 @@ class MultiFileContentReplacer:
"""Expands $!1, $!2, ... in the replacement template (same syntax as :class:`ContentReplacer`)."""
def expand(m: re.Match) -> str:
group_value = match.group(int(m.group(1)))
return group_value if group_value is not None else m.group(0)
group_num = int(m.group(1))
try:
group_value = match.group(group_num)
except IndexError as e:
raise ValueError(f"Backreference $!{group_num} refers to a group that does not exist in the search expression") from e
return group_value if group_value is not None else ""
return re.sub(r"\$!(\d+)", expand, repl_template)
+56 -1
View File
@@ -3,7 +3,14 @@ from collections.abc import Callable
import pytest
from serena.util.file_proxy import FileCollection, FileProxy
from serena.util.text_utils import GlobMatcher, LineType, MultiFileContentReplacer, search_files, search_text
from serena.util.text_utils import (
ContentReplacer,
GlobMatcher,
LineType,
MultiFileContentReplacer,
search_files,
search_text,
)
class TestSearchText:
@@ -656,3 +663,51 @@ class TestMultiFileContentReplacer:
occ = replacer.find_occurrences([(path, content)], "old_pkg", "new_pkg")[0]
with pytest.raises(AssertionError):
replacer.apply_to_content("completely different content", [occ])
class TestBackreferenceExpansion:
"""$!N backreferences in regex-mode replacements refer to matched groups. A group that
exists but did not participate in the match (e.g. inside an optional construct that was
skipped) must expand to the empty string; a reference to a group that the search
expression does not define at all must fail with an error naming the problem instead of
a raw IndexError. Literal mode has no backreference expansion at all: the replacement is
used verbatim (observed in practice when an agent tried to document the $!N convention
itself and the literal-mode replacement crashed instead of writing the text).
"""
def test_unmatched_group_expands_to_empty_string(self):
replacer = ContentReplacer(mode="regex", allow_multiple_occurrences=False)
needle = r"EA_INPUT(?:\((\w*)\))?"
# the group participated and captured an empty string (empty parentheses)
assert replacer.replace("EA_INPUT()\n", needle, r"EA_INPUT$!1(...)") == "EA_INPUT(...)\n"
# the group did not participate at all (no parentheses)
assert replacer.replace("EA_INPUT\n", needle, r"EA_INPUT$!1(...)") == "EA_INPUT(...)\n"
def test_matched_group_expands_to_its_value(self):
replacer = ContentReplacer(mode="regex", allow_multiple_occurrences=False)
assert replacer.replace("id=alpha", r"id=(\w+)", r"[$!1]") == "[alpha]"
def test_nonexistent_group_reference_raises_clear_error(self):
replacer = ContentReplacer(mode="regex", allow_multiple_occurrences=False)
with pytest.raises(ValueError, match="does not exist"):
replacer.replace("id=alpha", r"id=(\w+)", r"[$!2]")
def test_literal_mode_repl_is_verbatim(self):
"""Literal mode has no groups at all and no backreference expansion: a replacement
containing $!N sequences is written as-is instead of failing with a backreference error.
"""
replacer = ContentReplacer(mode="literal", allow_multiple_occurrences=False)
assert replacer.replace("literal needle", "literal needle", "$!1 stuff $!2") == "$!1 stuff $!2"
def test_multi_file_replacer_expands_unmatched_group_to_empty_string(self):
replacer = MultiFileContentReplacer(mode="regex")
files = [("f.txt", "EA_INPUT\n")]
occurrences = replacer.find_occurrences(files, r"EA_INPUT(?:\((\w*)\))?", r"EA_INPUT$!1(...)")
assert [o.replacement for o in occurrences] == ["EA_INPUT(...)"]
def test_multi_file_replacer_nonexistent_group_reference_raises_clear_error(self):
replacer = MultiFileContentReplacer(mode="regex")
files = [("f.txt", "id=alpha\n")]
with pytest.raises(ValueError, match="does not exist"):
replacer.find_occurrences(files, r"id=(\w+)", r"[$!2]")