Skip to content

Use Try node #7767

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.

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

Merged
merged 6 commits into from
Jul 10, 2023
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
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
24 changes: 11 additions & 13 deletions pylint/checkers/base/basic_checker.py
Original file line number Diff line number Diff line change
Expand Up @@ -272,13 +272,13 @@ class BasicChecker(_BasicChecker):

def __init__(self, linter: PyLinter) -> None:
super().__init__(linter)
self._tryfinallys: list[nodes.TryFinally] | None = None
self._trys: list[nodes.Try]

def open(self) -> None:
"""Initialize visit variables and statistics."""
py_version = self.linter.config.py_version
self._py38_plus = py_version >= (3, 8)
self._tryfinallys = []
self._trys = []
self.linter.stats.reset_node_count()

@utils.only_required_for_messages(
Expand Down Expand Up @@ -478,7 +478,7 @@ def visit_expr(self, node: nodes.Expr) -> None:
# side effects), else pointless-statement
if (
isinstance(expr, (nodes.Yield, nodes.Await))
or (isinstance(node.parent, nodes.TryExcept) and node.parent.body == [node])
or (isinstance(node.parent, nodes.Try) and node.parent.body == [node])
or (isinstance(expr, nodes.Const) and expr.value is Ellipsis)
):
return
Expand Down Expand Up @@ -768,19 +768,17 @@ def visit_set(self, node: nodes.Set) -> None:
)
values.add(value)

def visit_tryfinally(self, node: nodes.TryFinally) -> None:
"""Update try...finally flag."""
assert self._tryfinallys is not None
self._tryfinallys.append(node)
def visit_try(self, node: nodes.Try) -> None:
"""Update try block flag."""
self._trys.append(node)

for final_node in node.finalbody:
for return_node in final_node.nodes_of_class(nodes.Return):
self.add_message("return-in-finally", node=return_node, confidence=HIGH)

def leave_tryfinally(self, _: nodes.TryFinally) -> None:
"""Update try...finally flag."""
assert self._tryfinallys is not None
self._tryfinallys.pop()
def leave_try(self, _: nodes.Try) -> None:
"""Update try block flag."""
self._trys.pop()

def _check_unreachable(
self,
Expand Down Expand Up @@ -816,8 +814,8 @@ def _check_not_in_finally(
If we find a parent which type is in breaker_classes before
a 'try...finally' block we skip the whole check.
"""
# if self._tryfinallys is empty, we're not an in try...finally block
if not self._tryfinallys:
# if self._trys is empty, we're not an in try block
if not self._trys:
return
# the node could be a grand-grand...-child of the 'try...finally'
_parent = node.parent
Expand Down
2 changes: 1 addition & 1 deletion pylint/checkers/base/basic_error_checker.py
Original file line number Diff line number Diff line change
Expand Up @@ -496,7 +496,7 @@ def _check_in_loop(
if isinstance(parent, (nodes.ClassDef, nodes.FunctionDef)):
break
if (
isinstance(parent, nodes.TryFinally)
isinstance(parent, nodes.Try)
and node in parent.finalbody
and isinstance(node, nodes.Continue)
and not self._py38_plus
Expand Down
9 changes: 3 additions & 6 deletions pylint/checkers/design_analysis.py
Original file line number Diff line number Diff line change
Expand Up @@ -593,19 +593,16 @@ def visit_default(self, node: nodes.NodeNG) -> None:
if node.is_statement:
self._inc_all_stmts(1)

def visit_tryexcept(self, node: nodes.TryExcept) -> None:
def visit_try(self, node: nodes.Try) -> None:
"""Increments the branches counter."""
branches = len(node.handlers)
if node.orelse:
branches += 1
if node.finalbody:
branches += 1
self._inc_branch(node, branches)
self._inc_all_stmts(branches)

def visit_tryfinally(self, node: nodes.TryFinally) -> None:
"""Increments the branches counter."""
self._inc_branch(node, 2)
self._inc_all_stmts(2)

@only_required_for_messages("too-many-boolean-expressions", "too-many-branches")
def visit_if(self, node: nodes.If) -> None:
"""Increments the branches counter and checks boolean expressions."""
Expand Down
6 changes: 3 additions & 3 deletions pylint/checkers/exceptions.py
Original file line number Diff line number Diff line change
Expand Up @@ -348,7 +348,7 @@ def _check_misplaced_bare_raise(self, node: nodes.Raise) -> None:

current = node
# Stop when a new scope is generated or when the raise
# statement is found inside a TryFinally.
# statement is found inside a Try.
ignores = (nodes.ExceptHandler, nodes.FunctionDef)
while current and not isinstance(current.parent, ignores):
current = current.parent
Expand Down Expand Up @@ -473,7 +473,7 @@ def _check_catching_non_exception(
"catching-non-exception", node=handler.type, args=(exc.name,)
)

def _check_try_except_raise(self, node: nodes.TryExcept) -> None:
def _check_try_except_raise(self, node: nodes.Try) -> None:
def gather_exceptions_from_handler(
handler: nodes.ExceptHandler,
) -> list[InferenceResult] | None:
Expand Down Expand Up @@ -556,7 +556,7 @@ def visit_compare(self, node: nodes.Compare) -> None:
"catching-non-exception",
"duplicate-except",
)
def visit_tryexcept(self, node: nodes.TryExcept) -> None:
def visit_try(self, node: nodes.Try) -> None:
"""Check for empty except."""
self._check_try_except_raise(node)
exceptions_classes: list[Any] = []
Expand Down
14 changes: 0 additions & 14 deletions pylint/checkers/format.py
Original file line number Diff line number Diff line change
Expand Up @@ -496,14 +496,6 @@ def visit_default(self, node: nodes.NodeNG) -> None:
prev_sibl = node.previous_sibling()
if prev_sibl is not None:
prev_line = prev_sibl.fromlineno
# The line on which a 'finally': occurs in a 'try/finally'
# is not directly represented in the AST. We infer it
# by taking the last line of the body and adding 1, which
# should be the line of finally:
elif (
isinstance(node.parent, nodes.TryFinally) and node in node.parent.finalbody
):
prev_line = node.parent.body[0].tolineno + 1
elif isinstance(node.parent, nodes.Module):
prev_line = 0
else:
Expand Down Expand Up @@ -534,12 +526,6 @@ def _check_multi_statement_line(self, node: nodes.NodeNG, line: int) -> None:
# in with statements.
if isinstance(node, nodes.With):
return
# For try... except... finally..., the two nodes
# appear to be on the same line due to how the AST is built.
if isinstance(node, nodes.TryExcept) and isinstance(
node.parent, nodes.TryFinally
):
return
if (
isinstance(node.parent, nodes.If)
and not node.parent.orelse
Expand Down
15 changes: 4 additions & 11 deletions pylint/checkers/imports.py
Original file line number Diff line number Diff line change
Expand Up @@ -614,8 +614,7 @@ def compute_first_non_import_node(
| nodes.IfExp
| nodes.Assign
| nodes.AssignAttr
| nodes.TryExcept
| nodes.TryFinally,
| nodes.Try,
) -> None:
# if the node does not contain an import instruction, and if it is the
# first node of the module, keep a track of it (all the import positions
Expand All @@ -625,11 +624,7 @@ def compute_first_non_import_node(
return
if not isinstance(node.parent, nodes.Module):
return
nested_allowed = [nodes.TryExcept, nodes.TryFinally]
is_nested_allowed = [
allowed for allowed in nested_allowed if isinstance(node, allowed)
]
if is_nested_allowed and any(
if isinstance(node, nodes.Try) and any(
node.nodes_of_class((nodes.Import, nodes.ImportFrom))
):
return
Expand All @@ -646,9 +641,7 @@ def compute_first_non_import_node(
return
self._first_non_import_node = node

visit_tryfinally = (
visit_tryexcept
) = (
visit_try = (
visit_assignattr
) = (
visit_assign
Expand All @@ -672,7 +665,7 @@ def visit_functiondef(
while not isinstance(root.parent, nodes.Module):
root = root.parent

if isinstance(root, (nodes.If, nodes.TryFinally, nodes.TryExcept)):
if isinstance(root, (nodes.If, nodes.Try)):
if any(root.nodes_of_class((nodes.Import, nodes.ImportFrom))):
return

Expand Down
27 changes: 11 additions & 16 deletions pylint/checkers/refactoring/refactoring_checker.py
Original file line number Diff line number Diff line change
Expand Up @@ -27,9 +27,7 @@
from pylint.lint import PyLinter


NodesWithNestedBlocks = Union[
nodes.TryExcept, nodes.TryFinally, nodes.While, nodes.For, nodes.If
]
NodesWithNestedBlocks = Union[nodes.Try, nodes.While, nodes.For, nodes.If]

KNOWN_INFINITE_ITERATORS = {"itertools.count", "itertools.cycle"}
BUILTIN_EXIT_FUNCS = frozenset(("quit", "exit"))
Expand Down Expand Up @@ -71,7 +69,7 @@ def _if_statement_is_always_returning(


def _except_statement_is_always_returning(
node: nodes.TryExcept, returning_node_class: nodes.NodeNG
node: nodes.Try, returning_node_class: nodes.NodeNG
) -> bool:
"""Detect if all except statements return."""
return all(
Expand Down Expand Up @@ -653,15 +651,13 @@ def leave_module(self, _: nodes.Module) -> None:
self._init()

@utils.only_required_for_messages("too-many-nested-blocks", "no-else-return")
def visit_tryexcept(self, node: nodes.TryExcept | nodes.TryFinally) -> None:
def visit_try(self, node: nodes.Try) -> None:
self._check_nested_blocks(node)

if isinstance(node, nodes.TryExcept):
self._check_superfluous_else_return(node)
self._check_superfluous_else_raise(node)
self._check_superfluous_else_return(node)
self._check_superfluous_else_raise(node)

visit_tryfinally = visit_tryexcept
visit_while = visit_tryexcept
visit_while = visit_try

def _check_redefined_argument_from_local(self, name_node: nodes.AssignName) -> None:
if self._dummy_rgx and self._dummy_rgx.match(name_node.name):
Expand Down Expand Up @@ -723,13 +719,11 @@ def visit_with(self, node: nodes.With) -> None:

def _check_superfluous_else(
self,
node: nodes.If | nodes.TryExcept,
node: nodes.If | nodes.Try,
msg_id: str,
returning_node_class: nodes.NodeNG,
) -> None:
if isinstance(node, nodes.TryExcept) and isinstance(
node.parent, nodes.TryFinally
):
if isinstance(node, nodes.Try) and node.finalbody:
# Not interested in try/except/else/finally statements.
return

Expand All @@ -745,7 +739,8 @@ def _check_superfluous_else(
isinstance(node, nodes.If)
and _if_statement_is_always_returning(node, returning_node_class)
) or (
isinstance(node, nodes.TryExcept)
isinstance(node, nodes.Try)
and not node.finalbody
and _except_statement_is_always_returning(node, returning_node_class)
):
orelse = node.orelse[0]
Expand Down Expand Up @@ -1962,7 +1957,7 @@ def _is_node_return_ended(self, node: nodes.NodeNG) -> bool:
return self._is_raise_node_return_ended(node)
if isinstance(node, nodes.If):
return self._is_if_node_return_ended(node)
if isinstance(node, nodes.TryExcept):
if isinstance(node, nodes.Try):
handlers = {
_child
for _child in node.get_children()
Expand Down
19 changes: 9 additions & 10 deletions pylint/checkers/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -379,8 +379,7 @@ def is_defined_before(var_node: nodes.Name) -> bool:
nodes.For,
nodes.While,
nodes.With,
nodes.TryExcept,
nodes.TryFinally,
nodes.Try,
nodes.ExceptHandler,
),
):
Expand Down Expand Up @@ -988,10 +987,10 @@ def unimplemented_abstract_methods(

def find_try_except_wrapper_node(
node: nodes.NodeNG,
) -> nodes.ExceptHandler | nodes.TryExcept | None:
"""Return the ExceptHandler or the TryExcept node in which the node is."""
) -> nodes.ExceptHandler | nodes.Try | None:
"""Return the ExceptHandler or the Try node in which the node is."""
current = node
ignores = (nodes.ExceptHandler, nodes.TryExcept)
ignores = (nodes.ExceptHandler, nodes.Try)
while current and not isinstance(current.parent, ignores):
current = current.parent

Expand All @@ -1002,7 +1001,7 @@ def find_try_except_wrapper_node(

def find_except_wrapper_node_in_scope(
node: nodes.NodeNG,
) -> nodes.ExceptHandler | nodes.TryExcept | None:
) -> nodes.ExceptHandler | None:
"""Return the ExceptHandler in which the node is, without going out of scope."""
for current in node.node_ancestors():
if isinstance(current, astroid.scoped_nodes.LocalsDictNodeNG):
Expand Down Expand Up @@ -1062,7 +1061,7 @@ def get_exception_handlers(
list: the collection of handlers that are handling the exception or None.
"""
context = find_try_except_wrapper_node(node)
if isinstance(context, nodes.TryExcept):
if isinstance(context, nodes.Try):
return [
handler for handler in context.handlers if error_of_type(handler, exception)
]
Expand Down Expand Up @@ -1133,13 +1132,13 @@ def is_node_inside_try_except(node: nodes.Raise) -> bool:
bool: True if the node is inside a try/except statement, False otherwise.
"""
context = find_try_except_wrapper_node(node)
return isinstance(context, nodes.TryExcept)
return isinstance(context, nodes.Try)


def node_ignores_exception(
node: nodes.NodeNG, exception: type[Exception] | str = Exception
) -> bool:
"""Check if the node is in a TryExcept which handles the given exception.
"""Check if the node is in a Try which handles the given exception.

If the exception is not given, the function is going to look for bare
excepts.
Expand Down Expand Up @@ -1918,7 +1917,7 @@ def get_node_first_ancestor_of_type_and_its_child(
descendant visited directly before reaching the sought ancestor.

Useful for extracting whether a statement is guarded by a try, except, or finally
when searching for a TryFinally ancestor.
when searching for a Try ancestor.
"""
child = node
for ancestor in node.node_ancestors():
Expand Down
Loading