Skip to content

Commit 3f3c20b

Browse files
quazardousclaude
andcommitted
patchedast: don't match tokens inside string literals
The source walker locates tokens with a plain `str.index`, which cannot tell a real token from the same characters inside a literal. Two ways that went wrong: - `_good_token` looked for the last `#` before the token to decide whether it sat in a comment. A `#` inside a string opens no comment, but it made the walker skip the rest of that line -- and the closing paren with it. - `consume(")")` matched the `)` of `f"see (#123)"`, or one buried in a triple-quoted SQL blob, and carried on from a position the tree knows nothing about. Neither fails where it happens. The walker keeps going against a mis-parsed offset and surfaces much later as a MismatchedTokenError, or an AttributeError, naming an unrelated node several statements away. Replace the textual guess with the tokenizer: `_Source` now records the offset range of every string literal and comment once, and looks a candidate offset up in it. That is exact, where a scan for quotes cannot know whether it began inside one -- the reason the previous heuristic could not be repaired in place. f-strings need one distinction. From 3.12 the tokenizer splits them, so taking only FSTRING_MIDDLE keeps the replacement fields visible: they are code, and the walker consumes them. Before 3.12 an f-string is a single STRING token, fields included, so `consume` compares the literal the walker is itself working inside against the match's, rather than skipping every literal it meets. On an unparseable source the tokenizer yields nothing and the plain textual search of before is left untouched. Measured over a 250-file private codebase: 4 files failed to walk before, 2 after, with no file newly broken. Both remaining failures are a separate defect. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01V5GqJKaXZzFzmFEYgo4pPR
1 parent d2c5127 commit 3f3c20b

2 files changed

Lines changed: 120 additions & 15 deletions

File tree

rope/refactor/patchedast.py

Lines changed: 88 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,9 @@
1+
import bisect
12
import collections
3+
import io
24
import numbers
35
import re
6+
import tokenize
47
import warnings
58
from itertools import chain
69

@@ -907,15 +910,21 @@ class _Source:
907910
def __init__(self, source):
908911
self.source = source
909912
self.offset = 0
913+
self._ranges = None
910914

911915
def consume(self, token, skip_comment=True):
912916
try:
913917
while True:
914918
new_offset = self.source.index(token, self.offset)
915-
if self._good_token(token, new_offset) or not skip_comment:
919+
if not skip_comment:
916920
break
917-
else:
918-
self._skip_comment()
921+
here = self._enclosing_literal(self.offset)
922+
there = self._enclosing_literal(new_offset)
923+
if there is None or there == here:
924+
break
925+
# The match is buried in a literal the walker is not
926+
# itself working inside. Resume past that literal.
927+
self.offset = there[1]
919928
except (ValueError, TypeError):
920929
raise MismatchedTokenError(
921930
f"Token <{token}> at {self._get_location()} cannot be matched"
@@ -957,19 +966,83 @@ def consume_with_or_comma_context_manager(self):
957966
repattern = re.compile(r"with|,")
958967
return self._consume_pattern(repattern)
959968

960-
def _good_token(self, token, offset, start=None):
969+
def _good_token(self, token, offset):
961970
"""Checks whether consumed token is in comments"""
962-
if start is None:
963-
start = self.offset
964-
try:
965-
comment_index = self.source.rindex("#", start, offset)
966-
except ValueError:
967-
return True
971+
literal = self._enclosing_literal(offset)
972+
return literal is None or not literal[2]
973+
974+
def _enclosing_literal(self, offset):
975+
"""The string literal or comment containing ``offset``, if any.
976+
977+
A ``(start, end, is_comment)`` triple, or ``None`` when
978+
``offset`` sits in ordinary code.
979+
980+
The walker searches the source as plain text, which cannot tell
981+
a token from the same characters inside a literal. Asked for
982+
``)`` it would match the one in ``f"see (#123)"``, or the one in
983+
a triple-quoted ``CREATE TABLE t (id)`` blob, then carry on
984+
from a position the tree knows nothing about -- surfacing much
985+
later as an error naming an unrelated node.
986+
"""
987+
ranges, starts = self._get_ranges()
988+
index = bisect.bisect_right(starts, offset) - 1
989+
if index >= 0 and ranges[index][1] > offset:
990+
return ranges[index]
991+
return None
992+
993+
def _get_ranges(self):
994+
"""Every string literal and comment, as sorted offset ranges.
995+
996+
Paired with the list of their start offsets, to bisect on.
997+
998+
Tokenized once per source. The tokenizer is what makes this
999+
exact rather than another guess at where a literal starts: a
1000+
scan for quotes cannot know whether it began inside one.
1001+
1002+
On an unparseable source it yields nothing, leaving the plain
1003+
textual search of before.
1004+
"""
1005+
if self._ranges is None:
1006+
ranges = self._tokenize_ranges()
1007+
self._ranges = (ranges, [start for start, _, _ in ranges])
1008+
return self._ranges
1009+
1010+
def _tokenize_ranges(self):
1011+
line_starts = [0]
1012+
for line in self.source.splitlines(keepends=True):
1013+
line_starts.append(line_starts[-1] + len(line))
1014+
1015+
def offset_of(position):
1016+
row, column = position
1017+
if row >= len(line_starts):
1018+
return len(self.source)
1019+
return line_starts[row - 1] + column
1020+
1021+
# FSTRING_* exists from Python 3.12 on, where an f-string is
1022+
# tokenized in pieces. Taking only MIDDLE keeps the replacement
1023+
# fields out: they are code, and the walker consumes them.
1024+
# Before 3.12 an f-string is one STRING token, fields included --
1025+
# which is why `consume` compares the walker's own literal to the
1026+
# match's rather than skipping every literal it meets.
1027+
wanted = {tokenize.STRING, tokenize.COMMENT}
1028+
for name in ("FSTRING_START", "FSTRING_MIDDLE", "FSTRING_END"):
1029+
if hasattr(tokenize, name):
1030+
wanted.add(getattr(tokenize, name))
1031+
ranges = []
9681032
try:
969-
new_line_index = self.source.rindex("\n", start, offset)
970-
except ValueError:
971-
return False
972-
return comment_index < new_line_index
1033+
tokens = tokenize.generate_tokens(io.StringIO(self.source).readline)
1034+
for token in tokens:
1035+
if token.type in wanted:
1036+
ranges.append(
1037+
(
1038+
offset_of(token.start),
1039+
offset_of(token.end),
1040+
token.type == tokenize.COMMENT,
1041+
)
1042+
)
1043+
except (tokenize.TokenError, IndentationError, SyntaxError, ValueError):
1044+
return []
1045+
return ranges
9731046

9741047
def _skip_comment(self):
9751048
self.offset = self.source.index("\n", self.offset + 1)
@@ -999,7 +1072,7 @@ def rfind_token(self, token, start, end):
9991072
while True:
10001073
try:
10011074
index = self.source.rindex(token, start, end)
1002-
if self._good_token(token, index, start=start):
1075+
if self._good_token(token, index):
10031076
return index
10041077
else:
10051078
end = index

ropetest/refactor/patchedasttest.py

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -280,6 +280,38 @@ def test_handling_fstrings(self):
280280
checker = _ResultChecker(self, ast_frag)
281281
checker.check_children("BinOp", ["Num", " ", "+", " ", "JoinedStr"])
282282

283+
def test_handling_hash_inside_implicitly_concatenated_fstrings(self):
284+
# The walker searches the source as plain text. A `#` inside a
285+
# string opened no comment, yet it made the rest of that line be
286+
# skipped -- taking the closing paren with it -- and the walker
287+
# then matched a paren belonging to some later statement.
288+
source = dedent("""\
289+
assert flag, (
290+
f"one "
291+
f"two (#1693).")
292+
""")
293+
ast_frag = patchedast.get_patched_ast(source, True)
294+
self.assertEqual(source, patchedast.write_ast(ast_frag))
295+
296+
def test_handling_hash_inside_a_plain_string(self):
297+
source = dedent("""\
298+
assert flag, (
299+
"one "
300+
"two (#1693).")
301+
""")
302+
ast_frag = patchedast.get_patched_ast(source, True)
303+
self.assertEqual(source, patchedast.write_ast(ast_frag))
304+
305+
def test_handling_paren_inside_a_triple_quoted_string(self):
306+
source = dedent('''\
307+
SCHEMA = """
308+
PRIMARY KEY (a, b)
309+
"""
310+
run(SCHEMA)
311+
''')
312+
ast_frag = patchedast.get_patched_ast(source, True)
313+
self.assertEqual(source, patchedast.write_ast(ast_frag))
314+
283315
def test_handling_implicit_string_concatenation(self):
284316
source = "a = '1''2'"
285317
ast_frag = patchedast.get_patched_ast(source, True)

0 commit comments

Comments
 (0)