mirrors/ruff
1
0
Fork 0
mirror of https://github.com/astral-sh/ruff.git synced 2025-11-19 20:24:27 +00:00
Code Issues Projects Releases Packages Wiki Activity Actions 4

[ty] prefer submodule over module __getattr__ in from-imports (#21260)

Browse source 
Fixes https://github.com/astral-sh/ty/issues/1053

## Summary

Other type checkers prioritize a submodule over a package `__getattr__`
in `from mod import sub`, even though the runtime precedence is the
other direction. In effect, this is making an implicit assumption that a
module `__getattr__` will not handle (that is, will raise
`AttributeError`) for names that are also actual submodules, rather than
shadowing them. In practice this seems like a realistic assumption in
the ecosystem? Or at least the ecosystem has adapted to it, and we need
to adapt this precedence also, for ecosystem compatibility.

The implementation is a bit ugly, precisely because it departs from the
runtime semantics, and our implementation is oriented toward modeling
runtime semantics accurately. That is, `__getattr__` is modeled within
the member-lookup code, so it's hard to split "member lookup result from
module `__getattr__`" apart from other member lookup results. I did this
via a synthetic `TypeQualifier::FROM_MODULE_GETATTR` that we attach to a
type resulting from a member lookup, which isn't beautiful but it works
well and doesn't introduce inefficiency (e.g. redundant member lookups).

## Test Plan

Updated mdtests.

Also added a related mdtest formalizing our support for a module
`__getattr__` that is explicitly annotated to accept a limited set of
names. In principle this could be an alternative (more explicit) way to
handle the precedence problem without departing from runtime semantics,
if the ecosystem would adopt it.

### Ecosystem analysis

Lots of removed diagnostics which are an improvement because we now
infer the expected submodule.

Added diagnostics are mostly unrelated issues surfaced now because we
previously had an earlier attribute error resulting in `Unknown`; now we
correctly resolve the module so that earlier attribute error goes away,
we get an actual type instead of `Unknown`, and that triggers a new
error.

In scipy and sklearn, the module `__getattr__` which we were respecting
previously is un-annotated so returned a forgiving `Unknown`; now we
correctly see the actual module, which reveals some cases of
https://github.com/astral-sh/ty/issues/133 that were previously hidden
(`scipy/optimize/__init__.py` [imports `from
._tnc`](eff82ca575/scipy/optimize/__init__.py (L429)).)

---------

Co-authored-by: Alex Waygood <Alex.Waygood@Gmail.com>
This commit is contained in:
Carl Meyer 2025-11-03 15:24:01 -05:00 • committed by GitHub
parent 0433526897
commit fe4ee81b97
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
3 changed files with 92 additions and 18 deletions
Download patch file Download diff file Expand all files Collapse all files

9
crates/ty_python_semantic/src/types.rs
View file

@ -7909,6 +7909,10 @@ bitflags! {
/// instance attributes that are only implicitly defined via `self.x = …` in
/// the body of a class method.
const IMPLICIT_INSTANCE_ATTRIBUTE = 1 << 6;
/// A non-standard type qualifier that marks a type returned from a module-level
/// `__getattr__` function. We need this in order to implement precedence of submodules
/// over module-level `__getattr__`, for compatibility with other type checkers.
const FROM_MODULE_GETATTR = 1 << 7;
}
}
@ -11026,7 +11030,10 @@ impl<'db> ModuleLiteralType<'db> {
db,
&CallArguments::positional([Type::string_literal(db, name)]),
) {
return Place::Defined(outcome.return_type(db), origin, boundness).into();
return PlaceAndQualifiers {
place: Place::Defined(outcome.return_type(db), origin, boundness),
qualifiers: TypeQualifiers::FROM_MODULE_GETATTR,
};
}
}
}

50
crates/ty_python_semantic/src/types/infer/builder.rs
View file

@ -5347,6 +5347,10 @@ impl<'db, 'ast> TypeInferenceBuilder<'db, 'ast> {
.as_module_literal()
.is_some_and(|module| Some(self.file()) == module.module(self.db()).file(self.db()));
// Although it isn't the runtime semantics, we go to some trouble to prioritize a submodule
// over module `__getattr__`, because that's what other type checkers do.
let mut from_module_getattr = None;
// First try loading the requested attribute from the module.
if !import_is_self_referential {
if let PlaceAndQualifiers {
@ -5366,19 +5370,23 @@ impl<'db, 'ast> TypeInferenceBuilder<'db, 'ast> {
));
}
}
self.add_declaration_with_binding(
alias.into(),
definition,
&DeclaredAndInferredType::MightBeDifferent {
declared_ty: TypeAndQualifiers {
inner: ty,
origin: TypeOrigin::Declared,
qualifiers,
if qualifiers.contains(TypeQualifiers::FROM_MODULE_GETATTR) {
from_module_getattr = Some((ty, qualifiers));
} else {
self.add_declaration_with_binding(
alias.into(),
definition,
&DeclaredAndInferredType::MightBeDifferent {
declared_ty: TypeAndQualifiers {
inner: ty,
origin: TypeOrigin::Declared,
qualifiers,
},
inferred_ty: ty,
},
inferred_ty: ty,
},
);
return;
);
return;
}
}
}
@ -5418,6 +5426,24 @@ impl<'db, 'ast> TypeInferenceBuilder<'db, 'ast> {
return;
}
// We've checked for a submodule, so now we can go ahead and use a type from module
// `__getattr__`.
if let Some((ty, qualifiers)) = from_module_getattr {
self.add_declaration_with_binding(
alias.into(),
definition,
&DeclaredAndInferredType::MightBeDifferent {
declared_ty: TypeAndQualifiers {
inner: ty,
origin: TypeOrigin::Declared,
qualifiers,
},
inferred_ty: ty,
},
);
return;
}
self.add_unknown_declaration_with_binding(alias.into(), definition);
if &alias.name == "*" {