From 28bfcf82b8f0196bdbb2f3a724ce34b7ad8ff47a Mon Sep 17 00:00:00 2001 From: Alex Waygood Date: Fri, 16 Jan 2026 08:42:58 +0000 Subject: [PATCH] [ty] Make `ModuleType` and `object` attributes available on namespace packages (#22606) ## Summary Currently we don't think that namespace packages (e.g. `google` after you've pip-installed `google-cloud-ndb`) have attributes such as `__file__`, `__name__`, etc. This PR fixes that. ## Test Plan Mdtests and snapshots. --- crates/ty_ide/src/completion.rs | 33 +++++ .../mdtest/ide_support/all_members.md | 36 +++++ .../mdtest/import/dunder_file_attribute.md | 6 +- .../mdtest/scopes/moduletype_attrs.md | 25 +++- crates/ty_python_semantic/src/place.rs | 130 ++++++++---------- .../reachability_constraints.rs | 2 +- crates/ty_python_semantic/src/types.rs | 8 +- .../src/types/list_members.rs | 15 +- 8 files changed, 167 insertions(+), 88 deletions(-) diff --git a/crates/ty_ide/src/completion.rs b/crates/ty_ide/src/completion.rs index 56df9ae666..09295f7ad8 100644 --- a/crates/ty_ide/src/completion.rs +++ b/crates/ty_ide/src/completion.rs @@ -8099,6 +8099,39 @@ def f(x: Intersection[int, Any] | str): ); } + #[test] + fn dunder_file_attribute_completion_non_namespace_package() { + let builder = CursorTest::builder() + .source("module.py", "") + .source("main.py", "import module; module.__file") + .completion_test_builder(); + + // __file__ should be `str` when accessed as an attribute on a non-namespace-package module, + // not `str | None` + assert_snapshot!( + builder.skip_keywords().skip_auto_import().type_signatures().build().snapshot(), + @"__file__ :: str", + ); + } + + #[test] + fn dunder_file_attribute_completion_namespace_package() { + let builder = CursorTest::builder() + .source("namespace_package/foo.py", "") + .source( + "main.py", + "import namespace_package; namespace_package.__file", + ) + .completion_test_builder(); + + // __file__ should be `None` when accessed as an attribute on a namespace-package module, + // not `str | None` + assert_snapshot!( + builder.skip_keywords().skip_auto_import().type_signatures().build().snapshot(), + @"__file__ :: None", + ); + } + /// A way to create a simple single-file (named `main.py`) completion test /// builder. /// diff --git a/crates/ty_python_semantic/resources/mdtest/ide_support/all_members.md b/crates/ty_python_semantic/resources/mdtest/ide_support/all_members.md index 2d9734455b..4b0b7e4bf6 100644 --- a/crates/ty_python_semantic/resources/mdtest/ide_support/all_members.md +++ b/crates/ty_python_semantic/resources/mdtest/ide_support/all_members.md @@ -978,3 +978,39 @@ from ty_extensions import has_member, static_assert # TODO: this should ideally not be available: static_assert(not has_member(3, "__annotations__")) # error: [static-assert-error] ``` + +### `ModuleType` attributes are available on modules + +`namespace_package/foo.py`: + +```py +``` + +`regular_module.py`: + +```py +``` + +`regular_package/__init__.py`: + +```py +``` + +`main.py`: + +```py +import namespace_package +import regular_module +import regular_package +from ty_extensions import static_assert, has_member + +static_assert(has_member(namespace_package, "__file__")) +static_assert(has_member(namespace_package, "__name__")) +static_assert(has_member(namespace_package, "__eq__")) +static_assert(has_member(regular_module, "__file__")) +static_assert(has_member(regular_module, "__name__")) +static_assert(has_member(regular_module, "__eq__")) +static_assert(has_member(regular_package, "__file__")) +static_assert(has_member(regular_package, "__name__")) +static_assert(has_member(regular_package, "__eq__")) +``` diff --git a/crates/ty_python_semantic/resources/mdtest/import/dunder_file_attribute.md b/crates/ty_python_semantic/resources/mdtest/import/dunder_file_attribute.md index 05a2e328c0..b646555004 100644 --- a/crates/ty_python_semantic/resources/mdtest/import/dunder_file_attribute.md +++ b/crates/ty_python_semantic/resources/mdtest/import/dunder_file_attribute.md @@ -76,9 +76,5 @@ reveal_type(a.__file__) # revealed: str ```py import namespace -# TODO: `__file__` does exist on namespace packages but is set to `None`; -# this is a false positive -# -# error: [unresolved-attribute] "Module `namespace` has no member `__file__`" -reveal_type(namespace.__file__) # revealed: Unknown +reveal_type(namespace.__file__) # revealed: None ``` 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 af3f7dc892..0e8f1ee568 100644 --- a/crates/ty_python_semantic/resources/mdtest/scopes/moduletype_attrs.md +++ b/crates/ty_python_semantic/resources/mdtest/scopes/moduletype_attrs.md @@ -91,10 +91,18 @@ def nested_scope(): `ModuleType` attributes can also be accessed as attributes on module-literal types. The special attributes `__dict__` and `__init__`, and all attributes on `builtins.object`, can also be accessed as attributes on module-literal types, despite the fact that these are inaccessible as globals from -inside the module: +inside the module. They can even be accessed on namespace packages: + +`namespace_package/foo.py`: + +```py +``` + +`a.py`: ```py import typing +import namespace_package reveal_type(typing.__name__) # revealed: str reveal_type(typing.__init__) # revealed: bound method ModuleType.__init__(name: str, doc: str | None = ...) -> None @@ -110,17 +118,26 @@ reveal_type(typing.__file__) # revealed: str # These come from `builtins.object`, not `types.ModuleType`: reveal_type(typing.__eq__) # revealed: bound method ModuleType.__eq__(value: object, /) -> bool - reveal_type(typing.__class__) # revealed: - reveal_type(typing.__dict__) # revealed: dict[str, Any] + +reveal_type(namespace_package.__name__) # revealed: str +reveal_type(namespace_package.__init__) # revealed: bound method ModuleType.__init__(name: str, doc: str | None = ...) -> None +reveal_type(namespace_package.__file__) # revealed: None +reveal_type(namespace_package.__eq__) # revealed: bound method ModuleType.__eq__(value: object, /) -> bool +reveal_type(namespace_package.__class__) # revealed: +reveal_type(namespace_package.__dict__) # revealed: dict[str, Any] ``` Typeshed includes a fake `__getattr__` method in the stub for `types.ModuleType` to help out with dynamic imports; but we ignore that for module-literal types where we know exactly which module we're dealing with: +`b.py`: + ```py +import typing + # error: [unresolved-attribute] reveal_type(typing.__getattr__) # revealed: Unknown ``` @@ -128,6 +145,8 @@ 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`): +`c.py`: + ```py import types diff --git a/crates/ty_python_semantic/src/place.rs b/crates/ty_python_semantic/src/place.rs index 5267984154..19085c9b76 100644 --- a/crates/ty_python_semantic/src/place.rs +++ b/crates/ty_python_semantic/src/place.rs @@ -427,20 +427,14 @@ pub(crate) fn global_symbol<'db>( /// /// If `requires_explicit_reexport` is [`None`], it will be inferred from the file's source type. /// For stub files, explicit re-export will be required, while for non-stub files, it will not. +/// +/// `None` should be passed for the `file` parameter if looking up a symbol on a namespace package. pub(crate) fn imported_symbol<'db>( db: &'db dyn Db, - file: File, + file: Option, name: &str, requires_explicit_reexport: Option, ) -> PlaceAndQualifiers<'db> { - let requires_explicit_reexport = requires_explicit_reexport.unwrap_or_else(|| { - if file.is_stub(db) { - RequiresExplicitReExport::Yes - } else { - RequiresExplicitReExport::No - } - }); - // If it's not found in the global scope, check if it's present as an instance on // `types.ModuleType` or `builtins.object`. // @@ -456,22 +450,49 @@ pub(crate) fn imported_symbol<'db>( // ignore `__getattr__`. Typeshed has a fake `__getattr__` on `types.ModuleType` to help out with // dynamic imports; we shouldn't use it for `ModuleLiteral` types where we know exactly which // module we're dealing with. - symbol_impl( - db, - global_scope(db, file), - name, - requires_explicit_reexport, - ConsideredDefinitions::EndOfScope, - ) + file.map(|file| { + let requires_explicit_reexport = requires_explicit_reexport.unwrap_or_else(|| { + if file.is_stub(db) { + RequiresExplicitReExport::Yes + } else { + RequiresExplicitReExport::No + } + }); + + symbol_impl( + db, + global_scope(db, file), + name, + requires_explicit_reexport, + ConsideredDefinitions::EndOfScope, + ) + }) + .unwrap_or_default() .or_fall_back_to(db, || { - if name == "__getattr__" { - Place::Undefined.into() - } else if name == "__builtins__" { - Place::bound(Type::any()).into() - } else { - KnownClass::ModuleType + match name { + "__file__" => { + // We special-case `__file__` here because we know that for a successfully imported + // non-namespace-package Python module, that hasn't been explicitly overridden it + // is always a string, even though typeshed says `str | None`. For a namespace package, + // meanwhile, it will always be `None`. + // + // Note that C-extension modules (stdlib examples include `sys`, `itertools`, etc.) + // may not have a `__file__` attribute at runtime at all, but that doesn't really + // affect the *type* of the attribute, just the *boundness*. There's no way for us + // to know right now whether a stub represents a C extension or not, so for now we + // do not attempt to detect this; we just infer `str` still. This matches the + // behaviour of other major type checkers. + if file.is_some() { + Place::bound(KnownClass::Str.to_instance(db)).into() + } else { + Place::bound(Type::none(db)).into() + } + } + "__getattr__" => Place::Undefined.into(), + "__builtins__" => Place::bound(Type::any()).into(), + _ => KnownClass::ModuleType .to_instance(db) - .member_lookup_with_policy(db, name.into(), MemberLookupPolicy::NO_GETATTR_LOOKUP) + .member_lookup_with_policy(db, name.into(), MemberLookupPolicy::NO_GETATTR_LOOKUP), } }) } @@ -521,7 +542,7 @@ pub(crate) fn known_module_symbol<'db>( resolve_module_confident(db, &known_module.name()) .and_then(|module| { let file = module.file(db)?; - Some(imported_symbol(db, file, symbol, None)) + Some(imported_symbol(db, Some(file), symbol, None)) }) .unwrap_or_default() } @@ -1098,18 +1119,6 @@ fn symbol_impl<'db>( considered_definitions: ConsideredDefinitions, ) -> PlaceAndQualifiers<'db> { let _span = tracing::trace_span!("symbol", ?name).entered(); - let place = place_table(db, scope) - .symbol_id(name) - .map(|symbol| { - place_by_id( - db, - scope, - symbol.into(), - requires_explicit_reexport, - considered_definitions, - ) - }) - .unwrap_or_default(); if name == "platform" && file_to_module(db, scope.file(db)) @@ -1123,43 +1132,20 @@ fn symbol_impl<'db>( // Fall through to the looked up type } } - } else if name == "__file__" - && let Some(module) = file_to_module(db, scope.file(db)) - { - // We special-case `__file__` here because we know that for a successfully imported - // non-namespace-package Python module, that hasn't been explicitly overridden it - // is always a string, even though typeshed says `str | None`. For a namespace package, - // meanwhile, it will always be `None`. - // - // Note that C-extension modules (stdlib examples include `sys`, `itertools`, etc.) - // may not have a `__file__` attribute at runtime at all, but that doesn't really - // affect the *type* of the attribute, just the *boundness*. There's no way for us - // to know right now whether a stub represents a C extension or not, so for now we - // do not attempt to detect this; we just infer `str` still. This matches the - // behaviour of other major type checkers. - let default_type = if module.file(db).is_some() { - KnownClass::Str.to_instance(db) - } else { - Type::none(db) - }; - - match place.place { - Place::Defined(defined_place) => match defined_place.definedness { - Definedness::AlwaysDefined => return place, - Definedness::PossiblyUndefined => { - let new_type = UnionType::from_elements(db, [defined_place.ty, default_type]); - let def_place = DefinedPlace::new(new_type) - .with_definedness(Definedness::AlwaysDefined) - .with_widening(defined_place.widening) - .with_origin(defined_place.origin); - return Place::Defined(def_place).into(); - } - }, - Place::Undefined => return Place::bound(default_type).into(), - } } - place + place_table(db, scope) + .symbol_id(name) + .map(|symbol| { + place_by_id( + db, + scope, + symbol.into(), + requires_explicit_reexport, + considered_definitions, + ) + }) + .unwrap_or_default() } fn place_impl<'db>( @@ -1826,7 +1812,7 @@ pub(crate) mod implicit_globals { .chain(module_type_syms) .filter_map(move |name| { let place = module_type_implicit_global_symbol(db, name.as_str()); - // Only include bound symbols (not undefined or possibly-undefined) + // Only include bound symbols place.place.ignore_possibly_undefined().map(|ty| (name, ty)) }) } diff --git a/crates/ty_python_semantic/src/semantic_index/reachability_constraints.rs b/crates/ty_python_semantic/src/semantic_index/reachability_constraints.rs index 8e5ae7e683..8116bf6981 100644 --- a/crates/ty_python_semantic/src/semantic_index/reachability_constraints.rs +++ b/crates/ty_python_semantic/src/semantic_index/reachability_constraints.rs @@ -945,7 +945,7 @@ impl ReachabilityConstraints { match imported_symbol( db, - referenced_file, + Some(referenced_file), symbol.name(), requires_explicit_reexport, ) diff --git a/crates/ty_python_semantic/src/types.rs b/crates/ty_python_semantic/src/types.rs index 4622cce87a..0cafb153be 100644 --- a/crates/ty_python_semantic/src/types.rs +++ b/crates/ty_python_semantic/src/types.rs @@ -11343,7 +11343,7 @@ impl<'db> ModuleLiteralType<'db> { // For module literals, we want to try calling the module's own `__getattr__` function // if it exists. First, we need to look up the `__getattr__` function in the module's scope. if let Some(file) = self.module(db).file(db) { - let getattr_symbol = imported_symbol(db, file, "__getattr__", None); + let getattr_symbol = imported_symbol(db, Some(file), "__getattr__", None); // If we found a __getattr__ function, try to call it with the name argument if let Place::Defined(place) = getattr_symbol.place && let Ok(outcome) = place.ty.try_call( @@ -11389,11 +11389,7 @@ impl<'db> ModuleLiteralType<'db> { return Place::bound(submodule).into(); } - let place_and_qualifiers = self - .module(db) - .file(db) - .map(|file| imported_symbol(db, file, name, None)) - .unwrap_or_default(); + let place_and_qualifiers = imported_symbol(db, self.module(db).file(db), name, None); // If the normal lookup failed, try to call the module's `__getattr__` function if place_and_qualifiers.place.is_undefined() { diff --git a/crates/ty_python_semantic/src/types/list_members.rs b/crates/ty_python_semantic/src/types/list_members.rs index 1dc500e1b5..cca3f0972f 100644 --- a/crates/ty_python_semantic/src/types/list_members.rs +++ b/crates/ty_python_semantic/src/types/list_members.rs @@ -363,6 +363,19 @@ impl<'db> AllMembers<'db> { } Type::ModuleLiteral(literal) => { + // Looking up `__file__` on `types.ModuleType` will not give as precise a type + // as we infer in type inference, but it's confuisng if autocomplete etc. + // shows a different type in the tooltip to the one inferred by the type checker. + let dunder_file_type = if literal.module(db).file(db).is_some() { + KnownClass::Str.to_instance(db) + } else { + Type::none(db) + }; + self.members.insert(Member { + name: Name::new_static("__file__"), + ty: dunder_file_type, + }); + self.extend_with_type(db, KnownClass::ModuleType.to_instance(db)); let module = literal.module(db); @@ -377,7 +390,7 @@ impl<'db> AllMembers<'db> { for (symbol_id, _) in use_def_map.all_end_of_scope_symbol_declarations() { let symbol_name = place_table.symbol(symbol_id).name(); let Place::Defined(DefinedPlace { ty, .. }) = - imported_symbol(db, file, symbol_name, None).place + imported_symbol(db, Some(file), symbol_name, None).place else { continue; };