diff --git a/crates/ty_python_semantic/resources/mdtest/annotations/stdlib_typing_aliases.md b/crates/ty_python_semantic/resources/mdtest/annotations/stdlib_typing_aliases.md index 895d3263d9..82057136f8 100644 --- a/crates/ty_python_semantic/resources/mdtest/annotations/stdlib_typing_aliases.md +++ b/crates/ty_python_semantic/resources/mdtest/annotations/stdlib_typing_aliases.md @@ -102,8 +102,8 @@ reveal_type(SetSubclass.__mro__) class FrozenSetSubclass(typing.FrozenSet): ... -# TODO: should have `Generic`, should not have `Unknown` -# revealed: tuple[, , Unknown, ] +# TODO: generic protocols +# revealed: tuple[, , , , , , @Todo(`Protocol[]` subscript), typing.Generic, ] reveal_type(FrozenSetSubclass.__mro__) #################### diff --git a/crates/ty_python_semantic/resources/mdtest/import/conventions.md b/crates/ty_python_semantic/resources/mdtest/import/conventions.md index 73d9e1f525..c4950b5d1c 100644 --- a/crates/ty_python_semantic/resources/mdtest/import/conventions.md +++ b/crates/ty_python_semantic/resources/mdtest/import/conventions.md @@ -202,9 +202,9 @@ class AnyFoo: ... Here, the symbol is re-exported using the `__all__` variable. ```py -# TODO: This should *not* be an error but we don't understand `__all__` yet. -# error: "Module `a` has no member `Foo`" from a import Foo + +reveal_type(Foo) # revealed: ``` `a.pyi`: @@ -221,6 +221,44 @@ __all__ = ['Foo'] class Foo: ... ``` +## Re-exports with `__all__` + +If a symbol is re-exported via redundant alias but is not included in `__all__`, it shouldn't raise +an error when using named import. + +`named_import.py`: + +```py +from a import Foo + +reveal_type(Foo) # revealed: +``` + +`a.pyi`: + +```pyi +from b import Foo as Foo + +__all__ = [] +``` + +`b.pyi`: + +```pyi +class Foo: ... +``` + +However, a star import _would_ raise an error. + +`star_import.py`: + +```py +from a import * + +# error: [unresolved-reference] "Name `Foo` used when not defined" +reveal_type(Foo) # revealed: Unknown +``` + ## Re-exports in `__init__.pyi` Similarly, for an `__init__.pyi` (stub) file, importing a non-exported name should raise an error diff --git a/crates/ty_python_semantic/resources/mdtest/import/dunder_all.md b/crates/ty_python_semantic/resources/mdtest/import/dunder_all.md new file mode 100644 index 0000000000..96654b769a --- /dev/null +++ b/crates/ty_python_semantic/resources/mdtest/import/dunder_all.md @@ -0,0 +1,796 @@ +# `__all__` + +Reference: + + +NOTE: This file only includes the usage of `__all__` for named-imports i.e., +`from module import symbol`. For the usage of `__all__` in wildcard imports, refer to +[star.md](star.md). + +## Undefined + +`exporter.py`: + +```py +class A: ... +class B: ... +``` + +`importer.py`: + +```py +import exporter +from ty_extensions import dunder_all_names + +# revealed: None +reveal_type(dunder_all_names(exporter)) +``` + +## Global scope + +The `__all__` variable is only recognized from the global scope of the module. It is not recognized +from the local scope of a function or class. + +`exporter.py`: + +```py +__all__ = ["A"] + +def foo(): + __all__.append("B") + +class Foo: + __all__ += ["C"] + +class A: ... +class B: ... +class C: ... + +foo() +``` + +`importer.py`: + +```py +import exporter +from ty_extensions import dunder_all_names + +# revealed: tuple[Literal["A"]] +reveal_type(dunder_all_names(exporter)) +``` + +## Supported idioms + +According to the [specification], the following idioms are supported: + +### List assignment + +`exporter.py`: + +```py +__all__ = ["A", "B"] + +class A: ... +class B: ... +``` + +`exporter_annotated.py`: + +```py +__all__: list[str] = ["C", "D"] + +class C: ... +class D: ... +``` + +`importer.py`: + +```py +import exporter +import exporter_annotated +from ty_extensions import dunder_all_names + +# revealed: tuple[Literal["A"], Literal["B"]] +reveal_type(dunder_all_names(exporter)) + +# revealed: tuple[Literal["C"], Literal["D"]] +reveal_type(dunder_all_names(exporter_annotated)) +``` + +### List assignment (shadowed) + +`exporter.py`: + +```py +__all__ = ["A", "B"] + +class A: ... +class B: ... + +__all__ = ["C", "D"] + +class C: ... +class D: ... +``` + +`exporter_annotated.py`: + +```py +__all__ = ["X"] + +class X: ... + +__all__: list[str] = ["Y", "Z"] + +class Y: ... +class Z: ... +``` + +`importer.py`: + +```py +import exporter +import exporter_annotated +from ty_extensions import dunder_all_names + +# revealed: tuple[Literal["C"], Literal["D"]] +reveal_type(dunder_all_names(exporter)) + +# revealed: tuple[Literal["Y"], Literal["Z"]] +reveal_type(dunder_all_names(exporter_annotated)) +``` + +### Tuple assignment + +`exporter.py`: + +```py +__all__ = ("A", "B") + +class A: ... +class B: ... +``` + +`exporter_annotated.py`: + +```py +__all__: tuple[str, ...] = ("C", "D") + +class C: ... +class D: ... +``` + +`importer.py`: + +```py +import exporter +import exporter_annotated +from ty_extensions import dunder_all_names + +# revealed: tuple[Literal["A"], Literal["B"]] +reveal_type(dunder_all_names(exporter)) + +# revealed: tuple[Literal["C"], Literal["D"]] +reveal_type(dunder_all_names(exporter_annotated)) +``` + +### Tuple assignment (shadowed) + +`exporter.py`: + +```py +__all__ = ("A", "B") + +class A: ... +class B: ... + +__all__ = ("C", "D") + +class C: ... +class D: ... +``` + +`exporter_annotated.py`: + +```py +__all__ = ("X",) + +class X: ... + +__all__: tuple[str, ...] = ("Y", "Z") + +class Y: ... +class Z: ... +``` + +`importer.py`: + +```py +import exporter +import exporter_annotated +from ty_extensions import dunder_all_names + +# revealed: tuple[Literal["C"], Literal["D"]] +reveal_type(dunder_all_names(exporter)) + +# revealed: tuple[Literal["Y"], Literal["Z"]] +reveal_type(dunder_all_names(exporter_annotated)) +``` + +### Augmenting list with a list or submodule `__all__` + +`subexporter.py`: + +```py +__all__ = ["A", "B"] + +class A: ... +class B: ... +``` + +`exporter.py`: + +```py +import subexporter + +__all__ = [] +__all__ += ["C", "D"] +__all__ += subexporter.__all__ + +class C: ... +class D: ... +``` + +`importer.py`: + +```py +import exporter +from ty_extensions import dunder_all_names + +# revealed: tuple[Literal["A"], Literal["B"], Literal["C"], Literal["D"]] +reveal_type(dunder_all_names(exporter)) +``` + +### Extending with a list or submodule `__all__` + +`subexporter.py`: + +```py +__all__ = ["A", "B"] + +class A: ... +class B: ... +``` + +`exporter.py`: + +```py +import subexporter + +__all__ = [] +__all__.extend(["C", "D"]) +__all__.extend(subexporter.__all__) + +class C: ... +class D: ... +``` + +`importer.py`: + +```py +import exporter +from ty_extensions import dunder_all_names + +# revealed: tuple[Literal["A"], Literal["B"], Literal["C"], Literal["D"]] +reveal_type(dunder_all_names(exporter)) +``` + +### Appending a single symbol + +`exporter.py`: + +```py +__all__ = ["A"] +__all__.append("B") + +class A: ... +class B: ... +``` + +`importer.py`: + +```py +import exporter +from ty_extensions import dunder_all_names + +# revealed: tuple[Literal["A"], Literal["B"]] +reveal_type(dunder_all_names(exporter)) +``` + +### Removing a single symbol + +`exporter.py`: + +```py +__all__ = ["A", "B"] +__all__.remove("A") + +# Non-existant symbol in `__all__` at this point +# TODO: This raises `ValueError` at runtime, maybe we should raise a diagnostic as well? +__all__.remove("C") + +class A: ... +class B: ... +``` + +`importer.py`: + +```py +import exporter +from ty_extensions import dunder_all_names + +# revealed: tuple[Literal["B"]] +reveal_type(dunder_all_names(exporter)) +``` + +### Mixed + +`subexporter.py`: + +```py +__all__ = [] + +__all__ = ["A"] +__all__.append("B") +__all__.extend(["C"]) +__all__.remove("B") + +class A: ... +class B: ... +class C: ... +``` + +`exporter.py`: + +```py +import subexporter + +__all__ = [] +__all__ += ["D"] +__all__ += subexporter.__all__ + +class D: ... +``` + +`importer.py`: + +```py +import exporter +from ty_extensions import dunder_all_names + +# revealed: tuple[Literal["A"], Literal["C"], Literal["D"]] +reveal_type(dunder_all_names(exporter)) +``` + +## Invalid + +### Unsupported idioms + +Idioms that are not mentioned in the [specification] are not recognized by `ty` and if they're used, +`__all__` is considered to be undefined for that module. This is to avoid false positives. + +`bar.py`: + +```py +__all__ = ["A", "B"] + +class A: ... +class B: ... +``` + +`foo.py`: + +```py +import bar as bar +``` + +`exporter.py`: + +```py +import foo +from ty_extensions import dunder_all_names + +__all__ = [] + +# revealed: tuple[Literal["A"], Literal["B"]] +reveal_type(dunder_all_names(foo.bar)) + +# Only direct attribute access of modules are recognized +# TODO: warning diagnostic +__all__.extend(foo.bar.__all__) +# TODO: warning diagnostic +__all__ += foo.bar.__all__ + +# Augmented assignment is only allowed when the value is a list expression +# TODO: warning diagnostic +__all__ += ("C",) + +# Other methods on `list` are not recognized +# TODO: warning diagnostic +__all__.insert(0, "C") +# TODO: warning diagnostic +__all__.clear() + +__all__.append("C") +# `pop` is not valid; use `remove` instead +# TODO: warning diagnostic +__all__.pop() + +# Sets are not recognized +# TODO: warning diagnostic +__all__ = {"C", "D"} + +class C: ... +class D: ... +``` + +`importer.py`: + +```py +import exporter +from ty_extensions import dunder_all_names + +# revealed: None +reveal_type(dunder_all_names(exporter)) +``` + +### Non-string elements + +Similarly, if `__all__` contains any non-string elements, we will consider `__all__` to not be +defined for that module. This is also to avoid false positives. + +`subexporter.py`: + +```py +__all__ = ("A", "B") + +class A: ... +class B: ... +``` + +`exporter1.py`: + +```py +import subexporter +from ty_extensions import dunder_all_names + +# revealed: tuple[Literal["A"], Literal["B"]] +reveal_type(dunder_all_names(subexporter)) + +# TODO: warning diagnostic +__all__ = ("C", *subexporter.__all__) + +class C: ... +``` + +`importer.py`: + +```py +import exporter1 +from ty_extensions import dunder_all_names + +# revealed: None +reveal_type(dunder_all_names(exporter1)) +``` + +## Statically known branches + +### Python 3.10 + +```toml +[environment] +python-version = "3.10" +``` + +`exporter.py`: + +```py +import sys + +__all__ = ["AllVersion"] + +if sys.version_info >= (3, 12): + __all__ += ["Python312"] +elif sys.version_info >= (3, 11): + __all__ += ["Python311"] +else: + __all__ += ["Python310"] + +class AllVersion: ... + +if sys.version_info >= (3, 12): + class Python312: ... + +elif sys.version_info >= (3, 11): + class Python311: ... + +else: + class Python310: ... +``` + +`importer.py`: + +```py +import exporter +from ty_extensions import dunder_all_names + +# revealed: tuple[Literal["AllVersion"], Literal["Python310"]] +reveal_type(dunder_all_names(exporter)) +``` + +### Python 3.11 + +```toml +[environment] +python-version = "3.11" +``` + +`exporter.py`: + +```py +import sys + +__all__ = ["AllVersion"] + +if sys.version_info >= (3, 12): + __all__ += ["Python312"] +elif sys.version_info >= (3, 11): + __all__ += ["Python311"] +else: + __all__ += ["Python310"] + +class AllVersion: ... + +if sys.version_info >= (3, 12): + class Python312: ... + +elif sys.version_info >= (3, 11): + class Python311: ... + +else: + class Python310: ... +``` + +`importer.py`: + +```py +import exporter +from ty_extensions import dunder_all_names + +# revealed: tuple[Literal["AllVersion"], Literal["Python311"]] +reveal_type(dunder_all_names(exporter)) +``` + +### Python 3.12 + +```toml +[environment] +python-version = "3.12" +``` + +`exporter.py`: + +```py +import sys + +__all__ = ["AllVersion"] + +if sys.version_info >= (3, 12): + __all__ += ["Python312"] +elif sys.version_info >= (3, 11): + __all__ += ["Python311"] +else: + __all__ += ["Python310"] + +class AllVersion: ... + +if sys.version_info >= (3, 12): + class Python312: ... + +elif sys.version_info >= (3, 11): + class Python311: ... + +else: + class Python310: ... +``` + +`importer.py`: + +```py +import exporter +from ty_extensions import dunder_all_names + +# revealed: tuple[Literal["AllVersion"], Literal["Python312"]] +reveal_type(dunder_all_names(exporter)) +``` + +### Multiple `if` statements + +```toml +[environment] +python-version = "3.11" +``` + +`exporter.py`: + +```py +import sys + +__all__ = ["AllVersion"] + +if sys.version_info >= (3, 12): + __all__ += ["Python312"] + +if sys.version_info >= (3, 11): + __all__ += ["Python311"] + +if sys.version_info >= (3, 10): + __all__ += ["Python310"] + +class AllVersion: ... + +if sys.version_info >= (3, 12): + class Python312: ... + +if sys.version_info >= (3, 11): + class Python311: ... + +if sys.version_info >= (3, 10): + class Python310: ... +``` + +`importer.py`: + +```py +import exporter +from ty_extensions import dunder_all_names + +# revealed: tuple[Literal["AllVersion"], Literal["Python310"], Literal["Python311"]] +reveal_type(dunder_all_names(exporter)) +``` + +## Origin + +`__all__` can be defined in a module mainly in the following three ways: + +### Directly in the module + +`exporter.py`: + +```py +__all__ = ["A"] + +class A: ... +``` + +`importer.py`: + +```py +import exporter +from ty_extensions import dunder_all_names + +# revealed: tuple[Literal["A"]] +reveal_type(dunder_all_names(exporter)) +``` + +### Using named import + +`subexporter.py`: + +```py +__all__ = ["A"] + +class A: ... +``` + +`exporter.py`: + +```py +from subexporter import __all__ + +__all__.append("B") + +class B: ... +``` + +`importer.py`: + +```py +import exporter +import subexporter +from ty_extensions import dunder_all_names + +# revealed: tuple[Literal["A"]] +reveal_type(dunder_all_names(subexporter)) +# revealed: tuple[Literal["A"], Literal["B"]] +reveal_type(dunder_all_names(exporter)) +``` + +### Using wildcard import (1) + +Wildcard import doesn't export `__all__` unless it is explicitly included in the `__all__` of the +module. + +`subexporter.py`: + +```py +__all__ = ["A", "__all__"] + +class A: ... +``` + +`exporter.py`: + +```py +from subexporter import * + +# TODO: Should be `list[str]` +# TODO: Should we avoid including `Unknown` for this case? +reveal_type(__all__) # revealed: Unknown | list + +__all__.append("B") + +class B: ... +``` + +`importer.py`: + +```py +import exporter +import subexporter +from ty_extensions import dunder_all_names + +# revealed: tuple[Literal["A"], Literal["__all__"]] +reveal_type(dunder_all_names(subexporter)) +# revealed: tuple[Literal["A"], Literal["B"], Literal["__all__"]] +reveal_type(dunder_all_names(exporter)) +``` + +### Using wildcard import (2) + +`subexporter.py`: + +```py +__all__ = ["A"] + +class A: ... +``` + +`exporter.py`: + +```py +from subexporter import * + +# error: [unresolved-reference] +reveal_type(__all__) # revealed: Unknown + +# error: [unresolved-reference] +__all__.append("B") + +class B: ... +``` + +`importer.py`: + +```py +import exporter +import subexporter +from ty_extensions import dunder_all_names + +# revealed: tuple[Literal["A"]] +reveal_type(dunder_all_names(subexporter)) +# revealed: None +reveal_type(dunder_all_names(exporter)) +``` + +[specification]: https://typing.python.org/en/latest/spec/distributing.html#library-interface-public-and-private-symbols diff --git a/crates/ty_python_semantic/resources/mdtest/import/star.md b/crates/ty_python_semantic/resources/mdtest/import/star.md index 0603058358..4894074f2d 100644 --- a/crates/ty_python_semantic/resources/mdtest/import/star.md +++ b/crates/ty_python_semantic/resources/mdtest/import/star.md @@ -899,8 +899,8 @@ reveal_type(__protected) # revealed: bool reveal_type(__dunder__) # revealed: bool reveal_type(___thunder___) # revealed: bool -# TODO: should emit [unresolved-reference] diagnostic & reveal `Unknown` -reveal_type(Y) # revealed: bool +# error: [unresolved-reference] +reveal_type(Y) # revealed: Unknown ``` ### Simple list `__all__` @@ -921,8 +921,8 @@ from exporter import * reveal_type(X) # revealed: bool -# TODO: should emit [unresolved-reference] diagnostic & reveal `Unknown` -reveal_type(Y) # revealed: bool +# error: [unresolved-reference] +reveal_type(Y) # revealed: Unknown ``` ### `__all__` with additions later on in the global scope @@ -949,15 +949,13 @@ __all__ = ["A"] __all__ += ["B"] __all__.append("C") __all__.extend(["D"]) -__all__.extend(("E",)) __all__.extend(a.__all__) A: bool = True B: bool = True C: bool = True D: bool = True -E: bool = True -F: bool = False +E: bool = False ``` `c.py`: @@ -969,11 +967,10 @@ reveal_type(A) # revealed: bool reveal_type(B) # revealed: bool reveal_type(C) # revealed: bool reveal_type(D) # revealed: bool -reveal_type(E) # revealed: bool reveal_type(FOO) # revealed: bool -# TODO should error with [unresolved-reference] & reveal `Unknown` -reveal_type(F) # revealed: bool +# error: [unresolved-reference] +reveal_type(E) # revealed: Unknown ``` ### `__all__` with subtractions later on in the global scope @@ -985,7 +982,7 @@ one way of subtracting from `__all__` that type checkers are required to support ```py __all__ = ["A", "B"] -__all__.remove("A") +__all__.remove("B") A: bool = True B: bool = True @@ -998,8 +995,8 @@ from exporter import * reveal_type(A) # revealed: bool -# TODO should emit an [unresolved-reference] diagnostic & reveal `Unknown` -reveal_type(B) # revealed: bool +# error: [unresolved-reference] +reveal_type(B) # revealed: Unknown ``` ### Invalid `__all__` @@ -1125,8 +1122,8 @@ else: ```py from exporter import * -# TODO: should reveal `Unknown` and emit `[unresolved-reference]` -reveal_type(X) # revealed: bool +# error: [unresolved-reference] +reveal_type(X) # revealed: Unknown # error: [unresolved-reference] reveal_type(Y) # revealed: Unknown @@ -1199,8 +1196,8 @@ else: ```py from exporter import * -# TODO: should reveal `Unknown` & emit `[unresolved-reference] -reveal_type(X) # revealed: bool +# error: [unresolved-reference] +reveal_type(X) # revealed: Unknown # error: [unresolved-reference] reveal_type(Y) # revealed: Unknown @@ -1235,9 +1232,11 @@ __all__ = [] from a import * from b import * -# TODO: both of these should have [unresolved-reference] diagnostics and reveal `Unknown` -reveal_type(X) # revealed: bool -reveal_type(Y) # revealed: bool +# error: [unresolved-reference] +reveal_type(X) # revealed: Unknown + +# error: [unresolved-reference] +reveal_type(Y) # revealed: Unknown ``` ### `__all__` in a stub file @@ -1257,7 +1256,11 @@ Y: bool = True ```pyi from a import X, Y -__all__ = ["X"] +__all__ = ["X", "Z"] + +Z: bool = True + +Nope: bool = True ``` `c.py`: @@ -1265,18 +1268,21 @@ __all__ = ["X"] ```py from b import * -# TODO: should not error, should reveal `bool` -# (`X` is re-exported from `b.pyi` due to presence in `__all__`) -# See https://github.com/astral-sh/ruff/issues/16159 -# -# error: [unresolved-reference] -reveal_type(X) # revealed: Unknown +# `X` is re-exported from `b.pyi` due to presence in `__all__` +reveal_type(X) # revealed: bool # This diagnostic is accurate: `Y` does not use the "redundant alias" convention in `b.pyi`, -# nor is it included in `b.__all__`, so it is not exported from `b.pyi` +# nor is it included in `b.__all__`, so it is not exported from `b.pyi`. It would still be +# an error if it used the "redundant alias" convention as `__all__` would take precedence. # # error: [unresolved-reference] reveal_type(Y) # revealed: Unknown + +# `Z` is defined in `b.pyi` and included in `__all__` +reveal_type(Z) # revealed: bool + +# error: [unresolved-reference] +reveal_type(Nope) # revealed: Unknown ``` ## `global` statements in non-global scopes @@ -1355,10 +1361,7 @@ import collections.abc reveal_type(collections.abc.Sequence) # revealed: reveal_type(collections.abc.Callable) # revealed: typing.Callable - -# TODO: false positive as it's only re-exported from `_collections.abc` due to presence in `__all__` -# error: [unresolved-attribute] -reveal_type(collections.abc.Set) # revealed: Unknown +reveal_type(collections.abc.Set) # revealed: ``` ## Invalid `*` imports diff --git a/crates/ty_python_semantic/src/dunder_all.rs b/crates/ty_python_semantic/src/dunder_all.rs new file mode 100644 index 0000000000..73d1f034fb --- /dev/null +++ b/crates/ty_python_semantic/src/dunder_all.rs @@ -0,0 +1,470 @@ +use rustc_hash::FxHashSet; + +use ruff_db::files::File; +use ruff_db::parsed::parsed_module; +use ruff_python_ast::name::Name; +use ruff_python_ast::statement_visitor::{walk_stmt, StatementVisitor}; +use ruff_python_ast::{self as ast}; + +use crate::semantic_index::ast_ids::{HasScopedExpressionId, HasScopedUseId}; +use crate::semantic_index::symbol::ScopeId; +use crate::semantic_index::{global_scope, semantic_index, SemanticIndex}; +use crate::symbol::{symbol_from_bindings, Boundness, Symbol}; +use crate::types::{infer_expression_types, Truthiness}; +use crate::{resolve_module, Db, ModuleName}; + +#[allow(clippy::ref_option)] +fn dunder_all_names_cycle_recover( + _db: &dyn Db, + _value: &Option>, + _count: u32, + _file: File, +) -> salsa::CycleRecoveryAction>> { + salsa::CycleRecoveryAction::Iterate +} + +fn dunder_all_names_cycle_initial(_db: &dyn Db, _file: File) -> Option> { + None +} + +/// Returns a set of names in the `__all__` variable for `file`, [`None`] if it is not defined or +/// if it contains invalid elements. +pub(crate) fn dunder_all_names(db: &dyn Db, file: File) -> Option<&FxHashSet> { + #[allow(clippy::ref_option)] + #[salsa::tracked(return_ref, cycle_fn=dunder_all_names_cycle_recover, cycle_initial=dunder_all_names_cycle_initial)] + fn dunder_all_names_impl(db: &dyn Db, file: File) -> Option> { + let _span = tracing::trace_span!("dunder_all_names", file=?file.path(db)).entered(); + + let module = parsed_module(db.upcast(), file); + let index = semantic_index(db, file); + let mut collector = DunderAllNamesCollector::new(db, file, index); + collector.visit_body(module.suite()); + collector.into_names() + } + + dunder_all_names_impl(db, file).as_ref() +} + +/// A visitor that collects the names in the `__all__` variable of a module. +struct DunderAllNamesCollector<'db> { + db: &'db dyn Db, + file: File, + + /// The scope in which the `__all__` names are being collected from. + /// + /// This is always going to be the global scope of the module. + scope: ScopeId<'db>, + + /// The semantic index for the module. + index: &'db SemanticIndex<'db>, + + /// The origin of the `__all__` variable in the current module, [`None`] if it is not defined. + origin: Option, + + /// A flag indicating whether the module uses unrecognized `__all__` idioms or there are any + /// invalid elements in `__all__`. + invalid: bool, + + /// A set of names found in `__all__` for the current module. + names: FxHashSet, +} + +impl<'db> DunderAllNamesCollector<'db> { + fn new(db: &'db dyn Db, file: File, index: &'db SemanticIndex<'db>) -> Self { + Self { + db, + file, + scope: global_scope(db, file), + index, + origin: None, + invalid: false, + names: FxHashSet::default(), + } + } + + /// Updates the origin of `__all__` in the current module. + /// + /// This will clear existing names if the origin is changed to mimic the behavior of overriding + /// `__all__` in the current module. + fn update_origin(&mut self, origin: DunderAllOrigin) { + if self.origin.is_some() { + self.names.clear(); + } + self.origin = Some(origin); + } + + /// Extends the current set of names with the names from the given expression which can be + /// either a list of names or a module's `__all__` variable. + /// + /// Returns `true` if the expression is a valid list or module `__all__`, `false` otherwise. + fn extend_from_list_or_module(&mut self, expr: &ast::Expr) -> bool { + match expr { + // `__all__ += [...]` + // `__all__.extend([...])` + ast::Expr::List(ast::ExprList { elts, .. }) => self.add_names(elts), + + // `__all__ += module.__all__` + // `__all__.extend(module.__all__)` + ast::Expr::Attribute(ast::ExprAttribute { value, attr, .. }) => { + if attr != "__all__" { + return false; + } + let Some(name_node) = value.as_name_expr() else { + return false; + }; + let Symbol::Type(ty, Boundness::Bound) = symbol_from_bindings( + self.db, + self.index + .use_def_map(self.scope.file_scope_id(self.db)) + .bindings_at_use(name_node.scoped_use_id(self.db, self.scope)), + ) else { + return false; + }; + let Some(module_literal) = ty.into_module_literal() else { + return false; + }; + let Some(module_dunder_all_names) = + dunder_all_names(self.db, module_literal.module(self.db).file()) + else { + // The module either does not have a `__all__` variable or it is invalid. + return false; + }; + self.names.extend(module_dunder_all_names.iter().cloned()); + true + } + + _ => false, + } + } + + /// Processes a call idiom for `__all__` and updates the set of names accordingly. + /// + /// Returns `true` if the call idiom is recognized and valid, `false` otherwise. + fn process_call_idiom( + &mut self, + function_name: &ast::Identifier, + arguments: &ast::Arguments, + ) -> bool { + if arguments.len() != 1 { + return false; + } + let Some(argument) = arguments.find_positional(0) else { + return false; + }; + match function_name.as_str() { + // `__all__.extend([...])` + // `__all__.extend(module.__all__)` + "extend" => { + if !self.extend_from_list_or_module(argument) { + return false; + } + } + + // `__all__.append(...)` + "append" => { + let Some(name) = create_name(argument) else { + return false; + }; + self.names.insert(name); + } + + // `__all__.remove(...)` + "remove" => { + let Some(name) = create_name(argument) else { + return false; + }; + self.names.remove(&name); + } + + _ => return false, + } + true + } + + /// Returns the names in `__all__` from the module imported from the given `import_from` + /// statement. + /// + /// Returns [`None`] if module resolution fails, invalid syntax, or if the module does not have + /// a `__all__` variable. + fn dunder_all_names_for_import_from( + &self, + import_from: &ast::StmtImportFrom, + ) -> Option<&'db FxHashSet> { + let module_name = + ModuleName::from_import_statement(self.db, self.file, import_from).ok()?; + let module = resolve_module(self.db, &module_name)?; + dunder_all_names(self.db, module.file()) + } + + /// Evaluate the given expression and return its truthiness. + /// + /// Returns [`None`] if the expression type doesn't implement `__bool__` correctly. + fn evaluate_test_expr(&self, expr: &ast::Expr) -> Option { + infer_expression_types(self.db, self.index.expression(expr)) + .expression_type(expr.scoped_expression_id(self.db, self.scope)) + .try_bool(self.db) + .ok() + } + + /// Add valid names to the set. + /// + /// Returns `false` if any of the names are invalid. + fn add_names(&mut self, exprs: &[ast::Expr]) -> bool { + for expr in exprs { + let Some(name) = create_name(expr) else { + return false; + }; + self.names.insert(name); + } + true + } + + /// Consumes `self` and returns the collected set of names. + /// + /// Returns [`None`] if `__all__` is not defined in the current module or if it contains + /// invalid elements. + fn into_names(self) -> Option> { + if self.origin.is_none() { + None + } else if self.invalid { + tracing::debug!("Invalid `__all__` in `{}`", self.file.path(self.db)); + None + } else { + Some(self.names) + } + } +} + +impl<'db> StatementVisitor<'db> for DunderAllNamesCollector<'db> { + fn visit_stmt(&mut self, stmt: &'db ast::Stmt) { + if self.invalid { + return; + } + + match stmt { + ast::Stmt::ImportFrom(import_from @ ast::StmtImportFrom { names, .. }) => { + for ast::Alias { name, asname, .. } in names { + // `from module import *` where `module` is a module with a top-level `__all__` + // variable that contains the "__all__" element. + if name == "*" { + // Here, we need to use the `dunder_all_names` query instead of the + // `exported_names` query because a `*`-import does not import the + // `__all__` attribute unless it is explicitly included in the `__all__` of + // the module. + let Some(all_names) = self.dunder_all_names_for_import_from(import_from) + else { + self.invalid = true; + continue; + }; + + if all_names.contains(&Name::new_static("__all__")) { + self.update_origin(DunderAllOrigin::StarImport); + self.names.extend(all_names.iter().cloned()); + } + } else { + // `from module import __all__` + // `from module import __all__ as __all__` + if name != "__all__" + || asname.as_ref().is_some_and(|asname| asname != "__all__") + { + continue; + } + + // We could do the `__all__` lookup lazily in case it's not needed. This would + // happen if a `__all__` is imported from another module but then the module + // redefines it. For example: + // + // ```python + // from module import __all__ as __all__ + // + // __all__ = ["a", "b"] + // ``` + // + // I'm avoiding this for now because it doesn't seem likely to happen in + // practice. + let Some(all_names) = self.dunder_all_names_for_import_from(import_from) + else { + self.invalid = true; + continue; + }; + + self.update_origin(DunderAllOrigin::ExternalModule); + self.names.extend(all_names.iter().cloned()); + } + } + } + + ast::Stmt::Assign(ast::StmtAssign { targets, value, .. }) => { + let [target] = targets.as_slice() else { + return; + }; + if !is_dunder_all(target) { + return; + } + match &**value { + // `__all__ = [...]` + // `__all__ = (...)` + ast::Expr::List(ast::ExprList { elts, .. }) + | ast::Expr::Tuple(ast::ExprTuple { elts, .. }) => { + self.update_origin(DunderAllOrigin::CurrentModule); + if !self.add_names(elts) { + self.invalid = true; + } + } + _ => { + self.invalid = true; + } + } + } + + ast::Stmt::AugAssign(ast::StmtAugAssign { + target, + op: ast::Operator::Add, + value, + .. + }) => { + if self.origin.is_none() { + // We can't update `__all__` if it doesn't already exist. + return; + } + if !is_dunder_all(target) { + return; + } + if !self.extend_from_list_or_module(value) { + self.invalid = true; + } + } + + ast::Stmt::AnnAssign(ast::StmtAnnAssign { + target, + value: Some(value), + .. + }) => { + if !is_dunder_all(target) { + return; + } + match &**value { + // `__all__: list[str] = [...]` + // `__all__: tuple[str, ...] = (...)` + ast::Expr::List(ast::ExprList { elts, .. }) + | ast::Expr::Tuple(ast::ExprTuple { elts, .. }) => { + self.update_origin(DunderAllOrigin::CurrentModule); + if !self.add_names(elts) { + self.invalid = true; + } + } + _ => { + self.invalid = true; + } + } + } + + ast::Stmt::Expr(ast::StmtExpr { value: expr, .. }) => { + if self.origin.is_none() { + // We can't update `__all__` if it doesn't already exist. + return; + } + let Some(ast::ExprCall { + func, arguments, .. + }) = expr.as_call_expr() + else { + return; + }; + let Some(ast::ExprAttribute { + value, + attr, + ctx: ast::ExprContext::Load, + .. + }) = func.as_attribute_expr() + else { + return; + }; + if !is_dunder_all(value) { + return; + } + if !self.process_call_idiom(attr, arguments) { + self.invalid = true; + } + } + + ast::Stmt::If(ast::StmtIf { + test, + body, + elif_else_clauses, + .. + }) => match self.evaluate_test_expr(test) { + Some(Truthiness::AlwaysTrue) => self.visit_body(body), + Some(Truthiness::AlwaysFalse) => { + for ast::ElifElseClause { test, body, .. } in elif_else_clauses { + if let Some(test) = test { + match self.evaluate_test_expr(test) { + Some(Truthiness::AlwaysTrue) => { + self.visit_body(body); + break; + } + Some(Truthiness::AlwaysFalse) => {} + Some(Truthiness::Ambiguous) | None => { + break; + } + } + } else { + self.visit_body(body); + } + } + } + Some(Truthiness::Ambiguous) | None => {} + }, + + ast::Stmt::For(..) + | ast::Stmt::While(..) + | ast::Stmt::With(..) + | ast::Stmt::Match(..) + | ast::Stmt::Try(..) => { + walk_stmt(self, stmt); + } + + ast::Stmt::FunctionDef(..) | ast::Stmt::ClassDef(..) => { + // Avoid recursing into any nested scopes as `__all__` is only valid at the module + // level. + } + + ast::Stmt::AugAssign(..) + | ast::Stmt::AnnAssign(..) + | ast::Stmt::Delete(..) + | ast::Stmt::Return(..) + | ast::Stmt::Raise(..) + | ast::Stmt::Assert(..) + | ast::Stmt::Import(..) + | ast::Stmt::Global(..) + | ast::Stmt::Nonlocal(..) + | ast::Stmt::TypeAlias(..) + | ast::Stmt::Pass(..) + | ast::Stmt::Break(..) + | ast::Stmt::Continue(..) + | ast::Stmt::IpyEscapeCommand(..) => {} + } + } +} + +#[derive(Debug, Clone)] +enum DunderAllOrigin { + /// The `__all__` variable is defined in the current module. + CurrentModule, + + /// The `__all__` variable is imported from another module. + ExternalModule, + + /// The `__all__` variable is imported from a module via a `*`-import. + StarImport, +} + +/// Checks if the given expression is a name expression for `__all__`. +fn is_dunder_all(expr: &ast::Expr) -> bool { + matches!(expr, ast::Expr::Name(ast::ExprName { id, .. }) if id == "__all__") +} + +/// Create and return a [`Name`] from the given expression, [`None`] if it is an invalid expression +/// for a `__all__` element. +fn create_name(expr: &ast::Expr) -> Option { + Some(Name::new(expr.as_string_literal_expr()?.value.to_str())) +} diff --git a/crates/ty_python_semantic/src/lib.rs b/crates/ty_python_semantic/src/lib.rs index fd3b35e4df..b813e92ae7 100644 --- a/crates/ty_python_semantic/src/lib.rs +++ b/crates/ty_python_semantic/src/lib.rs @@ -14,6 +14,7 @@ pub use site_packages::SysPrefixPathOrigin; pub mod ast_node_ref; mod db; +mod dunder_all; pub mod lint; pub(crate) mod list; mod module_name; diff --git a/crates/ty_python_semantic/src/semantic_index/visibility_constraints.rs b/crates/ty_python_semantic/src/semantic_index/visibility_constraints.rs index 98175af6bf..4c656963a1 100644 --- a/crates/ty_python_semantic/src/semantic_index/visibility_constraints.rs +++ b/crates/ty_python_semantic/src/semantic_index/visibility_constraints.rs @@ -178,12 +178,13 @@ use std::cmp::Ordering; use ruff_index::{Idx, IndexVec}; use rustc_hash::FxHashMap; +use crate::dunder_all::dunder_all_names; use crate::semantic_index::expression::Expression; use crate::semantic_index::predicate::{ PatternPredicate, PatternPredicateKind, Predicate, PredicateNode, Predicates, ScopedPredicateId, }; use crate::semantic_index::symbol_table; -use crate::symbol::imported_symbol; +use crate::symbol::{imported_symbol, RequiresExplicitReExport}; use crate::types::{infer_expression_type, Truthiness, Type}; use crate::Db; @@ -655,7 +656,27 @@ impl VisibilityConstraints { PredicateNode::StarImportPlaceholder(star_import) => { let symbol_table = symbol_table(db, star_import.scope(db)); let symbol_name = symbol_table.symbol(star_import.symbol_id(db)).name(); - match imported_symbol(db, star_import.referenced_file(db), symbol_name).symbol { + let referenced_file = star_import.referenced_file(db); + + let requires_explicit_reexport = match dunder_all_names(db, referenced_file) { + Some(all_names) => { + if all_names.contains(symbol_name) { + Some(RequiresExplicitReExport::No) + } else { + tracing::debug!( + "Symbol `{}` (via star import) not found in `__all__` of `{}`", + symbol_name, + referenced_file.path(db) + ); + return Truthiness::AlwaysFalse; + } + } + None => None, + }; + + match imported_symbol(db, referenced_file, symbol_name, requires_explicit_reexport) + .symbol + { crate::symbol::Symbol::Type(_, crate::symbol::Boundness::Bound) => { Truthiness::AlwaysTrue } diff --git a/crates/ty_python_semantic/src/symbol.rs b/crates/ty_python_semantic/src/symbol.rs index 767d4392f2..6f28867a4c 100644 --- a/crates/ty_python_semantic/src/symbol.rs +++ b/crates/ty_python_semantic/src/symbol.rs @@ -1,5 +1,6 @@ use ruff_db::files::File; +use crate::dunder_all::dunder_all_names; use crate::module_resolver::file_to_module; use crate::semantic_index::definition::Definition; use crate::semantic_index::symbol::{ScopeId, ScopedSymbolId}; @@ -285,11 +286,23 @@ pub(crate) fn global_symbol<'db>( } /// Infers the public type of an imported symbol. +/// +/// If `requires_explicit_reexport` is [`None`], it will be inferred from the file's source type. +/// For stub files, explicit re-export will be required, while for non-stub files, it will not. pub(crate) fn imported_symbol<'db>( db: &'db dyn Db, file: File, name: &str, + requires_explicit_reexport: Option, ) -> SymbolAndQualifiers<'db> { + let requires_explicit_reexport = requires_explicit_reexport.unwrap_or_else(|| { + if file.is_stub(db.upcast()) { + RequiresExplicitReExport::Yes + } else { + RequiresExplicitReExport::No + } + }); + // If it's not found in the global scope, check if it's present as an instance on // `types.ModuleType` or `builtins.object`. // @@ -305,13 +318,16 @@ pub(crate) fn imported_symbol<'db>( // ignore `__getattr__`. Typeshed has a fake `__getattr__` on `types.ModuleType` to help out with // dynamic imports; we shouldn't use it for `ModuleLiteral` types where we know exactly which // module we're dealing with. - external_symbol_impl(db, file, name).or_fall_back_to(db, || { - if name == "__getattr__" { - Symbol::Unbound.into() - } else { - KnownClass::ModuleType.to_instance(db).member(db, name) - } - }) + symbol_impl(db, global_scope(db, file), name, requires_explicit_reexport).or_fall_back_to( + db, + || { + if name == "__getattr__" { + Symbol::Unbound.into() + } else { + KnownClass::ModuleType.to_instance(db).member(db, name) + } + }, + ) } /// Lookup the type of `symbol` in the builtins namespace. @@ -324,7 +340,13 @@ pub(crate) fn imported_symbol<'db>( pub(crate) fn builtins_symbol<'db>(db: &'db dyn Db, symbol: &str) -> SymbolAndQualifiers<'db> { resolve_module(db, &KnownModule::Builtins.name()) .map(|module| { - external_symbol_impl(db, module.file(), symbol).or_fall_back_to(db, || { + symbol_impl( + db, + global_scope(db, module.file()), + symbol, + RequiresExplicitReExport::Yes, + ) + .or_fall_back_to(db, || { // We're looking up in the builtins namespace and not the module, so we should // do the normal lookup in `types.ModuleType` and not the special one as in // `imported_symbol`. @@ -343,7 +365,7 @@ pub(crate) fn known_module_symbol<'db>( symbol: &str, ) -> SymbolAndQualifiers<'db> { resolve_module(db, &known_module.name()) - .map(|module| imported_symbol(db, module.file(), symbol)) + .map(|module| imported_symbol(db, module.file(), symbol, None)) .unwrap_or_default() } @@ -702,7 +724,7 @@ fn symbol_from_bindings_impl<'db>( let mut bindings_with_constraints = bindings_with_constraints.peekable(); let is_non_exported = |binding: Definition<'db>| { - requires_explicit_reexport.is_yes() && !binding.is_reexported(db) + requires_explicit_reexport.is_yes() && !is_reexported(db, binding) }; let unbound_visibility_constraint = match bindings_with_constraints.peek() { @@ -833,7 +855,7 @@ fn symbol_from_declarations_impl<'db>( let mut declarations = declarations.peekable(); let is_non_exported = |declaration: Definition<'db>| { - requires_explicit_reexport.is_yes() && !declaration.is_reexported(db) + requires_explicit_reexport.is_yes() && !is_reexported(db, declaration) }; let undeclared_visibility = match declarations.peek() { @@ -911,6 +933,27 @@ fn symbol_from_declarations_impl<'db>( } } +// Returns `true` if the `definition` is re-exported. +// +// This will first check if the definition is using the "redundant alias" pattern like `import foo +// as foo` or `from foo import bar as bar`. If it's not, it will check whether the symbol is being +// exported via `__all__`. +fn is_reexported(db: &dyn Db, definition: Definition<'_>) -> bool { + // This information is computed by the semantic index builder. + if definition.is_reexported(db) { + return true; + } + // At this point, the definition should either be an `import` or `from ... import` statement. + // This is because the default value of `is_reexported` is `true` for any other kind of + // definition. + let Some(all_names) = dunder_all_names(db, definition.file(db)) else { + return false; + }; + let table = symbol_table(db, definition.scope(db)); + let symbol_name = table.symbol(definition.symbol(db)).name(); + all_names.contains(symbol_name) +} + mod implicit_globals { use ruff_python_ast as ast; @@ -1015,26 +1058,8 @@ mod implicit_globals { } } -/// Implementation of looking up a module-global symbol as seen from outside the file (e.g. via -/// imports). -/// -/// This will take into account whether the definition of the symbol is being explicitly -/// re-exported from a stub file or not. -fn external_symbol_impl<'db>(db: &'db dyn Db, file: File, name: &str) -> SymbolAndQualifiers<'db> { - symbol_impl( - db, - global_scope(db, file), - name, - if file.is_stub(db.upcast()) { - RequiresExplicitReExport::Yes - } else { - RequiresExplicitReExport::No - }, - ) -} - #[derive(Copy, Clone, Debug, PartialEq, Eq, Hash)] -enum RequiresExplicitReExport { +pub(crate) enum RequiresExplicitReExport { Yes, No, } diff --git a/crates/ty_python_semantic/src/types.rs b/crates/ty_python_semantic/src/types.rs index 7b26a77dc6..a8ff1fd8ab 100644 --- a/crates/ty_python_semantic/src/types.rs +++ b/crates/ty_python_semantic/src/types.rs @@ -6992,6 +6992,8 @@ pub enum KnownFunction { IsSingleValued, /// `ty_extensions.generic_context` GenericContext, + /// `ty_extensions.dunder_all_names` + DunderAllNames, } impl KnownFunction { @@ -7048,6 +7050,7 @@ impl KnownFunction { | Self::IsSingleton | Self::IsSubtypeOf | Self::GenericContext + | Self::DunderAllNames | Self::StaticAssert => module.is_ty_extensions(), } } @@ -7355,7 +7358,7 @@ impl<'db> ModuleLiteralType<'db> { } } - imported_symbol(db, self.module(db).file(), name).symbol + imported_symbol(db, self.module(db).file(), name, None).symbol } } @@ -8444,6 +8447,7 @@ pub(crate) mod tests { KnownFunction::IsSingleton | KnownFunction::IsSubtypeOf | KnownFunction::GenericContext + | KnownFunction::DunderAllNames | KnownFunction::StaticAssert | KnownFunction::IsFullyStatic | KnownFunction::IsDisjointFrom diff --git a/crates/ty_python_semantic/src/types/call/bind.rs b/crates/ty_python_semantic/src/types/call/bind.rs index 8f28f5f141..ccaae94f66 100644 --- a/crates/ty_python_semantic/src/types/call/bind.rs +++ b/crates/ty_python_semantic/src/types/call/bind.rs @@ -10,6 +10,7 @@ use super::{ InferContext, Signature, Signatures, Type, }; use crate::db::Db; +use crate::dunder_all::dunder_all_names; use crate::symbol::{Boundness, Symbol}; use crate::types::diagnostic::{ CALL_NON_CALLABLE, CONFLICTING_ARGUMENT_FORMS, INVALID_ARGUMENT_TYPE, MISSING_ARGUMENT, @@ -585,6 +586,30 @@ impl<'db> Bindings<'db> { } } + Some(KnownFunction::DunderAllNames) => { + if let [Some(ty)] = overload.parameter_types() { + overload.set_return_type(match ty { + Type::ModuleLiteral(module_literal) => { + match dunder_all_names(db, module_literal.module(db).file()) + { + Some(names) => { + let mut names = names.iter().collect::>(); + names.sort(); + TupleType::from_elements( + db, + names.iter().map(|name| { + Type::string_literal(db, name.as_str()) + }), + ) + } + None => Type::none(db), + } + } + _ => Type::none(db), + }); + } + } + Some(KnownFunction::Len) => { if let [Some(first_arg)] = overload.parameter_types() { if let Some(len_ty) = first_arg.len(db) { diff --git a/crates/ty_vendored/ty_extensions/ty_extensions.pyi b/crates/ty_vendored/ty_extensions/ty_extensions.pyi index 163a1d1cc6..0ed447e112 100644 --- a/crates/ty_vendored/ty_extensions/ty_extensions.pyi +++ b/crates/ty_vendored/ty_extensions/ty_extensions.pyi @@ -30,3 +30,7 @@ def is_single_valued(type: Any) -> bool: ... # Returns the generic context of a type as a tuple of typevars, or `None` if the # type is not generic. def generic_context(type: Any) -> Any: ... + +# Returns the `__all__` names of a module as a tuple of sorted strings, or `None` if +# either the module does not have `__all__` or it has invalid elements. +def dunder_all_names(module: Any) -> Any: ...