Skip to content

Commit

Permalink
Fix false positives for superfluous-parens
Browse files Browse the repository at this point in the history
This fixes the false positives identified in pylint-dev#2818, pylint-dev#3249, pylint-dev#3608 & pylint-dev#4346
All false positives reported fell under keywords before walrus operator
or if-keyword within generators/comprehension.
This closes pylint-dev#2818, closes pylint-dev#3429, closes pylint-dev#3608, closes pylint-dev#4346
  • Loading branch information
DanielNoord committed Aug 1, 2021
1 parent 2ad8247 commit e9133ad
Show file tree
Hide file tree
Showing 2 changed files with 31 additions and 18 deletions.
7 changes: 5 additions & 2 deletions pylint/checkers/format.py
Expand Up @@ -404,8 +404,7 @@ def _check_keyword_parentheses(
# The empty tuple () is always accepted.
if contains_walrus_operator and walrus_operator_depth - 1 == depth:
# Reset variable for possible following expressions
contains_walrus_operator = False
continue
return
if i == start + 2:
return
if keyword_token == "not":
Expand Down Expand Up @@ -440,6 +439,10 @@ def _check_keyword_parentheses(
# without an error.
elif token[1] == "for":
return
# A generator expression can have a 'if' token in it.
# This legitimizes the parens so bail without an error.
elif token[1] == "if":
return

def _prepare_token_dispatcher(self):
dispatch = {}
Expand Down
42 changes: 26 additions & 16 deletions tests/checkers/unittest_format.py
Expand Up @@ -152,6 +152,8 @@ def testCheckKeywordParensHandlesValidCases(self):
"for x in (x for x in x):",
"not (foo or bar)",
"not (foo or bar) and baz",
"return [x for x in (3 if 1 else [4])]",
"return (x for x in ((3, 4) if 2 > 1 else (5, 6)))",
]
with self.assertNoMessages():
for code in cases:
Expand Down Expand Up @@ -181,31 +183,39 @@ def testCheckKeywordParensHandlesUnnecessaryParens(self):
"if (1) in (1, 2, 3):",
0,
),
(
Message("superfluous-parens", line=1, args="in"),
"return (x for x in ((3, 4)))",
0,
),
]
for msg, code, offset in cases:
with self.assertAddsMessages(msg):
self.checker._check_keyword_parentheses(_tokenize_str(code), offset)

def testNoSuperfluousParensWalrusOperatorIf(self):
"""Parenthesis change the meaning of assignment in the walrus operator
and so are not superfluous:"""
code = "if (odd := is_odd(i))"
offset = 0
with self.assertNoMessages():
self.checker._check_keyword_parentheses(_tokenize_str(code), offset)
and so are not always superfluous:"""
cases = [
("if (odd := is_odd(i))", 0),
(
"not (foo := 5)",
0,
),
]
for code, offset in cases:
with self.assertNoMessages():
self.checker._check_keyword_parentheses(_tokenize_str(code), offset)

def testPositiveSuperfluousParensWalrusOperatorIf(self):
"""Test positive superfluous parens with the walrus operator"""
code = "if ((odd := is_odd(i))):"
msg = Message("superfluous-parens", line=1, args="if")
with self.assertAddsMessages(msg):
self.checker._check_keyword_parentheses(_tokenize_str(code), 0)

def testNoSuperfluousParensWalrusOperatorNot(self):
"""Test superfluous-parens with the not operator"""
code = "not (foo := 5)"
with self.assertNoMessages():
self.checker._check_keyword_parentheses(_tokenize_str(code), 0)
"""Test positive superfluous parens cases with the walrus operator"""
cases = [
(Message("superfluous-parens", line=1, args="if"), "if ((x := y)):", 0),
(Message("superfluous-parens", line=1, args="if"), "if not ((x := y)):", 0),
]
for msg, code, offset in cases:
with self.assertAddsMessages(msg):
self.checker._check_keyword_parentheses(_tokenize_str(code), offset)

def testCheckIfArgsAreNotUnicode(self):
cases = [("if (foo):", 0), ("assert (1 == 1)", 0)]
Expand Down

0 comments on commit e9133ad

Please sign in to comment.