Skip to content

Commit

Permalink
Use Try node (#7767)
Browse files Browse the repository at this point in the history
Bump astroid to 3.0.0a7
  • Loading branch information
cdce8p authored Jul 10, 2023
1 parent 4798f57 commit 5bdc221
Show file tree
Hide file tree
Showing 27 changed files with 100 additions and 118 deletions.
4 changes: 4 additions & 0 deletions doc/whatsnew/fragments/7767.breaking
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
Disables placed in a ``try`` block now apply to the ``except`` block.
Previously, they only happened to do so in the presence of an ``else`` clause.

Refs #7767
5 changes: 5 additions & 0 deletions doc/whatsnew/fragments/7767.false_positive
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
Fix false negatives and false positives for ``too-many-try-statements``,
``too-complex``, and ``too-many-branches`` by correctly counting statements
under a ``try``.

Refs #7767
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 @@ -617,8 +617,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 @@ -628,11 +627,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 @@ -649,9 +644,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 @@ -675,7 +668,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

0 comments on commit 5bdc221

Please sign in to comment.