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

Fix in place properties #2553

Merged
merged 2 commits into from
Sep 10, 2024
Merged

Fix in place properties #2553

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
b659f1f
fix construction of in-place properties
temyurchenko Sep 9, 2024
3dfd5ff
fixup! fix construction of in-place properties
temyurchenko Sep 10, 2024
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
9 changes: 6 additions & 3 deletions astroid/brain/brain_builtin_inference.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -645,12 +645,15 @@ def infer_property(

prop_func = objects.Property(
function=inferred,
name=inferred.name,
name="<property>",
lineno=node.lineno,
col_offset=node.col_offset,
# ↓ semantically, the definition of the class of property isn't within
# node.frame. It's somewhere in the builtins module, but we are special
# casing it for each "property()" call, so we are making up the
# definition on the spot, ad-hoc.
parent=AstroidManager().adhoc_module,
)
# Set parent outside __init__: https://github.com/pylint-dev/astroid/issues/1490
prop_func.parent = node
prop_func.postinit(
body=[],
args=inferred.args,
Expand Down
11 changes: 6 additions & 5 deletions tests/brain/test_builtin.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,22 +14,23 @@

class BuiltinsTest(unittest.TestCase):
def test_infer_property(self):
class_with_property = _extract_single_node(
property_assign = _extract_single_node(
Copy link
Collaborator

Choose a reason for hiding this comment

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

Shall we add a test that property doesn't end up in the locals of Something so we don't regress here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've changed the assert below to check that there are no Property nodes in the locals.

"""
class Something:
def getter():
return 5
asd = property(getter) #@
"""
)
inferred_property = next(iter(class_with_property.value.infer()))
inferred_property = next(iter(property_assign.value.infer()))
self.assertTrue(isinstance(inferred_property, objects.Property))
class_parent = inferred_property.parent.parent.parent
class_parent = property_assign.scope()
self.assertIsInstance(class_parent, nodes.ClassDef)
self.assertFalse(
any(
isinstance(getter, objects.Property)
for getter in class_parent.locals["getter"]
isinstance(def_, objects.Property)
for def_list in class_parent.locals.values()
for def_ in def_list
)
)
self.assertTrue(hasattr(inferred_property, "args"))
Expand Down
Loading