Skip to content

Fix crash on Super.getattr for previously uninferable attributes #1370

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 4 commits into from
Jan 26, 2022
Merged
Show file tree
Hide file tree
Changes from 2 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
7 changes: 7 additions & 0 deletions ChangeLog
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,13 @@ Release date: TBA
* Fixed builtin inferenence on `property` calls not calling the `postinit` of the new node, which
resulted in instance arguments missing on these nodes.

* Fixed a crash on ``Super.getattr`` when the attribute was previously uninferable due to a cache
limit size.
This limit can be hit when the inheritance pattern of a class (and therefore of the ``__init__``
attribute) is very large.

Closes PyCQA/pylint#5679

What's New in astroid 2.9.4?
============================
Release date: TBA
Expand Down
4 changes: 3 additions & 1 deletion astroid/nodes/node_ng.py
Original file line number Diff line number Diff line change
Expand Up @@ -163,7 +163,9 @@ def infer(self, context=None, **kwargs):
limit = AstroidManager().max_inferable_values
for i, result in enumerate(generator):
if i >= limit or (context.nodes_inferred > context.max_inferred):
yield util.Uninferable
uninferable = util.Uninferable
results.append(uninferable)
yield uninferable
break
results.append(result)
yield result
Expand Down
5 changes: 3 additions & 2 deletions tests/resources.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,14 +13,15 @@

import os
import sys
from pathlib import Path
from typing import Optional

from astroid import builder
from astroid.manager import AstroidManager
from astroid.nodes.scoped_nodes import Module

DATA_DIR = os.path.join("testdata", "python3")
RESOURCE_PATH = os.path.join(os.path.dirname(__file__), DATA_DIR, "data")
DATA_DIR = Path("testdata") / "python3"
RESOURCE_PATH = Path(__file__).parent / DATA_DIR / "data"


def find(name: str) -> str:
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
"""This example is based on sqlalchemy.

See https://github.com/PyCQA/pylint/issues/5679
"""
from other_funcs import FromClause

from .nodes import roles


class HasMemoized(object):
...


class Generative(HasMemoized):
...


class ColumnElement(
roles.ColumnArgumentOrKeyRole,
roles.BinaryElementRole,
roles.OrderByRole,
roles.ColumnsClauseRole,
roles.LimitOffsetRole,
roles.DMLColumnRole,
roles.DDLConstraintColumnRole,
roles.StatementRole,
Generative,
):
...


class FunctionElement(ColumnElement, FromClause):
...


class months_between(FunctionElement):
def __init__(self):
super().__init__()
Original file line number Diff line number Diff line change
@@ -0,0 +1,82 @@
class SQLRole(object):
...


class UsesInspection(object):
...


class AllowsLambdaRole(object):
...


class ColumnArgumentRole(SQLRole):
...


class ColumnArgumentOrKeyRole(ColumnArgumentRole):
...


class ColumnListRole(SQLRole):
...


class ColumnsClauseRole(AllowsLambdaRole, UsesInspection, ColumnListRole):
...


class LimitOffsetRole(SQLRole):
...


class ByOfRole(ColumnListRole):
...


class OrderByRole(AllowsLambdaRole, ByOfRole):
...


class StructuralRole(SQLRole):
...


class ExpressionElementRole(SQLRole):
...


class BinaryElementRole(ExpressionElementRole):
...


class JoinTargetRole(AllowsLambdaRole, UsesInspection, StructuralRole):
...


class FromClauseRole(ColumnsClauseRole, JoinTargetRole):
...


class StrictFromClauseRole(FromClauseRole):
...


class AnonymizedFromClauseRole(StrictFromClauseRole):
...


class ReturnsRowsRole(SQLRole):
...


class StatementRole(SQLRole):
...


class DMLColumnRole(SQLRole):
...


class DDLConstraintColumnRole(SQLRole):
...
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
from operator import attrgetter

from .nodes import roles


class HasCacheKey(object):
...


class HasMemoized(object):
...


class MemoizedHasCacheKey(HasCacheKey, HasMemoized):
...


class ClauseElement(MemoizedHasCacheKey):
...


class ReturnsRows(roles.ReturnsRowsRole, ClauseElement):
...


class Selectable(ReturnsRows):
...


class FromClause(roles.AnonymizedFromClauseRole, Selectable):
c = property(attrgetter("columns"))
32 changes: 31 additions & 1 deletion tests/unittest_regrtest.py
Original file line number Diff line number Diff line change
Expand Up @@ -24,11 +24,13 @@

import pytest

from astroid import MANAGER, Instance, nodes, parse, test_utils
from astroid import MANAGER, Instance, bases, nodes, parse, test_utils
from astroid.builder import AstroidBuilder, extract_node
from astroid.const import PY38_PLUS
from astroid.context import InferenceContext
from astroid.exceptions import InferenceError
from astroid.raw_building import build_module
from astroid.util import Uninferable

from . import resources

Expand Down Expand Up @@ -399,5 +401,33 @@ class Another(subclass):
parse(code)


def test_max_inferred_for_complicated_class_hierarchy() -> None:
"""Regression test for a crash reported in https://github.com/PyCQA/pylint/issues/5679.

The class hierarchy of 'sqlalchemy' is so intricate that it becomes uninferable with
the standard max_inferred of 100. We used to crash when this happened.
"""
# Create module and get relevant nodes
module = resources.build_file(
str(resources.RESOURCE_PATH / "max_inferable_limit_for_classes" / "main.py")
)
init_attr_node = module.body[-1].body[0].body[0].value.func
init_object_node = module.body[-1].mro()[-1]["__init__"]
super_node = next(init_attr_node.expr.infer())

# Arbitrarily limit the max number of infered nodes per context
InferenceContext.max_inferred = -1
context = InferenceContext()

# Try to infer 'object.__init__' > because of limit is impossible
for _ in bases._infer_stmts([init_object_node], context, frame=super):
pass

# Reset inference limit
InferenceContext.max_inferred = 100
# Check that we don't crash on a previously uninferable node
assert super_node.getattr("__init__", context=context)[0] == Uninferable


if __name__ == "__main__":
unittest.main()