astral-sh / ruff

An extremely fast Python linter and code formatter, written in Rust.
https://docs.astral.sh/ruff
MIT License
33.21k stars 1.11k forks source link

`useless-import-alias` (`PLC0414`) rule contradicts PEP484 #6294

Open DetachHead opened 1 year ago

DetachHead commented 1 year ago
import numpy as numpy

this means explicit re-export, as mentioned in PEP484:

Modules and variables imported into the stub are not considered exported from the stub unless the import uses the import ... as ... form or the equivalent from ... import ... as ... form. (UPDATE: To clarify, the intention here is that only names imported using the form X as X will be exported, i.e. the name before and after as must be the same.)

perhaps this rule should be removed, since it contradicts type checking rules

qdegraaf commented 1 year ago

This is only the case for stub files. Ruff takes this into account by only running the rule for non-stub files

See https://github.com/astral-sh/ruff/blob/9f3567dea6f04e9163ab66ad2fc1fafbadb3e790/crates/ruff/src/checkers/ast/analyze/statement.rs#L568-L572 and https://github.com/astral-sh/ruff/blob/9f3567dea6f04e9163ab66ad2fc1fafbadb3e790/crates/ruff/src/checkers/ast/analyze/statement.rs#L888-L892

zanieb commented 1 year ago

Hm per https://github.com/microsoft/pyright/blob/ada1ea63e8360fe3bd203bff3b0069c34d142817/docs/typed-libraries.md#library-interface this also applies to non-stub files. I think we've considered turning this rule off in __init__.py files.

charliermarsh commented 1 year ago

I feel like this rule shouldn’t exist, those explicit aliases do have semantic meaning now, but it’s a bit strange to remove entirely since it does exist in pylint.

charliermarsh commented 1 year ago

Some projects do have this explicitly enabled (home-assistant, for example).

KotlinIsland commented 1 year ago

I feel like this rule shouldn’t exist

I agree

Some projects do have this explicitly enabled (home-assistant, for example).

They use mypy, but they don't export their types. From the looks of it, they don't use any explicit reexports, perhaps they will at some point. If they do, then they would certainly remove this rule.

but it’s a bit strange to remove entirely since it does exist in pylint.

So the rule was invented before this new semantic meaning was introduced, now it seems invalid.

Perhaps if a project were to forgo typing (or just doesn't like the concept of explicit reexport) then this rule would be applicable.

Avasam commented 8 months ago

Ignoring this rule from __init__.py definitely makes sense to me. Could still be considered a code smell (ie: did you really mean to re-export?) elsewhere. For instance in pywin32 I found a handful of old actual aliases that are no longer relevant after the Python 3 transition, but were accidentally left in. Maybe such a distinction would make more sense after #1774 / #1256

In the meantime it can be configured as such:

[lint.per-file-ignores]
# Explicit re-exports is fine in __init__.py, still a code smell elsewhere.
"__init__.py" = ["PLC0414"]
Hawk777 commented 5 months ago

I agree it would be good to ignore this in __init__.py. It contradicts F401; as far as I know there’s no non-noqa way to have both F401 and PLC0414 enabled and re-export things from an __init__.py right now, which is quite unfortunate.