From 50c6729b37570b0f4a822977a74012a14189c659 Mon Sep 17 00:00:00 2001 From: LuisFigueroaG Date: Tue, 27 Oct 2026 16:05:35 -0300 Subject: [PATCH 1/2] B018: don't flag map() and friends the module binds to something else B018 reported every bare map(), filter(), reversed(), zip() and enumerate() statement as a lazy builtin, even when the module defines its own `def map()`, imports one from another library or assigns the name to some other callable. Those calls can do real work. The check B031 and B911 use to tell whether a bare groupby()/batched() is the itertools one now takes the module to compare against, and B018 uses it with builtins. `from builtins import map`, `map = builtins.map` and names the module never binds are still reported. --- README.rst | 7 +++ bugbear.py | 56 +++++++++++++----------- tests/eval_files/b018_shadowed.py | 21 +++++++++ tests/eval_files/b018_shadowed_alias.py | 11 +++++ tests/eval_files/b018_shadowed_import.py | 14 ++++++ 5 files changed, 83 insertions(+), 26 deletions(-) create mode 100644 tests/eval_files/b018_shadowed.py create mode 100644 tests/eval_files/b018_shadowed_alias.py create mode 100644 tests/eval_files/b018_shadowed_import.py diff --git a/README.rst b/README.rst index 9869e0b..29c43bb 100644 --- a/README.rst +++ b/README.rst @@ -565,6 +565,13 @@ MIT Change Log ---------- +UNRELEASED +~~~~~~~~~~ + +* B018: only report a ``map()``, ``filter()``, ``reversed()``, ``zip()`` or + ``enumerate()`` statement when the name is the builtin, not when the module + defines or imports its own function under that name, which may well do the work + 26.10.27 ~~~~~~~~ diff --git a/bugbear.py b/bugbear.py index 6664dfd..62b701b 100644 --- a/bugbear.py +++ b/bugbear.py @@ -707,16 +707,16 @@ def _b906_definition_exprs( return exprs -def _module_assigns_itertools_attr(module: ast.Module, name: str) -> bool: - """Whether the module has ``name = itertools.``, e.g. in a fallback.""" - itertools_modules: set[str] = set() +def _module_assigns_attr(module: ast.Module, source: str, name: str) -> bool: + """Whether the module has ``name = .``, e.g. in a fallback.""" + source_modules: set[str] = set() bases: set[str] = set() for node in ast.walk(module): if isinstance(node, ast.Import): - itertools_modules.update( + source_modules.update( alias.asname or alias.name for alias in node.names - if alias.name == "itertools" + if alias.name == source ) elif ( isinstance(node, ast.Assign) @@ -726,15 +726,15 @@ def _module_assigns_itertools_attr(module: ast.Module, name: str) -> bool: and any(isinstance(t, ast.Name) and t.id == name for t in node.targets) ): bases.add(node.value.value.id) - return not itertools_modules.isdisjoint(bases) + return not source_modules.isdisjoint(bases) -def _module_name_is_itertools(module: ast.Module, name: str) -> bool: - """Whether a bare ``name`` is ``itertools.`` or never bound. +def _module_name_is_from(module: ast.Module, source: str, name: str) -> bool: + """Whether a bare ``name`` is ``.`` or never bound. Whole-module ``ast.walk``; nested scopes are intentionally not modeled. """ - from_itertools = False + from_source = False other = False for node in ast.walk(module): if isinstance(node, (ast.FunctionDef, ast.AsyncFunctionDef, ast.ClassDef)): @@ -742,15 +742,15 @@ def _module_name_is_itertools(module: ast.Module, name: str) -> bool: other = True elif isinstance(node, (ast.Import, ast.ImportFrom)): for alias in node.names: - # Absolute ``from itertools import *`` binds ``name``. + # Absolute ``from import *`` binds ``name``. # Other star imports do not. ``import name.sub`` does. if alias.name == "*": if ( isinstance(node, ast.ImportFrom) and node.level == 0 - and node.module == "itertools" + and node.module == source ): - from_itertools = True + from_source = True continue bound = alias.asname or alias.name.split(".", 1)[0] if bound != name: @@ -758,10 +758,10 @@ def _module_name_is_itertools(module: ast.Module, name: str) -> bool: if ( isinstance(node, ast.ImportFrom) and node.level == 0 - and node.module == "itertools" + and node.module == source and alias.name == name ): - from_itertools = True + from_source = True else: other = True elif ( @@ -775,7 +775,7 @@ def _module_name_is_itertools(module: ast.Module, name: str) -> bool: and node.name == name ) or (isinstance(node, ast.MatchMapping) and node.rest == name): other = True - return from_itertools or not other or _module_assigns_itertools_attr(module, name) + return from_source or not other or _module_assigns_attr(module, source, name) @attr.s @@ -805,8 +805,9 @@ class BugBearVisitor(ast.NodeVisitor): _b008_imports: dict[str, str | None] = attr.ib(factory=dict, init=False) _b008_class_imports: list[dict[str, str | None]] = attr.ib(factory=list, init=False) _b008_class_globals: list[set[str]] = attr.ib(factory=list, init=False) - # Bare itertools names (groupby, batched) already resolved for this module. - _bare_itertools_names: dict[str, bool] = attr.ib(factory=dict, init=False) + # Bare names (groupby, batched, map) already resolved for this module, keyed + # by the module they are expected to come from. + _bare_names: dict[tuple[str, str], bool] = attr.ib(factory=dict, init=False) # set to "*" when inside a try/except*, for correctly printing errors in_trystar: str = attr.ib(default="") @@ -2168,17 +2169,17 @@ def _check_b031_group_usage( ) return num_usages - def _bare_name_is_itertools(self, name: str) -> bool: - """Whether a bare ``name()`` in this module is ``itertools.``.""" - if name not in self._bare_itertools_names: + def _bare_name_is_from(self, source: str, name: str) -> bool: + """Whether a bare ``name()`` in this module is ``.``.""" + if (source, name) not in self._bare_names: module = self.contexts[0].node if self.contexts else None # No module to inspect: keep the historical report. - self._bare_itertools_names[name] = ( - _module_name_is_itertools(module, name) + self._bare_names[source, name] = ( + _module_name_is_from(module, source, name) if isinstance(module, ast.Module) else True ) - return self._bare_itertools_names[name] + return self._bare_names[source, name] def check_for_b031(self, loop_node: ast.For) -> None: # noqa: C901 """Check that `itertools.groupby` isn't iterated over more than once. @@ -2193,7 +2194,7 @@ def check_for_b031(self, loop_node: ast.For) -> None: # noqa: C901 if ( isinstance(node.func, ast.Name) and node.func.id == "groupby" - and self._bare_name_is_itertools("groupby") + and self._bare_name_is_from("itertools", "groupby") ) or ( isinstance(node.func, ast.Attribute) and node.func.attr == "groupby" @@ -2528,11 +2529,14 @@ def check_for_b018(self, node: ast.AST) -> None: ) ) # `map()`, `filter()` and the like are lazy too, so the - # function or loop they were meant for never runs + # function or loop they were meant for never runs. Only the + # builtins count: a module's own `def map()` or one imported + # from elsewhere may well run it or ( isinstance(node.value, ast.Call) and isinstance(node.value.func, ast.Name) and node.value.func.id in B018_LAZY_FUNCTIONS + and self._bare_name_is_from("builtins", node.value.func.id) ) ) # directly in a `try` or `with` block, an operation is often run @@ -3244,7 +3248,7 @@ def check_for_b911(self, node: ast.Call) -> None: ( isinstance(node.func, ast.Name) and node.func.id == "batched" - and self._bare_name_is_itertools("batched") + and self._bare_name_is_from("itertools", "batched") ) or ( isinstance(node.func, ast.Attribute) diff --git a/tests/eval_files/b018_shadowed.py b/tests/eval_files/b018_shadowed.py new file mode 100644 index 0000000..ebe4460 --- /dev/null +++ b/tests/eval_files/b018_shadowed.py @@ -0,0 +1,21 @@ +""" +Should emit: +B018 - on line 21 +""" + +# The module's own map() and filter() are not the lazy builtins, so calling +# them as a statement may well do the work. +import concurrent.futures + + +def map(func, items): + for item in items: + func(item) + + +filter = concurrent.futures.ThreadPoolExecutor().map + +map(print, [1, 2]) +filter(print, [1, 2]) +# zip() is still the builtin +zip([1], [2], strict=True) # B018: 0, "Call" diff --git a/tests/eval_files/b018_shadowed_alias.py b/tests/eval_files/b018_shadowed_alias.py new file mode 100644 index 0000000..692d00f --- /dev/null +++ b/tests/eval_files/b018_shadowed_alias.py @@ -0,0 +1,11 @@ +""" +Should emit: +B018 - on line 11 +""" + +# Assigning builtins.map to map still means the lazy builtin. +import builtins + +map = builtins.map + +map(print, [1, 2]) # B018: 0, "Call" diff --git a/tests/eval_files/b018_shadowed_import.py b/tests/eval_files/b018_shadowed_import.py new file mode 100644 index 0000000..bf62b98 --- /dev/null +++ b/tests/eval_files/b018_shadowed_import.py @@ -0,0 +1,14 @@ +""" +Should emit: +B018 - on lines 13-14 +""" + +from builtins import filter + +# map() and reversed() from another library are not the lazy builtins. +from mylib import map, reversed + +map(print, [1, 2]) +reversed([1, 2]) +filter(None, [1, 2]) # B018: 0, "Call" +enumerate([1, 2]) # B018: 0, "Call" From 84c90d533c0e0da06a56ec24e30995e79bde8f2f Mon Sep 17 00:00:00 2001 From: LuisFigueroaG Date: Wed, 28 Oct 2026 16:52:11 -0300 Subject: [PATCH 2/2] Let a later module-level def or assignment override the imported name --- README.rst | 2 + bugbear.py | 131 ++++++++++++------ tests/eval_files/b018_shadowed_rebound.py | 23 +++ tests/eval_files/b031_shadowed_rebound.py | 25 ++++ .../eval_files/b911_shadowed_alias_rebound.py | 14 ++ tests/eval_files/b911_shadowed_rebound.py | 17 +++ 6 files changed, 166 insertions(+), 46 deletions(-) create mode 100644 tests/eval_files/b018_shadowed_rebound.py create mode 100644 tests/eval_files/b031_shadowed_rebound.py create mode 100644 tests/eval_files/b911_shadowed_alias_rebound.py create mode 100644 tests/eval_files/b911_shadowed_rebound.py diff --git a/README.rst b/README.rst index 29c43bb..81fbd28 100644 --- a/README.rst +++ b/README.rst @@ -571,6 +571,8 @@ UNRELEASED * B018: only report a ``map()``, ``filter()``, ``reversed()``, ``zip()`` or ``enumerate()`` statement when the name is the builtin, not when the module defines or imports its own function under that name, which may well do the work +* B031, B911: a bare ``groupby``/``batched`` imported from ``itertools`` no longer + counts as ``itertools`` when a later module-level ``def`` or assignment replaces it 26.10.27 ~~~~~~~~ diff --git a/bugbear.py b/bugbear.py index 62b701b..1e27e6c 100644 --- a/bugbear.py +++ b/bugbear.py @@ -707,26 +707,81 @@ def _b906_definition_exprs( return exprs -def _module_assigns_attr(module: ast.Module, source: str, name: str) -> bool: - """Whether the module has ``name = .``, e.g. in a fallback.""" - source_modules: set[str] = set() - bases: set[str] = set() - for node in ast.walk(module): - if isinstance(node, ast.Import): - source_modules.update( - alias.asname or alias.name +def _binds_from_source( + node: ast.AST, source: str, name: str, source_modules: set[str] +) -> bool: + """Whether ``node`` binds ``name`` to ``.``. + + That is ``from import ``, ``from import *`` or + ``name = .`` with ```` imported as a module. + """ + if isinstance(node, ast.ImportFrom): + return ( + node.level == 0 + and node.module == source + and any( + alias.name == "*" + or (alias.name == name and alias.asname in (None, name)) for alias in node.names - if alias.name == source ) - elif ( - isinstance(node, ast.Assign) - and isinstance(node.value, ast.Attribute) - and node.value.attr == name - and isinstance(node.value.value, ast.Name) - and any(isinstance(t, ast.Name) and t.id == name for t in node.targets) + ) + return ( + isinstance(node, ast.Assign) + and isinstance(node.value, ast.Attribute) + and node.value.attr == name + and isinstance(node.value.value, ast.Name) + and node.value.value.id in source_modules + and any(isinstance(t, ast.Name) and t.id == name for t in node.targets) + ) + + +def _rebinds_elsewhere(stmt: ast.stmt, name: str) -> bool: + """Whether a module-level ``stmt`` itself binds ``name`` to something else. + + ``name = name`` keeps the binding it had. + """ + if isinstance(stmt, (ast.FunctionDef, ast.AsyncFunctionDef, ast.ClassDef)): + return stmt.name == name + if isinstance(stmt, (ast.Import, ast.ImportFrom)): + return any( + (alias.asname or alias.name.split(".", 1)[0]) == name + for alias in stmt.names + ) + if isinstance(stmt, ast.Assign): + targets, value = stmt.targets, stmt.value + elif isinstance(stmt, ast.AnnAssign) and stmt.value is not None: + targets, value = [stmt.target], stmt.value + else: + return False + return not (isinstance(value, ast.Name) and value.id == name) and any( + isinstance(t, ast.Name) and t.id == name for t in targets + ) + + +def _module_binding_is_from(module: ast.Module, source: str, name: str) -> bool: + """Whether the last module-level binding of ``name`` is ``.``. + + Statements are taken in source order. A binding from ```` anywhere + in a statement counts for that statement, so a ``try``/``except`` fallback + for older Pythons still means ``.``. + """ + source_modules = { + alias.asname or alias.name + for node in ast.walk(module) + if isinstance(node, ast.Import) + for alias in node.names + if alias.name == source + } + is_from = False + for stmt in module.body: + if any( + _binds_from_source(node, source, name, source_modules) + for node in ast.walk(stmt) ): - bases.add(node.value.value.id) - return not source_modules.isdisjoint(bases) + is_from = True + elif _rebinds_elsewhere(stmt, name): + is_from = False + return is_from def _module_name_is_from(module: ast.Module, source: str, name: str) -> bool: @@ -734,48 +789,32 @@ def _module_name_is_from(module: ast.Module, source: str, name: str) -> bool: Whole-module ``ast.walk``; nested scopes are intentionally not modeled. """ - from_source = False - other = False + bound = False for node in ast.walk(module): if isinstance(node, (ast.FunctionDef, ast.AsyncFunctionDef, ast.ClassDef)): if node.name == name: - other = True + bound = True elif isinstance(node, (ast.Import, ast.ImportFrom)): - for alias in node.names: - # Absolute ``from import *`` binds ``name``. - # Other star imports do not. ``import name.sub`` does. - if alias.name == "*": - if ( - isinstance(node, ast.ImportFrom) - and node.level == 0 - and node.module == source - ): - from_source = True - continue - bound = alias.asname or alias.name.split(".", 1)[0] - if bound != name: - continue - if ( - isinstance(node, ast.ImportFrom) - and node.level == 0 - and node.module == source - and alias.name == name - ): - from_source = True - else: - other = True + # Star imports are left to ``_module_binding_is_from``: only an + # absolute ``from import *`` binds ``name`` there. + # ``import name.sub`` binds ``name``. + if any( + (alias.asname or alias.name.split(".", 1)[0]) == name + for alias in node.names + ): + bound = True elif ( isinstance(node, ast.Name) and node.id == name and isinstance(node.ctx, ast.Store) ): - other = True + bound = True elif ( isinstance(node, (ast.MatchAs, ast.MatchStar, ast.ExceptHandler)) and node.name == name ) or (isinstance(node, ast.MatchMapping) and node.rest == name): - other = True - return from_source or not other or _module_assigns_attr(module, source, name) + bound = True + return not bound or _module_binding_is_from(module, source, name) @attr.s diff --git a/tests/eval_files/b018_shadowed_rebound.py b/tests/eval_files/b018_shadowed_rebound.py new file mode 100644 index 0000000..a2ce488 --- /dev/null +++ b/tests/eval_files/b018_shadowed_rebound.py @@ -0,0 +1,23 @@ +""" +Should emit: +B018 - on line 21 +""" + +# A later definition or assignment replaces the imported builtin. +from builtins import filter, map, zip + +import mylib + + +def map(func, items): + for item in items: + func(item) + + +filter = mylib.run_filter +zip = mylib.zip +from builtins import zip + +zip([1], [2], strict=True) # B018: 0, "Call" +map(print, [1, 2]) +filter(print, [1, 2]) diff --git a/tests/eval_files/b031_shadowed_rebound.py b/tests/eval_files/b031_shadowed_rebound.py new file mode 100644 index 0000000..d551d3b --- /dev/null +++ b/tests/eval_files/b031_shadowed_rebound.py @@ -0,0 +1,25 @@ +""" +Should emit: +B031 - on line 24 +""" + +# A later definition replaces the imported itertools.groupby. +import itertools +from itertools import groupby + + +def groupby(x, y): + return dict().items() + + +for _, v in groupby(None, None): + if v: + pass + if v: + pass + +for _, v in itertools.groupby(None, None): + if v: + pass + if v: # B031: 7, "v" + pass diff --git a/tests/eval_files/b911_shadowed_alias_rebound.py b/tests/eval_files/b911_shadowed_alias_rebound.py new file mode 100644 index 0000000..068f3fe --- /dev/null +++ b/tests/eval_files/b911_shadowed_alias_rebound.py @@ -0,0 +1,14 @@ +""" +Should emit: +B911 - on line 13 +""" + +# A later assignment replaces batched = itertools.batched. +import itertools + +import mylib + +batched = itertools.batched +batched = mylib.batched +itertools.batched(range(3), 2) # B911: 0 +batched(range(3), 2) diff --git a/tests/eval_files/b911_shadowed_rebound.py b/tests/eval_files/b911_shadowed_rebound.py new file mode 100644 index 0000000..b9c3b3e --- /dev/null +++ b/tests/eval_files/b911_shadowed_rebound.py @@ -0,0 +1,17 @@ +""" +Should emit: +B911 - on line 17 +""" + +# A later definition replaces the imported itertools.batched. +import itertools +from itertools import batched + + +def batched(iterable, n): + it = iter(iterable) + return iter(lambda: tuple(itertools.islice(it, n)), ()) + + +batched(range(3), 2) +itertools.batched(range(3), 2) # B911: 0