mirror of
https://github.com/astral-sh/ruff.git
synced 2025-11-18 19:41:34 +00:00
[ty] Make __getattr__ available for ModuleType instances (#21450)
## Summary Typeshed has a (fake) `__getattr__` method on `types.ModuleType` with a return type of `Any`. We ignore this method when accessing attributes on module *literals*, but with this PR, we respect this method when dealing with `ModuleType` itself. That is, we allow arbitrary attribute accesses on instances of `types.ModuleType`. This is useful because dynamic import mechanisms such as `importlib.import_module` use `ModuleType` as a return type. closes https://github.com/astral-sh/ty/issues/1346 ## Ecosystem Massive reduction in diagnostics. The few new diagnostics are true positives. ## Test Plan Added regression test.
This commit is contained in:
parent
d0314131fb
commit
6a26f86778
4 changed files with 44 additions and 25 deletions
|
|
@ -49,12 +49,9 @@ def _(flag: bool):
|
|||
a = reveal_type(__import__("a.b.c")) # revealed: ModuleType
|
||||
|
||||
# TODO: Should be `int`, `str`, `bytes`
|
||||
# error: [unresolved-attribute]
|
||||
reveal_type(a.a) # revealed: Unknown
|
||||
# error: [unresolved-attribute]
|
||||
reveal_type(a.b.b) # revealed: Unknown
|
||||
# error: [unresolved-attribute]
|
||||
reveal_type(a.b.c.c) # revealed: Unknown
|
||||
reveal_type(a.a) # revealed: Any
|
||||
reveal_type(a.b.b) # revealed: Any
|
||||
reveal_type(a.b.c.c) # revealed: Any
|
||||
```
|
||||
|
||||
`a/__init__.py`:
|
||||
|
|
|
|||
|
|
@ -120,6 +120,21 @@ we're dealing with:
|
|||
reveal_type(typing.__getattr__) # revealed: Unknown
|
||||
```
|
||||
|
||||
However, if we have a `ModuleType` instance, we make `__getattr__` available. This means that
|
||||
arbitrary attribute accesses are allowed (with a result type of `Any`):
|
||||
|
||||
```py
|
||||
import types
|
||||
|
||||
reveal_type(types.ModuleType.__getattr__) # revealed: def __getattr__(self, name: str) -> Any
|
||||
|
||||
def f(module: types.ModuleType):
|
||||
reveal_type(module.__getattr__) # revealed: bound method ModuleType.__getattr__(name: str) -> Any
|
||||
|
||||
reveal_type(module.__all__) # revealed: Any
|
||||
reveal_type(module.whatever) # revealed: Any
|
||||
```
|
||||
|
||||
## `types.ModuleType.__dict__` takes precedence over global variable `__dict__`
|
||||
|
||||
It's impossible to override the `__dict__` attribute of `types.ModuleType` instances from inside the
|
||||
|
|
|
|||
|
|
@ -10,9 +10,9 @@ use crate::semantic_index::{
|
|||
};
|
||||
use crate::semantic_index::{DeclarationWithConstraint, global_scope, use_def_map};
|
||||
use crate::types::{
|
||||
ApplyTypeMappingVisitor, DynamicType, KnownClass, MaterializationKind, Truthiness, Type,
|
||||
TypeAndQualifiers, TypeQualifiers, UnionBuilder, UnionType, binding_type, declaration_type,
|
||||
todo_type,
|
||||
ApplyTypeMappingVisitor, DynamicType, KnownClass, MaterializationKind, MemberLookupPolicy,
|
||||
Truthiness, Type, TypeAndQualifiers, TypeQualifiers, UnionBuilder, UnionType, binding_type,
|
||||
declaration_type, todo_type,
|
||||
};
|
||||
use crate::{Db, FxOrderSet, Program, resolve_module};
|
||||
|
||||
|
|
@ -364,7 +364,9 @@ pub(crate) fn imported_symbol<'db>(
|
|||
} else if name == "__builtins__" {
|
||||
Place::bound(Type::any()).into()
|
||||
} else {
|
||||
KnownClass::ModuleType.to_instance(db).member(db, name)
|
||||
KnownClass::ModuleType
|
||||
.to_instance(db)
|
||||
.member_lookup_with_policy(db, name.into(), MemberLookupPolicy::NO_GETATTR_LOOKUP)
|
||||
}
|
||||
})
|
||||
}
|
||||
|
|
@ -1374,7 +1376,9 @@ mod implicit_globals {
|
|||
use crate::place::{Definedness, PlaceAndQualifiers, TypeOrigin};
|
||||
use crate::semantic_index::symbol::Symbol;
|
||||
use crate::semantic_index::{place_table, use_def_map};
|
||||
use crate::types::{CallableType, KnownClass, Parameter, Parameters, Signature, Type};
|
||||
use crate::types::{
|
||||
CallableType, KnownClass, MemberLookupPolicy, Parameter, Parameters, Signature, Type,
|
||||
};
|
||||
use ruff_python_ast::PythonVersion;
|
||||
|
||||
use super::{Place, place_from_declarations};
|
||||
|
|
@ -1473,7 +1477,13 @@ mod implicit_globals {
|
|||
.iter()
|
||||
.any(|module_type_member| &**module_type_member == name) =>
|
||||
{
|
||||
KnownClass::ModuleType.to_instance(db).member(db, name)
|
||||
KnownClass::ModuleType
|
||||
.to_instance(db)
|
||||
.member_lookup_with_policy(
|
||||
db,
|
||||
name.into(),
|
||||
MemberLookupPolicy::NO_GETATTR_LOOKUP,
|
||||
)
|
||||
}
|
||||
|
||||
_ => Place::Undefined.into(),
|
||||
|
|
|
|||
|
|
@ -336,6 +336,9 @@ bitflags! {
|
|||
|
||||
/// Skip looking up attributes on the builtin `int` and `str` classes.
|
||||
const MRO_NO_INT_OR_STR_LOOKUP = 1 << 3;
|
||||
|
||||
/// Do not call `__getattr__` during member lookup.
|
||||
const NO_GETATTR_LOOKUP = 1 << 4;
|
||||
}
|
||||
}
|
||||
|
||||
|
|
@ -363,6 +366,11 @@ impl MemberLookupPolicy {
|
|||
pub(crate) const fn mro_no_int_or_str_fallback(self) -> bool {
|
||||
self.contains(Self::MRO_NO_INT_OR_STR_LOOKUP)
|
||||
}
|
||||
|
||||
/// Do not call `__getattr__` during member lookup.
|
||||
pub(crate) const fn no_getattr_lookup(self) -> bool {
|
||||
self.contains(Self::NO_GETATTR_LOOKUP)
|
||||
}
|
||||
}
|
||||
|
||||
impl Default for MemberLookupPolicy {
|
||||
|
|
@ -4222,7 +4230,7 @@ impl<'db> Type<'db> {
|
|||
/// Similar to [`Type::member`], but allows the caller to specify what policy should be used
|
||||
/// when looking up attributes. See [`MemberLookupPolicy`] for more information.
|
||||
#[salsa::tracked(cycle_initial=member_lookup_cycle_initial, heap_size=ruff_memory_usage::heap_size)]
|
||||
fn member_lookup_with_policy(
|
||||
pub(crate) fn member_lookup_with_policy(
|
||||
self,
|
||||
db: &'db dyn Db,
|
||||
name: Name,
|
||||
|
|
@ -4512,18 +4520,7 @@ impl<'db> Type<'db> {
|
|||
);
|
||||
|
||||
let custom_getattr_result = || {
|
||||
// Typeshed has a fake `__getattr__` on `types.ModuleType` to help out with
|
||||
// dynamic imports. We explicitly hide it here to prevent arbitrary attributes
|
||||
// from being available on modules. Same for `types.GenericAlias` - its
|
||||
// `__getattr__` method will delegate to `__origin__` to allow looking up
|
||||
// attributes on the original type. But in typeshed its return type is `Any`.
|
||||
// It will need a special handling, so it remember the origin type to properly
|
||||
// resolve the attribute.
|
||||
if matches!(
|
||||
self.as_nominal_instance()
|
||||
.and_then(|instance| instance.known_class(db)),
|
||||
Some(KnownClass::ModuleType | KnownClass::GenericAlias)
|
||||
) {
|
||||
if policy.no_getattr_lookup() {
|
||||
return Place::Undefined.into();
|
||||
}
|
||||
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue