Fix false positives for superfluous-parens

ChangeLog

@@ -68,6 +68,13 @@ Release date: TBA
 
   Closes #4681
 
+* 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:
@@ -188,24 +191,24 @@ def testCheckKeywordParensHandlesUnnecessaryParens(self):
 
     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))\n"),
+            ("not (foo := 5)\n"),
+        ]
+        for code in cases:
+            with self.assertNoMessages():
+                self.checker.process_tokens(_tokenize_str(code))
 
     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)):\n"),
+            (Message("superfluous-parens", line=1, args="not"), "if not ((x := y)):\n"),
+        ]
+        for msg, code in cases:
+            with self.assertAddsMessages(msg):
+                self.checker.process_tokens(_tokenize_str(code))
 
     def testCheckIfArgsAreNotUnicode(self):
         cases = [("if (foo):", 0), ("assert (1 == 1)", 0)]

tests/functional/s/superfluous_parens.py → tests/functional/s/super/superfluous_parens.py

@@ -1,6 +1,6 @@
 """Test the superfluous-parens warning."""
 from __future__ import print_function
-# pylint: disable=unneeded-not
+# pylint: disable=unneeded-not, unnecessary-comprehension, missing-function-docstring
 i = 3
 if (i == 5):  # [superfluous-parens]
     pass
@@ -17,3 +17,12 @@
 DICT = {'a': 1, 'b': 2}
 del(DICT['b'])  # [superfluous-parens]
 del DICT['a']
+
+i = [x for x in ((3, 4))]  # [superfluous-parens]
+i = [x for x in ((3, 4) if 1 > 0 else (5, 6))]
+i = [x for x in ((3, 4) if 1 > 0 else ((5, 6)))]  # [superfluous-parens]
+i = [x for x in ((3, 4) if 1 > 0 else ((((5, 6)))))]  # [superfluous-parens]
+
+
+def a_function():
+    return (x for x in ((3, 4)))  # [superfluous-parens]

tests/functional/s/superfluous_parens.txt → tests/functional/s/super/superfluous_parens.txt

@@ -3,3 +3,7 @@ superfluous-parens:7:0::Unnecessary parens after 'not' keyword
 superfluous-parens:11:0::Unnecessary parens after 'for' keyword
 superfluous-parens:13:0::Unnecessary parens after 'if' keyword
 superfluous-parens:18:0::Unnecessary parens after 'del' keyword
+superfluous-parens:21:0::Unnecessary parens after 'in' keyword
+superfluous-parens:23:0::Unnecessary parens after 'else' keyword
+superfluous-parens:24:0::Unnecessary parens after 'else' keyword
+superfluous-parens:28:0::Unnecessary parens after 'in' keyword

tests/functional/s/super/superfluous_parens_walrus_py38.py

@@ -0,0 +1,24 @@
+"""Test the superfluous-parens warning with python 3.8 functionality (walrus operator)"""
+# pylint: disable=missing-function-docstring,invalid-name
+if not (x := False):
+    print(x)
+
+if not ((x := 1)):  # [superfluous-parens]
+    pass
+
+if not ((((x := 1)))):  # [superfluous-parens]
+    pass
+
+i = 1
+y = 1
+
+if odd := is_odd(i): # [undefined-variable]
+    pass
+if not (foo := 5):
+    pass
+
+if not ((x := y)):  # [superfluous-parens]
+    pass
+
+if ((x := y)):   # [superfluous-parens]
+    pass

tests/functional/s/super/superfluous_parens_walrus_py38.rc

@@ -0,0 +1,2 @@
+[testoptions]
+min_pyver=3.8

tests/functional/s/super/superfluous_parens_walrus_py38.txt

@@ -0,0 +1,5 @@
+superfluous-parens:6:0::Unnecessary parens after 'not' keyword:HIGH
+superfluous-parens:9:0::Unnecessary parens after 'not' keyword:HIGH
+undefined-variable:15:10::Undefined variable 'is_odd':HIGH
+superfluous-parens:20:0::Unnecessary parens after 'not' keyword:HIGH
+superfluous-parens:23:0::Unnecessary parens after 'if' keyword:HIGH