Skip to content

Commit

Permalink
w605 fixes for identical tokens
Browse files Browse the repository at this point in the history
An infinite loop was occuring when two w605 tokens appeared on the same line because the string find() function would only return the position of the first instance to be fixed.  The Token datastrucute (tuple) already contained the start position where the fix was needed, so it is used directly instead of doing a search.  Unit tests added that catch the original issue.
  • Loading branch information
bigredengineer committed Oct 31, 2018
1 parent 4343c41 commit 8c5f4d0
Show file tree
Hide file tree
Showing 2 changed files with 22 additions and 2 deletions.
5 changes: 3 additions & 2 deletions autopep8.py
Original file line number Diff line number Diff line change
Expand Up @@ -1363,7 +1363,7 @@ def get_w605_position(tokens):
'N', 'u', 'U',
]

for token_type, text, start, end, line in tokens:
for token_type, text, start_pos, end_pos, line in tokens:
if token_type == tokenize.STRING:
quote = text[-3:] if text[-3:] in ('"""', "'''") else text[-1]
# Extract string modifiers (e.g. u or r)
Expand All @@ -1378,7 +1378,8 @@ def get_w605_position(tokens):
pos += 1
if string[pos] not in valid:
yield (
line.find(text),
# No need to search line, token stores position
start_pos[1],
"W605 invalid escape sequence '\\%s'" %
string[pos],
)
Expand Down
19 changes: 19 additions & 0 deletions test/test_autopep8.py
Original file line number Diff line number Diff line change
Expand Up @@ -4734,6 +4734,25 @@ def test_w605_simple(self):
with autopep8_context(line, options=['--aggressive']) as result:
self.assertEqual(fixed, result)

def test_w605_identical_token(self):
# ***NOTE***: The --pep8-passes option is requred to prevent an infinite loop in
# the old, failing code. DO NOT REMOVE.
line = "escape = foo('\.bar', '\.kilroy')\n"
fixed = "escape = foo(r'\.bar', r'\.kilroy')\n"
with autopep8_context(line, options=['--aggressive', '--pep8-passes', '5']) as result:
self.assertEqual(fixed, result, "Two tokens get r added")

line = "escape = foo('\.bar', r'\.kilroy')\n"
fixed = "escape = foo(r'\.bar', r'\.kilroy')\n"
with autopep8_context(line, options=['--aggressive', '--pep8-passes', '5']) as result:
self.assertEqual(fixed, result, "r not added if already there")

# Test Case to catch bad behavior reported in Issue #449
line = "escape = foo('\.bar', '\.bar')\n"
fixed = "escape = foo(r'\.bar', r'\.bar')\n"
with autopep8_context(line, options=['--aggressive', '--pep8-passes', '5']) as result:
self.assertEqual(fixed, result)

def test_trailing_whitespace_in_multiline_string(self):
line = 'x = """ \nhello""" \n'
fixed = 'x = """ \nhello"""\n'
Expand Down

0 comments on commit 8c5f4d0

Please sign in to comment.