Skip to content

Commit

Permalink
Add new check: unguarded-typing-import
Browse files Browse the repository at this point in the history
  • Loading branch information
nickdrozd committed Oct 17, 2024
1 parent 0d7b0d7 commit d78d4ee
Show file tree
Hide file tree
Showing 4 changed files with 92 additions and 11 deletions.
79 changes: 68 additions & 11 deletions pylint/checkers/variables.py
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -409,6 +410,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",
Expand Down Expand Up @@ -482,6 +491,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.
Expand All @@ -498,6 +508,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()
Expand All @@ -506,30 +517,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.
Expand Down Expand Up @@ -572,6 +599,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 = [
Expand Down Expand Up @@ -1356,7 +1386,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)
Expand All @@ -1368,7 +1399,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:
Expand Down Expand Up @@ -1672,7 +1703,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:
Expand Down Expand Up @@ -3163,7 +3198,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[
Expand Down Expand Up @@ -3251,8 +3290,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]] = []
Expand Down
21 changes: 21 additions & 0 deletions tests/functional/u/unguarded_typing_import.py
Original file line number Diff line number Diff line change
@@ -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]
2 changes: 2 additions & 0 deletions tests/functional/u/unguarded_typing_import.rc
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
[MESSAGES CONTROL]
enable = unguarded-typing-import
1 change: 1 addition & 0 deletions tests/functional/u/unguarded_typing_import.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
unguarded-typing-import:3:0:3:17::`A` used only for typechecking but imported outside of a typechecking block:UNDEFINED

0 comments on commit d78d4ee

Please sign in to comment.