Fix false positive superfluous-parens for the walrus operator

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

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

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.

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)