Merge branch 'main' into issue-626

Author: david-yz-liu

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
+
 * Fix false negative for ``used-before-assignment`` when the variable is assigned
   in an exception handler, but used outside of the handler.
 

doc/whatsnew/2.10.rst

@@ -55,6 +55,13 @@ Other Changes
   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
+
 * Fix false negative for ``used-before-assignment`` when the variable is assigned
   in an exception handler, but used outside of the handler.
 

pylint/checkers/format.py

@@ -37,6 +37,7 @@
 # Copyright (c) 2019 Hugo van Kemenade <[email protected]>
 # Copyright (c) 2020 Raphael Gaschignard <[email protected]>
 # Copyright (c) 2021 Marc Mueller <[email protected]>
+# Copyright (c) 2021 Daniel van Noord <[email protected]>
 
 # 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 = {}

pylint/pyreverse/diagrams.py

@@ -45,6 +45,19 @@ def __init__(self, title="No name", node=None):
         self.node = node
 
 
+class PackageEntity(DiagramEntity):
+    """A diagram object representing a package"""
+
+
+class ClassEntity(DiagramEntity):
+    """A diagram object representing a class"""
+
+    def __init__(self, title, node):
+        super().__init__(title=title, node=node)
+        self.attrs = None
+        self.methods = None
+
+
 class ClassDiagram(Figure, FilterMixIn):
     """main class diagram handling"""
 

pylint/pyreverse/dot_printer.py

@@ -0,0 +1,129 @@
+# Copyright (c) 2021 Andreas Finkler <[email protected]>
+
+# 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
+
+"""
+Class to generate files in dot format and image formats supported by Graphviz.
+"""
+import os
+import subprocess
+import sys
+import tempfile
+from pathlib import Path
+from typing import Dict, FrozenSet, Optional
+
+from pylint.pyreverse.printer import EdgeType, Layout, NodeProperties, NodeType, Printer
+from pylint.pyreverse.utils import check_graphviz_availability
+
+ALLOWED_CHARSETS: FrozenSet[str] = frozenset(("utf-8", "iso-8859-1", "latin1"))
+SHAPES: Dict[NodeType, str] = {
+    NodeType.PACKAGE: "box",
+    NodeType.INTERFACE: "record",
+    NodeType.CLASS: "record",
+}
+ARROWS: Dict[EdgeType, Dict] = {
+    EdgeType.INHERITS: dict(arrowtail="none", arrowhead="empty"),
+    EdgeType.IMPLEMENTS: dict(arrowtail="node", arrowhead="empty", style="dashed"),
+    EdgeType.ASSOCIATION: dict(
+        fontcolor="green", arrowtail="none", arrowhead="diamond", style="solid"
+    ),
+    EdgeType.USES: dict(arrowtail="none", arrowhead="open"),
+}
+
+
+class DotPrinter(Printer):
+    def __init__(
+        self,
+        title: str,
+        layout: Optional[Layout] = None,
+        use_automatic_namespace: Optional[bool] = None,
+    ):
+        self.charset = "utf-8"
+        self.node_style = "solid"
+        super().__init__(title, layout, use_automatic_namespace)
+
+    def _open_graph(self) -> None:
+        """Emit the header lines"""
+        self.emit(f'digraph "{self.title}" {{')
+        if self.layout:
+            self.emit(f"rankdir={self.layout.value}")
+        if self.charset:
+            assert (
+                self.charset.lower() in ALLOWED_CHARSETS
+            ), f"unsupported charset {self.charset}"
+            self.emit(f'charset="{self.charset}"')
+
+    def emit_node(
+        self,
+        name: str,
+        type_: NodeType,
+        properties: Optional[NodeProperties] = None,
+    ) -> None:
+        """Create a new node. Nodes can be classes, packages, participants etc."""
+        if properties is None:
+            properties = NodeProperties(label=name)
+        shape = SHAPES[type_]
+        color = properties.color if properties.color is not None else "black"
+        label = properties.label
+        if label:
+            if type_ is NodeType.INTERFACE:
+                label = "<<interface>>\\n" + label
+            label_part = f', label="{label}"'
+        else:
+            label_part = ""
+        fontcolor_part = (
+            f', fontcolor="{properties.fontcolor}"' if properties.fontcolor else ""
+        )
+        self.emit(
+            f'"{name}" [color="{color}"{fontcolor_part}{label_part}, shape="{shape}", style="{self.node_style}"];'
+        )
+
+    def emit_edge(
+        self,
+        from_node: str,
+        to_node: str,
+        type_: EdgeType,
+        label: Optional[str] = None,
+    ) -> None:
+        """Create an edge from one node to another to display relationships."""
+        arrowstyle = ARROWS[type_]
+        attrs = [f'{prop}="{value}"' for prop, value in arrowstyle.items()]
+        if label:
+            attrs.append(f'label="{label}"')
+        self.emit(f'"{from_node}" -> "{to_node}" [{", ".join(sorted(attrs))}];')
+
+    def generate(self, outputfile: str) -> None:
+        self._close_graph()
+        graphviz_extensions = ("dot", "gv")
+        name = self.title
+        if outputfile is None:
+            target = "png"
+            pdot, dot_sourcepath = tempfile.mkstemp(".gv", name)
+            ppng, outputfile = tempfile.mkstemp(".png", name)
+            os.close(pdot)
+            os.close(ppng)
+        else:
+            target = Path(outputfile).suffix.lstrip(".")
+            if not target:
+                target = "png"
+                outputfile = outputfile + "." + target
+            if target not in graphviz_extensions:
+                pdot, dot_sourcepath = tempfile.mkstemp(".gv", name)
+                os.close(pdot)
+            else:
+                dot_sourcepath = outputfile
+        with open(dot_sourcepath, "w", encoding="utf8") as outfile:
+            outfile.writelines(self.lines)
+        if target not in graphviz_extensions:
+            check_graphviz_availability()
+            use_shell = sys.platform == "win32"
+            subprocess.call(
+                ["dot", "-T", target, dot_sourcepath, "-o", outputfile],
+                shell=use_shell,
+            )
+            os.unlink(dot_sourcepath)
+
+    def _close_graph(self) -> None:
+        """Emit the lines needed to properly close the graph."""
+        self.emit("}\n")

pylint/pyreverse/main.py

@@ -21,15 +21,14 @@
   create UML diagrams for classes and modules in <packages>
 """
 import os
-import subprocess
 import sys
 from typing import Iterable
 
 from pylint.config import ConfigurationMixIn
 from pylint.pyreverse import writer
 from pylint.pyreverse.diadefslib import DiadefsHandler
 from pylint.pyreverse.inspector import Linker, project_from_files
-from pylint.pyreverse.utils import insert_default_options
+from pylint.pyreverse.utils import check_graphviz_availability, insert_default_options
 
 OPTIONS = (
     (
@@ -175,19 +174,6 @@
 )
 
 
-def _check_graphviz_available(output_format):
-    """check if we need graphviz for different output format"""
-    try:
-        subprocess.call(["dot", "-V"], stdout=subprocess.PIPE, stderr=subprocess.PIPE)
-    except OSError:
-        print(
-            "The output format '%s' is currently not available.\n"
-            "Please install 'Graphviz' to have other output formats "
-            "than 'dot' or 'vcg'." % output_format
-        )
-        sys.exit(32)
-
-
 class Run(ConfigurationMixIn):
     """base class providing common behaviour for pyreverse commands"""
 
@@ -198,7 +184,7 @@ def __init__(self, args: Iterable[str]):
         insert_default_options()
         args = self.load_command_line_configuration(args)
         if self.config.output_format not in ("dot", "vcg"):
-            _check_graphviz_available(self.config.output_format)
+            check_graphviz_availability()
 
         sys.exit(self.run(args))
 

pylint/pyreverse/printer.py

@@ -0,0 +1,92 @@
+# Copyright (c) 2021 Andreas Finkler <[email protected]>
+
+# 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
+
+"""
+Base class defining the interface for a printer.
+"""
+from abc import ABC, abstractmethod
+from enum import Enum
+from typing import List, NamedTuple, Optional
+
+
+class NodeType(Enum):
+    CLASS = "class"
+    INTERFACE = "interface"
+    PACKAGE = "package"
+
+
+class EdgeType(Enum):
+    INHERITS = "inherits"
+    IMPLEMENTS = "implements"
+    ASSOCIATION = "association"
+    USES = "uses"
+
+
+class Layout(Enum):
+    LEFT_TO_RIGHT = "LR"
+    RIGHT_TO_LEFT = "RL"
+    TOP_TO_BOTTOM = "TB"
+    BOTTOM_TO_TOP = "BT"
+
+
+class NodeProperties(NamedTuple):
+    label: str
+    color: Optional[str] = None
+    fontcolor: Optional[str] = None
+    body: Optional[str] = None
+
+
+class Printer(ABC):
+    """Base class defining the interface for a printer"""
+
+    def __init__(
+        self,
+        title: str,
+        layout: Optional[Layout] = None,
+        use_automatic_namespace: Optional[bool] = None,
+    ):
+        self.title: str = title
+        self.layout = layout
+        self.use_automatic_namespace = use_automatic_namespace
+        self.lines: List[str] = []
+        self._open_graph()
+
+    @abstractmethod
+    def _open_graph(self) -> None:
+        """Emit the header lines, i.e. all boilerplate code that defines things like layout etc."""
+
+    def emit(self, line: str, force_newline: Optional[bool] = True) -> None:
+        if force_newline and not line.endswith("\n"):
+            line += "\n"
+        self.lines.append(line)
+
+    @abstractmethod
+    def emit_node(
+        self,
+        name: str,
+        type_: NodeType,
+        properties: Optional[NodeProperties] = None,
+    ) -> None:
+        """Create a new node. Nodes can be classes, packages, participants etc."""
+
+    @abstractmethod
+    def emit_edge(
+        self,
+        from_node: str,
+        to_node: str,
+        type_: EdgeType,
+        label: Optional[str] = None,
+    ) -> None:
+        """Create an edge from one node to another to display relationships."""
+
+    def generate(self, outputfile: str) -> None:
+        """Generate and save the final outputfile."""
+        self._close_graph()
+        with open(outputfile, "w", encoding="utf-8") as outfile:
+            outfile.writelines(self.lines)
+
+    @abstractmethod
+    def _close_graph(self) -> None:
+        """Emit the lines needed to properly close the graph."""