Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -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`:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
22 changes: 16 additions & 6 deletions crates/ty_python_semantic/src/place.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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};

Expand Down Expand Up @@ -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)
}
})
}
Expand Down Expand Up @@ -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};
Expand Down Expand Up @@ -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(),
Expand Down
23 changes: 10 additions & 13 deletions crates/ty_python_semantic/src/types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
}

Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't understand why the GenericAlias exception was here. No tests seem to fail when I remove it?

// `__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();
}

Expand Down
Loading