astral-sh / ruff

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

[red-knot] implement re-export conventions for imports #14099

Open carljm opened 2 weeks ago

carljm commented 2 weeks ago

See https://typing.readthedocs.io/en/latest/spec/distributing.html#import-conventions

A normal from foo import bar in module a should not allow another module to do from a import bar; an explicit from foo import bar as bar should be required to specify a re-export.

In the current typing spec, these rules are worded as applicable to all modules. Mypy does apply them in all modules, pyright applies them only in type stubs.

I have a vague memory that these originated as stub-only conventions, but I haven't been able to dig up confirmation.

We need to decide if we will apply these conventions in all modules, or only in stubs, and then implement them accordingly.

In particular this is problematic at the moment because we don't apply these conventions in builtins.pyi, meaning we understand all symbols imported into builtins.pyi (e.g. various typing module symbols) as builtin names.

AlexWaygood commented 2 weeks ago

As you say, it's very important that we apply these semantics in stub files.

For runtime files, my instinct is to say that we should copy the runtime semantics exactly, but have an additional disabled-by-default error code (let's call it no-implicit-reexport) where we warn if you try to import something that's only implicitly re-exported from a module. (I actually believe that's what mypy does?) I think that should work fine for most cases — the only problem I can see is imports inside if TYPE_CHECKING blocks, which won't exist at runtime but which we will assume to exist if we're running without the no-implicit-reexport error code enabled. I'm not sure the unsoundness there is significant, though — I can't remember it ever coming up in the mypy issue tracker.

charliermarsh commented 2 weeks ago

I could look into this.

carljm commented 2 weeks ago

For runtime files, my instinct is to say that we should copy the runtime semantics exactly, but have an additional disabled-by-default error code (let's call it no-implicit-reexport) where we warn if you try to import something that's only implicitly re-exported from a module. (I actually believe that's what mypy does?)

This makes sense to me. I definitely think that if we see an import of a not-explicitly-re-exported import, we should still infer the correct type that we are able to observe from the runtime semantics, even if we also issue a warning. I think the only difference between what you suggest here and what mypy does (according to my local testing) is that it appears to me the warning is enabled by default in mypy, not disabled by default. I get the warning without using a mypy config file or any additional CLI flags.

One additional wrinkle to throw in -- I got clarification at https://github.com/microsoft/pyright/discussions/9385 that pyright makes a distinction I hadn't realized here: it only applies these import rules to non-stub modules if they are third-party modules, not first-party ones. The idea is that these conventions are how you clarify the public API of a typed library, but they don't apply within a codebase. I can see the rationale there, and I can also see how the structure of the specification suggests that: the "import conventions" section is within the page about distributing typed libraries. So I think I would weakly favor making the same distinction?

hauntsaninja commented 2 weeks ago

Mypy's behaviour is:

Random thoughts:

carljm commented 2 weeks ago

Thanks, that's useful!

One note on pyright's first vs third party distinction; I think it works in the opposite way from what you're suggesting. Pyright applies strict re-export rules to imports from third party libraries, but not to imports from your own first-party code. The idea is that this allow library authors more control over their exported api surface.