Extend redundant-assignment check to parallel and multi-target assign…

…ments (#1098)

CHANGELOG.md

@@ -12,6 +12,7 @@ and adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
 - Added `include_frames` filter to `snapshot`
 - Added `exclude_vars` filter to `snapshot`
 - Added new `python_ta.debug` module with an `SnapshotTracer` context manager for generating memory models
+- Included the name of redundant variable in `E9959 redundant-assignment` message
 - Update to pylint v3.3 and and astroid v3.3. This added support for Python 3.13 and dropped support for Python 3.8.
 - Added a STRICT_NUMERIC_TYPES configuration to `python_ta.contracts` allowing to enable/disable stricter type checking of numeric types
 
@@ -23,6 +24,7 @@ and adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
 
 - Fixed issue where `snapshot` errors on unserializable values
 - Fixed issue within `Snapshot.py` where the `memory_viz_version` parameter was not respected
+- Fixed issue where parallel assignment statements and assignment to multiple targets were not checked by `redundant_assignment_checker`
 - Fixed issue where annotated assignment statements were not checked by `redundant_assignment_checker`
 
 ### 🔧 Internal changes

python_ta/checkers/redundant_assignment_checker.py

@@ -34,33 +34,39 @@ class RedundantAssignmentChecker(BaseChecker):
     name = "redundant_assignment"
     msgs = {
         "E9959": (
-            "This assignment statement is redundant;" " You can remove it from the program.",
+            "Assigning to variable(s) %s here is redundant, because they are not used before getting reassigned later."
+            " You can remove the assignment(s) without changing the behaviour of this code.",
             "redundant-assignment",
-            "This assignment statement is redundant;" " You can remove it from the program.",
+            "Assigning to variable(s) is redundant, because they are not used before getting reassigned later."
+            " You can remove the assignment(s) without changing the behaviour of this code.",
         )
     }
 
     def __init__(self, linter=None) -> None:
         super().__init__(linter=linter)
-        self._redundant_assignment: set[nodes.Assign] = set()
+        self._redundant_assignment: dict[
+            nodes.Assign | nodes.AugAssign | nodes.AnnAssign, list[str]
+        ] = {}
 
     @only_required_for_messages("redundant-assignment")
     def visit_assign(self, node: nodes.Assign) -> None:
         """Visit the assign node"""
         if node in self._redundant_assignment:
-            self.add_message("redundant-assignment", node=node)
+            self.add_message(
+                "redundant-assignment", node=node, args=", ".join(self._redundant_assignment[node])
+            )
 
     @only_required_for_messages("redundant-assignment")
     def visit_augassign(self, node: nodes.AugAssign) -> None:
         """ "Visit the augmented assign node"""
         if node in self._redundant_assignment:
-            self.add_message("redundant-assignment", node=node)
+            self.add_message("redundant-assignment", node=node, args=node.target.name)
 
     @only_required_for_messages("redundant-assignment")
     def visit_annassign(self, node: nodes.AnnAssign) -> None:
         """Visit the annotated assign node"""
         if node in self._redundant_assignment:
-            self.add_message("redundant-assignment", node=node)
+            self.add_message("redundant-assignment", node=node, args=node.target.name)
 
     def visit_module(self, node: nodes.Module) -> None:
         """Visit the module"""
@@ -120,10 +126,24 @@ def _transfer(self, block: CFGBlock, out_facts: set[str]) -> set[str]:
                 nodes.FunctionDef,
             ):
                 if isinstance(node, nodes.AssignName):
+                    # checking for parent node that accounts for parallel assignment
+                    parent = (
+                        node.parent.parent if isinstance(node.parent, nodes.Tuple) else node.parent
+                    )
                     if node.name in gen.difference(kill):
-                        self._redundant_assignment.add(node.parent)
-                    elif node.parent in self._redundant_assignment:
-                        self._redundant_assignment.remove(node.parent)
+                        # add redundant assignment
+                        if parent not in self._redundant_assignment:
+                            self._redundant_assignment[parent] = []
+                        if node.name not in self._redundant_assignment[parent]:
+                            self._redundant_assignment[parent].append(node.name)
+                    elif (
+                        parent in self._redundant_assignment
+                        and node.name in self._redundant_assignment[parent]
+                    ):
+                        # remove redundant assignment
+                        self._redundant_assignment[parent].remove(node.name)
+                        if len(self._redundant_assignment[parent]) == 0:
+                            self._redundant_assignment.pop(parent)
 
                     # When node.parent is an AugAssign, the name counts as a use of the variable,
                     # and so is added to kill.

tests/test_custom_checkers/test_redundant_assignment_checker.py

@@ -38,7 +38,7 @@ def test_message_simple(self):
 
         self.checker.visit_module(mod)
         with self.assertAddsMessages(
-            pylint.testutils.MessageTest(msg_id="redundant-assignment", node=assign_1),
+            pylint.testutils.MessageTest(msg_id="redundant-assignment", node=assign_1, args="x"),
             ignore_position=True,
         ):
             self.checker.visit_assign(assign_1)
@@ -60,7 +60,7 @@ def test_message_if_stmt(self):
 
         self.checker.visit_module(mod)
         with self.assertAddsMessages(
-            pylint.testutils.MessageTest(msg_id="redundant-assignment", node=assign_x),
+            pylint.testutils.MessageTest(msg_id="redundant-assignment", node=assign_x, args="x"),
             ignore_position=True,
         ):
             self.checker.visit_assign(assign_x)
@@ -80,8 +80,8 @@ def test_message_loop_complex(self):
 
         self.checker.visit_module(mod)
         with self.assertAddsMessages(
-            pylint.testutils.MessageTest(msg_id="redundant-assignment", node=assign_y),
-            pylint.testutils.MessageTest(msg_id="redundant-assignment", node=assign_x1),
+            pylint.testutils.MessageTest(msg_id="redundant-assignment", node=assign_y, args="y"),
+            pylint.testutils.MessageTest(msg_id="redundant-assignment", node=assign_x1, args="x"),
             ignore_position=True,
         ):
             self.checker.visit_assign(assign_y)
@@ -105,7 +105,7 @@ def func2():
 
         self.checker.visit_module(mod)
         with self.assertAddsMessages(
-            pylint.testutils.MessageTest(msg_id="redundant-assignment", node=assign_x),
+            pylint.testutils.MessageTest(msg_id="redundant-assignment", node=assign_x, args="x"),
             ignore_position=True,
         ):
             self.checker.visit_assign(assign_x)
@@ -240,7 +240,9 @@ def test_augassign_redundant(self):
 
         self.checker.visit_module(mod)
         with self.assertAddsMessages(
-            pylint.testutils.MessageTest(msg_id="redundant-assignment", node=augassign_node),
+            pylint.testutils.MessageTest(
+                msg_id="redundant-assignment", node=augassign_node, args="y_pos"
+            ),
             ignore_position=True,
         ):
             self.checker.visit_augassign(augassign_node)
@@ -256,7 +258,79 @@ def test_annassign_redundant(self):
 
         self.checker.visit_module(mod)
         with self.assertAddsMessages(
-            pylint.testutils.MessageTest(msg_id="redundant-assignment", node=annassign_node),
+            pylint.testutils.MessageTest(
+                msg_id="redundant-assignment", node=annassign_node, args="y_pos"
+            ),
             ignore_position=True,
         ):
             self.checker.visit_annassign(annassign_node)
+
+    def test_parallel_assign_redundant(self):
+        src = """
+        x, y = 0, 0
+        x, y = 10, 10
+        """
+        mod = astroid.parse(src)
+        mod.accept(CFGVisitor())
+        assign_node, *_ = mod.nodes_of_class(nodes.Assign)
+
+        self.checker.visit_module(mod)
+        with self.assertAddsMessages(
+            pylint.testutils.MessageTest(
+                msg_id="redundant-assignment", node=assign_node, args="x, y"
+            ),
+            ignore_position=True,
+        ):
+            self.checker.visit_assign(assign_node)
+
+    def test_parallel_assign_one_variable_redundant(self):
+        src = """
+        x, y = 0, 0
+        y = 10
+        """
+        mod = astroid.parse(src)
+        mod.accept(CFGVisitor())
+        assign_node, *_ = mod.nodes_of_class(nodes.Assign)
+
+        self.checker.visit_module(mod)
+        with self.assertAddsMessages(
+            pylint.testutils.MessageTest(msg_id="redundant-assignment", node=assign_node, args="y"),
+            ignore_position=True,
+        ):
+            self.checker.visit_assign(assign_node)
+
+    def test_multiple_target_assign_redundant(self):
+        src = """
+        x = y = z = 10
+        x = 11
+        y = 45
+        z = 14
+        """
+        mod = astroid.parse(src)
+        mod.accept(CFGVisitor())
+        assign_node, *_ = mod.nodes_of_class(nodes.Assign)
+
+        self.checker.visit_module(mod)
+        with self.assertAddsMessages(
+            pylint.testutils.MessageTest(
+                msg_id="redundant-assignment", node=assign_node, args="x, y, z"
+            ),
+            ignore_position=True,
+        ):
+            self.checker.visit_assign(assign_node)
+
+    def test_multiple_target_assign_one_variable_redundant(self):
+        src = """
+        x = y = z = 10
+        y = 6
+        """
+        mod = astroid.parse(src)
+        mod.accept(CFGVisitor())
+        assign_node, *_ = mod.nodes_of_class(nodes.Assign)
+
+        self.checker.visit_module(mod)
+        with self.assertAddsMessages(
+            pylint.testutils.MessageTest(msg_id="redundant-assignment", node=assign_node, args="y"),
+            ignore_position=True,
+        ):
+            self.checker.visit_assign(assign_node)