Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Return directly from incompatible branches #9997

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
+91 −77
Open

Return directly from incompatible branches #9997

Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
b41b190
Return directly from incompatible branches in _defines_name_raises_or…
nickdrozd Oct 1, 2024
06f6213
Add Scope type
nickdrozd Oct 1, 2024
8bdfaaa
Return directly from incompatible branches in _should_node_be_skipped
nickdrozd Oct 1, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
168 changes: 91 additions & 77 deletions pylint/checkers/variables.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@

if TYPE_CHECKING:
from collections.abc import Generator, Iterable, Iterator
from typing import Literal

from astroid.nodes import _base_nodes
from astroid.typing import InferenceResult
Expand All @@ -42,6 +43,8 @@

Consumption = dict[str, list[nodes.NodeNG]]

Scope = Literal["class", "comprehension", "function", "lambda", "module"]


SPECIAL_OBJ = re.compile("^_{2}[a-z]+_{2}$")
FUTURE = "__future__"
Expand Down Expand Up @@ -478,7 +481,7 @@
"""A simple class to handle consumed, to consume and scope type info of node locals."""

node: nodes.NodeNG
scope_type: str
scope_type: Scope

to_consume: Consumption
consumed: Consumption
Expand All @@ -492,7 +495,7 @@
(e.g. for unused-variable) may need to add them back.
"""

def __init__(self, node: nodes.NodeNG, scope_type: str):
def __init__(self, node: nodes.NodeNG, scope_type: Scope):
self.node = node
self.scope_type = scope_type

Expand Down Expand Up @@ -872,77 +875,91 @@
def _defines_name_raises_or_returns(name: str, node: nodes.NodeNG) -> bool:
if isinstance(node, (nodes.Raise, nodes.Assert, nodes.Return, nodes.Continue)):
return True
if isinstance(node, nodes.Expr) and isinstance(node.value, nodes.Call):
if utils.is_terminating_func(node.value):
return True
if (
isinstance(node.value.func, nodes.Name)
and node.value.func.name == "assert_never"
):
return True
if (
isinstance(node, nodes.AnnAssign)
and node.value
and isinstance(node.target, nodes.AssignName)
and node.target.name == name
):
return True

if isinstance(node, nodes.Expr):
return isinstance(node.value, nodes.Call) and (
(
isinstance(node.value.func, nodes.Name)
and node.value.func.name == "assert_never"
)
or utils.is_terminating_func(node.value)
)

if isinstance(node, nodes.AnnAssign):
return (
node.value
and isinstance(node.target, nodes.AssignName)
and node.target.name == name
)

if isinstance(node, nodes.Assign):
for target in node.targets:
for elt in utils.get_all_elements(target):
if isinstance(elt, nodes.Starred):
elt = elt.value
if isinstance(elt, nodes.AssignName) and elt.name == name:
return True
return any(
isinstance(
elt_or_val := elt.value if isinstance(elt, nodes.Starred) else elt,
nodes.AssignName,
)
and elt_or_val.name == name
for target in node.targets
for elt in utils.get_all_elements(target)
)

if isinstance(node, nodes.If):
if any(
return any(
child_named_expr.target.name == name
for child_named_expr in node.nodes_of_class(nodes.NamedExpr)
):
return True
if isinstance(node, (nodes.Import, nodes.ImportFrom)) and any(
(node_name[1] and node_name[1] == name) or (node_name[0] == name)
for node_name in node.names
):
return True
if isinstance(node, nodes.With) and any(
isinstance(item[1], nodes.AssignName) and item[1].name == name
for item in node.items
):
return True
if isinstance(node, (nodes.ClassDef, nodes.FunctionDef)) and node.name == name:
return True
if (
isinstance(node, nodes.ExceptHandler)
and node.name
and node.name.name == name
):
return True
)

if isinstance(node, (nodes.Import, nodes.ImportFrom)):
return any(
(alias and alias == name) or (node_name == name)
for node_name, alias in node.names
)

if isinstance(node, nodes.With):
return any(
isinstance(item[1], nodes.AssignName) and item[1].name == name
for item in node.items
)

if isinstance(node, (nodes.ClassDef, nodes.FunctionDef)):
assert isinstance(node.name, str)
return node.name == name

if isinstance(node, nodes.ExceptHandler):
return (
node.name
and isinstance(node_name := node.name.name, str)
and node_name == name
)

return False

@staticmethod
def _defines_name_raises_or_returns_recursive(
name: str,
node: nodes.NodeNG,
) -> bool:
"""Return True if some child of `node` defines the name `name`,
"""Return whether some child of `node` defines the name `name`,
raises, or returns.
"""
for stmt in node.get_children():
if NamesConsumer._defines_name_raises_or_returns(name, stmt):
return True

if isinstance(stmt, (nodes.If, nodes.With)):
if any(
return any(
NamesConsumer._defines_name_raises_or_returns(name, nested_stmt)
for nested_stmt in stmt.get_children()
):
return True
if (
isinstance(stmt, nodes.Try)
and not stmt.finalbody
and NamesConsumer._defines_name_raises_or_returns_recursive(name, stmt)
):
return True
)

if isinstance(stmt, nodes.Try):
return (

Check warning on line 956 in pylint/checkers/variables.py

View check run for this annotation

Codecov / codecov/patch

pylint/checkers/variables.py#L956 

Added line #L956 was not covered by tests
not stmt.finalbody
and NamesConsumer._defines_name_raises_or_returns_recursive(
name, stmt
)
)

return False

@staticmethod
Expand Down Expand Up @@ -1709,32 +1726,29 @@
# scope, ignore it. This prevents to access this scope instead of
# the globals one in function members when there are some common
# names.
if utils.is_ancestor_name(consumer.node, node) or (
not is_start_index and self._ignore_class_scope(node)
):
return True

# Ignore inner class scope for keywords in class definition
if isinstance(node.parent, nodes.Keyword) and isinstance(
node.parent.parent, nodes.ClassDef
):
return True

elif consumer.scope_type == "function" and self._defined_in_function_definition(
node, consumer.node
):
if any(node.name == param.name.name for param in consumer.node.type_params):
return False
return (
utils.is_ancestor_name(consumer.node, node)
or (not is_start_index and self._ignore_class_scope(node))
# Ignore inner class scope for keywords in class definition
or (
isinstance(node.parent, nodes.Keyword)
and isinstance(node.parent.parent, nodes.ClassDef)
)
)

if consumer.scope_type == "function":
# If the name node is used as a function default argument's value or as
# a decorator, then start from the parent frame of the function instead
# of the function frame - and thus open an inner class scope
return True
return self._defined_in_function_definition(
node,
consumer.node,
) and not any(
node.name == param.name.name for param in consumer.node.type_params
)

elif consumer.scope_type == "lambda" and utils.is_default_argument(
node, consumer.node
):
return True
if consumer.scope_type == "lambda":
return utils.is_default_argument(node, consumer.node)

return False

Expand All @@ -1745,7 +1759,7 @@
stmt: nodes.NodeNG,
frame: nodes.LocalsDictNodeNG,
current_consumer: NamesConsumer,
base_scope_type: str,
base_scope_type: Scope,
) -> tuple[VariableVisitConsumerAction, list[nodes.NodeNG] | None]:
"""Checks a consumer for conditions that should trigger messages."""
# If the name has already been consumed, only check it's not a loop
Expand Down Expand Up @@ -2176,7 +2190,7 @@
defstmt: _base_nodes.Statement,
frame: nodes.LocalsDictNodeNG, # scope of statement of node
defframe: nodes.LocalsDictNodeNG,
base_scope_type: str,
base_scope_type: Scope,
is_recursive_klass: bool,
) -> tuple[bool, bool, bool]:
maybe_before_assign = True
Expand Down
Loading