diff --git a/README.rst b/README.rst index 0a16825..c288f18 100644 --- a/README.rst +++ b/README.rst @@ -500,6 +500,8 @@ Change Log UNRELEASED ~~~~~~~~~~ +* B008: resolve direct module-level imports and aliases when matching + ``extend-immutable-calls`` (#252) * B019: also flag `async_lru.alru_cache` and check cache decorators on `async def` methods (#488) * B023: don't flag a function whose every reference is a direct call inside the loop body: such a function cannot outlive the iteration it was defined in (#468, #380) diff --git a/bugbear.py b/bugbear.py index d445418..2bd874a 100644 --- a/bugbear.py +++ b/bugbear.py @@ -448,6 +448,8 @@ class BugBearVisitor(ast.NodeVisitor): _b023_seen: set[ast.Name] = attr.ib(factory=set, init=False) _b023_scopes: dict[int, tuple] = attr.ib(factory=dict, init=False) _b005_imports: set[str] = attr.ib(factory=set, init=False) + # None marks an imported name that has since been rebound at module scope. + _b008_imports: dict[str, str | None] = attr.ib(factory=dict, init=False) # set to "*" when inside a try/except*, for correctly printing errors in_trystar: str = attr.ib(default="") @@ -470,6 +472,23 @@ def node_stack(self) -> list[ast.AST]: context, stack = self.contexts[-1] return stack + def _b008_shadow_imports(self, names: Iterable[str]) -> None: + for name in names: + if self._b008_imports.get(name) is not None: + self._b008_imports[name] = None + + def _b008_in_module_scope(self) -> bool: + return len(self.contexts) == 1 and isinstance(self.contexts[0].node, ast.Module) + + def _b008_is_direct_module_statement(self) -> bool: + return self._b008_in_module_scope() and len(self.node_stack) == 2 + + def _b008_in_module_child(self) -> bool: + return len(self.contexts) == 2 and isinstance(self.contexts[0].node, ast.Module) + + def _b008_in_direct_module_child(self) -> bool: + return self._b008_in_module_child() and len(self.contexts[0].stack) == 1 + def in_class_init(self) -> bool: return ( len(self.contexts) >= 2 @@ -540,6 +559,8 @@ def visit_ExceptHandler(self, node: ast.ExceptHandler) -> None: ): self.add_error("B040", node) self.b040_caught_exception = old_b040_caught_exception + if self._b008_in_module_scope() and node.name is not None: + self._b008_shadow_imports((node.name,)) def visit_UAdd(self, node: ast.UAdd) -> None: trailing_nodes = list(map(type, self.node_window[-4:])) @@ -616,6 +637,12 @@ def visit_Call(self, node: ast.Call) -> None: def visit_Module(self, node: ast.Module) -> None: self.generic_visit(node) + def visit_Name( # noqa: B906 # names don't contain other names + self, node: ast.Name + ) -> None: + if self._b008_in_module_scope() and isinstance(node.ctx, (ast.Store, ast.Del)): + self._b008_shadow_imports((node.id,)) + def visit_Assign(self, node: ast.Assign) -> None: self.check_for_b040_usage(node.value) if len(node.targets) == 1: @@ -668,6 +695,8 @@ def visit_AsyncFunctionDef(self, node: ast.AsyncFunctionDef) -> None: self.check_for_b006_and_b008(node) self.check_for_b019(node) self.generic_visit(node) + if self._b008_in_module_child(): + self._b008_shadow_imports((node.name,)) def visit_FunctionDef(self, node: ast.FunctionDef) -> None: self.check_for_b901(node) @@ -677,6 +706,8 @@ def visit_FunctionDef(self, node: ast.FunctionDef) -> None: self.check_for_b021(node) self.check_for_b906(node) self.generic_visit(node) + if self._b008_in_module_child(): + self._b008_shadow_imports((node.name,)) def visit_ClassDef(self, node: ast.ClassDef) -> None: self.check_for_b903(node) @@ -684,6 +715,8 @@ def visit_ClassDef(self, node: ast.ClassDef) -> None: self.check_for_b024_and_b027(node) self.check_for_b042(node) self.generic_visit(node) + if self._b008_in_module_child(): + self._b008_shadow_imports((node.name,)) def visit_Try(self, node: ast.Try | ast.TryStar) -> None: self.check_for_b012(node) @@ -716,6 +749,21 @@ def visit_With(self, node: ast.With) -> None: self.check_for_b908(node) self.generic_visit(node) + def visit_MatchAs(self, node: ast.MatchAs) -> None: + if self._b008_in_module_scope() and node.name is not None: + self._b008_shadow_imports((node.name,)) + self.generic_visit(node) + + def visit_MatchMapping(self, node: ast.MatchMapping) -> None: + if self._b008_in_module_scope() and node.rest is not None: + self._b008_shadow_imports((node.rest,)) + self.generic_visit(node) + + def visit_MatchStar(self, node: ast.MatchStar) -> None: + if self._b008_in_module_scope() and node.name is not None: + self._b008_shadow_imports((node.name,)) + self.generic_visit(node) + def visit_JoinedStr(self, node: ast.JoinedStr) -> None: self.check_for_b907(node) self.generic_visit(node) @@ -727,10 +775,32 @@ def visit_AnnAssign(self, node: ast.AnnAssign) -> None: def visit_Import(self, node: ast.Import) -> None: self.check_for_b005(node) + if self.b008_b039_extend_immutable_calls and self._b008_in_module_scope(): + for name in node.names: + bound_name = name.asname or name.name.partition(".")[0] + qualified_name = name.name if name.asname else bound_name + if self._b008_is_direct_module_statement(): + self._b008_imports[bound_name] = qualified_name + else: + self._b008_shadow_imports((bound_name,)) self.generic_visit(node) def visit_ImportFrom(self, node: ast.ImportFrom) -> None: self.check_for_b005(node) + if self.b008_b039_extend_immutable_calls and self._b008_in_module_scope(): + for name in node.names: + if name.name == "*": + self._b008_shadow_imports(self._b008_imports) + elif ( + self._b008_is_direct_module_statement() + and node.level == 0 + and node.module is not None + ): + self._b008_imports[name.asname or name.name] = ( + f"{node.module}.{name.name}" + ) + else: + self._b008_shadow_imports((name.asname or name.name,)) self.generic_visit(node) def visit_Set(self, node: ast.Set) -> None: @@ -803,10 +873,17 @@ def check_for_b005(self, node: ast.Import | ast.ImportFrom | ast.Call) -> None: def check_for_b006_and_b008( self, node: ast.FunctionDef | ast.AsyncFunctionDef ) -> None: + imported_names = None + if ( + self.b008_b039_extend_immutable_calls + and self._b008_in_direct_module_child() + ): + imported_names = self._b008_imports visitor = FunctionDefDefaultsVisitor( error_codes["B006"], error_codes["B008"], self.b008_b039_extend_immutable_calls, + imported_names, ) visitor.visit(node.args.defaults + node.args.kw_defaults) self.errors.extend(visitor.errors) @@ -2391,12 +2468,14 @@ def __init__( error_code_calls: "Error", # B006 or B039 error_code_literals: "Error", # B008 or B039 b008_b039_extend_immutable_calls: set[str] | None = None, + imported_names: dict[str, str | None] | None = None, ) -> None: self.b008_b039_extend_immutable_calls = ( b008_b039_extend_immutable_calls or set() ) self.error_code_calls = error_code_calls self.error_code_literals = error_code_literals + self.imported_names = imported_names or {} for node in B006_MUTABLE_LITERALS + B006_MUTABLE_COMPREHENSIONS: setattr(self, f"visit_{node}", self.visit_mutable_literal_or_comprehension) self.errors: list[error] = [] @@ -2426,7 +2505,22 @@ def visit_Call(self, node: ast.Call) -> None: self.generic_visit(node) return - if call_path in B008_IMMUTABLE_CALLS | self.b008_b039_extend_immutable_calls: + if call_path in B008_IMMUTABLE_CALLS: + self.generic_visit(node) + return + + head, separator, tail = call_path.partition(".") + if head not in self.imported_names: + extended_call_paths = {call_path} + elif (qualified_name := self.imported_names[head]) is None: + extended_call_paths = set() + else: + resolved_call_path = qualified_name + if separator: + resolved_call_path = f"{resolved_call_path}.{tail}" + extended_call_paths = {call_path, resolved_call_path} + + if extended_call_paths & self.b008_b039_extend_immutable_calls: self.generic_visit(node) return diff --git a/tests/eval_files/b008_extended.py b/tests/eval_files/b008_extended.py index 4f2c179..2a1d710 100644 --- a/tests/eval_files/b008_extended.py +++ b/tests/eval_files/b008_extended.py @@ -2,7 +2,15 @@ from typing import List import fastapi +import fastapi as fastapi_alias +from fastapi import Depends +from fastapi import Depends as DependsAlias +from fastapi import Depends as loop_depends from fastapi import Query +from other import Depends as OtherDepends + +if condition: + from fastapi import Depends as conditional_depends def this_is_okay_extended(db=fastapi.Depends(get_db)): ... @@ -11,5 +19,37 @@ def this_is_okay_extended(db=fastapi.Depends(get_db)): ... def this_is_okay_extended_second(data: List[str] = fastapi.Query(None)): ... -# not okay, relative import not listed -def not_okay(data: List[str] = Query(None)): ... # B008: 31 +def this_is_okay_imported(db=Depends(get_db)): ... + + +def this_is_okay_imported_alias(db=DependsAlias(get_db)): ... + + +def this_is_okay_module_alias(db=fastapi_alias.Depends(get_db)): ... + + +def not_okay_other_import(db=OtherDepends(get_db)): ... # B008: 29 + + +def Depends(): ... + + +def not_okay_redefined(db=Depends(get_db)): ... # B008: 26 + + +Query = lambda value: value + + +def not_okay_reassigned(data: List[str] = Query(None)): ... # B008: 42 + + +def not_okay_conditional_import( + db=conditional_depends(get_db), # B008: 7 +): ... + + +for loop_depends in providers: + _ = loop_depends + + +def not_okay_loop_rebound(db=loop_depends(get_db)): ... # B008: 29 diff --git a/tests/eval_files/b008_extended_shadowing.py b/tests/eval_files/b008_extended_shadowing.py new file mode 100644 index 0000000..d235ce9 --- /dev/null +++ b/tests/eval_files/b008_extended_shadowing.py @@ -0,0 +1,11 @@ +# OPTIONS: extend_immutable_calls=["Depends"] +from fastapi import Depends + + +def this_is_okay_imported(db=Depends(get_db)): ... + + +def Depends(): ... + + +def not_okay_redefined(db=Depends(get_db)): ... # B008: 26