Skip to content

Commit

Permalink
Add new check: import-used-only-for-typechecking
Browse files Browse the repository at this point in the history
  • Loading branch information
nickdrozd committed Sep 24, 2024
1 parent 83cc31c commit 83e81c6
Showing 1 changed file with 76 additions and 13 deletions.
89 changes: 76 additions & 13 deletions pylint/checkers/variables.py
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,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 @@ -459,6 +460,11 @@ 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.",
),
"W0615": (
"`%s` used only for typechecking but imported outside of a typechecking block",
"import-used-only-for-typechecking",
"Used when an import is used only for typechecking but imported outside of a typechecking block.",
),
"W0621": (
"Redefining name %r from outer scope (line %s)",
"redefined-outer-name",
Expand Down Expand Up @@ -529,6 +535,7 @@ class ScopeConsumer(NamedTuple):

to_consume: dict[str, list[nodes.NodeNG]]
consumed: dict[str, list[nodes.NodeNG]]
consumed_as_type: dict[str, list[nodes.NodeNG]]
consumed_uncertain: defaultdict[str, list[nodes.NodeNG]]
scope_type: str

Expand All @@ -538,7 +545,11 @@ class NamesConsumer:

def __init__(self, node: nodes.NodeNG, scope_type: str) -> None:
self._atomic = ScopeConsumer(
copy.copy(node.locals), {}, collections.defaultdict(list), scope_type
copy.copy(node.locals),
{},
{},
collections.defaultdict(list),
scope_type,
)
self.node = node
self.names_under_always_false_test: set[str] = set()
Expand All @@ -547,15 +558,20 @@ def __init__(self, node: nodes.NodeNG, scope_type: str) -> None:
def __repr__(self) -> str:
_to_consumes = [f"{k}->{v}" for k, v in self._atomic.to_consume.items()]
_consumed = [f"{k}->{v}" for k, v in self._atomic.consumed.items()]
_consumed_as_type = [
f"{k}->{v}" for k, v in self._atomic.consumed_as_type.items()
]
_consumed_uncertain = [
f"{k}->{v}" for k, v in self._atomic.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._atomic.scope_type}
"""
Expand All @@ -571,6 +587,10 @@ def to_consume(self) -> dict[str, list[nodes.NodeNG]]:
def consumed(self) -> dict[str, list[nodes.NodeNG]]:
return self._atomic.consumed

@property
def consumed_as_type(self) -> dict[str, list[nodes.NodeNG]]:
return self._atomic.consumed_as_type

@property
def consumed_uncertain(self) -> defaultdict[str, list[nodes.NodeNG]]:
"""Retrieves nodes filtered out by get_next_to_consume() that may not
Expand All @@ -587,19 +607,32 @@ def consumed_uncertain(self) -> defaultdict[str, list[nodes.NodeNG]]:
def scope_type(self) -> str:
return self._atomic.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 @@ -642,6 +675,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 @@ -1415,7 +1451,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 @@ -1427,7 +1464,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 @@ -1729,7 +1766,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 @@ -3195,7 +3236,11 @@ def _check_globals(self, not_consumed: dict[str, nodes.NodeNG]) -> None:
self.add_message("unused-variable", args=(name,), node=node)

# pylint: disable = too-many-branches
def _check_imports(self, not_consumed: dict[str, list[nodes.NodeNG]]) -> None:
def _check_imports(
self,
not_consumed: dict[str, list[nodes.NodeNG]],
consumed_as_type: dict[str, list[nodes.NodeNG]],
) -> None:
local_names = _fix_dot_imports(not_consumed)
checked = set()
unused_wildcard_imports: defaultdict[
Expand Down Expand Up @@ -3282,8 +3327,26 @@ def _check_imports(self, not_consumed: dict[str, list[nodes.NodeNG]]) -> 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(
"import-used-only-for-typechecking",
args=name,
node=import_node,
)

def _check_metaclasses(self, node: nodes.Module | nodes.FunctionDef) -> None:
"""Update consumption analysis for metaclasses."""
consumed: list[tuple[dict[str, list[nodes.NodeNG]], str]] = []
Expand Down Expand Up @@ -3325,7 +3388,7 @@ def _check_classdef_metaclasses(
name = METACLASS_NAME_TRANSFORMS.get(name, name)
if name:
# check enclosing scopes starting from most local
for scope_locals, _, _, _ in self._to_consume[::-1]:
for scope_locals, _, _, _, _ in self._to_consume[::-1]:
found_nodes = scope_locals.get(name, [])
for found_node in found_nodes:
if found_node.lineno <= klass.lineno:
Expand Down

0 comments on commit 83e81c6

Please sign in to comment.