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.

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

Inferring property fields in a class context when metaclass is present #941

Merged
merged 7 commits into from
May 15, 2021

Conversation

nelfin
Copy link
Contributor

@nelfin nelfin commented Apr 11, 2021

Steps

  • For new features or bug fixes, add a ChangeLog entry describing what your PR does.
  • Write a good description on what the PR does.

Description

If we are accessing an attribute and it is found on
type(A).__dict__ and it is a data descriptor, then we resolve that
descriptor. For the case of inferring a property which is accessed as a
class attribute, this equates to the property being defined on the
metaclass (which may be anywhere in the class hierarchy).

Type of Changes

Type
🐛 Bug fix

Related Issue

Closes #940

@nelfin nelfin force-pushed the fix/940-metaclass-subclass-property branch 2 times, most recently from 643d548 to 89fac51 Compare April 12, 2021 09:00
@nelfin nelfin marked this pull request as ready for review April 12, 2021 09:04
@nelfin nelfin force-pushed the fix/940-metaclass-subclass-property branch from da5f55a to 35d2941 Compare April 12, 2021 09:05
@nelfin nelfin mentioned this pull request Apr 23, 2021
@nelfin nelfin force-pushed the fix/940-metaclass-subclass-property branch from 35d2941 to b389805 Compare May 11, 2021 00:31
@nelfin nelfin force-pushed the fix/940-metaclass-subclass-property branch from b389805 to 39d359d Compare May 11, 2021 23:22
Copy link
Contributor

@hippo91 hippo91 left a comment

Choose a reason for hiding this comment

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

Very nice job @nelfin! The fix is clear, documented and heavily tested! 👍
I approved this PR but let a few comments.
We just have to wait for @cdce8p review before merging.

tests/unittest_scoped_nodes.py Outdated Show resolved Hide resolved
tests/unittest_scoped_nodes.py Outdated Show resolved Hide resolved
astroid/brain/brain_namedtuple_enum.py Show resolved Hide resolved
Copy link
Member

@cdce8p cdce8p left a comment

Choose a reason for hiding this comment

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

Overall it looks good. Just some small comments and a merge conflict. After those are resolved, this one can be merged.

astroid/brain/brain_namedtuple_enum.py Show resolved Hide resolved
astroid/brain/brain_namedtuple_enum.py Outdated Show resolved Hide resolved
tests/unittest_scoped_nodes.py Outdated Show resolved Hide resolved
@nelfin nelfin force-pushed the fix/940-metaclass-subclass-property branch from 5dcd0ed to f6f43ab Compare May 13, 2021 22:33
Ref pylint-dev#940. If we are accessing an attribute and it is found on
type(A).__dict__ *and* it is a data descriptor, then we resolve that
descriptor. For the case of inferring a property which is accessed as a
class attribute, this equates to the property being defined on the
metaclass (which may be anywhere in the class hierarchy).
@nelfin nelfin force-pushed the fix/940-metaclass-subclass-property branch from f6f43ab to 8afdc7b Compare May 13, 2021 23:20
astroid/brain/brain_namedtuple_enum.py Show resolved Hide resolved
ChangeLog Outdated Show resolved Hide resolved
tests/unittest_scoped_nodes.py Outdated Show resolved Hide resolved
Ref pylint-dev/pylint#3535. Ref pylint-dev/pylint#4358. This updates the
namedtuple/enum brain to add a dictionary for __members__
Copy link
Member

@cdce8p cdce8p left a comment

Choose a reason for hiding this comment

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

👍🏻 Thanks again

@cdce8p cdce8p merged commit 8181dc9 into pylint-dev:master May 15, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

@property members defined in metaclasses of a base class are not correctly inferred
3 participants