Merge pull request #588 from LuisFigueroaG/feat/dict-fromkeys-mutable

Add B046: mutable value passed to dict.fromkeys()

Author: cooperlees

README.rst

@@ -325,6 +325,12 @@ Names beginning with an underscore are ignored to allow intentional discards.
 Separate function/class/comprehension scopes and a loop's ``else`` suite do not
 count as active enclosing loops.
 
+.. _B046:
+
+**B046**: Mutable value passed to ``dict.fromkeys()``. The same object is shared by every key,
+so changing it for one key changes it for all of them. Use a dict comprehension instead, e.g.
+``{key: [] for key in keys}``.
+
 
 Opinionated warnings
 ~~~~~~~~~~~~~~~~~~~~
@@ -529,6 +535,8 @@ UNRELEASED
 
 * B045: detect reuse of an enclosing loop's control variable in a nested loop
   in the same scope (#360).
+* B046: New check for ``dict.fromkeys()`` called with a mutable value, which is
+  shared by every key (#387)
 * B005: say in the message that only strings with repeated characters are
   reported, since ``.strip("abc")`` style character sets are fine (#91)
 * B031: only treat a bare ``groupby()`` call as ``itertools.groupby`` when the

bugbear.py

@@ -762,6 +762,7 @@ def visit_Call(self, node: ast.Call) -> None:
         self.check_for_b028(node)
         self.check_for_b034(node)
         self.check_for_b039(node)
+        self.check_for_b046(node)
         self.check_for_b905(node)
         self.check_for_b910(node)
         self.check_for_b911(node)
@@ -2615,6 +2616,31 @@ def check_for_b044(self, node: ast.Assert) -> None:
         if isinstance(node.test, ast.GeneratorExp):
             self.add_error("B044", node)
 
+    def check_for_b046(self, node: ast.Call) -> None:
+        if not (
+            isinstance(node.func, ast.Attribute)
+            and node.func.attr == "fromkeys"
+            and ".".join(compose_call_path(node.func.value)) in B046_DICT_TYPES
+        ):
+            return
+
+        if len(node.args) == 2:
+            value = node.args[1]
+        elif len(node.args) == 1:
+            # OrderedDict.fromkeys() also takes the value as a keyword argument
+            value = next((kw.value for kw in node.keywords if kw.arg == "value"), None)
+        else:
+            return
+
+        if isinstance(
+            value,
+            (ast.List, ast.Dict, ast.Set, ast.ListComp, ast.DictComp, ast.SetComp),
+        ) or (
+            isinstance(value, ast.Call)
+            and ".".join(compose_call_path(value.func)) in B006_MUTABLE_CALLS
+        ):
+            self.add_error("B046", value)
+
     def check_for_b909(self, node: ast.For) -> None:
         if isinstance(node.iter, ast.Name):
             name = _to_name_str(node.iter)
@@ -3127,6 +3153,11 @@ def visit_Lambda(self, node: ast.Lambda) -> None:
     "async_lru.alru_cache",
     "alru_cache",
 }
+B046_DICT_TYPES = {
+    "dict",
+    "OrderedDict",
+    "collections.OrderedDict",
+}
 B902_IMPLICIT_CLASSMETHODS = {"__new__", "__init_subclass__", "__class_getitem__"}
 B902_SELF = ["self"]  # it's a list because the first is preferred
 B902_CLS = ["cls", "klass"]  # ditto.
@@ -3419,6 +3450,13 @@ def __call__(self, lineno: int, col: int, vars: tuple[object, ...] = ()) -> erro
             "Use a different name for the inner loop variable."
         )
     ),
+    "B046": Error(
+        message=(
+            "B046 Mutable value passed to `dict.fromkeys()`. The same object is "
+            "shared by every key, so changing it for one key changes it for all of "
+            "them. Use a dict comprehension instead, e.g. `{{key: [] for key in keys}}`."
+        )
+    ),
     # Warnings disabled by default.
     "B901": Error(
         message=(

tests/eval_files/b046.py

@@ -0,0 +1,35 @@
+import collections
+from collections import OrderedDict
+
+keys = ("a", "b")
+
+# bad
+dict.fromkeys(keys, [])  # B046: 20
+dict.fromkeys(keys, {})  # B046: 20
+dict.fromkeys(keys, {1, 2})  # B046: 20
+dict.fromkeys(keys, [x for x in range(3)])  # B046: 20
+dict.fromkeys(keys, {x: x for x in range(3)})  # B046: 20
+dict.fromkeys(keys, {x for x in range(3)})  # B046: 20
+dict.fromkeys(keys, list())  # B046: 20
+dict.fromkeys(keys, dict())  # B046: 20
+dict.fromkeys(keys, set())  # B046: 20
+dict.fromkeys(keys, collections.defaultdict(list))  # B046: 20
+dict.fromkeys(keys, collections.Counter())  # B046: 20
+OrderedDict.fromkeys(keys, [])  # B046: 27
+collections.OrderedDict.fromkeys(keys, [])  # B046: 39
+OrderedDict.fromkeys(keys, value=[])  # B046: 33
+dict.fromkeys(["a", "b"], [])  # B046: 26
+
+# good
+dict.fromkeys(keys)
+dict.fromkeys(keys, None)
+dict.fromkeys(keys, 0)
+dict.fromkeys(keys, "")
+dict.fromkeys(keys, ())
+dict.fromkeys(keys, tuple())
+dict.fromkeys(keys, frozenset())
+dict.fromkeys(keys, value)
+dict.fromkeys(keys, make_default())
+OrderedDict.fromkeys(keys, value=None)
+{key: [] for key in keys}
+other.fromkeys(keys, [])