False negative and false positive 'no-member' errors with +=

[yanqd0]

Steps to reproduce

Given a file a.py:

# pylint: disable=missing-module-docstring
# pylint: disable=too-few-public-methods
# pylint: disable=missing-class-docstring
# pylint: disable=invalid-name


class A:
    value: int


obj = A()
obj.value += 1

Given a file b.py:

# pylint: disable=missing-module-docstring
# pylint: disable=too-few-public-methods
# pylint: disable=missing-class-docstring
# pylint: disable=invalid-name


class A:
    value: int


obj = A()
obj.value = 1 + obj.value

Given a file c.py:

# pylint: disable=missing-module-docstring
# pylint: disable=too-few-public-methods
# pylint: disable=missing-class-docstring
# pylint: disable=invalid-name


class A:
    value: int


obj = A()
obj.value += 1
obj.value = 1 + obj.value

Current behavior

Results:

$ pylint a.py b.py c.py
************* Module a
a.py:12:0: E1101: Instance of 'A' has no 'value' member (no-member)

-------------------------------------------------------------------
Your code has been rated at 6.15/10

Expected behavior

There should also be no-member issues in b.py and c.py.

pylint --version output

Result of pylint --version output:

pylint 2.8.2
astroid 2.5.6
Python 3.8.10 (default, May 13 2021, 15:41:32) 
[GCC 9.3.0]

[yanqd0]

There is also a false positive case.

Prerequisite

pip install pydantic

Steps to reproduce

Given a file d.py:

# pylint: disable=missing-module-docstring
# pylint: disable=too-few-public-methods
# pylint: disable=missing-class-docstring
# pylint: disable=invalid-name
from pydantic import BaseModel


class A(BaseModel):
    value: int


obj = A(value=1)
obj.value += 1

Current behavior

$ pylint d.py
************* Module d
d.py:13:0: E1101: Instance of 'A' has no 'value' member (no-member)

-----------------------------------
Your code has been rated at 0.00/10

Expected behavior

This one should be OK.

[areveny]

The false negative in the original post is related to the below code in typecheck.py TypeChecker.visit_attribute where we check usages of node.attrname.

            try:
                if not [
                    n
                    for n in owner.getattr(node.attrname)
                    if not isinstance(n.statement(future=True), nodes.AugAssign)
                ]:
                    missingattr.add((owner, name))
                    continue

We correctly exclude instances of nodes.AugAssign, which combine an operator and an assignment, e.g. += because these rely on the attribute being already defined. These are not added to missingattr on which messages are emitted.

However, we do not exclude cases like in the false negative where the assignment is reliant on the value being checked less explicitly, e.g. obj.value = 1 + obj.value. A fix would check the value (right side) of the assignment more thoroughly to ensure that the tested attribute (obj.value) is not being utilized.

[Dulakd]

Hi, I'm going to try to work on this issue as a first issue.

[clavedeluna]

I took a stab at fixing this issue but came across a roadblock.

Basically, pylint correctly identifies obj.value += 1 because of this code

                    # Skip augmented assignments
                    try:
                        if isinstance(
                            attr_node.statement(future=True), nodes.AugAssign
                        ):
                            continue

but it doesn't identify obj.value = 1 + obj.value because this isn't an AugAssign, even though these two statements are equivalent.

So I tried to add breakpoints and try to make the two be equivalent, by adding

                    # Skip augmented assignments
                    try:
                        if isinstance(
                            attr_node.statement(future=True), nodes.AugAssign
                        )
                        or any([isinstance(x, nodes.BinOp) for x in attr_parent.get_children()]):
                            continue

As a starting point, but this gave some other failures in other tests.
I'm still trying to figure out how to write code that says "consider obj.value += 1 equal to obj.value = 1 + obj.value because obj.value is being operated on in the right hand side of the assign", but I haven't quite figured out.

Any ideas from the maintainers?

[Pierre-Sassoulas]

Any ideas from the maintainers?

I would check what is inside the AST node when the code is obj.value = 1 + obj.value (with prints or pdb) and then also skip that kind of node on top of augmented assignment.

[DanielNoord]

I think you'll need a isinstance(x, nodes.BinOp) and x.op == "+=". I believe the = is part of the op attribute.

[clavedeluna]

any([isinstance(x, nodes.BinOp) for x in attr_parent.get_children()]):

= will not be part of the op attribute because the operation in obj.value = 1 + obj.value is +.

I'm getting a bit closer with this check

                def _binop(x):
                    return isinstance(x, nodes.BinOp) and x.op == "+"
....
....
                    try:
                        if isinstance(
                            attr_node.statement(future=True), nodes.AugAssign
                        ) or (
                            isinstance(attr_node, nodes.AssignAttr)
                            and any([_binop(x) for x in attr_parent.get_children()])
                        ):
                            continue

because it checks that we're assigning and that there's a bin operation to +
the problem is that now this is flagged as no-member in regression_2964.py

self.__path = path + (self,)

which is fair... I have to somehow figure out if the assigning is to ITSELF, but the object ids are different:

in obj.value = 1 + obj.value the object id for the obj.value in the left hand side is not the same as the obj.value in the right hand side (make sense python), but that also means I have no way to identify if it's auto-assigning. OTher than maybe checking for the name of the object? could be risky, given example of self.__path = path. ... above , the name is so similar.

[DanielNoord]

You might want to check if we don't have a helper function for this in pylint.checkers.utils. If not something like this might work:

attr_node.target.name in {x.left.name, x.right.name}