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``
+
+  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``
+
+  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
@@ -404,8 +405,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":
@@ -440,6 +440,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 = {}

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,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)]