From 6a26f86778d8fe7b253aa1b761d1e6374589b2af Mon Sep 17 00:00:00 2001 From: David Peter Date: Fri, 14 Nov 2025 13:59:14 +0100 Subject: [PATCH] [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. --- .../resources/mdtest/call/dunder_import.md | 9 +++----- .../mdtest/scopes/moduletype_attrs.md | 15 ++++++++++++ crates/ty_python_semantic/src/place.rs | 22 +++++++++++++----- crates/ty_python_semantic/src/types.rs | 23 ++++++++----------- 4 files changed, 44 insertions(+), 25 deletions(-) diff --git a/crates/ty_python_semantic/resources/mdtest/call/dunder_import.md b/crates/ty_python_semantic/resources/mdtest/call/dunder_import.md index 24e7db6449..7cbd382fcf 100644 --- a/crates/ty_python_semantic/resources/mdtest/call/dunder_import.md +++ b/crates/ty_python_semantic/resources/mdtest/call/dunder_import.md @@ -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`: diff --git a/crates/ty_python_semantic/resources/mdtest/scopes/moduletype_attrs.md b/crates/ty_python_semantic/resources/mdtest/scopes/moduletype_attrs.md index b9d92d7fed..3f84588803 100644 --- a/crates/ty_python_semantic/resources/mdtest/scopes/moduletype_attrs.md +++ b/crates/ty_python_semantic/resources/mdtest/scopes/moduletype_attrs.md @@ -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 diff --git a/crates/ty_python_semantic/src/place.rs b/crates/ty_python_semantic/src/place.rs index c0b3428345..0f22048ccc 100644 --- a/crates/ty_python_semantic/src/place.rs +++ b/crates/ty_python_semantic/src/place.rs @@ -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(), diff --git a/crates/ty_python_semantic/src/types.rs b/crates/ty_python_semantic/src/types.rs index b7f4327ad6..952609b3cd 100644 --- a/crates/ty_python_semantic/src/types.rs +++ b/crates/ty_python_semantic/src/types.rs @@ -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(); }