pylint
pylint copied to clipboard
False negative and false positive 'no-member' errors with `+=`
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]
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.
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.
Hi, I'm going to try to work on this issue as a first issue.
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?
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.
I think you'll need a isinstance(x, nodes.BinOp) and x.op == "+="
. I believe the =
is part of the op
attribute.
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.
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}