diff --git a/ChangeLog b/ChangeLog index 2dc3aba713..33128066dd 100644 --- a/ChangeLog +++ b/ChangeLog @@ -26,6 +26,10 @@ Release date: TBA * Add an faq detailing which messages to disable to avoid duplicates w/ other popular linters +* Fix superfluous-parens false-positive for the walrus operator + + Close #3383 + * Fix a bug with `ignore-docstrings` ignoring all lines in a module * Fix `pre-commit` config that could lead to undetected duplicate lines of code diff --git a/doc/whatsnew/2.6.rst b/doc/whatsnew/2.6.rst index 5ab2028c16..dc659d6562 100644 --- a/doc/whatsnew/2.6.rst +++ b/doc/whatsnew/2.6.rst @@ -23,3 +23,5 @@ Other Changes * The `no-space-check` option has been removed, it's no longer possible to consider empty line like a `trailing-whitespace` by using clever options. * `mixed-indentation` has been removed, it is no longer useful since TabError is included directly in python3 + +* Fix superfluous-parens false-positive for the walrus operator diff --git a/pylint/checkers/format.py b/pylint/checkers/format.py index ae7cf4260e..52d852a258 100644 --- a/pylint/checkers/format.py +++ b/pylint/checkers/format.py @@ -52,6 +52,7 @@ import tokenize from functools import reduce # pylint: disable=redefined-builtin +from tokenize import TokenInfo from typing import List from astroid import nodes @@ -351,9 +352,9 @@ def new_line(self, tokens, line_end, line_start): self.check_lines(line, line_num) def process_module(self, _module): - self._keywords_with_parens = set() + pass - def _check_keyword_parentheses(self, tokens, start): + def _check_keyword_parentheses(self, tokens: List[TokenInfo], start: int) -> None: """Check that there are not unnecessary parens after a keyword. Parens are unnecessary if there is exactly one balanced outer pair on a @@ -365,30 +366,45 @@ def _check_keyword_parentheses(self, tokens, start): start: int; the position of the keyword in the token list. """ # If the next token is not a paren, we're fine. - if self._bracket_stack[-1] == ":" and tokens[start][1] == "for": + if self._bracket_stack[-1] == ":" and tokens[start].string == "for": self._bracket_stack.pop() - if tokens[start + 1][1] != "(": + if tokens[start + 1].string != "(": return found_and_or = False + contains_walrus_operator = False + walrus_operator_depth = 0 depth = 0 - keyword_token = str(tokens[start][1]) - line_num = tokens[start][2][0] + keyword_token = str(tokens[start].string) + line_num = tokens[start].start[0] for i in range(start, len(tokens) - 1): token = tokens[i] + # If we hit a newline, then assume any parens were for continuation. - if token[0] == tokenize.NL: + if token.type == tokenize.NL: return - if token[1] == "(": + # Since the walrus operator doesn't exist below python3.8, the tokenizer + # generates independent tokens + if ( + token.string == ":=" # <-- python3.8+ path + or token.string + tokens[i + 1].string == ":=" + ): + contains_walrus_operator = True + walrus_operator_depth = depth + if token.string == "(": depth += 1 - elif token[1] == ")": + elif token.string == ")": depth -= 1 if depth: continue # ')' can't happen after if (foo), since it would be a syntax error. - if tokens[i + 1][1] in (":", ")", "]", "}", "in") or tokens[i + 1][ - 0 - ] in (tokenize.NEWLINE, tokenize.ENDMARKER, tokenize.COMMENT): + if tokens[i + 1].string in (":", ")", "]", "}", "in") or tokens[ + i + 1 + ].type in (tokenize.NEWLINE, tokenize.ENDMARKER, tokenize.COMMENT): # 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 if i == start + 2: return if keyword_token == "not": @@ -400,11 +416,10 @@ def _check_keyword_parentheses(self, tokens, start): self.add_message( "superfluous-parens", line=line_num, args=keyword_token ) - elif keyword_token not in self._keywords_with_parens: - if not found_and_or: - self.add_message( - "superfluous-parens", line=line_num, args=keyword_token - ) + elif not found_and_or: + self.add_message( + "superfluous-parens", line=line_num, args=keyword_token + ) return elif depth == 1: # This is a tuple, which is always acceptable. diff --git a/tests/checkers/unittest_format.py b/tests/checkers/unittest_format.py index 75b0c79674..562649eb93 100644 --- a/tests/checkers/unittest_format.py +++ b/tests/checkers/unittest_format.py @@ -137,7 +137,6 @@ class TestSuperfluousParentheses(CheckerTestCase): CHECKER_CLASS = FormatChecker def testCheckKeywordParensHandlesValidCases(self): - self.checker._keywords_with_parens = set() cases = [ "if foo:", "if foo():", @@ -157,7 +156,6 @@ def testCheckKeywordParensHandlesValidCases(self): self.checker._check_keyword_parentheses(_tokenize_str(code), 0) def testCheckKeywordParensHandlesUnnecessaryParens(self): - self.checker._keywords_with_parens = set() cases = [ (Message("superfluous-parens", line=1, args="if"), "if (foo):", 0), (Message("superfluous-parens", line=1, args="if"), "if ((foo, bar)):", 0), @@ -186,8 +184,28 @@ def testCheckKeywordParensHandlesUnnecessaryParens(self): 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) + + 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) + def testCheckIfArgsAreNotUnicode(self): - self.checker._keywords_with_parens = set() cases = [("if (foo):", 0), ("assert (1 == 1)", 0)] for code, offset in cases: @@ -205,7 +223,6 @@ def testFuturePrintStatementWithoutParensWarning(self): self.checker.process_tokens(_tokenize_str(code)) def testKeywordParensFalsePositive(self): - self.checker._keywords_with_parens = set() code = "if 'bar' in (DICT or {}):" with self.assertNoMessages(): self.checker._check_keyword_parentheses(_tokenize_str(code), start=2)