Include subclasses of standard property classes as property decorators

[mharding-hpe]

Type of Changes

	Type
✓	🐛 Bug fix
✓	✨ New feature

Description

This is my attempt to get astroid to automatically recognize subclasses of functools.cached_property as property decorators. In my local testing, this change corrects the problem I reported in issue 10377, but I am not at all familiar with the overall codebase here, so I am not confident that this is the appropriate change to make.

I also wasn't sure if I should describe this as a bug fix or a new feature. :)

Refs #2306
Closes #10377

[DanielNoord]

Do you mind adding a test for this? There should be other tests for property that we can adapt :)

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 93.30%. Comparing base (3636bc2) to head (1ae4a2f).
Report is 6 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #2735      +/-   ##
==========================================
+ Coverage   93.21%   93.30%   +0.08%     
==========================================
  Files          93       93              
  Lines       11072    11080       +8     
==========================================
+ Hits        10321    10338      +17     
+ Misses        751      742       -9     

Flag	Coverage Δ
linux	93.16% <100.00%> (+0.08%)	⬆️
pypy	93.30% <100.00%> (+0.08%)	⬆️
windows	93.28% <100.00%> (+0.08%)	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines	Coverage Δ
astroid/bases.py	89.37% <100.00%> (+0.46%)	⬆️

... and 8 files with indirect coverage changes

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[mharding-hpe]

Actually, I have hit a problem with this PR as written. It works if my class inherits from cached_property, but not if it inherits from cached_property[T]. Will need to dig further. I'll move this PR into draft state in the meantime.

[Pierre-Sassoulas]

Looks great so far, thank you for digging into this !

[mharding-hpe]

I have a possible fix to the subscripting issue. I have started working on the tests.

[mharding-hpe]

Ok, I've updated my original fix so that it handles subclasses of functools.cached_property, whether or not it is subscripted. I added to the existing property tests in test_nodes.py to cover these cases, as well as a couple of others that were not currently being covered by that test.

[mharding-hpe]

I ran the full test suite locally without my PR, and got:

1653 passed, 50 skipped, 15 xfailed, 1 xpassed

And with my PR, including the 1 additional test (test_is_property_py312) it created:

1654 passed, 50 skipped, 15 xfailed, 1 xpassed

So at least based on my local pytest execution, the PR does not appear to cause any regressions.

[mharding-hpe]

I see the tests don't cover a few of the non-property cases in the code. I think I know which ones I can add to address this. Hold, please.

[mharding-hpe]

In working on filling out the code coverage, I ran into a question about how we want pylint to behave in a particular case.

Some context -- from looking at the _is_property function in bases.py, it is clear that pylint is willing to decide that a decorator is a property decorator just based on its name. For example, if the decorator is named @cached_property or @reify, then it will be assumed to be a property, even in cases where this is clearly not the case (like in the existing test, where an empty function is named cached_property and used successfully as a property decorator).

I am assuming that this is the desired philosophy into which my PR should fit. But I realized that my PR is making pylint pickier in some sense. Not in a regression sense -- my changes don't break any current behavior. But for example, consider the following code:

import functools

class cached_property:
    pass

class user_property(cached_property):
    pass

class user_property_from_functools(functools.cached_property):
    pass

class A:
    @cached_property
    def cached_property(self): return 42
    @user_property
    def user_property(self): return 42
    @user_property_from_functools
    def user_property_from_functools(self): return 42

Without my PR in place, then only A.cached_property is considered by pylint to be a property.
With my PR in place, A.cached_property and A.user_property_from_functools are considered by pylint to be properties, but A.user_property is not. In other words, when I am validating the parent classes, I am not being as generous as pylint normally has been. This new behavior feels inconsistent, since I can use an empty class named cached_property and have it be considered a property, but if I subclass it, then it no longer works.

If we want to retain this idea that pylint will use the name as an assurance that it is a property, then I can easily change my PR so that in the above example, all of those will work. BUT, there is another case. Consider:

cached_property = { int: float }
class user_property(cached_property[int]):
    pass

Do we want user_property here to also be interpreted as a property if used as a decorator? In this case, cached_property is not even referring to a class, but instead to a dict. I do see that pylint will not interpret THIS cached_property as a property decorator by itself, so maybe it's okay to also forbid the subclassing to be considered as such.

I'm going to modify my PR to allow all 3 cases of my first example, but I will retain the fact that it balks at that last example. Let me know if you disagree with any of that, or if I'm not being clear about it.

[mharding-hpe]

Thinking about it more, it's not quite so simple.

My above discussion centered around cached_property, but pylint takes its lenient view for any decorators with names in PROPERTIES or POSSIBLE_PROPERTIES. So presumably if I am going to allow subclasses of any class named cached_property, then I should also do so for subclasses of the others.

For the non-subscripted case, this is handled by calling inferred.is_subtype_of right now, but if I wanted to allow subtypes of ANY user class with any of those names, I'd need to make the handling of the non-subscripted case more elaborate.

For the subscripted case, I'm already adding code to look at what is being subscripted, and what its base class is. The problem here is that several of the official property types (builtins,property, enum.property, and abc.abstractproperty), subscripting is not supported. You'll get a runtime error if you try to do it. So it's probably not great for pylint to allow these and interpret them as properties.

I'm actually now thinking that when looking at user classes, we should really just look to see if they inherit from one of the provided property classes. If not, and if the user wants pylint to handle them, then they can add them to the POSSIBLE_PROPERTIES list when they run pylint.

[mharding-hpe]

Ok, I've added tests to cover all of the situations I described in my earlier comments. I also added tests for other things that had been missing, like for the enum.property class.

[mharding-hpe]

I see that the 3.10 tests are failing with the new enum.property tests I added.
When looking into this failure, I noticed that enum.property was actually not added until Python 3.11.
I had been using checks for Python 3.10 because that it what was present in the existing astroid code in bases.py.
I think that needs to be changed to reflect when it was actually added. I can cover that with this same PR.

[mharding-hpe]

This PR seems to be the one which claimed enum.properties was added in 3.10.
The Python docs say 3.11, which matches what I see when I try to import it under 3.10.

[mharding-hpe]

I've added a commit to change the tests and bases.py to reflect that enum.property was added in Python 3.11, not 3.10. I'll also add the original PR author as a reviewer in case there is something going on here that is not apparent to me.

Oh, I guess I don't have authority to add reviewers. Well, @nelfin if you happen to see this, please give your input.

[nelfin]

LGTM. For context, 3.10 wasn't released in Jul of 2021, that change was for the alpha release. enum.property was removed in python/cpython#27010 commit b58dc58

…

On Thu., 15 May 2025, 8:40 am Mitch Harding, ***@***.***> wrote: *mharding-hpe* left a comment (pylint-dev/astroid#2735) <#2735 (comment)> I've added a commit to change the tests and bases.py to reflect that enum.property was added in Python 3.11, not 3.10. I'll also add the original PR author as a reviewer in case there is something going on here that is not apparent to me. Oh, I guess I don't have authority to add reviewers. Well, @nelfin <https://github.com/nelfin> if you happen to see this, please give your input. — Reply to this email directly, view it on GitHub <#2735 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AABSH3KPGEDQX7VMZPMAW6L26OS2XAVCNFSM6AAAAAB46YWS32VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDQOBRGUYDCOBYGQ> . You are receiving this because you were mentioned.Message ID: ***@***.***>

[mharding-hpe]

Aha -- that makes sense. I should have paid closer attention to the date of the original commit. Thank you for the insight!

LGTM. For context, 3.10 wasn't released in Jul of 2021, that change was for
the alpha release. enum.property was removed in
python/cpython#27010 commit b58dc58

[mharding-hpe]

Working on this PR gave me the thought that there may be other cases that are currently overlooked in the tests (like I saw here for tests validating the other standard property classes). @DanielNoord @Pierre-Sassoulas are you open to further PRs that are purely test improvements/additions, if I see anything worth doing?

[Pierre-Sassoulas]

We're open to any PR making the code base better :) your work and in depth digging into issues is appreciated

[DanielNoord]

This looks awesome!

@Pierre-Sassoulas Do you want to review as well?

[Pierre-Sassoulas]

This is great !

[Pierre-Sassoulas]

Should this be based on pylint's py-version instead ? Not sure if we have the ability to propagate this option to astroid yet. If not I don't know what exactly the design should be. (this is more a question for @DanielNoord)

[DanielNoord]

We don't really have an option to...
Since we haven't had anybody complain about this for 2-3 years after making the mistake I wouldn't bother with adding complicated support for it.