diff --git a/CHANGELOG.md b/CHANGELOG.md index 37b517051..5f2f8d3a7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,6 @@ # **Upcoming release** +- #858 Fix `find_definition`/Rename mis-handling class methods defined after an f-string with an unmatched literal bracket (Python 3.12+) (@gadievron) - #850 Update and pin black version in pre-commit and Github Actions - #851 Bump supported python version to up to Python 3.14 - #852 Implement patchedast handlers for TypeAlias diff --git a/rope/base/simplify.py b/rope/base/simplify.py index 3198c26de..b2a58960c 100644 --- a/rope/base/simplify.py +++ b/rope/base/simplify.py @@ -3,7 +3,10 @@ This module is here to help source code analysis. """ +import io import re +import sys +import tokenize from rope.base import codeanalyze, utils @@ -23,16 +26,25 @@ def real_code(source): only in offsets. """ collector = codeanalyze.ChangeCollector(source) + has_fstring = False for start, end, matchgroups in ignored_regions(source): if source[start] == "#": replacement = " " * (end - start) elif "f" in matchgroups.get("prefix", "").lower(): replacement = None + has_fstring = True else: replacement = '"%s"' % (" " * (end - start - 2)) if replacement is not None: collector.add_change(start, end, replacement) source = collector.get_changed() or source + if has_fstring: + # f-strings are left untouched above so their expression parts keep + # their real offsets, but that leaves any literal or format-spec + # `()[]{}` text (e.g. the `[` in `f"[{x}"`) sitting in `source`, + # where the `_parens` pass below would miscount it as a real + # unmatched bracket and corrupt every following offset. Blank it. + source = _blank_fstring_literal_brackets(source) collector = codeanalyze.ChangeCollector(source) parens = 0 for match in _parens.finditer(source): @@ -48,6 +60,60 @@ def real_code(source): return source.replace("\\\n", " ").replace("\t", " ").replace(";", "\n") +def _blank_fstring_literal_brackets(source): + """Blank `()[]{}` chars that are f-string literal/format-spec text. + + Blanked characters become a single space each, so length and every + character offset are preserved exactly. + + Approach: tokenize `source` and protect every character covered by an + `OP` token. A `()[]{}` character that no `OP` token covers cannot be + real syntax -- comments and non-f strings are already blanked, so the + only place it can occur is inside an f-string's literal or format-spec + text. Real expression brackets living inside an f-string (e.g. the + `[`/`]` in `f"{[1,2][0]}"`, or `{width}` in `f"{x:{width}}"`) are `OP` + tokens, so they stay untouched. A bracket *character* that is literal + data inside a nested string (the `}` in `f"{d['}']}"`) is not an `OP` + token, so it is blanked to a space -- correct, since it is data, not a + real bracket, and must not sway the `_parens` continuation count. This + also handles `{{`/`}}` escapes: tokenize covers the doubled brace with + a single FSTRING_MIDDLE token, leaving the escaping second brace under + no token span -- and, not being an `OP` either, the complement-of-OP + rule correctly blanks it. + + Only Python 3.12+ can make this distinction (PEP 701 added + FSTRING_START/MIDDLE/END; earlier an f-string is one opaque STRING + token). On earlier versions this is a no-op and the pre-existing + behaviour is unchanged. + """ + if sys.version_info < (3, 12): + return source + try: + tokens = list(tokenize.generate_tokens(io.StringIO(source).readline)) + except (tokenize.TokenError, SyntaxError, IndentationError, ValueError): + # `source` may be incomplete/invalid mid-edit (e.g. an unterminated + # f-string or an unbalanced paren -- exactly what the `_parens` pass + # handles). Fall back to the pre-existing behaviour. + return source + + lines = codeanalyze.SourceLinesAdapter(source) + + def offset(position): + row, col = position + return lines.get_line_start(row) + col + + protected = set() + for tok in tokens: + if tok.type == tokenize.OP: + protected.update(range(offset(tok.start), offset(tok.end))) + + collector = codeanalyze.ChangeCollector(source) + for i, c in enumerate(source): + if c in "()[]{}" and i not in protected: + collector.add_change(i, i + 1, " ") + return collector.get_changed() or source + + @utils.cached(7) def ignored_regions(source): """Return ignored regions like strings and comments in `source`""" diff --git a/ropetest/contrib/findittest.py b/ropetest/contrib/findittest.py index c436428b4..d1b68f1d3 100644 --- a/ropetest/contrib/findittest.py +++ b/ropetest/contrib/findittest.py @@ -3,6 +3,7 @@ from rope.base import exceptions from rope.contrib.findit import find_definition, find_implementations, find_occurrences +from rope.refactor.rename import Rename from ropetest import testutils @@ -145,3 +146,60 @@ def test_find_definition_in_other_modules(self): result = find_definition(self.project, code, code.index("var")) self.assertEqual(mod1, result.resource) self.assertEqual(0, result.offset) + + # A class-method def following an f-string with an unmatched literal + # bracket: the unmatched bracket used to fold every following newline, + # desyncing offsets so worder mis-read the method def header -- + # find_definition returned None (or the call site) and Rename silently + # corrupted the file (renamed the call, orphaned the def). Module-level + # defs after the same f-string stayed immune. + + _FSTRING_METHOD_CODE = dedent("""\ + class C(object): + def run(self): + label = f"[{self}" + self.target() + + def target(self): + pass + """) + + @testutils.only_for_versions_higher("3.12") + def test_find_definition_of_method_after_fstring_bracket(self): + code = self._FSTRING_METHOD_CODE + call_offset = code.index("self.target()") + len("self.") + result = find_definition(self.project, code, call_offset) + def_offset = code.index("def target") + len("def ") + self.assertIsNotNone(result) + self.assertEqual(def_offset, result.offset) + + @testutils.only_for_versions_higher("3.12") + def test_rename_heals_method_after_fstring_bracket(self): + mod = testutils.create_module(self.project, "mod") + code = self._FSTRING_METHOD_CODE + mod.write(code) + call_offset = code.index("self.target()") + len("self.") + changes = Rename(self.project, mod, call_offset).get_changes("renamed") + self.project.do(changes) + result = mod.read() + # both the call and the def must be renamed -- not silently orphaned + self.assertEqual(0, result.count("target")) + self.assertEqual(2, result.count("renamed")) + + @testutils.only_for_versions_higher("3.12") + def test_module_function_after_fstring_still_resolves(self): + # module-level defs after the same f-string are immune and must stay working + code = dedent("""\ + def run(): + label = f"[{run}" + + def target(): + pass + + target() + """) + call_offset = code.rindex("target") + result = find_definition(self.project, code, call_offset) + def_offset = code.index("def target") + len("def ") + self.assertIsNotNone(result) + self.assertEqual(def_offset, result.offset) diff --git a/ropetest/simplifytest.py b/ropetest/simplifytest.py index a8795b988..f9d9a82b5 100644 --- a/ropetest/simplifytest.py +++ b/ropetest/simplifytest.py @@ -1,6 +1,7 @@ import unittest from rope.base import simplify +from ropetest import testutils class SimplifyTest(unittest.TestCase): @@ -62,3 +63,76 @@ def test_simplifying_f_string_containing_quotes(self): def test_simplifying_uppercase_f_string_containing_quotes(self): code = """s = Fr"..'{hello}'.."\n""" self.assertEqual("""s = Fr"..'{hello}'.."\n""", simplify.real_code(code)) + + # --- f-string literal/format-spec bracket folding (see CHANGELOG) --- + # An unmatched ``()[]{}`` in an f-string's *literal* or *format-spec* + # text must not be counted by the implicit-continuation pass, or every + # following newline is blanked and later offsets desync. Each of the + # following asserts the newline count is preserved (i.e. not folded). + + @testutils.only_for_versions_higher("3.12") + def test_fstring_literal_open_bracket_not_folded(self): + code = 'f"[{x}"\ndef g(): pass\n' + self.assertEqual(code.count("\n"), simplify.real_code(code).count("\n")) + + @testutils.only_for_versions_higher("3.12") + def test_fstring_literal_open_paren_not_folded(self): + code = 'f"({n} items"\ndef g(): pass\n' + self.assertEqual(code.count("\n"), simplify.real_code(code).count("\n")) + + @testutils.only_for_versions_higher("3.12") + def test_fstring_escaped_brace_not_folded(self): + code = 'f"{{"\ndef g(): pass\n' + self.assertEqual(code.count("\n"), simplify.real_code(code).count("\n")) + + @testutils.only_for_versions_higher("3.12") + def test_fstring_format_spec_bracket_not_folded(self): + code = 'f"{x:>[}"\ndef g(): pass\n' + self.assertEqual(code.count("\n"), simplify.real_code(code).count("\n")) + + @testutils.only_for_versions_higher("3.12") + def test_fstring_ansi_escape_not_folded(self): + code = 'f"\\x1b[K{x}"\ndef g(): pass\n' + self.assertEqual(code.count("\n"), simplify.real_code(code).count("\n")) + + @testutils.only_for_versions_higher("3.12") + def test_fstring_multiline_bracket_not_folded(self): + code = 'f"""\n[{x}\n"""\ndef g(): pass\n' + self.assertEqual(code.count("\n"), simplify.real_code(code).count("\n")) + + @testutils.only_for_versions_higher("3.12") + def test_fstring_nested_bracket_not_folded(self): + code = 'f"{f\'[{x}\'}"\ndef g(): pass\n' + self.assertEqual(code.count("\n"), simplify.real_code(code).count("\n")) + + # --- PRESERVED: real expression brackets inside an f-string must stay --- + # The fix must not over-blank real syntax. + + @testutils.only_for_versions_higher("3.12") + def test_fstring_real_list_brackets_preserved(self): + code = 'f"{[1,2][0]}"\ndef g(): pass\n' + self.assertEqual(code, simplify.real_code(code)) + + @testutils.only_for_versions_higher("3.12") + def test_fstring_nested_string_literal_bracket_blanked(self): + # A bracket char that is literal data inside a nested string is not + # real syntax: it is blanked to a space (length preserved) so it + # cannot sway the continuation count. The structural brackets stay + # intact and the code below does not fold. + code = "f\"{d['}']}\"\ndef g(): pass\n" + expected = "f\"{d[' ']}\"\ndef g(): pass\n" + result = simplify.real_code(code) + self.assertEqual(expected, result) + self.assertEqual(len(code), len(result)) + + @testutils.only_for_versions_higher("3.12") + def test_fstring_format_spec_nested_expr_preserved(self): + code = 'f"{x:{w}}"\ndef g(): pass\n' + self.assertEqual(code, simplify.real_code(code)) + + def test_unterminated_fstring_falls_back_without_crashing(self): + # an invalid/incomplete f-string must not crash real_code or + # over-blank the code that follows it + code = 'f"[{x}\ndef g(): pass\n' + result = simplify.real_code(code) + self.assertIn("def g", result)