From 538e7daeca4f900e8b325ff92d96b18c2d0aa3d7 Mon Sep 17 00:00:00 2001 From: Alex Waygood Date: Tue, 13 May 2025 15:24:24 -0400 Subject: [PATCH 1/2] [ty] Check assignments to implicit global symbols are assignable to the types declared on `types.ModuleType` --- .../mdtest/scopes/moduletype_attrs.md | 39 +++++++++++++++++-- crates/ty_python_semantic/src/symbol.rs | 36 ++++++++++++++--- crates/ty_python_semantic/src/types/infer.rs | 32 ++++++++++----- 3 files changed, 88 insertions(+), 19 deletions(-) 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 09fdefa94278e..08815bc519a8f 100644 --- a/crates/ty_python_semantic/resources/mdtest/scopes/moduletype_attrs.md +++ b/crates/ty_python_semantic/resources/mdtest/scopes/moduletype_attrs.md @@ -40,6 +40,37 @@ reveal_type(__dict__) reveal_type(__init__) ``` +## `ModuleType` globals combined with explicit assignments and declarations + +A `ModuleType` attribute can be overridden in the global scope with a different type, but it must be +a type assignable to the declaration on `ModuleType` unless it is accompanied by an explicit +redeclaration: + +`module.py`: + +```py +__file__ = None +__path__: list[str] = [] +__doc__: int = 42 +__spec__ = 42 # error: [invalid-assignment] "Object of type `Literal[42]` is not assignable to `ModuleSpec | None`" +``` + +`main.py`: + +```py +import module + +reveal_type(module.__file__) # revealed: Unknown | None +reveal_type(module.__path__) # revealed: list[str] +reveal_type(module.__doc__) # revealed: int +reveal_type(module.__spec__) # revealed: Unknown | ModuleSpec | None + +def nested_scope(): + global __loader__ + reveal_type(__loader__) # revealed: LoaderProtocol | None + __loader__ = 56 # error: [invalid-assignment] "Object of type `Literal[56]` is not assignable to `LoaderProtocol | None`" +``` + ## Accessed as attributes `ModuleType` attributes can also be accessed as attributes on module-literal types. The special @@ -105,16 +136,16 @@ defined as a global, however, a name lookup should union the `ModuleType` type w conditionally defined type: ```py -__file__ = 42 +__file__ = "foo" def returns_bool() -> bool: return True if returns_bool(): - __name__ = 1 + __name__ = 1 # error: [invalid-assignment] "Object of type `Literal[1]` is not assignable to `str`" -reveal_type(__file__) # revealed: Literal[42] -reveal_type(__name__) # revealed: Literal[1] | str +reveal_type(__file__) # revealed: Literal["foo"] +reveal_type(__name__) # revealed: str ``` ## Conditionally global or `ModuleType` attribute, with annotation diff --git a/crates/ty_python_semantic/src/symbol.rs b/crates/ty_python_semantic/src/symbol.rs index b18dba7733228..1d1156767985c 100644 --- a/crates/ty_python_semantic/src/symbol.rs +++ b/crates/ty_python_semantic/src/symbol.rs @@ -14,7 +14,9 @@ use crate::types::{ }; use crate::{resolve_module, Db, KnownModule, Program}; -pub(crate) use implicit_globals::module_type_implicit_global_symbol; +pub(crate) use implicit_globals::{ + module_type_implicit_global_declaration, module_type_implicit_global_symbol, +}; #[derive(Debug, Clone, Copy, Hash, PartialEq, Eq)] pub(crate) enum Boundness { @@ -275,7 +277,6 @@ pub(crate) fn explicit_global_symbol<'db>( /// rather than being looked up as symbols explicitly defined/declared in the global scope. /// /// Use [`imported_symbol`] to perform the lookup as seen from outside the file (e.g. via imports). -#[cfg(test)] pub(crate) fn global_symbol<'db>( db: &'db dyn Db, file: File, @@ -958,11 +959,36 @@ mod implicit_globals { use ruff_python_ast as ast; use crate::db::Db; - use crate::semantic_index::{self, symbol_table}; + use crate::semantic_index::{self, symbol_table, use_def_map}; use crate::symbol::SymbolAndQualifiers; - use crate::types::KnownClass; + use crate::types::{KnownClass, Type}; - use super::Symbol; + use super::{symbol_from_declarations, Symbol, SymbolFromDeclarationsResult}; + + pub(crate) fn module_type_implicit_global_declaration<'db>( + db: &'db dyn Db, + name: &str, + ) -> SymbolFromDeclarationsResult<'db> { + if !module_type_symbols(db) + .iter() + .any(|module_type_member| &**module_type_member == name) + { + return Ok(Symbol::Unbound.into()); + } + let Type::ClassLiteral(module_type_class) = KnownClass::ModuleType.to_class_literal(db) + else { + return Ok(Symbol::Unbound.into()); + }; + let module_type_scope = module_type_class.body_scope(db); + let symbol_table = symbol_table(db, module_type_scope); + let Some(symbol_id) = symbol_table.symbol_id_by_name(name) else { + return Ok(Symbol::Unbound.into()); + }; + symbol_from_declarations( + db, + use_def_map(db, module_type_scope).public_declarations(symbol_id), + ) + } /// Looks up the type of an "implicit global symbol". Returns [`Symbol::Unbound`] if /// `name` is not present as an implicit symbol in module-global namespaces. diff --git a/crates/ty_python_semantic/src/types/infer.rs b/crates/ty_python_semantic/src/types/infer.rs index 16ebb5b830f80..c1eb45d1d8e48 100644 --- a/crates/ty_python_semantic/src/types/infer.rs +++ b/crates/ty_python_semantic/src/types/infer.rs @@ -60,9 +60,10 @@ use crate::semantic_index::symbol::{ }; use crate::semantic_index::{semantic_index, EagerSnapshotResult, SemanticIndex}; use crate::symbol::{ - builtins_module_scope, builtins_symbol, explicit_global_symbol, - module_type_implicit_global_symbol, symbol, symbol_from_bindings, symbol_from_declarations, - typing_extensions_symbol, Boundness, LookupError, + builtins_module_scope, builtins_symbol, explicit_global_symbol, global_symbol, + module_type_implicit_global_declaration, module_type_implicit_global_symbol, symbol, + symbol_from_bindings, symbol_from_declarations, typing_extensions_symbol, Boundness, + LookupError, }; use crate::types::call::{Argument, Bindings, CallArgumentTypes, CallArguments, CallError}; use crate::types::class::{MetaclassErrorKind, SliceLiteral}; @@ -1420,8 +1421,9 @@ impl<'db> TypeInferenceBuilder<'db> { let symbol_id = binding.symbol(self.db()); let global_use_def_map = self.index.use_def_map(FileScopeId::global()); - let declarations = if self.skip_non_global_scopes(file_scope_id, symbol_id) { - let symbol_name = symbol_table.symbol(symbol_id).name(); + let symbol_name = symbol_table.symbol(symbol_id).name(); + let skip_non_global_scopes = self.skip_non_global_scopes(file_scope_id, symbol_id); + let declarations = if skip_non_global_scopes { match self .index .symbol_table(FileScopeId::global()) @@ -1436,6 +1438,20 @@ impl<'db> TypeInferenceBuilder<'db> { }; let declared_ty = symbol_from_declarations(self.db(), declarations) + .and_then(|symbol| { + let symbol = if matches!(symbol.symbol, Symbol::Type(_, Boundness::Bound)) { + symbol + } else if skip_non_global_scopes + || self.scope().file_scope_id(self.db()).is_global() + { + let module_type_declarations = + module_type_implicit_global_declaration(self.db(), symbol_name)?; + symbol.or_fall_back_to(self.db(), || module_type_declarations) + } else { + symbol + }; + Ok(symbol) + }) .map(|SymbolAndQualifiers { symbol, .. }| { symbol.ignore_possibly_unbound().unwrap_or(Type::unknown()) }) @@ -5386,11 +5402,7 @@ impl<'db> TypeInferenceBuilder<'db> { .is_some_and(|symbol_id| self.skip_non_global_scopes(file_scope_id, symbol_id)); if skip_non_global_scopes { - return symbol( - db, - FileScopeId::global().to_scope_id(db, current_file), - symbol_name, - ); + return global_symbol(self.db(), self.file(), symbol_name); } // If it's a function-like scope and there is one or more binding in this scope (but From e9b73f001f65e92908e5b4a5990f1e8c26fa6016 Mon Sep 17 00:00:00 2001 From: Alex Waygood Date: Tue, 13 May 2025 16:28:54 -0400 Subject: [PATCH 2/2] add diagnostics for declarations that are not assignable to declarations on `types.ModuleType` --- .../mdtest/scopes/moduletype_attrs.md | 8 +++- crates/ty_python_semantic/src/types/infer.rs | 45 +++++++++++++++++++ 2 files changed, 51 insertions(+), 2 deletions(-) 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 08815bc519a8f..152ab6edb3dc3 100644 --- a/crates/ty_python_semantic/resources/mdtest/scopes/moduletype_attrs.md +++ b/crates/ty_python_semantic/resources/mdtest/scopes/moduletype_attrs.md @@ -51,7 +51,9 @@ redeclaration: ```py __file__ = None __path__: list[str] = [] -__doc__: int = 42 +__doc__: int # error: [invalid-declaration] "Cannot declare type `int` for inferred type `str | None`" +# error: [invalid-declaration] "Cannot shadow implicit global attribute `__package__` with declaration of type `int`" +__package__: int = 42 __spec__ = 42 # error: [invalid-assignment] "Object of type `Literal[42]` is not assignable to `ModuleSpec | None`" ``` @@ -62,7 +64,7 @@ import module reveal_type(module.__file__) # revealed: Unknown | None reveal_type(module.__path__) # revealed: list[str] -reveal_type(module.__doc__) # revealed: int +reveal_type(module.__doc__) # revealed: Unknown reveal_type(module.__spec__) # revealed: Unknown | ModuleSpec | None def nested_scope(): @@ -153,12 +155,14 @@ reveal_type(__name__) # revealed: str The same is true if the name is annotated: ```py +# error: [invalid-declaration] "Cannot shadow implicit global attribute `__file__` with declaration of type `int`" __file__: int = 42 def returns_bool() -> bool: return True if returns_bool(): + # error: [invalid-declaration] "Cannot shadow implicit global attribute `__name__` with declaration of type `int`" __name__: int = 1 reveal_type(__file__) # revealed: Literal[42] diff --git a/crates/ty_python_semantic/src/types/infer.rs b/crates/ty_python_semantic/src/types/infer.rs index c1eb45d1d8e48..5b7533d714e3a 100644 --- a/crates/ty_python_semantic/src/types/infer.rs +++ b/crates/ty_python_semantic/src/types/infer.rs @@ -1503,6 +1503,23 @@ impl<'db> TypeInferenceBuilder<'db> { let prior_bindings = use_def.bindings_at_declaration(declaration); // unbound_ty is Never because for this check we don't care about unbound let inferred_ty = symbol_from_bindings(self.db(), prior_bindings) + .with_qualifiers(TypeQualifiers::empty()) + .or_fall_back_to(self.db(), || { + // Fallback to bindings declared on `types.ModuleType` if it's a global symbol + let scope = self.scope().file_scope_id(self.db()); + if scope.is_global() { + module_type_implicit_global_symbol( + self.db(), + self.index + .symbol_table(scope) + .symbol(declaration.symbol(self.db())) + .name(), + ) + } else { + Symbol::Unbound.into() + } + }) + .symbol .ignore_possibly_unbound() .unwrap_or(Type::Never); let ty = if inferred_ty.is_assignable_to(self.db(), ty.inner_type()) { @@ -1541,6 +1558,34 @@ impl<'db> TypeInferenceBuilder<'db> { declared_ty, inferred_ty, } => { + let file_scope_id = self.scope().file_scope_id(self.db()); + if file_scope_id.is_global() { + let symbol_table = self.index.symbol_table(file_scope_id); + let symbol_name = symbol_table.symbol(definition.symbol(self.db())).name(); + if let Some(module_type_implicit_declaration) = + module_type_implicit_global_declaration(self.db(), symbol_name) + .ok() + .and_then(|sym| sym.symbol.ignore_possibly_unbound()) + { + let declared_type = declared_ty.inner_type(); + if !declared_type + .is_assignable_to(self.db(), module_type_implicit_declaration) + { + if let Some(builder) = + self.context.report_lint(&INVALID_DECLARATION, node) + { + let mut diagnostic = builder.into_diagnostic(format_args!( + "Cannot shadow implicit global attribute `{symbol_name}` with declaration of type `{}`", + declared_type.display(self.db()) + )); + diagnostic.info(format_args!("The global symbol `{}` must always have a type assignable to `{}`", + symbol_name, + module_type_implicit_declaration.display(self.db()) + )); + } + } + } + } if inferred_ty.is_assignable_to(self.db(), declared_ty.inner_type()) { (declared_ty, inferred_ty) } else {