New checker `unbalanced dict unpacking` (#7750)

Co-authored-by: Pierre Sassoulas <pierre.sassoulas@gmail.com>

10 files changed, 231 insertions, 29 deletions

diff --git a/doc/data/messages/u/unbalanced-dict-unpacking/bad.py b/doc/data/messages/u/unbalanced-dict-unpacking/bad.py
new file mode 100644
index 000000000..9162ccc45
--- /dev/null
+++ b/doc/data/messages/u/unbalanced-dict-unpacking/bad.py
@@ -0,0 +1,4 @@
+FRUITS = {"apple": 2, "orange": 3, "mellon": 10}
+
+for fruit, price in FRUITS.values():  # [unbalanced-dict-unpacking]
+    print(fruit)
diff --git a/doc/data/messages/u/unbalanced-dict-unpacking/good.py b/doc/data/messages/u/unbalanced-dict-unpacking/good.py
new file mode 100644
index 000000000..450e03489
--- /dev/null
+++ b/doc/data/messages/u/unbalanced-dict-unpacking/good.py
@@ -0,0 +1,4 @@
+FRUITS = {"apple": 2, "orange": 3, "mellon": 10}
+
+for fruit, price in FRUITS.items():
+    print(fruit)
diff --git a/doc/user_guide/checkers/features.rst b/doc/user_guide/checkers/features.rst
index 4494c7d33..8b166855a 100644
--- a/doc/user_guide/checkers/features.rst
+++ b/doc/user_guide/checkers/features.rst
@@ -1309,6 +1309,8 @@ Variables checker Messages
   variable is not defined in the module scope.
 :self-cls-assignment (W0642): *Invalid assignment to %s in method*
   Invalid assignment to self or cls in instance or class method respectively.
+:unbalanced-dict-unpacking (W0644): *Possible unbalanced dict unpacking with %s: left side has %d label%s, right side has %d value%s*
+  Used when there is an unbalanced dict unpacking in assignment or for loop
 :unbalanced-tuple-unpacking (W0632): *Possible unbalanced tuple unpacking with sequence %s: left side has %d label%s, right side has %d value%s*
   Used when there is an unbalanced tuple unpacking in assignment
 :possibly-unused-variable (W0641): *Possibly unused variable %r*
diff --git a/doc/user_guide/messages/messages_overview.rst b/doc/user_guide/messages/messages_overview.rst
index faabacc28..c292c88c6 100644
--- a/doc/user_guide/messages/messages_overview.rst
+++ b/doc/user_guide/messages/messages_overview.rst
@@ -313,6 +313,7 @@ All messages in the warning category:
    warning/super-without-brackets
    warning/too-many-try-statements
    warning/try-except-raise
+   warning/unbalanced-dict-unpacking
    warning/unbalanced-tuple-unpacking
    warning/undefined-loop-variable
    warning/unknown-option-value
diff --git a/doc/whatsnew/fragments/5797.new_check b/doc/whatsnew/fragments/5797.new_check
new file mode 100644
index 000000000..f82abe09d
--- /dev/null
+++ b/doc/whatsnew/fragments/5797.new_check
@@ -0,0 +1,4 @@
+Add new check called ``unbalanced-dict-unpacking`` to check for unbalanced dict unpacking
+in assignment and for loops.
+
+Closes #5797
diff --git a/pylint/checkers/variables.py b/pylint/checkers/variables.py
index e258c917b..9efb9e1be 100644
--- a/pylint/checkers/variables.py
+++ b/pylint/checkers/variables.py
@@ -113,6 +113,13 @@ TYPING_NAMES = frozenset(
     }
 )
 
+DICT_TYPES = (
+    astroid.objects.DictValues,
+    astroid.objects.DictKeys,
+    astroid.objects.DictItems,
+    astroid.nodes.node_classes.Dict,
+)
+
 
 class VariableVisitConsumerAction(Enum):
     """Reported by _check_consumer() and its sub-methods to determine the
@@ -161,9 +168,16 @@ def overridden_method(
 
 def _get_unpacking_extra_info(node: nodes.Assign, inferred: InferenceResult) -> str:
     """Return extra information to add to the message for unpacking-non-sequence
-    and unbalanced-tuple-unpacking errors.
+    and unbalanced-tuple/dict-unpacking errors.
     """
     more = ""
+    if isinstance(inferred, DICT_TYPES):
+        if isinstance(node, nodes.Assign):
+            more = node.value.as_string()
+        elif isinstance(node, nodes.For):
+            more = node.iter.as_string()
+        return more
+
     inferred_module = inferred.root().name
     if node.root().name == inferred_module:
         if node.lineno == inferred.lineno:
@@ -518,6 +532,12 @@ MSGS: dict[str, MessageDefinitionTuple] = {
         "Emitted when an index used on an iterable goes beyond the length of that "
         "iterable.",
     ),
+    "W0644": (
+        "Possible unbalanced dict unpacking with %s: "
+        "left side has %d label%s, right side has %d value%s",
+        "unbalanced-dict-unpacking",
+        "Used when there is an unbalanced dict unpacking in assignment or for loop",
+    ),
 }
 
 
@@ -1108,6 +1128,41 @@ class VariablesChecker(BaseChecker):
         """This is a queue, last in first out."""
         self._postponed_evaluation_enabled = False
 
+    @utils.only_required_for_messages(
+        "unbalanced-dict-unpacking",
+    )
+    def visit_for(self, node: nodes.For) -> None:
+        if not isinstance(node.target, nodes.Tuple):
+            return
+
+        targets = node.target.elts
+
+        inferred = utils.safe_infer(node.iter)
+        if not isinstance(inferred, DICT_TYPES):
+            return
+
+        values = self._nodes_to_unpack(inferred)
+        if not values:
+            # no dict items returned
+            return
+
+        if isinstance(inferred, astroid.objects.DictItems):
+            # dict.items() is a bit special because values will be a tuple
+            # So as long as there are always 2 targets and values each are
+            # a tuple with two items, this will unpack correctly.
+            # Example: `for key, val in {1: 2, 3: 4}.items()`
+            if len(targets) == 2 and all(len(x.elts) == 2 for x in values):
+                return
+
+            # Starred nodes indicate ambiguous unpacking
+            # if `dict.items()` is used so we won't flag them.
+            if any(isinstance(target, nodes.Starred) for target in targets):
+                return
+
+        if len(targets) != len(values):
+            details = _get_unpacking_extra_info(node, inferred)
+            self._report_unbalanced_unpacking(node, inferred, targets, values, details)
+
     def leave_for(self, node: nodes.For) -> None:
         self._store_type_annotation_names(node)
 
@@ -1745,7 +1800,10 @@ class VariablesChecker(BaseChecker):
             self._check_module_attrs(node, module, name.split("."))
 
     @utils.only_required_for_messages(
-        "unbalanced-tuple-unpacking", "unpacking-non-sequence", "self-cls-assignment"
+        "unbalanced-tuple-unpacking",
+        "unpacking-non-sequence",
+        "self-cls-assignment",
+        "unbalanced_dict_unpacking",
     )
     def visit_assign(self, node: nodes.Assign) -> None:
         """Check unbalanced tuple unpacking for assignments and unpacking
@@ -1756,6 +1814,11 @@ class VariablesChecker(BaseChecker):
             return
 
         targets = node.targets[0].itered()
+
+        # Check if we have starred nodes.
+        if any(isinstance(target, nodes.Starred) for target in targets):
+            return
+
         try:
             inferred = utils.safe_infer(node.value)
             if inferred is not None:
@@ -2670,32 +2733,20 @@ class VariablesChecker(BaseChecker):
         # Attempt to check unpacking is properly balanced
         values = self._nodes_to_unpack(inferred)
         details = _get_unpacking_extra_info(node, inferred)
+
         if values is not None:
             if len(targets) != len(values):
-                # Check if we have starred nodes.
-                if any(isinstance(target, nodes.Starred) for target in targets):
-                    return
-                self.add_message(
-                    "unbalanced-tuple-unpacking",
-                    node=node,
-                    args=(
-                        details,
-                        len(targets),
-                        "" if len(targets) == 1 else "s",
-                        len(values),
-                        "" if len(values) == 1 else "s",
-                    ),
+                self._report_unbalanced_unpacking(
+                    node, inferred, targets, values, details
                 )
         # attempt to check unpacking may be possible (i.e. RHS is iterable)
         elif not utils.is_iterable(inferred):
-            if details and not details.startswith(" "):
-                details = f" {details}"
-            self.add_message("unpacking-non-sequence", node=node, args=details)
+            self._report_unpacking_non_sequence(node, details)
 
     @staticmethod
     def _nodes_to_unpack(node: nodes.NodeNG) -> list[nodes.NodeNG] | None:
         """Return the list of values of the `Assign` node."""
-        if isinstance(node, (nodes.Tuple, nodes.List)):
+        if isinstance(node, (nodes.Tuple, nodes.List) + DICT_TYPES):
             return node.itered()  # type: ignore[no-any-return]
         if isinstance(node, astroid.Instance) and any(
             ancestor.qname() == "typing.NamedTuple" for ancestor in node.ancestors()
@@ -2703,6 +2754,34 @@ class VariablesChecker(BaseChecker):
             return [i for i in node.values() if isinstance(i, nodes.AssignName)]
         return None
 
+    def _report_unbalanced_unpacking(
+        self,
+        node: nodes.NodeNG,
+        inferred: InferenceResult,
+        targets: list[nodes.NodeNG],
+        values: list[nodes.NodeNG],
+        details: str,
+    ) -> None:
+        args = (
+            details,
+            len(targets),
+            "" if len(targets) == 1 else "s",
+            len(values),
+            "" if len(values) == 1 else "s",
+        )
+
+        symbol = (
+            "unbalanced-dict-unpacking"
+            if isinstance(inferred, DICT_TYPES)
+            else "unbalanced-tuple-unpacking"
+        )
+        self.add_message(symbol, node=node, args=args, confidence=INFERENCE)
+
+    def _report_unpacking_non_sequence(self, node: nodes.NodeNG, details: str) -> None:
+        if details and not details.startswith(" "):
+            details = f" {details}"
+        self.add_message("unpacking-non-sequence", node=node, args=details)
+
     def _check_module_attrs(
         self,
         node: _base_nodes.ImportNode,
diff --git a/tests/functional/u/unbalanced_dict_unpacking.py b/tests/functional/u/unbalanced_dict_unpacking.py
new file mode 100644
index 000000000..2c4d3b103
--- /dev/null
+++ b/tests/functional/u/unbalanced_dict_unpacking.py
@@ -0,0 +1,91 @@
+"""Check possible unbalanced dict unpacking """
+# pylint: disable=missing-function-docstring, invalid-name
+# pylint: disable=unused-variable, redefined-outer-name, line-too-long
+
+def dict_vals():
+    a, b, c, d, e, f, g = {1: 2}.values()  # [unbalanced-dict-unpacking]
+    return a, b
+
+def dict_keys():
+    a, b, c, d, e, f, g = {1: 2, "hi": 20}.keys()  # [unbalanced-dict-unpacking]
+    return a, b
+
+
+def dict_items():
+    tupe_one, tuple_two = {1: 2, "boo": 3}.items()
+    tupe_one, tuple_two, tuple_three = {1: 2, "boo": 3}.items()  # [unbalanced-dict-unpacking]
+    return tuple_three
+
+def all_dict():
+    a, b, c, d, e, f, g = {1: 2, 3: 4}  # [unbalanced-dict-unpacking]
+    return a
+
+for a, b, c, d, e, f, g in {1: 2}.items():  # [unbalanced-dict-unpacking]
+    pass
+
+for key, value in {1: 2}:  # [unbalanced-dict-unpacking]
+    pass
+
+for key, value in {1: 2}.keys():  # [unbalanced-dict-unpacking, consider-iterating-dictionary]
+    pass
+
+for key, value in {1: 2}.values():  # [unbalanced-dict-unpacking]
+    pass
+
+empty = {}
+
+# this should not raise unbalanced-dict because it is valid code using `items()`
+for key, value in empty.items():
+    print(key)
+    print(value)
+
+for key, val in {1: 2}.items():
+    print(key)
+
+populated = {2: 1}
+for key, val in populated.items():
+    print(key)
+
+key, val = populated.items()  # [unbalanced-dict-unpacking]
+
+for key, val in {1: 2, 3: 4, 5: 6}.items():
+    print(key)
+
+key, val = {1: 2, 3: 4, 5: 6}.items()  # [unbalanced-dict-unpacking]
+
+a, b, c = {}  # [unbalanced-dict-unpacking]
+
+for k in {'key': 'value', 1: 2}.items():
+    print(k)
+
+for k, _ in {'key': 'value'}.items():
+    print(k)
+
+for _, _ in {'key': 'value'}.items():
+    print(_)
+
+for _, val in {'key': 'value'}.values():  # [unbalanced-dict-unpacking]
+    print(val)
+
+for key, *val in {'key': 'value', 1: 2}.items():
+    print(key)
+
+for *key, val in {'key': 'value', 1: 2}.items():
+    print(key)
+
+
+for key, *val in {'key': 'value', 1: 2, 20: 21}.values():  # [unbalanced-dict-unpacking]
+    print(key)
+
+for *key, val in {'key': 'value', 1: 2, 20: 21}.values():  # [unbalanced-dict-unpacking]
+    print(key)
+
+one, *others = {1: 2, 3: 4, 5: 6}.items()
+one, *others, last = {1: 2, 3: 4, 5: 6}.items()
+
+one, *others = {1: 2, 3: 4, 5: 6}.values()
+one, *others, last = {1: 2, 3: 4, 5: 6}.values()
+
+_, *others = {1: 2, 3: 4, 5: 6}.items()
+_, *others = {1: 2, 3: 4, 5: 6}.values()
+_, others = {1: 2, 3: 4, 5: 6}.values()  # [unbalanced-dict-unpacking]
diff --git a/tests/functional/u/unbalanced_dict_unpacking.txt b/tests/functional/u/unbalanced_dict_unpacking.txt
new file mode 100644
index 000000000..b31d89b40
--- /dev/null
+++ b/tests/functional/u/unbalanced_dict_unpacking.txt
@@ -0,0 +1,16 @@
+unbalanced-dict-unpacking:6:4:6:41:dict_vals:"Possible unbalanced dict unpacking with {1: 2}.values(): left side has 7 labels, right side has 1 value":INFERENCE
+unbalanced-dict-unpacking:10:4:10:49:dict_keys:"Possible unbalanced dict unpacking with {1: 2, 'hi': 20}.keys(): left side has 7 labels, right side has 2 values":INFERENCE
+unbalanced-dict-unpacking:16:4:16:63:dict_items:"Possible unbalanced dict unpacking with {1: 2, 'boo': 3}.items(): left side has 3 labels, right side has 2 values":INFERENCE
+unbalanced-dict-unpacking:20:4:20:38:all_dict:"Possible unbalanced dict unpacking with {1: 2, 3: 4}: left side has 7 labels, right side has 2 values":INFERENCE
+unbalanced-dict-unpacking:23:0:24:8::"Possible unbalanced dict unpacking with {1: 2}.items(): left side has 7 labels, right side has 1 value":INFERENCE
+unbalanced-dict-unpacking:26:0:27:8::"Possible unbalanced dict unpacking with {1: 2}: left side has 2 labels, right side has 1 value":INFERENCE
+consider-iterating-dictionary:29:18:29:31::Consider iterating the dictionary directly instead of calling .keys():INFERENCE
+unbalanced-dict-unpacking:29:0:30:8::"Possible unbalanced dict unpacking with {1: 2}.keys(): left side has 2 labels, right side has 1 value":INFERENCE
+unbalanced-dict-unpacking:32:0:33:8::"Possible unbalanced dict unpacking with {1: 2}.values(): left side has 2 labels, right side has 1 value":INFERENCE
+unbalanced-dict-unpacking:49:0:49:28::"Possible unbalanced dict unpacking with populated.items(): left side has 2 labels, right side has 1 value":INFERENCE
+unbalanced-dict-unpacking:54:0:54:37::"Possible unbalanced dict unpacking with {1: 2, 3: 4, 5: 6}.items(): left side has 2 labels, right side has 3 values":INFERENCE
+unbalanced-dict-unpacking:56:0:56:12::"Possible unbalanced dict unpacking with {}: left side has 3 labels, right side has 0 values":INFERENCE
+unbalanced-dict-unpacking:67:0:68:14::"Possible unbalanced dict unpacking with {'key': 'value'}.values(): left side has 2 labels, right side has 1 value":INFERENCE
+unbalanced-dict-unpacking:77:0:78:14::"Possible unbalanced dict unpacking with {'key': 'value', 1: 2, 20: 21}.values(): left side has 2 labels, right side has 3 values":INFERENCE
+unbalanced-dict-unpacking:80:0:81:14::"Possible unbalanced dict unpacking with {'key': 'value', 1: 2, 20: 21}.values(): left side has 2 labels, right side has 3 values":INFERENCE
+unbalanced-dict-unpacking:91:0:91:39::"Possible unbalanced dict unpacking with {1: 2, 3: 4, 5: 6}.values(): left side has 2 labels, right side has 3 values":INFERENCE
diff --git a/tests/functional/u/unbalanced_tuple_unpacking.txt b/tests/functional/u/unbalanced_tuple_unpacking.txt
index a4e1df2db..651e09840 100644
--- a/tests/functional/u/unbalanced_tuple_unpacking.txt
+++ b/tests/functional/u/unbalanced_tuple_unpacking.txt
@@ -1,9 +1,9 @@
-unbalanced-tuple-unpacking:11:4:11:27:do_stuff:"Possible unbalanced tuple unpacking with sequence '(1, 2, 3)': left side has 2 labels, right side has 3 values":UNDEFINED
-unbalanced-tuple-unpacking:17:4:17:29:do_stuff1:"Possible unbalanced tuple unpacking with sequence '[1, 2, 3]': left side has 2 labels, right side has 3 values":UNDEFINED
-unbalanced-tuple-unpacking:23:4:23:29:do_stuff2:"Possible unbalanced tuple unpacking with sequence '(1, 2, 3)': left side has 2 labels, right side has 3 values":UNDEFINED
-unbalanced-tuple-unpacking:82:4:82:28:do_stuff9:"Possible unbalanced tuple unpacking with sequence defined at line 7 of functional.u.unpacking.unpacking: left side has 2 labels, right side has 3 values":UNDEFINED
-unbalanced-tuple-unpacking:96:8:96:33:UnbalancedUnpacking.test:"Possible unbalanced tuple unpacking with sequence defined at line 7 of functional.u.unpacking.unpacking: left side has 2 labels, right side has 3 values":UNDEFINED
-unbalanced-tuple-unpacking:140:8:140:43:MyClass.sum_unpack_3_into_4:"Possible unbalanced tuple unpacking with sequence defined at line 128: left side has 4 labels, right side has 3 values":UNDEFINED
-unbalanced-tuple-unpacking:145:8:145:28:MyClass.sum_unpack_3_into_2:"Possible unbalanced tuple unpacking with sequence defined at line 128: left side has 2 labels, right side has 3 values":UNDEFINED
-unbalanced-tuple-unpacking:157:0:157:24::"Possible unbalanced tuple unpacking with sequence defined at line 151: left side has 2 labels, right side has 0 values":UNDEFINED
-unbalanced-tuple-unpacking:162:0:162:16::"Possible unbalanced tuple unpacking with sequence '(1, 2)': left side has 3 labels, right side has 2 values":UNDEFINED
+unbalanced-tuple-unpacking:11:4:11:27:do_stuff:"Possible unbalanced tuple unpacking with sequence '(1, 2, 3)': left side has 2 labels, right side has 3 values":INFERENCE
+unbalanced-tuple-unpacking:17:4:17:29:do_stuff1:"Possible unbalanced tuple unpacking with sequence '[1, 2, 3]': left side has 2 labels, right side has 3 values":INFERENCE
+unbalanced-tuple-unpacking:23:4:23:29:do_stuff2:"Possible unbalanced tuple unpacking with sequence '(1, 2, 3)': left side has 2 labels, right side has 3 values":INFERENCE
+unbalanced-tuple-unpacking:82:4:82:28:do_stuff9:"Possible unbalanced tuple unpacking with sequence defined at line 7 of functional.u.unpacking.unpacking: left side has 2 labels, right side has 3 values":INFERENCE
+unbalanced-tuple-unpacking:96:8:96:33:UnbalancedUnpacking.test:"Possible unbalanced tuple unpacking with sequence defined at line 7 of functional.u.unpacking.unpacking: left side has 2 labels, right side has 3 values":INFERENCE
+unbalanced-tuple-unpacking:140:8:140:43:MyClass.sum_unpack_3_into_4:"Possible unbalanced tuple unpacking with sequence defined at line 128: left side has 4 labels, right side has 3 values":INFERENCE
+unbalanced-tuple-unpacking:145:8:145:28:MyClass.sum_unpack_3_into_2:"Possible unbalanced tuple unpacking with sequence defined at line 128: left side has 2 labels, right side has 3 values":INFERENCE
+unbalanced-tuple-unpacking:157:0:157:24::"Possible unbalanced tuple unpacking with sequence defined at line 151: left side has 2 labels, right side has 0 values":INFERENCE
+unbalanced-tuple-unpacking:162:0:162:16::"Possible unbalanced tuple unpacking with sequence '(1, 2)': left side has 3 labels, right side has 2 values":INFERENCE
diff --git a/tests/functional/u/unbalanced_tuple_unpacking_py30.py b/tests/functional/u/unbalanced_tuple_unpacking_py30.py
index dd3e4b6d3..c45cccdd1 100644
--- a/tests/functional/u/unbalanced_tuple_unpacking_py30.py
+++ b/tests/functional/u/unbalanced_tuple_unpacking_py30.py
@@ -1,10 +1,11 @@
 """ Test that using starred nodes in unpacking
 does not trigger a false positive on Python 3.
 """
-
+# pylint: disable=unused-variable
 
 def test():
     """ Test that starred expressions don't give false positives. """
     first, second, *last = (1, 2, 3, 4)
+    one, two, three, *four = (1, 2, 3, 4)
     *last, = (1, 2)
     return (first, second, last)