Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

PLC2801 triggers for cases that are not necessarily errors #9789

Closed
ThiefMaster opened this issue Feb 2, 2024 · 7 comments · Fixed by #9791
Closed

PLC2801 triggers for cases that are not necessarily errors #9789

ThiefMaster opened this issue Feb 2, 2024 · 7 comments · Fixed by #9791
Labels
bug Something isn't working help wanted Contributions especially welcome

Comments

@ThiefMaster
Copy link
Contributor

Is PLC2801 really supposed to trigger in this case, and suggest using a get method that does not exist? I can of course easily silence the rule, but just in case it's overzealous here I'm opening this issue.

class classproperty(property):
    def __get__(self, obj, type=None):
        return self.fget.__get__(None, type)()


class A:
    @classproperty
    @classmethod
    def prop(cls):
        return 'aaa'


class B:
    prop = 'bbb'


class C:
    pass


class Foo(A, B, C):
    @classmethod
    def test(cls):
        for class_ in reversed(cls.mro()[:-1]):
            prop = class_.__dict__.get('prop')
            if isinstance(prop, classproperty):
                prop = prop.__get__(None, class_)
            print(class_.__name__, prop)


Foo.test()
[adrian@eluvian:~/dev/indico/py3/src:update-ruff +$]> ruff --version
ruff 0.2.0

[adrian@eluvian:~/dev/indico/py3/src:update-ruff +$]> ruff --isolated --select PLC2801 --preview --no-cache rufftest/ruff_sample.py
rufftest/ruff_sample.py:27:24: PLC2801 Unnecessary dunder call to `__get__`. Use `get` method.
   |
25 |             prop = class_.__dict__.get('prop')
26 |             if isinstance(prop, classproperty):
27 |                 prop = prop.__get__(None, class_)
   |                        ^^^^^^^^^^^^^^^^^^^^^^^^^^ PLC2801
28 |             print(class_.__name__, prop)
   |
   = help: Use `get` method

Found 1 error.

@zanieb
Copy link
Member

zanieb commented Feb 2, 2024

Huh... reading https://docs.python.org/3/howto/descriptor.html this sounds like a totally incorrect suggestion for how __get__ is supposed to work in the first place?

@zanieb zanieb added the bug Something isn't working label Feb 2, 2024
@ThiefMaster
Copy link
Contributor Author

Yes, __get__ is a property thing and has nothing to do with a dict's .get()

@zanieb zanieb added the help wanted Contributions especially welcome label Feb 2, 2024
@charliermarsh
Copy link
Member

@LefterisJP
Copy link

We also get lots of unecessary hits on this rule. We also use the dunder set in a hacky way to get around immutability somewhere temporarily so it's also on us, but then this rule also hits.

@charliermarsh
Copy link
Member

I believe we're removing __get__ and __set__.

charliermarsh pushed a commit that referenced this issue Feb 5, 2024
These are for descriptors which affects the behavior of the object _as a
property_; I do not think they should be called directly but there is no
alternative when working with the object directly.

Closes #9789
@s-banach
Copy link

Why would calling a dunder method ever be an error?
Nobody is doing it accidentally, they do it because they're method chaining.

@ThiefMaster
Copy link
Contributor Author

Just because something is possible it's not a good idea. And you can disable the rule altogether if you like doing it and want to keep doing it in your own codebase...

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
bug Something isn't working help wanted Contributions especially welcome
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants