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

use the ClassDef attribute inference process in Instances #2563

Closed
wants to merge 1 commit into from
Closed

use the ClassDef attribute inference process in Instances #2563

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
15 changes: 7 additions & 8 deletions astroid/bases.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -273,27 +273,26 @@ def igetattr(
try:
context.lookupname = name
# XXX frame should be self._proxied, or not ?
get_attr = self.getattr(name, context, lookupclass=False)
yield from _infer_stmts(
self._wrap_attr(get_attr, context), context, frame=self
)
attrs = self.getattr(name, context, lookupclass=False)
iattrs = self._proxied._infer_attrs(attrs, context, class_context=False)
yield from self._wrap_attr(iattrs)
except AttributeInferenceError:
try:
# fallback to class.igetattr since it has some logic to handle
# descriptors
# But only if the _proxied is the Class.
if self._proxied.__class__.__name__ != "ClassDef":
raise
attrs = self._proxied.igetattr(name, context, class_context=False)
yield from self._wrap_attr(attrs, context)
iattrs = self._proxied.igetattr(name, context, class_context=False)
yield from self._wrap_attr(iattrs, context)
except AttributeInferenceError as error:
raise InferenceError(**vars(error)) from error

def _wrap_attr(
self, attrs: Iterable[InferenceResult], context: InferenceContext | None = None
self, iattrs: Iterable[InferenceResult], context: InferenceContext | None = None
) -> Iterator[InferenceResult]:
"""Wrap bound methods of attrs in a InstanceMethod proxies."""
for attr in attrs:
for attr in iattrs:
if isinstance(attr, UnboundMethod):
if _is_property(attr):
yield from attr.infer_call_result(self, context)
Expand Down
85 changes: 44 additions & 41 deletions astroid/nodes/scoped_nodes/scoped_nodes.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2465,14 +2465,11 @@ def igetattr(

:returns: The inferred possible values.
"""
from astroid import objects # pylint: disable=import-outside-toplevel

# set lookup name since this is necessary to infer on import nodes for
# instance
context = copy_context(context)
context.lookupname = name

metaclass = self.metaclass(context=context)
try:
attributes = self.getattr(name, context, class_context=class_context)
# If we have more than one attribute, make sure that those starting from
Expand All @@ -2495,44 +2492,7 @@ def igetattr(
for a in attributes
if a not in functions or a is last_function or bases._is_property(a)
]

for inferred in bases._infer_stmts(attributes, context, frame=self):
# yield Uninferable object instead of descriptors when necessary
if not isinstance(inferred, node_classes.Const) and isinstance(
inferred, bases.Instance
):
try:
inferred._proxied.getattr("__get__", context)
except AttributeInferenceError:
yield inferred
else:
yield util.Uninferable
elif isinstance(inferred, objects.Property):
function = inferred.function
if not class_context:
if not context.callcontext:
context.callcontext = CallContext(
args=function.args.arguments, callee=function
)
# Through an instance so we can solve the property
yield from function.infer_call_result(
caller=self, context=context
)
# If we're in a class context, we need to determine if the property
# was defined in the metaclass (a derived class must be a subclass of
# the metaclass of all its bases), in which case we can resolve the
# property. If not, i.e. the property is defined in some base class
# instead, then we return the property object
elif metaclass and function.parent.scope() is metaclass:
# Resolve a property as long as it is not accessed through
# the class itself.
yield from function.infer_call_result(
caller=self, context=context
)
else:
yield inferred
else:
yield function_to_method(inferred, self)
yield from self._infer_attrs(attributes, context, class_context)
except AttributeInferenceError as error:
if not name.startswith("__") and self.has_dynamic_getattr(context):
# class handle some dynamic attributes, return a Uninferable object
Expand All @@ -2542,6 +2502,49 @@ def igetattr(
str(error), target=self, attribute=name, context=context
) from error

def _infer_attrs(
self,
attributes: list[InferenceResult],
context: InferenceContext,
class_context: bool = True,
) -> Iterator[InferenceResult]:
from astroid import objects # pylint: disable=import-outside-toplevel

metaclass = self.metaclass(context=context)
for inferred in bases._infer_stmts(attributes, context, frame=self):
# yield Uninferable object instead of descriptors when necessary
if not isinstance(inferred, node_classes.Const) and isinstance(
inferred, bases.Instance
):
try:
inferred._proxied.getattr("__get__", context)
except AttributeInferenceError:
yield inferred
else:
yield util.Uninferable
elif isinstance(inferred, objects.Property):
function = inferred.function
if not class_context:
if not context.callcontext:
context.callcontext = CallContext(
args=function.args.arguments, callee=function
)
# Through an instance so we can solve the property
yield from function.infer_call_result(caller=self, context=context)
# If we're in a class context, we need to determine if the property
# was defined in the metaclass (a derived class must be a subclass of
# the metaclass of all its bases), in which case we can resolve the
# property. If not, i.e. the property is defined in some base class
# instead, then we return the property object
elif metaclass and function.parent.scope() is metaclass:
# Resolve a property as long as it is not accessed through
# the class itself.
yield from function.infer_call_result(caller=self, context=context)
else:
yield inferred
else:
yield function_to_method(inferred, self)

def has_dynamic_getattr(self, context: InferenceContext | None = None) -> bool:
"""Check if the class has a custom __getattr__ or __getattribute__.

Expand Down
2 changes: 1 addition & 1 deletion tests/test_scoped_nodes.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1295,7 +1295,7 @@ def func(arg1, arg2):
self.assertIsInstance(inferred[0], BoundMethod)
inferred = list(Instance(cls).igetattr("m4"))
self.assertEqual(len(inferred), 1)
self.assertIsInstance(inferred[0], nodes.FunctionDef)
self.assertIsInstance(inferred[0], BoundMethod)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this change is incorrect, and therefore shows something is up with this PR.

To reproduce:

class Clazz(object): ...


def func(arg1, arg2):
    "function that will be used as a method"
    return arg1.value + arg2


Clazz.m3 = func
inst = Clazz()
inst.m4 = func

print(Clazz.m3)
print(inst.m3)
print(inst.m4)
<function func at 0x109582340>
<bound method func of <__main__.Clazz object at 0x1095ab9b0>>
<function func at 0x109582340>

This should remain a FunctionDef and not a BoundMethod.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, you are right. Then, I think the issue is in the semantics of objectmodel. For ordinary Instances, special_attributes are «instance» attributes. They should remain FunctionDef. However, in some cases, for specialized instances, e.g. Generator, the objectmodel is used for «class» methods. So, the FunctionDefs should get wrapped into BoundMethods.
So, the fix should lie on that side, but it also feels quite error-prone to use the same special_attributes for the two different purposes.

I'll do a fix, but a bit later, I'm a bit short on time.


def test_getattr_from_grandpa(self) -> None:
data = """
Expand Down