Fix false positives for superfluous-parens

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

ChangeLog

@@ -60,6 +60,13 @@ Release date: TBA
 
   Closes #4673
 
+* Fix false positives for ``superfluous-parens`` with walrus operator, ternary operator and inside list comprehension.
+
+  Closes #2818
+  Closes #3249
+  Closes #3608
+  Closes #4346
+
 
 What's New in Pylint 2.9.6?
 ===========================

doc/whatsnew/2.10.rst

@@ -54,3 +54,10 @@ Other Changes
   Ref #1162
   Closes #1990
   Closes #4168
+
+* Fix false positives for ``superfluous-parens`` with walrus operator, ternary operator and inside list comprehension.
+
+  Closes #2818
+  Closes #3249
+  Closes #3608
+  Closes #4346

pylint/checkers/format.py

@@ -37,6 +37,7 @@
 # Copyright (c) 2019 Hugo van Kemenade <hugovk@users.noreply.github.com>
 # Copyright (c) 2020 Raphael Gaschignard <raphael@rtpg.co>
 # Copyright (c) 2021 Marc Mueller <30130371+cdce8p@users.noreply.github.com>
+# Copyright (c) 2021 Daniel van Noord <13665637+DanielNoord@users.noreply.github.com>
 
 # Licensed under the GPL: https://www.gnu.org/licenses/old-licenses/gpl-2.0.html
 # For details: https://github.com/PyCQA/pylint/blob/main/LICENSE
@@ -374,6 +375,7 @@ def _check_keyword_parentheses(
         found_and_or = False
         contains_walrus_operator = False
         walrus_operator_depth = 0
+        contains_double_parens = 0
         depth = 0
         keyword_token = str(tokens[start].string)
         line_num = tokens[start].start[0]
@@ -393,19 +395,25 @@ def _check_keyword_parentheses(
                 walrus_operator_depth = depth
             if token.string == "(":
                 depth += 1
+                if tokens[i + 1].string == "(":
+                    contains_double_parens = 1
             elif token.string == ")":
                 depth -= 1
                 if depth:
+                    if contains_double_parens and tokens[i + 1].string == ")":
+                        self.add_message(
+                            "superfluous-parens", line=line_num, args=keyword_token
+                        )
+                        return
+                    contains_double_parens = 0
                     continue
                 # ')' can't happen after if (foo), since it would be a syntax error.
                 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
+                        return
+                    # The empty tuple () is always accepted.
                     if i == start + 2:
                         return
                     if keyword_token == "not":
@@ -417,7 +425,7 @@ def _check_keyword_parentheses(
                         self.add_message(
                             "superfluous-parens", line=line_num, args=keyword_token
                         )
-                    elif not found_and_or:
+                    elif not found_and_or and keyword_token != "in":
                         self.add_message(
                             "superfluous-parens", line=line_num, args=keyword_token
                         )
@@ -440,6 +448,13 @@ def _check_keyword_parentheses(
                 # without an error.
                 elif token[1] == "for":
                     return
+                # A generator expression can have a 'else' token in it.
+                # We check the rest of the tokens to see if any problems incure after
+                # the 'else'.
+                elif token[1] == "else":
+                    if "(" in (i.string for i in tokens[i:]):
+                        self._check_keyword_parentheses(tokens[i:], 0)
+                    return
 
     def _prepare_token_dispatcher(self):
         dispatch = {}

tests/checkers/unittest_format.py

@@ -21,6 +21,7 @@
 # Copyright (c) 2020 hippo91 <guillaume.peillex@gmail.com>
 # Copyright (c) 2021 Marc Mueller <30130371+cdce8p@users.noreply.github.com>
 # Copyright (c) 2021 Andreas Finkler <andi.finkler@gmail.com>
+# Copyright (c) 2021 Daniel van Noord <13665637+DanielNoord@users.noreply.github.com>
 
 # Licensed under the GPL: https://www.gnu.org/licenses/old-licenses/gpl-2.0.html
 # For details: https://github.com/PyCQA/pylint/blob/main/LICENSE
@@ -152,6 +153,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:
@@ -181,31 +184,43 @@ 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="not"),
+                "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)]