diff --git a/CHANGELOG.md b/CHANGELOG.md index 5f661bb3..8d518a05 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -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 diff --git a/src/serena/util/text_utils.py b/src/serena/util/text_utils.py index 986eed85..a926abec 100644 --- a/src/serena/util/text_utils.py +++ b/src/serena/util/text_utils.py @@ -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) diff --git a/test/serena/test_text_utils.py b/test/serena/test_text_utils.py index 865b71b4..6fbea535 100644 --- a/test/serena/test_text_utils.py +++ b/test/serena/test_text_utils.py @@ -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]")