From ea1649f806a7a3b3bb09a92b49df78ce4cf479fa Mon Sep 17 00:00:00 2001 From: Nick Drozd Date: Tue, 24 Sep 2024 16:37:21 -0400 Subject: [PATCH] Add new check: unguarded-typing-import --- pylint/checkers/variables.py | 79 ++++++++++++++++--- tests/functional/u/unguarded_typing_import.py | 21 +++++ tests/functional/u/unguarded_typing_import.rc | 2 + .../functional/u/unguarded_typing_import.txt | 1 + 4 files changed, 92 insertions(+), 11 deletions(-) create mode 100644 tests/functional/u/unguarded_typing_import.py create mode 100644 tests/functional/u/unguarded_typing_import.rc create mode 100644 tests/functional/u/unguarded_typing_import.txt diff --git a/pylint/checkers/variables.py b/pylint/checkers/variables.py index 9064bd120c..cdbc88cb09 100644 --- a/pylint/checkers/variables.py +++ b/pylint/checkers/variables.py @@ -24,6 +24,7 @@ from pylint.checkers.utils import ( in_type_checking_block, is_module_ignored, + is_node_in_type_annotation_context, is_postponed_evaluation_enabled, is_sys_guard, overridden_method, @@ -434,6 +435,14 @@ def _has_locals_call_after_node(stmt: nodes.NodeNG, scope: nodes.FunctionDef) -> "Used when an imported module or variable is not used from a " "`'from X import *'` style import.", ), + "R0615": ( + "`%s` used only for typechecking but imported outside of a typechecking block", + "unguarded-typing-import", + "Used when an import is used only for typechecking but imported outside of a typechecking block.", + { + "default_enabled": False, + }, + ), "W0621": ( "Redefining name %r from outer scope (line %s)", "redefined-outer-name", @@ -507,6 +516,7 @@ class NamesConsumer: to_consume: Consumption consumed: Consumption + consumed_as_type: Consumption consumed_uncertain: Consumption """Retrieves nodes filtered out by get_next_to_consume() that may not have executed. @@ -523,6 +533,7 @@ def __init__(self, node: nodes.NodeNG, scope_type: str): self.to_consume = copy.copy(node.locals) self.consumed = {} + self.consumed_as_type = {} self.consumed_uncertain = defaultdict(list) self.names_under_always_false_test: set[str] = set() @@ -531,30 +542,46 @@ def __init__(self, node: nodes.NodeNG, scope_type: str): def __repr__(self) -> str: _to_consumes = [f"{k}->{v}" for k, v in self.to_consume.items()] _consumed = [f"{k}->{v}" for k, v in self.consumed.items()] + _consumed_as_type = [f"{k}->{v}" for k, v in self.consumed_as_type.items()] _consumed_uncertain = [f"{k}->{v}" for k, v in self.consumed_uncertain.items()] to_consumes = ", ".join(_to_consumes) consumed = ", ".join(_consumed) + consumed_as_type = ", ".join(_consumed_as_type) consumed_uncertain = ", ".join(_consumed_uncertain) return f""" to_consume : {to_consumes} consumed : {consumed} +consumed_as_type : {consumed_as_type} consumed_uncertain: {consumed_uncertain} scope_type : {self.scope_type} """ - def mark_as_consumed(self, name: str, consumed_nodes: list[nodes.NodeNG]) -> None: + def mark_as_consumed( + self, + name: str, + consumed_nodes: list[nodes.NodeNG], + consumed_as_type: bool = False, + ) -> None: """Mark the given nodes as consumed for the name. If all of the nodes for the name were consumed, delete the name from the to_consume dictionary """ - unconsumed = [n for n in self.to_consume[name] if n not in set(consumed_nodes)] - self.consumed[name] = consumed_nodes + consumed = self.consumed_as_type if consumed_as_type else self.consumed + consumed[name] = consumed_nodes - if unconsumed: - self.to_consume[name] = unconsumed - else: - del self.to_consume[name] + if name in self.to_consume: + unconsumed = [ + n for n in self.to_consume[name] if n not in set(consumed_nodes) + ] + + if unconsumed: + self.to_consume[name] = unconsumed + else: + del self.to_consume[name] + + if not consumed_as_type and name in self.consumed_as_type: + del self.consumed_as_type[name] def get_next_to_consume(self, node: nodes.Name) -> list[nodes.NodeNG] | None: """Return a list of the nodes that define `node` from this scope. @@ -594,6 +621,9 @@ def get_next_to_consume(self, node: nodes.Name) -> list[nodes.NodeNG] | None: if VariablesChecker._comprehension_between_frame_and_node(node): return found_nodes + if found_nodes is None: + found_nodes = self.consumed_as_type.get(name) + # Filter out assignments in ExceptHandlers that node is not contained in if found_nodes: found_nodes = [ @@ -1384,7 +1414,8 @@ def leave_module(self, node: nodes.Module) -> None: assert len(self._to_consume) == 1 self._check_metaclasses(node) - not_consumed = self._to_consume.pop().to_consume + consumer = self._to_consume.pop() + not_consumed = consumer.to_consume # attempt to check for __all__ if defined if "__all__" in node.locals: self._check_all(node, not_consumed) @@ -1396,7 +1427,7 @@ def leave_module(self, node: nodes.Module) -> None: if not self.linter.config.init_import and node.package: return - self._check_imports(not_consumed) + self._check_imports(not_consumed, consumer.consumed_as_type) self._type_annotation_names = [] def visit_classdef(self, node: nodes.ClassDef) -> None: @@ -1700,7 +1731,11 @@ def _undefined_and_used_before_checker( # They will have already had a chance to emit used-before-assignment. # We check here instead of before every single return in _check_consumer() nodes_to_consume += current_consumer.consumed_uncertain[node.name] - current_consumer.mark_as_consumed(node.name, nodes_to_consume) + current_consumer.mark_as_consumed( + node.name, + nodes_to_consume, + consumed_as_type=is_node_in_type_annotation_context(node), + ) if action is VariableVisitConsumerAction.CONTINUE: continue if action is VariableVisitConsumerAction.RETURN: @@ -3218,7 +3253,11 @@ def _check_globals(self, not_consumed: Consumption) -> None: self.add_message("unused-variable", args=(name,), node=node) # pylint: disable = too-many-branches - def _check_imports(self, not_consumed: Consumption) -> None: + def _check_imports( + self, + not_consumed: Consumption, + consumed_as_type: Consumption, + ) -> None: local_names = _fix_dot_imports(not_consumed) checked = set() unused_wildcard_imports: defaultdict[ @@ -3306,8 +3345,26 @@ def _check_imports(self, not_consumed: Consumption) -> None: self.add_message( "unused-wildcard-import", args=(arg_string, module[0]), node=module[1] ) + + self._check_type_imports(consumed_as_type) + del self._to_consume + def _check_type_imports( + self, + consumed_as_type: dict[str, list[nodes.NodeNG]], + ) -> None: + for name, import_node in _fix_dot_imports(consumed_as_type): + if import_node.names[0][0] == "*": + continue + + if not in_type_checking_block(import_node): + self.add_message( + "unguarded-typing-import", + args=name, + node=import_node, + ) + def _check_metaclasses(self, node: nodes.Module | nodes.FunctionDef) -> None: """Update consumption analysis for metaclasses.""" consumed: list[tuple[Consumption, str]] = [] diff --git a/tests/functional/u/unguarded_typing_import.py b/tests/functional/u/unguarded_typing_import.py new file mode 100644 index 0000000000..780352bf6a --- /dev/null +++ b/tests/functional/u/unguarded_typing_import.py @@ -0,0 +1,21 @@ +# pylint: disable = import-error, missing-module-docstring, missing-function-docstring, missing-class-docstring, too-few-public-methods, + +from mod import A # [unguarded-typing-import] +from mod import B + +def f(_: A): + pass + +def g(x: B): + assert isinstance(x, B) + +class C: + pass + +class D: + c: C + + def h(self): + # --> BUG <-- + # pylint: disable = undefined-variable + return [C() for _ in self.c] diff --git a/tests/functional/u/unguarded_typing_import.rc b/tests/functional/u/unguarded_typing_import.rc new file mode 100644 index 0000000000..a90770c614 --- /dev/null +++ b/tests/functional/u/unguarded_typing_import.rc @@ -0,0 +1,2 @@ +[MESSAGES CONTROL] +enable = unguarded-typing-import diff --git a/tests/functional/u/unguarded_typing_import.txt b/tests/functional/u/unguarded_typing_import.txt new file mode 100644 index 0000000000..8fd2f1cc00 --- /dev/null +++ b/tests/functional/u/unguarded_typing_import.txt @@ -0,0 +1 @@ +unguarded-typing-import:3:0:3:17::`A` used only for typechecking but imported outside of a typechecking block:UNDEFINED