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 unused-import (W0611) when assigning class properties with the same name #6089

Closed
teake opened this issue Apr 1, 2022 · 4 comments · Fixed by #6096
Closed
Labels
Bug 🪲 False Positive 🦟 A message is emitted but nothing is wrong with the code
Milestone

Comments

@teake
Copy link

teake commented Apr 1, 2022

Bug description

This code snippet

from other import x, y  # raises unused-import (W0611) for x but not for y


class MyClass:
    x = float(x)
    y = y

will start raising a false positive from ea13058 up until the current main branch (6505c74).

Related: #4993.

Configuration

No response

Command used

A false positive is only raised in combination with --disable=all:

$ python -m pylint --disable=all --enable=W0611 <path_to_file>

But when invoked as

$ python -m pylint <path_to_file>

no false positive is raised, even though unused-import (W0611) is enabled by default.

Pylint output

************* Module mycodesnippet
/path/to/mycodesnippet.py:1:0: W0611: Unused x imported from other (unused-import)

------------------------------------------------------------------
Your code has been rated at 7.50/10 (previous run: 7.50/10, +0.00)

Expected behavior

No false positive should be raised.

Pylint version

pylint 2.14.0-dev0
astroid 2.11.2
Python 3.6.15 | packaged by conda-forge | (default, Dec  3 2021, 18:49:43) 
[GCC Clang 11.1.0]

OS / Environment

macOS 12.3

Additional dependencies

No response

@teake teake added the Needs triage 📥 Just created, needs acknowledgment, triage, and proper labelling label Apr 1, 2022
@DudeNr33 DudeNr33 added False Positive 🦟 A message is emitted but nothing is wrong with the code Bug 🪲 and removed Needs triage 📥 Just created, needs acknowledgment, triage, and proper labelling labels Apr 1, 2022
@DudeNr33
Copy link
Collaborator

DudeNr33 commented Apr 1, 2022

While the original bug introduced by ea13058 really was an issue with the unused-import not being included in the check_messages call, this is not the root cause in the current main branch.

The error is also slightly different:
When running with ea13058, the output for pylint --disable=all --enable=unused-import a.py contains error messages for both x and y:

************* Module a
/Users/andreas/programming/forks/pylint/.notes/a.py:1:0: W0611: Unused x imported from other (unused-import)
/Users/andreas/programming/forks/pylint/.notes/a.py:1:0: W0611: Unused y imported from other (unused-import)

With the current main branch, the output only complains about x:

************* Module a
a.py:1:0: W0611: Unused x imported from other (unused-import)

It makes no difference if we short-circuit the check_messages decorator.
The name x is not marked as consumed when only unused-import is enabled, but I have not yet figured out why.

@teake
Copy link
Author

teake commented Apr 1, 2022

FWIW, enabling either used-before-assignment (E0601) or undefined-variable (E0602) together with W0611 makes the issue go away:

$ python -m pylint  --disable=all --enable=E0602,W0611 <path_to_file>

--------------------------------------------------------------------
Your code has been rated at 10.00/10 (previous run: 10.00/10, +0.00)

Those are the only two rules that seem to make a difference though -- enabling any of the others does not make the issue go away.

@DudeNr33
Copy link
Collaborator

DudeNr33 commented Apr 1, 2022

Thanks, this explains why in the current version the check_messages decorator does not play a role.
used-before-assignment and undefined-variable translate to internal flags that are used to change the control flow in the checker class and how the consumption of names is handled.

This was introduced with c518188 which apparently already aimed at solving similar problems, but did not catch all of them. It explicitly dealt with the --disable=all --enable=unused-import configuration.
Before that commit, we get false positives for both x and y.
Afterwards is the situation we have now, with only a false positive for x.

I just saw that the issue that this commit fixed was one of yours as well. 😁

@teake
Copy link
Author

teake commented Apr 4, 2022

Thanks guys!

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
3 participants