ruff icon indicating copy to clipboard operation
ruff copied to clipboard

F401 unused-import for star-imports: false negative?

Open spaceone opened this issue 2 years ago • 5 comments

flake8 and ruff differ for * imports:

$ flake8 - <<<"from foo import *"
stdin:1:1: F403 'from foo import *' used; unable to detect undefined names
stdin:1:1: F401 'foo.*' imported but unused
$ ruff - <<<"from foo import *"
-:1:1: F403 `from foo import *` used; unable to detect undefined names
Found 1 error.

I detected this as prior it had a # noqa: F401 comment which was removed by RUF100 when I compared any leftovers with flake8.

spaceone avatar Jan 31 '23 18:01 spaceone

I filed an upstream bug: https://github.com/PyCQA/pyflakes/issues/766 I would vote for fixing it in pyflakes.

spaceone avatar Jan 31 '23 18:01 spaceone

So they think that if F405 doesn't occur they mark the star import as F401.

F405 can be triggered by:

$ python3 -m pyflakes <<< $'from foo import *; __all__ = ("a",)'
<stdin>:1:1: 'from foo import *' used; unable to detect undefined names
<stdin>:1:20: 'a' may be undefined, or defined from star imports: foo
$ python3 -m pyflakes <<< $'from foo import *; a'
<stdin>:1:1: 'from foo import *' used; unable to detect undefined names
<stdin>:1:20: 'a' may be undefined, or defined from star imports: foo

spaceone avatar Jan 31 '23 19:01 spaceone

I think I'm okay with Ruff's behavior here but open to being convinced otherwise. What do you think?

charliermarsh avatar Jan 31 '23 19:01 charliermarsh

I am unsure. I like ruffs behavior more. Will think what I can do for the time I need to support both flake8 and ruff (aka: until #2402 is fixed).

spaceone avatar Jan 31 '23 19:01 spaceone

For a file containing just the following (which is often seen in __init__.py):

from math import *
❯ flake8 bug.py
bug.py:1:1: F403 'from math import *' used; unable to detect undefined names
bug.py:1:1: F401 'math.*' imported but unused
❯ ruff bug.py 
bug.py:1:1: F403 `from math import *` used; unable to detect undefined names
Found 1 error.

Using something from the math module:

from math import *
pi
❯ flake8 bug.py
bug.py:1:1: F403 'from math import *' used; unable to detect undefined names
bug.py:2:1: F405 'pi' may be undefined, or defined from star imports: math
❯ ruff bug.py 
bug.py:1:1: F403 `from math import *` used; unable to detect undefined names
bug.py:2:1: F405 `pi` may be undefined, or defined from star imports: `math`
Found 2 errors.

Using something that is not in the math module:

from math import *
invalid_name
❯ flake8 bug.py
bug.py:1:1: F403 'from math import *' used; unable to detect undefined names
bug.py:2:1: F405 'invalid_name' may be undefined, or defined from star imports: math
❯ ruff bug.py 
bug.py:1:1: F403 `from math import *` used; unable to detect undefined names
bug.py:2:1: F405 `invalid_name` may be undefined, or defined from star imports: `math`
Found 2 errors.

It has always bugged me getting F401 and F403 when the latter feels like it gives the expectation of the former, so I do quite like the way ruff does this... But I wouldn't be upset if it was implemented like the original.

ngnpope avatar Jan 31 '23 20:01 ngnpope

Going to close for now because I'm comfortable with our current behavior.

charliermarsh avatar Feb 02 '23 03:02 charliermarsh

I think if all is defined it should recognize the star import, only throw if all is also undefined.

enriquebos avatar Dec 07 '23 20:12 enriquebos

I think if all is defined it should recognize the star import, only throw if all is also undefined.

I think I agree with this, but it would be good to hear if someone thinks there is a reason why this is bad practice for __init__.py files. Anyways, it was really bothering me so I used the following in my pyproject.toml, adapted from here

[tool.ruff.lint.per-file-ignores]
"__init__.py" = ["F401", "F403"]

niniack avatar Mar 02 '24 12:03 niniack