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

false positive "assigning-non-slot" error for inherited descriptor when slots are used #6001

Closed
jab opened this issue Mar 27, 2022 · 8 comments · Fixed by #7987
Closed

false positive "assigning-non-slot" error for inherited descriptor when slots are used #6001

jab opened this issue Mar 27, 2022 · 8 comments · Fixed by #7987
Assignees
Labels
Bug 🪲 False Positive 🦟 A message is emitted but nothing is wrong with the code

Comments

@jab
Copy link
Contributor

jab commented Mar 27, 2022

Bug description

Minimized reproducer:

"""Repro false positive bug in pylint."""


# pylint: disable=too-few-public-methods


class MyDescriptor:
    """Basic descriptor."""

    def __get__(self, instance, owner):
        return 42

    def __set__(self, instance, value):
        pass


class Base:
    """Base class with slots."""

    __slots__ = ()

    attr2 = MyDescriptor()


class Repro(Base):
    """Reproduce false positive error using this class."""

    __slots__ = ()


repro = Repro()
repro.attr2 = 'false positive "assigning-non-slot" error'  # false positive error here

A real-world (non-minimized) example where pylint produces this false positive error can be found on this line: https://github.com/jab/bidict/blob/caf703e959ed4471bc391a7794411864c1d6ab9d/bidict/_orderedbase.py#L101

Configuration

No response

Command used

pylint repro.py

Pylint output

************* Module repro
repro.py:32:0: E0237: Assigning to attribute 'attr2' not defined in class slots (assigning-non-slot)

Expected behavior

No error should be flagged here.

Pylint version

pylint 2.13.2
astroid 2.11.2
Python 3.10.2 (main, Feb  2 2022, 07:36:01) [Clang 12.0.0 (clang-1200.0.32.29)]

OS / Environment

No response

Additional dependencies

No response

@jab jab added the Needs triage 📥 Just created, needs acknowledgment, triage, and proper labelling label Mar 27, 2022
@mbyrnepr2
Copy link
Member

Regarding the provided example pasted above, I think this is expected Pylint behaviour due to attr2 not being one of the slots.

__slots__ = ("attr2",) vs. __slots__ = ()

@mbyrnepr2
Copy link
Member

The answer here and here may be useful to achieve what you are looking for.

@jab
Copy link
Contributor Author

jab commented Mar 27, 2022

Slots are for instance attributes, not class attributes. In this example, attr2 is (and should be) a class attribute, therefore it is omitted from slots, as it should be. Pylint is incorrectly treating it as an instance attribute, not the (inherited) class attribute that it actually is.

@mbyrnepr2
Copy link
Member

mbyrnepr2 commented Mar 28, 2022

Thanks for the info @jab. I certainly don't disagree with your analysis. The correct behaviour is also seen with the simpler case:

class Example:
    __slots__ = ('a',)


myinstance = Example()

myinstance.b = 1 # a.py:7:0: E0237: Assigning to attribute 'b' not defined in class slots (assigning-non-slot)

I think it does look like a false positive in your case :D Perhaps this line can be used to fix this also: https://github.com/PyCQA/pylint/blob/main/pylint/checkers/classes/class_checker.py#L1556

@jab
Copy link
Contributor Author

jab commented Mar 28, 2022

@mbyrnepr2, your simplified example is not the same as what's going on here. In your example, you are assigning an instance attribute b. Since b is not present in slots for Example (or any of its base classes), pylint is correctly flagging the assigning-non-slot error.

This issue is about pylint incorrectly flagging an assigning-non-slot error when an inherited descriptor is used. (And note, the descriptor is a class attribute, so it should not be present in slots.) If you try to simplify the example I gave in the description to a case where a non-inherited descriptor is used, you can see that pylint correctly does not flag any assigning-non-slot error:

class MyDescriptor:
    def __get__(self, instance, owner):
        return 42
    def __set__(self, instance, value):
        instance.a = 42


class Example:
    __slots__ = ('a',)

    b = MyDescriptor()


myinstance = Example()
myinstance.b = 'set a to 42'  # no error on this line, as we expect
print(myinstance.a)  # prints 42

@jacobtylerwalls jacobtylerwalls added Bug 🪲 False Positive 🦟 A message is emitted but nothing is wrong with the code and removed Needs triage 📥 Just created, needs acknowledgment, triage, and proper labelling labels Mar 28, 2022
@mbyrnepr2
Copy link
Member

@jab I agree with you. It's an example of correct usage vs. what you are reporting. Sorry that I didn't communicate that clearly.

@Hubrrr88
Copy link

I have seen the same issue on latest PyLint version 2.13.8. Any news on when will this be fixed yet?

@clavedeluna clavedeluna self-assigned this Dec 26, 2022
@clavedeluna
Copy link
Contributor

It appears this case was considered and tested here and deemed to need to emit this message. But I believe this is incorrect, as slots is meant for instance attributes, not class attributes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug 🪲 False Positive 🦟 A message is emitted but nothing is wrong with the code
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants