WIP 5

pylint-dev · Aug 27, 2021 · 8455efd · 8455efd
1 parent 43ab448
commit 8455efd
Show file tree

Hide file tree

Showing 2 changed files with 60 additions and 60 deletions.
diff --git a/pylint/extensions/code_style.py b/pylint/extensions/code_style.py
@@ -182,6 +182,7 @@ def _check_consider_using_assignment_expr(self, node: nodes.If) -> None:
 
         Note: Assignment expressions were added in Python 3.8
         """
+        # Check if `node.test` contains a `Name` node
         node_name: Optional[nodes.Name] = None
         if isinstance(node.test, nodes.Name):
             node_name = node.test
@@ -200,74 +201,68 @@ def _check_consider_using_assignment_expr(self, node: nodes.If) -> None:
         else:
             return
 
+        # Make sure the previous node is an assignment to the same name
+        # used in `node.test`. Furthermore, ignore if assignment spans multiple lines.
         prev_sibling = node.previous_sibling()
-        if self._check_prev_sibling_to_if_stmt(prev_sibling, node_name.name):
-            if isinstance(node.test, nodes.Compare):
-                # Check if match statement would be a better fit
-                #     x = 2
-                #     if x == 1: ...
-                #     elif x == 2: ...
-                #     elif x: ...
-                next_if_node: Optional[nodes.If] = None
-                next_sibling = node.next_sibling()
-                if len(node.orelse) == 1 and isinstance(node.orelse[0], nodes.If):
-                    next_if_node = node.orelse[0]
-                elif isinstance(next_sibling, nodes.If):
-                    next_if_node = next_sibling
-
-                if (  # pylint: disable=too-many-boolean-expressions
-                    next_if_node is not None
-                    and (
-                        isinstance(next_if_node.test, nodes.Compare)
-                        and isinstance(next_if_node.test.left, nodes.Name)
-                        and next_if_node.test.left.name == node_name.name
-                        or isinstance(next_if_node.test, nodes.Name)
-                        and next_if_node.test.name == node_name.name
-                    )
-                ):
-                    return
-
-            test_str = node.test.as_string().replace(
-                node_name.name,
-                f"({node_name.name} := {prev_sibling.value.as_string()})",  # type: ignore
-                1,
-            )
-            suggestion = f"if {test_str}:"
+        if (
+            isinstance(prev_sibling, nodes.Assign)
+            and len(prev_sibling.targets) == 1
+            and isinstance(prev_sibling.targets[0], nodes.AssignName)
+        ):
+            assign_node = prev_sibling.targets[0]
+        elif isinstance(prev_sibling, nodes.AnnAssign) and isinstance(
+            prev_sibling.target, nodes.AssignName
+        ):
+            assign_node = prev_sibling.target
+        else:
+            return
+        if (
+            assign_node.name != node_name.name
+            or prev_sibling.tolineno - prev_sibling.fromlineno != 0
+        ):
+            return
 
-            if (
-                node.col_offset is not None
-                and len(suggestion) + node.col_offset > self._max_length
+        # Check if match statement would be a better fit.
+        # I.e. multiple ifs that test the same name.
+        if isinstance(node.test, nodes.Compare):
+            next_if_node: Optional[nodes.If] = None
+            next_sibling = node.next_sibling()
+            if len(node.orelse) == 1 and isinstance(node.orelse[0], nodes.If):
+                next_if_node = node.orelse[0]
+            elif isinstance(next_sibling, nodes.If):
+                next_if_node = next_sibling
+
+            if (  # pylint: disable=too-many-boolean-expressions
+                next_if_node is not None
+                and (
+                    isinstance(next_if_node.test, nodes.Compare)
+                    and isinstance(next_if_node.test.left, nodes.Name)
+                    and next_if_node.test.left.name == node_name.name
+                    or isinstance(next_if_node.test, nodes.Name)
+                    and next_if_node.test.name == node_name.name
+                )
             ):
-                # Don't emit suggestions if resulting line would be too long
                 return
 
-            self.add_message(
-                "consider-using-assignment-expr",
-                node=node_name,
-                args=(suggestion,),
-            )
-
-    @staticmethod
-    def _check_prev_sibling_to_if_stmt(
-        prev_sibling: Optional[nodes.NodeNG], name: Optional[str]
-    ) -> bool:
-        """Check if previous sibiling is an assignment with the same name."""
+        # Build suggestion string. Check length of suggestion
+        # does not exceed max-line-length-suggestions
+        test_str = node.test.as_string().replace(
+            node_name.name,
+            f"({node_name.name} := {prev_sibling.value.as_string()})",
+            1,
+        )
+        suggestion = f"if {test_str}:"
         if (
-            not isinstance(prev_sibling, nodes.NodeNG)
-            or prev_sibling.tolineno - prev_sibling.fromlineno != 0
+            node.col_offset is not None
+            and len(suggestion) + node.col_offset > self._max_length
         ):
-            # Don't suggest assignment expressions
-            # if the assignment spans multiple lines
-            return False
+            return
 
-        return (  # pylint: disable=too-many-boolean-expressions
-            isinstance(prev_sibling, nodes.Assign)
-            and len(prev_sibling.targets) == 1
-            and isinstance(prev_sibling.targets[0], nodes.AssignName)
-            and prev_sibling.targets[0].name == name
-            or isinstance(prev_sibling, nodes.AnnAssign)
-            and isinstance(prev_sibling.target, nodes.AssignName)
-            and prev_sibling.target.name == name
+        # Emit suggestion
+        self.add_message(
+            "consider-using-assignment-expr",
+            node=node_name,
+            args=(suggestion,),
         )
 
 

diff --git a/tests/functional/ext/code_style/code_style_consider_using_assignment_expr.py b/tests/functional/ext/code_style/code_style_consider_using_assignment_expr.py
@@ -31,6 +31,11 @@ def func_a():
     if a6 is None:
         ...
 
+# Previous unrelate note should not match
+print("")
+if a7:
+    ...
+
 
 b1: int = 2
 if b1:  # [consider-using-assignment-expr]