Skip to content

Commit

Permalink
Log RecursionErrors out as warnings during node transformation (#2385)
Browse files Browse the repository at this point in the history 
* Trap RecursionError in `visit_attribute`

Co-authored-by: Marc Mueller <[email protected]>
  • Loading branch information
@jacobtylerwalls @cdce8p
jacobtylerwalls and cdce8p authored Feb 23, 2024
1 parent 6519a0e commit 8c7106e
Show file tree
Hide file tree
Showing 6 changed files with 237 additions and 4 deletions.
5 changes: 4 additions & 1 deletion ChangeLog
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,12 +27,15 @@ Release date: TBA
* Include modname in AST warnings. Useful for ``invalid escape sequence`` warnings
with Python 3.12.

* ``RecursionError`` is now trapped and logged out as ``UserWarning`` during astroid node transformations with instructions about raising the system recursion limit.

Closes pylint-dev/pylint#8842

* Suppress ``SyntaxWarning`` for invalid escape sequences on Python 3.12 when parsing modules.

Closes pylint-dev/pylint#9322



What's New in astroid 3.0.3?
============================
Release date: 2024-02-04
Expand Down
11 changes: 10 additions & 1 deletion astroid/nodes/as_string.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@

from __future__ import annotations

import warnings
from collections.abc import Iterator
from typing import TYPE_CHECKING

Expand Down Expand Up @@ -363,7 +364,15 @@ def visit_generatorexp(self, node) -> str:

def visit_attribute(self, node) -> str:
"""return an astroid.Getattr node as string"""
left = self._precedence_parens(node, node.expr)
try:
left = self._precedence_parens(node, node.expr)
except RecursionError:
warnings.warn(
"Recursion limit exhausted; defaulting to adding parentheses.",
UserWarning,
stacklevel=2,
)
left = f"({node.expr.accept(self)})"
if left.isdigit():
left = f"({left})"
return f"{left}.{node.attrname}"
Expand Down
14 changes: 13 additions & 1 deletion astroid/transforms.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

from __future__ import annotations

import warnings
from collections import defaultdict
from collections.abc import Callable
from typing import TYPE_CHECKING, List, Optional, Tuple, TypeVar, Union, cast, overload
Expand Down Expand Up @@ -110,7 +111,18 @@ def _visit_generic(self, node: _Vistables) -> _VisitReturns:
if not node or isinstance(node, str):
return node

return self._visit(node)
try:
return self._visit(node)
except RecursionError:
# Returning the node untransformed is better than giving up.
warnings.warn(
f"Astroid was unable to transform {node}.\n"
"Some functionality will be missing unless the system recursion limit is lifted.\n"
"From pylint, try: --init-hook='import sys; sys.setrecursionlimit(2000)' or higher.",
UserWarning,
stacklevel=0,
)
return node

def register_transform(
self,
Expand Down
17 changes: 16 additions & 1 deletion tests/test_nodes.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@
transforms,
util,
)
from astroid.const import PY310_PLUS, PY312_PLUS, Context
from astroid.const import IS_PYPY, PY310_PLUS, PY312_PLUS, Context
from astroid.context import InferenceContext
from astroid.exceptions import (
AstroidBuildingError,
Expand All @@ -47,6 +47,7 @@
Tuple,
)
from astroid.nodes.scoped_nodes import ClassDef, FunctionDef, GeneratorExp, Module
from tests.testdata.python3.recursion_error import LONG_CHAINED_METHOD_CALL

from . import resources

Expand Down Expand Up @@ -279,6 +280,20 @@ def test_as_string_unknown() -> None:
assert nodes.Unknown().as_string() == "Unknown.Unknown()"
assert nodes.Unknown(lineno=1, col_offset=0).as_string() == "Unknown.Unknown()"

@staticmethod
@pytest.mark.skipif(
IS_PYPY,
reason="Test requires manipulating the recursion limit, which cannot "
"be undone in a finally block without polluting other tests on PyPy.",
)
def test_recursion_error_trapped() -> None:
with pytest.warns(UserWarning, match="unable to transform"):
ast = abuilder.string_build(LONG_CHAINED_METHOD_CALL)

attribute = ast.body[1].value.func
with pytest.raises(UserWarning):
attribute.as_string()


@pytest.mark.skipif(not PY312_PLUS, reason="Uses 3.12 type param nodes")
class AsStringTypeParamNodes(unittest.TestCase):
Expand Down
24 changes: 24 additions & 0 deletions tests/test_transforms.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,14 +5,20 @@
from __future__ import annotations

import contextlib
import sys
import time
import unittest
from collections.abc import Callable, Iterator

import pytest

from astroid import MANAGER, builder, nodes, parse, transforms
from astroid.brain.brain_dataclasses import _looks_like_dataclass_field_call
from astroid.const import IS_PYPY
from astroid.manager import AstroidManager
from astroid.nodes.node_classes import Call, Compare, Const, Name
from astroid.nodes.scoped_nodes import FunctionDef, Module
from tests.testdata.python3.recursion_error import LONG_CHAINED_METHOD_CALL


@contextlib.contextmanager
Expand Down Expand Up @@ -258,3 +264,21 @@ def transform_class(cls):
import UserDict
"""
)

def test_transform_aborted_if_recursion_limited(self):
def transform_call(node: Call) -> Const:
return node

self.transformer.register_transform(
nodes.Call, transform_call, _looks_like_dataclass_field_call
)

original_limit = sys.getrecursionlimit()
sys.setrecursionlimit(500 if IS_PYPY else 1000)

try:
with pytest.warns(UserWarning) as records:
self.parse_transform(LONG_CHAINED_METHOD_CALL)
assert "sys.setrecursionlimit" in records[0].message.args[0]
finally:
sys.setrecursionlimit(original_limit)
170 changes: 170 additions & 0 deletions tests/testdata/python3/recursion_error.py
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,170 @@
LONG_CHAINED_METHOD_CALL = """
from a import b
(
b.builder('name')
.add('name', value='value')
.add('name', value='value')
.add('name', value='value')
.add('name', value='value')
.add('name', value='value')
.add('name', value='value')
.add('name', value='value')
.add('name', value='value')
.add('name', value='value')
.add('name', value='value')
.add('name', value='value')
.add('name', value='value')
.add('name', value='value')
.add('name', value='value')
.add('name', value='value')
.add('name', value='value')
.add('name', value='value')
.add('name', value='value')
.add('name', value='value')
.add('name', value='value')
.add('name', value='value')
.add('name', value='value')
.add('name', value='value')
.add('name', value='value')
.add('name', value='value')
.add('name', value='value')
.add('name', value='value')
.add('name', value='value')
.add('name', value='value')
.add('name', value='value')
.add('name', value='value')
.add('name', value='value')
.add('name', value='value')
.add('name', value='value')
.add('name', value='value')
.add('name', value='value')
.add('name', value='value')
.add('name', value='value')
.add('name', value='value')
.add('name', value='value')
.add('name', value='value')
.add('name', value='value')
.add('name', value='value')
.add('name', value='value')
.add('name', value='value')
.add('name', value='value')
.add('name', value='value')
.add('name', value='value')
.add('name', value='value')
.add('name', value='value')
.add('name', value='value')
.add('name', value='value')
.add('name', value='value')
.add('name', value='value')
.add('name', value='value')
.add('name', value='value')
.add('name', value='value')
.add('name', value='value')
.add('name', value='value')
.add('name', value='value')
.add('name', value='value')
.add('name', value='value')
.add('name', value='value')
.add('name', value='value')
.add('name', value='value')
.add('name', value='value')
.add('name', value='value')
.add('name', value='value')
.add('name', value='value')
.add('name', value='value')
.add('name', value='value')
.add('name', value='value')
.add('name', value='value')
.add('name', value='value')
.add('name', value='value')
.add('name', value='value')
.add('name', value='value')
.add('name', value='value')
.add('name', value='value')
.add('name', value='value')
.add('name', value='value')
.add('name', value='value')
.add('name', value='value')
.add('name', value='value')
.add('name', value='value')
.add('name', value='value')
.add('name', value='value')
.add('name', value='value')
.add('name', value='value')
.add('name', value='value')
.add('name', value='value')
.add('name', value='value')
.add('name', value='value')
.add('name', value='value')
.add('name', value='value')
.add('name', value='value')
.add('name', value='value')
.add('name', value='value')
.add('name', value='value')
.add('name', value='value')
.add('name', value='value')
.add('name', value='value')
.add('name', value='value')
.add('name', value='value')
.add('name', value='value')
.add('name', value='value')
.add('name', value='value')
.add('name', value='value')
.add('name', value='value')
.add('name', value='value')
.add('name', value='value')
.add('name', value='value')
.add('name', value='value')
.add('name', value='value')
.add('name', value='value')
.add('name', value='value')
.add('name', value='value')
.add('name', value='value')
.add('name', value='value')
.add('name', value='value')
.add('name', value='value')
.add('name', value='value')
.add('name', value='value')
.add('name', value='value')
.add('name', value='value')
.add('name', value='value')
.add('name', value='value')
.add('name', value='value')
.add('name', value='value')
.add('name', value='value')
.add('name', value='value')
.add('name', value='value')
.add('name', value='value')
.add('name', value='value')
.add('name', value='value')
.add('name', value='value')
.add('name', value='value')
.add('name', value='value')
.add('name', value='value')
.add('name', value='value')
.add('name', value='value')
.add('name', value='value')
.add('name', value='value')
.add('name', value='value')
.add('name', value='value')
.add('name', value='value')
.add('name', value='value')
.add('name', value='value')
.add('name', value='value')
.add('name', value='value')
.add('name', value='value')
.add('name', value='value')
.add('name', value='value')
.add('name', value='value')
.add('name', value='value')
.add('name', value='value')
.add('name', value='value')
.add('name', value='value')
.add('name', value='value')
.add('name', value='value')
.add('name', value='value')
.add('name', value='value')
.add('name', value='value')
.Build()
)"""

0 comments on commit 8c7106e

Please sign in to comment.