mirror of
https://github.com/astral-sh/ruff.git
synced 2025-09-28 04:45:01 +00:00
[ty] Add support for __all__
(#17856)
## Summary This PR adds support for the `__all__` module variable. Reference spec: https://typing.python.org/en/latest/spec/distributing.html#library-interface-public-and-private-symbols This PR adds a new `dunder_all_names` query that returns a set of `Name`s defined in the `__all__` variable of the given `File`. The query works by implementing the `StatementVisitor` and collects all the names by recognizing the supported idioms as mentioned in the spec. Any idiom that's not recognized are ignored. The current implementation is minimum to what's required for us to remove all the false positives that this is causing. Refer to the "Follow-ups" section below to see what we can do next. I'll a open separate issue to keep track of them. Closes: astral-sh/ty#106 Closes: astral-sh/ty#199 ### Follow-ups * Diagnostics: * Add warning diagnostics for unrecognized `__all__` idioms, `__all__` containing non-string element * Add an error diagnostic for elements that are present in `__all__` but not defined in the module. This could lead to runtime error * Maybe we should return `<type>` instead of `Unknown | <type>` for `module.__all__`. For example: https://playknot.ruff.rs/2a6fe5d7-4e16-45b1-8ec3-d79f2d4ca894 * Mark a symbol that's mentioned in `__all__` as used otherwise it could raise (possibly in the future) "unused-name" diagnostic Supporting diagnostics will require that we update the return type of the query to be something other than `Option<FxHashSet<Name>>`, something that behaves like a result and provides a way to check whether a name exists in `__all__`, loop over elements in `__all__`, loop over the invalid elements, etc. ## Ecosystem analysis The following are the maximum amount of diagnostics **removed** in the ecosystem: * "Type <module '...'> has no attribute ..." * `collections.abc` - 14 * `numpy` - 35534 * `numpy.ma` - 296 * `numpy.char` - 37 * `numpy.testing` - 175 * `hashlib` - 311 * `scipy.fft` - 2 * `scipy.stats` - 38 * "Module '...' has no member ..." * `collections.abc` - 85 * `numpy` - 508 * `numpy.testing` - 741 * `hashlib` - 36 * `scipy.stats` - 68 * `scipy.interpolate` - 7 * `scipy.signal` - 5 The following modules have dynamic `__all__` definition, so `ty` assumes that `__all__` doesn't exists in that module: * `scipy.stats` (95a5d6ea8b/scipy/stats/__init__.py (L665)
) * `scipy.interpolate` (95a5d6ea8b/scipy/interpolate/__init__.py (L221)
) * `scipy.signal` (indirectly via95a5d6ea8b/scipy/signal/_signal_api.py (L30)
) * `numpy.testing` (de784cd6ee/numpy/testing/__init__.py (L16-L18)
) ~There's this one category of **false positives** that have been added:~ Fixed the false positives by also ignoring `__all__` from a module that uses unrecognized idioms. <details><summary>Details about the false postivie:</summary> <p> The `scipy.stats` module has dynamic `__all__` and it imports a bunch of symbols via star imports. Some of those modules have a mix of valid and invalid `__all__` idioms. For example, in95a5d6ea8b/scipy/stats/distributions.py (L18-L24)
, 2 out of 4 `__all__` idioms are invalid but currently `ty` recognizes two of them and says that the module has a `__all__` with 5 values. This leads to around **2055** newly added false positives of the form: ``` Type <module 'scipy.stats'> has no attribute ... ``` I think the fix here is to completely ignore `__all__`, not only if there are invalid elements in it, but also if there are unrecognized idioms used in the module. </p> </details> ## Test Plan Add a bunch of test cases using the new `ty_extensions.dunder_all_names` function to extract a module's `__all__` names. Update various test cases to remove false positives around `*` imports and re-export convention. Add new test cases for named import behavior as `*` imports covers all of it already (thanks Alex!).
This commit is contained in:
parent
c6f4929cdc
commit
78054824c0
11 changed files with 1456 additions and 69 deletions
|
@ -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
|
||||
}
|
||||
|
|
Loading…
Add table
Add a link
Reference in a new issue