From 9f111eaebf60bd73a7a3df9d5722403a4cc4780e Mon Sep 17 00:00:00 2001 From: Dhruv Manilawala Date: Mon, 17 Feb 2025 17:45:38 +0530 Subject: [PATCH] red-knot: move symbol lookups in `symbol.rs` (#16152) ## Summary This PR does the following: * Moves the following from `types.rs` in `symbol.rs`: * `symbol` * `global_symbol` * `imported_symbol` * `symbol_from_bindings` * `symbol_from_declarations` * `SymbolAndQualifiers` * `SymbolFromDeclarationsResult` * Moves the following from `stdlib.rs` in `symbol.rs` and removes `stdlib.rs`: * `known_module_symbol` * `builtins_symbol` * `typing_symbol` (only for tests) * `typing_extensions_symbol` * `builtins_module_scope` * `core_module_scope` * Add `symbol_from_bindings_impl` and `symbol_from_declarations_impl` to keep `RequiresExplicitReExport` an implementation detail * Make `declaration_type` a `pub(crate)` as it's required in `symbol_from_declarations` (`binding_type` is already `pub(crate)` The main motivation is to keep the implementation details private and only expose an ergonomic API which uses sane defaults for various scenario to avoid any mistakes from the caller. Refer to https://github.com/astral-sh/ruff/pull/16133#discussion_r1955262772, https://github.com/astral-sh/ruff/pull/16133#issue-2850146612 for details. --- crates/red_knot_python_semantic/src/lib.rs | 1 - crates/red_knot_python_semantic/src/stdlib.rs | 50 -- crates/red_knot_python_semantic/src/symbol.rs | 615 +++++++++++++++++- crates/red_knot_python_semantic/src/types.rs | 543 +--------------- .../src/types/infer.rs | 73 +-- .../src/types/property_tests.rs | 4 +- 6 files changed, 663 insertions(+), 623 deletions(-) delete mode 100644 crates/red_knot_python_semantic/src/stdlib.rs diff --git a/crates/red_knot_python_semantic/src/lib.rs b/crates/red_knot_python_semantic/src/lib.rs index 4e18bb73171090..ba8cf29d8fd72c 100644 --- a/crates/red_knot_python_semantic/src/lib.rs +++ b/crates/red_knot_python_semantic/src/lib.rs @@ -22,7 +22,6 @@ mod python_platform; pub mod semantic_index; mod semantic_model; pub(crate) mod site_packages; -mod stdlib; mod suppression; pub(crate) mod symbol; pub mod types; diff --git a/crates/red_knot_python_semantic/src/stdlib.rs b/crates/red_knot_python_semantic/src/stdlib.rs deleted file mode 100644 index c4eea665453afd..00000000000000 --- a/crates/red_knot_python_semantic/src/stdlib.rs +++ /dev/null @@ -1,50 +0,0 @@ -use crate::module_resolver::{resolve_module, KnownModule}; -use crate::semantic_index::global_scope; -use crate::semantic_index::symbol::ScopeId; -use crate::symbol::Symbol; -use crate::types::imported_symbol; -use crate::Db; - -/// Lookup the type of `symbol` in a given known module -/// -/// Returns `Symbol::Unbound` if the given known module cannot be resolved for some reason -pub(crate) fn known_module_symbol<'db>( - db: &'db dyn Db, - known_module: KnownModule, - symbol: &str, -) -> Symbol<'db> { - resolve_module(db, &known_module.name()) - .map(|module| imported_symbol(db, &module, symbol)) - .unwrap_or(Symbol::Unbound) -} - -/// Lookup the type of `symbol` in the `typing` module namespace. -/// -/// Returns `Symbol::Unbound` if the `typing` module isn't available for some reason. -#[inline] -#[cfg(test)] -pub(crate) fn typing_symbol<'db>(db: &'db dyn Db, symbol: &str) -> Symbol<'db> { - known_module_symbol(db, KnownModule::Typing, symbol) -} - -/// Lookup the type of `symbol` in the `typing_extensions` module namespace. -/// -/// Returns `Symbol::Unbound` if the `typing_extensions` module isn't available for some reason. -#[inline] -pub(crate) fn typing_extensions_symbol<'db>(db: &'db dyn Db, symbol: &str) -> Symbol<'db> { - known_module_symbol(db, KnownModule::TypingExtensions, symbol) -} - -/// Get the scope of a core stdlib module. -/// -/// Can return `None` if a custom typeshed is used that is missing the core module in question. -fn core_module_scope(db: &dyn Db, core_module: KnownModule) -> Option> { - resolve_module(db, &core_module.name()).map(|module| global_scope(db, module.file())) -} - -/// Get the `builtins` module scope. -/// -/// Can return `None` if a custom typeshed is used that is missing `builtins.pyi`. -pub(crate) fn builtins_module_scope(db: &dyn Db) -> Option> { - core_module_scope(db, KnownModule::Builtins) -} diff --git a/crates/red_knot_python_semantic/src/symbol.rs b/crates/red_knot_python_semantic/src/symbol.rs index 0d3bdd8eadc420..e64ac1bb112bc7 100644 --- a/crates/red_knot_python_semantic/src/symbol.rs +++ b/crates/red_knot_python_semantic/src/symbol.rs @@ -1,7 +1,18 @@ -use crate::{ - types::{todo_type, Type, UnionType}, - Db, +use ruff_db::files::File; +use ruff_python_ast as ast; + +use crate::module_resolver::file_to_module; +use crate::semantic_index::definition::Definition; +use crate::semantic_index::symbol::{ScopeId, ScopedSymbolId}; +use crate::semantic_index::{self, global_scope, use_def_map, DeclarationWithConstraint}; +use crate::semantic_index::{ + symbol_table, BindingWithConstraints, BindingWithConstraintsIterator, DeclarationsIterator, +}; +use crate::types::{ + binding_type, declaration_type, narrowing_constraint, todo_type, IntersectionBuilder, + KnownClass, Truthiness, Type, TypeAndQualifiers, TypeQualifiers, UnionBuilder, UnionType, }; +use crate::{resolve_module, Db, KnownModule, Module, Program}; #[derive(Debug, Clone, Copy, PartialEq, Eq)] pub(crate) enum Boundness { @@ -166,6 +177,592 @@ impl<'db> LookupError<'db> { /// In the future, we could possibly consider removing `Symbol` and using this type everywhere instead. pub(crate) type LookupResult<'db> = Result, LookupError<'db>>; +/// Infer the public type of a symbol (its type as seen from outside its scope) in the given +/// `scope`. +pub(crate) fn symbol<'db>(db: &'db dyn Db, scope: ScopeId<'db>, name: &str) -> Symbol<'db> { + symbol_impl(db, scope, name, RequiresExplicitReExport::No) +} + +/// Infers the public type of a module-global symbol as seen from within the same file. +/// +/// If it's not defined explicitly in the global scope, it will look it up in `types.ModuleType` +/// with a few very special exceptions. +/// +/// Use [`imported_symbol`] to perform the lookup as seen from outside the file (e.g. via imports). +pub(crate) fn global_symbol<'db>(db: &'db dyn Db, file: File, name: &str) -> Symbol<'db> { + symbol_impl( + db, + global_scope(db, file), + name, + RequiresExplicitReExport::No, + ) + .or_fall_back_to(db, || module_type_symbol(db, name)) +} + +/// Infers the public type of an imported symbol. +pub(crate) fn imported_symbol<'db>(db: &'db dyn Db, module: &Module, name: &str) -> Symbol<'db> { + // If it's not found in the global scope, check if it's present as an instance on + // `types.ModuleType` or `builtins.object`. + // + // We do a more limited version of this in `global_symbol`, but there are two crucial + // differences here: + // - If a member is looked up as an attribute, `__init__` is also available on the module, but + // it isn't available as a global from inside the module + // - If a member is looked up as an attribute, members on `builtins.object` are also available + // (because `types.ModuleType` inherits from `object`); these attributes are also not + // available as globals from inside the module. + // + // The same way as in `global_symbol`, however, we need to be careful to 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. + external_symbol_impl(db, module.file(), name).or_fall_back_to(db, || { + if name == "__getattr__" { + Symbol::Unbound + } else { + KnownClass::ModuleType.to_instance(db).member(db, name) + } + }) +} + +/// Lookup the type of `symbol` in the builtins namespace. +/// +/// Returns `Symbol::Unbound` if the `builtins` module isn't available for some reason. +/// +/// Note that this function is only intended for use in the context of the builtins *namespace* +/// and should not be used when a symbol is being explicitly imported from the `builtins` module +/// (e.g. `from builtins import int`). +pub(crate) fn builtins_symbol<'db>(db: &'db dyn Db, symbol: &str) -> Symbol<'db> { + resolve_module(db, &KnownModule::Builtins.name()) + .map(|module| { + external_symbol_impl(db, module.file(), symbol).or_fall_back_to(db, || { + // We're looking up in the builtins namespace and not the module, so we should + // do the normal lookup in `types.ModuleType` and not the special one as in + // `imported_symbol`. + module_type_symbol(db, symbol) + }) + }) + .unwrap_or(Symbol::Unbound) +} + +/// Lookup the type of `symbol` in a given known module. +/// +/// Returns `Symbol::Unbound` if the given known module cannot be resolved for some reason. +pub(crate) fn known_module_symbol<'db>( + db: &'db dyn Db, + known_module: KnownModule, + symbol: &str, +) -> Symbol<'db> { + resolve_module(db, &known_module.name()) + .map(|module| imported_symbol(db, &module, symbol)) + .unwrap_or(Symbol::Unbound) +} + +/// Lookup the type of `symbol` in the `typing` module namespace. +/// +/// Returns `Symbol::Unbound` if the `typing` module isn't available for some reason. +#[inline] +#[cfg(test)] +pub(crate) fn typing_symbol<'db>(db: &'db dyn Db, symbol: &str) -> Symbol<'db> { + known_module_symbol(db, KnownModule::Typing, symbol) +} + +/// Lookup the type of `symbol` in the `typing_extensions` module namespace. +/// +/// Returns `Symbol::Unbound` if the `typing_extensions` module isn't available for some reason. +#[inline] +pub(crate) fn typing_extensions_symbol<'db>(db: &'db dyn Db, symbol: &str) -> Symbol<'db> { + known_module_symbol(db, KnownModule::TypingExtensions, symbol) +} + +/// Get the `builtins` module scope. +/// +/// Can return `None` if a custom typeshed is used that is missing `builtins.pyi`. +pub(crate) fn builtins_module_scope(db: &dyn Db) -> Option> { + core_module_scope(db, KnownModule::Builtins) +} + +/// Get the scope of a core stdlib module. +/// +/// Can return `None` if a custom typeshed is used that is missing the core module in question. +fn core_module_scope(db: &dyn Db, core_module: KnownModule) -> Option> { + resolve_module(db, &core_module.name()).map(|module| global_scope(db, module.file())) +} + +/// Infer the combined type from an iterator of bindings, and return it +/// together with boundness information in a [`Symbol`]. +/// +/// The type will be a union if there are multiple bindings with different types. +pub(crate) fn symbol_from_bindings<'db>( + db: &'db dyn Db, + bindings_with_constraints: BindingWithConstraintsIterator<'_, 'db>, +) -> Symbol<'db> { + symbol_from_bindings_impl(db, bindings_with_constraints, RequiresExplicitReExport::No) +} + +/// Build a declared type from a [`DeclarationsIterator`]. +/// +/// If there is only one declaration, or all declarations declare the same type, returns +/// `Ok(..)`. If there are conflicting declarations, returns an `Err(..)` variant with +/// a union of the declared types as well as a list of all conflicting types. +/// +/// This function also returns declaredness information (see [`Symbol`]) and a set of +/// [`TypeQualifiers`] that have been specified on the declaration(s). +pub(crate) fn symbol_from_declarations<'db>( + db: &'db dyn Db, + declarations: DeclarationsIterator<'_, 'db>, +) -> SymbolFromDeclarationsResult<'db> { + symbol_from_declarations_impl(db, declarations, RequiresExplicitReExport::No) +} + +/// The result of looking up a declared type from declarations; see [`symbol_from_declarations`]. +pub(crate) type SymbolFromDeclarationsResult<'db> = + Result, (TypeAndQualifiers<'db>, Box<[Type<'db>]>)>; + +/// A type with declaredness information, and a set of type qualifiers. +/// +/// This is used to represent the result of looking up the declared type. Consider this +/// example: +/// ```py +/// class C: +/// if flag: +/// variable: ClassVar[int] +/// ``` +/// If we look up the declared type of `variable` in the scope of class `C`, we will get +/// the type `int`, a "declaredness" of [`Boundness::PossiblyUnbound`], and the information +/// that this comes with a [`CLASS_VAR`] type qualifier. +/// +/// [`CLASS_VAR`]: crate::types::TypeQualifiers::CLASS_VAR +#[derive(Debug)] +pub(crate) struct SymbolAndQualifiers<'db>(pub(crate) Symbol<'db>, pub(crate) TypeQualifiers); + +impl SymbolAndQualifiers<'_> { + /// Constructor that creates a [`SymbolAndQualifiers`] instance with a [`TodoType`] type + /// and no qualifiers. + /// + /// [`TodoType`]: crate::types::TodoType + pub(crate) fn todo(message: &'static str) -> Self { + Self(Symbol::todo(message), TypeQualifiers::empty()) + } + + /// Returns `true` if the symbol has a `ClassVar` type qualifier. + pub(crate) fn is_class_var(&self) -> bool { + self.1.contains(TypeQualifiers::CLASS_VAR) + } + + /// Returns `true` if the symbol has a `Final` type qualifier. + pub(crate) fn is_final(&self) -> bool { + self.1.contains(TypeQualifiers::FINAL) + } +} + +impl<'db> From> for SymbolAndQualifiers<'db> { + fn from(symbol: Symbol<'db>) -> Self { + SymbolAndQualifiers(symbol, TypeQualifiers::empty()) + } +} + +/// Implementation of [`symbol`]. +fn symbol_impl<'db>( + db: &'db dyn Db, + scope: ScopeId<'db>, + name: &str, + requires_explicit_reexport: RequiresExplicitReExport, +) -> Symbol<'db> { + #[salsa::tracked] + fn symbol_by_id<'db>( + db: &'db dyn Db, + scope: ScopeId<'db>, + symbol_id: ScopedSymbolId, + requires_explicit_reexport: RequiresExplicitReExport, + ) -> Symbol<'db> { + let use_def = use_def_map(db, scope); + + // If the symbol is declared, the public type is based on declarations; otherwise, it's based + // on inference from bindings. + + let declarations = use_def.public_declarations(symbol_id); + let declared = symbol_from_declarations_impl(db, declarations, requires_explicit_reexport); + let is_final = declared.as_ref().is_ok_and(SymbolAndQualifiers::is_final); + let declared = declared.map(|SymbolAndQualifiers(symbol, _)| symbol); + + match declared { + // Symbol is declared, trust the declared type + Ok(symbol @ Symbol::Type(_, Boundness::Bound)) => symbol, + // Symbol is possibly declared + Ok(Symbol::Type(declared_ty, Boundness::PossiblyUnbound)) => { + let bindings = use_def.public_bindings(symbol_id); + let inferred = symbol_from_bindings_impl(db, bindings, requires_explicit_reexport); + + match inferred { + // Symbol is possibly undeclared and definitely unbound + Symbol::Unbound => { + // TODO: We probably don't want to report `Bound` here. This requires a bit of + // design work though as we might want a different behavior for stubs and for + // normal modules. + Symbol::Type(declared_ty, Boundness::Bound) + } + // Symbol is possibly undeclared and (possibly) bound + Symbol::Type(inferred_ty, boundness) => Symbol::Type( + UnionType::from_elements(db, [inferred_ty, declared_ty]), + boundness, + ), + } + } + // Symbol is undeclared, return the union of `Unknown` with the inferred type + Ok(Symbol::Unbound) => { + let bindings = use_def.public_bindings(symbol_id); + let inferred = symbol_from_bindings_impl(db, bindings, requires_explicit_reexport); + + // `__slots__` is a symbol with special behavior in Python's runtime. It can be + // modified externally, but those changes do not take effect. We therefore issue + // a diagnostic if we see it being modified externally. In type inference, we + // can assign a "narrow" type to it even if it is not *declared*. This means, we + // do not have to call [`widen_type_for_undeclared_public_symbol`]. + let is_considered_non_modifiable = + is_final || symbol_table(db, scope).symbol(symbol_id).name() == "__slots__"; + + widen_type_for_undeclared_public_symbol(db, inferred, is_considered_non_modifiable) + } + // Symbol has conflicting declared types + Err((declared_ty, _)) => { + // Intentionally ignore conflicting declared types; that's not our problem, + // it's the problem of the module we are importing from. + Symbol::bound(declared_ty.inner_type()) + } + } + + // TODO (ticket: https://github.com/astral-sh/ruff/issues/14297) Our handling of boundness + // currently only depends on bindings, and ignores declarations. This is inconsistent, since + // we only look at bindings if the symbol may be undeclared. Consider the following example: + // ```py + // x: int + // + // if flag: + // y: int + // else + // y = 3 + // ``` + // If we import from this module, we will currently report `x` as a definitely-bound symbol + // (even though it has no bindings at all!) but report `y` as possibly-unbound (even though + // every path has either a binding or a declaration for it.) + } + + let _span = tracing::trace_span!("symbol", ?name).entered(); + + // We don't need to check for `typing_extensions` here, because `typing_extensions.TYPE_CHECKING` + // is just a re-export of `typing.TYPE_CHECKING`. + if name == "TYPE_CHECKING" + && file_to_module(db, scope.file(db)) + .is_some_and(|module| module.is_known(KnownModule::Typing)) + { + return Symbol::bound(Type::BooleanLiteral(true)); + } + if name == "platform" + && file_to_module(db, scope.file(db)) + .is_some_and(|module| module.is_known(KnownModule::Sys)) + { + match Program::get(db).python_platform(db) { + crate::PythonPlatform::Identifier(platform) => { + return Symbol::bound(Type::string_literal(db, platform.as_str())); + } + crate::PythonPlatform::All => { + // Fall through to the looked up type + } + } + } + + symbol_table(db, scope) + .symbol_id_by_name(name) + .map(|symbol| symbol_by_id(db, scope, symbol, requires_explicit_reexport)) + .unwrap_or(Symbol::Unbound) +} + +/// Implementation of [`symbol_from_bindings`]. +fn symbol_from_bindings_impl<'db>( + db: &'db dyn Db, + bindings_with_constraints: BindingWithConstraintsIterator<'_, 'db>, + requires_explicit_reexport: RequiresExplicitReExport, +) -> Symbol<'db> { + let visibility_constraints = bindings_with_constraints.visibility_constraints; + let mut bindings_with_constraints = bindings_with_constraints.peekable(); + + let is_non_exported = |binding: Definition<'db>| { + requires_explicit_reexport.is_yes() && !binding.is_reexported(db) + }; + + let unbound_visibility = match bindings_with_constraints.peek() { + Some(BindingWithConstraints { + binding, + visibility_constraint, + constraints: _, + }) if binding.map_or(true, is_non_exported) => { + visibility_constraints.evaluate(db, *visibility_constraint) + } + _ => Truthiness::AlwaysFalse, + }; + + let mut types = bindings_with_constraints.filter_map( + |BindingWithConstraints { + binding, + constraints, + visibility_constraint, + }| { + let binding = binding?; + + if is_non_exported(binding) { + return None; + } + + let static_visibility = visibility_constraints.evaluate(db, visibility_constraint); + + if static_visibility.is_always_false() { + return None; + } + + let mut constraint_tys = constraints + .filter_map(|constraint| narrowing_constraint(db, constraint, binding)) + .peekable(); + + let binding_ty = binding_type(db, binding); + if constraint_tys.peek().is_some() { + let intersection_ty = constraint_tys + .fold( + IntersectionBuilder::new(db).add_positive(binding_ty), + IntersectionBuilder::add_positive, + ) + .build(); + Some(intersection_ty) + } else { + Some(binding_ty) + } + }, + ); + + if let Some(first) = types.next() { + let boundness = match unbound_visibility { + Truthiness::AlwaysTrue => { + unreachable!("If we have at least one binding, the scope-start should not be definitely visible") + } + Truthiness::AlwaysFalse => Boundness::Bound, + Truthiness::Ambiguous => Boundness::PossiblyUnbound, + }; + + if let Some(second) = types.next() { + Symbol::Type( + UnionType::from_elements(db, [first, second].into_iter().chain(types)), + boundness, + ) + } else { + Symbol::Type(first, boundness) + } + } else { + Symbol::Unbound + } +} + +/// Implementation of [`symbol_from_declarations`]. +fn symbol_from_declarations_impl<'db>( + db: &'db dyn Db, + declarations: DeclarationsIterator<'_, 'db>, + requires_explicit_reexport: RequiresExplicitReExport, +) -> SymbolFromDeclarationsResult<'db> { + let visibility_constraints = declarations.visibility_constraints; + let mut declarations = declarations.peekable(); + + let is_non_exported = |declaration: Definition<'db>| { + requires_explicit_reexport.is_yes() && !declaration.is_reexported(db) + }; + + let undeclared_visibility = match declarations.peek() { + Some(DeclarationWithConstraint { + declaration, + visibility_constraint, + }) if declaration.map_or(true, is_non_exported) => { + visibility_constraints.evaluate(db, *visibility_constraint) + } + _ => Truthiness::AlwaysFalse, + }; + + let mut types = declarations.filter_map( + |DeclarationWithConstraint { + declaration, + visibility_constraint, + }| { + let declaration = declaration?; + + if is_non_exported(declaration) { + return None; + } + + let static_visibility = visibility_constraints.evaluate(db, visibility_constraint); + + if static_visibility.is_always_false() { + None + } else { + Some(declaration_type(db, declaration)) + } + }, + ); + + if let Some(first) = types.next() { + let mut conflicting: Vec> = vec![]; + let declared_ty = if let Some(second) = types.next() { + let ty_first = first.inner_type(); + let mut qualifiers = first.qualifiers(); + + let mut builder = UnionBuilder::new(db).add(ty_first); + for other in std::iter::once(second).chain(types) { + let other_ty = other.inner_type(); + if !ty_first.is_equivalent_to(db, other_ty) { + conflicting.push(other_ty); + } + builder = builder.add(other_ty); + qualifiers = qualifiers.union(other.qualifiers()); + } + TypeAndQualifiers::new(builder.build(), qualifiers) + } else { + first + }; + if conflicting.is_empty() { + let boundness = match undeclared_visibility { + Truthiness::AlwaysTrue => { + unreachable!("If we have at least one declaration, the scope-start should not be definitely visible") + } + Truthiness::AlwaysFalse => Boundness::Bound, + Truthiness::Ambiguous => Boundness::PossiblyUnbound, + }; + + Ok(SymbolAndQualifiers( + Symbol::Type(declared_ty.inner_type(), boundness), + declared_ty.qualifiers(), + )) + } else { + Err(( + declared_ty, + std::iter::once(first.inner_type()) + .chain(conflicting) + .collect(), + )) + } + } else { + Ok(Symbol::Unbound.into()) + } +} + +/// Return a list of the symbols that typeshed declares in the body scope of +/// the stub for the class `types.ModuleType`. +/// +/// Conceptually this could be a `Set` rather than a list, +/// but the number of symbols declared in this scope is likely to be very small, +/// so the cost of hashing the names is likely to be more expensive than it's worth. +#[salsa::tracked(return_ref)] +fn module_type_symbols<'db>(db: &'db dyn Db) -> smallvec::SmallVec<[ast::name::Name; 8]> { + let Some(module_type) = KnownClass::ModuleType + .to_class_literal(db) + .into_class_literal() + else { + // The most likely way we get here is if a user specified a `--custom-typeshed-dir` + // without a `types.pyi` stub in the `stdlib/` directory + return smallvec::SmallVec::default(); + }; + + let module_type_scope = module_type.body_scope(db); + let module_type_symbol_table = symbol_table(db, module_type_scope); + + // `__dict__` and `__init__` are very special members that can be accessed as attributes + // on the module when imported, but cannot be accessed as globals *inside* the module. + // + // `__getattr__` is even more special: it doesn't exist at runtime, but typeshed includes it + // to reduce false positives associated with functions that dynamically import modules + // and return `Instance(types.ModuleType)`. We should ignore it for any known module-literal type. + module_type_symbol_table + .symbols() + .filter(|symbol| symbol.is_declared()) + .map(semantic_index::symbol::Symbol::name) + .filter(|symbol_name| !matches!(&***symbol_name, "__dict__" | "__getattr__" | "__init__")) + .cloned() + .collect() +} + +/// Return the symbol for a member of `types.ModuleType`. +/// +/// ## Notes +/// +/// In general we wouldn't check to see whether a symbol exists on a class before doing the +/// [`member`] call on the instance type -- we'd just do the [`member`] call on the instance +/// type, since it has the same end result. The reason to only call [`member`] on [`ModuleType`] +/// instance when absolutely necessary is that it was a fairly significant performance regression +/// to fallback to doing that for every name lookup that wasn't found in the module's globals +/// ([`global_symbol`]). So we use less idiomatic (and much more verbose) code here as a +/// micro-optimisation because it's used in a very hot path. +/// +/// [`member`]: Type::member +/// [`ModuleType`]: KnownClass::ModuleType +fn module_type_symbol<'db>(db: &'db dyn Db, name: &str) -> Symbol<'db> { + if module_type_symbols(db) + .iter() + .any(|module_type_member| &**module_type_member == name) + { + KnownClass::ModuleType.to_instance(db).member(db, name) + } else { + Symbol::Unbound + } +} + +/// Implementation of looking up a module-global symbol as seen from outside the file (e.g. via +/// imports). +/// +/// This will take into account whether the definition of the symbol is being explicitly +/// re-exported from a stub file or not. +fn external_symbol_impl<'db>(db: &'db dyn Db, file: File, name: &str) -> Symbol<'db> { + symbol_impl( + db, + global_scope(db, file), + name, + if file.is_stub(db.upcast()) { + RequiresExplicitReExport::Yes + } else { + RequiresExplicitReExport::No + }, + ) +} + +#[derive(Copy, Clone, Debug, PartialEq, Eq, Hash)] +enum RequiresExplicitReExport { + Yes, + No, +} + +impl RequiresExplicitReExport { + const fn is_yes(self) -> bool { + matches!(self, RequiresExplicitReExport::Yes) + } +} + +/// Computes a possibly-widened type `Unknown | T_inferred` from the inferred type `T_inferred` +/// of a symbol, unless the type is a known-instance type (e.g. `typing.Any`) or the symbol is +/// considered non-modifiable (e.g. when the symbol is `@Final`). We need this for public uses +/// of symbols that have no declared type. +fn widen_type_for_undeclared_public_symbol<'db>( + db: &'db dyn Db, + inferred: Symbol<'db>, + is_considered_non_modifiable: bool, +) -> Symbol<'db> { + // We special-case known-instance types here since symbols like `typing.Any` are typically + // not declared in the stubs (e.g. `Any = object()`), but we still want to treat them as + // such. + let is_known_instance = inferred + .ignore_possibly_unbound() + .is_some_and(|ty| matches!(ty, Type::KnownInstance(_))); + + if is_considered_non_modifiable || is_known_instance { + inferred + } else { + inferred.map_type(|ty| UnionType::from_elements(db, [Type::unknown(), ty])) + } +} + #[cfg(test)] mod tests { use super::*; @@ -222,4 +819,16 @@ mod tests { Symbol::Type(ty1, Bound) ); } + + #[test] + fn module_type_symbols_includes_declared_types_but_not_referenced_types() { + let db = setup_db(); + let symbol_names = module_type_symbols(&db); + + let dunder_name_symbol_name = ast::name::Name::new_static("__name__"); + assert!(symbol_names.contains(&dunder_name_symbol_name)); + + let property_symbol_name = ast::name::Name::new_static("property"); + assert!(!symbol_names.contains(&property_symbol_name)); + } } diff --git a/crates/red_knot_python_semantic/src/types.rs b/crates/red_knot_python_semantic/src/types.rs index a309887fdc9e61..d7e58fd841d9b8 100644 --- a/crates/red_knot_python_semantic/src/types.rs +++ b/crates/red_knot_python_semantic/src/types.rs @@ -27,15 +27,15 @@ use crate::semantic_index::ast_ids::HasScopedExpressionId; use crate::semantic_index::attribute_assignment::AttributeAssignment; use crate::semantic_index::definition::Definition; use crate::semantic_index::expression::Expression; -use crate::semantic_index::symbol::{self as symbol, ScopeId, ScopedSymbolId}; +use crate::semantic_index::symbol::ScopeId; use crate::semantic_index::{ - attribute_assignments, global_scope, imported_modules, semantic_index, symbol_table, - use_def_map, BindingWithConstraints, BindingWithConstraintsIterator, DeclarationWithConstraint, - DeclarationsIterator, + attribute_assignments, imported_modules, semantic_index, symbol_table, use_def_map, }; -use crate::stdlib::{known_module_symbol, typing_extensions_symbol}; use crate::suppression::check_suppressions; -use crate::symbol::{Boundness, LookupError, LookupResult, Symbol}; +use crate::symbol::{ + global_symbol, imported_symbol, known_module_symbol, symbol, symbol_from_bindings, + symbol_from_declarations, Boundness, LookupError, LookupResult, Symbol, SymbolAndQualifiers, +}; use crate::types::call::{ bind_call, CallArguments, CallBinding, CallDunderResult, CallOutcome, StaticAssertionErrorKind, }; @@ -43,7 +43,7 @@ use crate::types::class_base::ClassBase; use crate::types::diagnostic::INVALID_TYPE_FORM; use crate::types::infer::infer_unpack_types; use crate::types::mro::{Mro, MroError, MroIterator}; -use crate::types::narrow::narrowing_constraint; +pub(crate) use crate::types::narrow::narrowing_constraint; use crate::{Db, FxOrderSet, Module, Program}; mod builder; @@ -84,284 +84,6 @@ pub fn check_types(db: &dyn Db, file: File) -> TypeCheckDiagnostics { diagnostics } -/// Computes a possibly-widened type `Unknown | T_inferred` from the inferred type `T_inferred` -/// of a symbol, unless the type is a known-instance type (e.g. `typing.Any`) or the symbol is -/// considered non-modifiable (e.g. when the symbol is `@Final`). We need this for public uses -/// of symbols that have no declared type. -fn widen_type_for_undeclared_public_symbol<'db>( - db: &'db dyn Db, - inferred: Symbol<'db>, - is_considered_non_modifiable: bool, -) -> Symbol<'db> { - // We special-case known-instance types here since symbols like `typing.Any` are typically - // not declared in the stubs (e.g. `Any = object()`), but we still want to treat them as - // such. - let is_known_instance = inferred - .ignore_possibly_unbound() - .is_some_and(|ty| matches!(ty, Type::KnownInstance(_))); - - if is_considered_non_modifiable || is_known_instance { - inferred - } else { - inferred.map_type(|ty| UnionType::from_elements(db, [Type::unknown(), ty])) - } -} - -#[derive(Copy, Clone, Debug, PartialEq, Eq, Hash)] -enum RequiresExplicitReExport { - Yes, - No, -} - -impl RequiresExplicitReExport { - const fn is_yes(self) -> bool { - matches!(self, RequiresExplicitReExport::Yes) - } -} - -fn symbol_impl<'db>( - db: &'db dyn Db, - scope: ScopeId<'db>, - name: &str, - requires_explicit_reexport: RequiresExplicitReExport, -) -> Symbol<'db> { - #[salsa::tracked] - fn symbol_by_id<'db>( - db: &'db dyn Db, - scope: ScopeId<'db>, - symbol_id: ScopedSymbolId, - requires_explicit_reexport: RequiresExplicitReExport, - ) -> Symbol<'db> { - let use_def = use_def_map(db, scope); - - // If the symbol is declared, the public type is based on declarations; otherwise, it's based - // on inference from bindings. - - let declarations = use_def.public_declarations(symbol_id); - let declared = symbol_from_declarations(db, declarations, requires_explicit_reexport); - let is_final = declared.as_ref().is_ok_and(SymbolAndQualifiers::is_final); - let declared = declared.map(|SymbolAndQualifiers(symbol, _)| symbol); - - match declared { - // Symbol is declared, trust the declared type - Ok(symbol @ Symbol::Type(_, Boundness::Bound)) => symbol, - // Symbol is possibly declared - Ok(Symbol::Type(declared_ty, Boundness::PossiblyUnbound)) => { - let bindings = use_def.public_bindings(symbol_id); - let inferred = symbol_from_bindings(db, bindings, requires_explicit_reexport); - - match inferred { - // Symbol is possibly undeclared and definitely unbound - Symbol::Unbound => { - // TODO: We probably don't want to report `Bound` here. This requires a bit of - // design work though as we might want a different behavior for stubs and for - // normal modules. - Symbol::Type(declared_ty, Boundness::Bound) - } - // Symbol is possibly undeclared and (possibly) bound - Symbol::Type(inferred_ty, boundness) => Symbol::Type( - UnionType::from_elements(db, [inferred_ty, declared_ty]), - boundness, - ), - } - } - // Symbol is undeclared, return the union of `Unknown` with the inferred type - Ok(Symbol::Unbound) => { - let bindings = use_def.public_bindings(symbol_id); - let inferred = symbol_from_bindings(db, bindings, requires_explicit_reexport); - - // `__slots__` is a symbol with special behavior in Python's runtime. It can be - // modified externally, but those changes do not take effect. We therefore issue - // a diagnostic if we see it being modified externally. In type inference, we - // can assign a "narrow" type to it even if it is not *declared*. This means, we - // do not have to call [`widen_type_for_undeclared_public_symbol`]. - let is_considered_non_modifiable = - is_final || symbol_table(db, scope).symbol(symbol_id).name() == "__slots__"; - - widen_type_for_undeclared_public_symbol(db, inferred, is_considered_non_modifiable) - } - // Symbol has conflicting declared types - Err((declared_ty, _)) => { - // Intentionally ignore conflicting declared types; that's not our problem, - // it's the problem of the module we are importing from. - Symbol::bound(declared_ty.inner_type()) - } - } - - // TODO (ticket: https://github.com/astral-sh/ruff/issues/14297) Our handling of boundness - // currently only depends on bindings, and ignores declarations. This is inconsistent, since - // we only look at bindings if the symbol may be undeclared. Consider the following example: - // ```py - // x: int - // - // if flag: - // y: int - // else - // y = 3 - // ``` - // If we import from this module, we will currently report `x` as a definitely-bound symbol - // (even though it has no bindings at all!) but report `y` as possibly-unbound (even though - // every path has either a binding or a declaration for it.) - } - - let _span = tracing::trace_span!("symbol", ?name).entered(); - - // We don't need to check for `typing_extensions` here, because `typing_extensions.TYPE_CHECKING` - // is just a re-export of `typing.TYPE_CHECKING`. - if name == "TYPE_CHECKING" - && file_to_module(db, scope.file(db)) - .is_some_and(|module| module.is_known(KnownModule::Typing)) - { - return Symbol::bound(Type::BooleanLiteral(true)); - } - if name == "platform" - && file_to_module(db, scope.file(db)) - .is_some_and(|module| module.is_known(KnownModule::Sys)) - { - match Program::get(db).python_platform(db) { - crate::PythonPlatform::Identifier(platform) => { - return Symbol::bound(Type::string_literal(db, platform.as_str())); - } - crate::PythonPlatform::All => { - // Fall through to the looked up type - } - } - } - - symbol_table(db, scope) - .symbol_id_by_name(name) - .map(|symbol| symbol_by_id(db, scope, symbol, requires_explicit_reexport)) - .unwrap_or(Symbol::Unbound) -} - -/// Return a list of the symbols that typeshed declares in the body scope of -/// the stub for the class `types.ModuleType`. -/// -/// Conceptually this could be a `Set` rather than a list, -/// but the number of symbols declared in this scope is likely to be very small, -/// so the cost of hashing the names is likely to be more expensive than it's worth. -#[salsa::tracked(return_ref)] -fn module_type_symbols<'db>(db: &'db dyn Db) -> smallvec::SmallVec<[ast::name::Name; 8]> { - let Some(module_type) = KnownClass::ModuleType - .to_class_literal(db) - .into_class_literal() - else { - // The most likely way we get here is if a user specified a `--custom-typeshed-dir` - // without a `types.pyi` stub in the `stdlib/` directory - return smallvec::SmallVec::default(); - }; - - let module_type_scope = module_type.class.body_scope(db); - let module_type_symbol_table = symbol_table(db, module_type_scope); - - // `__dict__` and `__init__` are very special members that can be accessed as attributes - // on the module when imported, but cannot be accessed as globals *inside* the module. - // - // `__getattr__` is even more special: it doesn't exist at runtime, but typeshed includes it - // to reduce false positives associated with functions that dynamically import modules - // and return `Instance(types.ModuleType)`. We should ignore it for any known module-literal type. - module_type_symbol_table - .symbols() - .filter(|symbol| symbol.is_declared()) - .map(symbol::Symbol::name) - .filter(|symbol_name| !matches!(&***symbol_name, "__dict__" | "__getattr__" | "__init__")) - .cloned() - .collect() -} - -/// Return the symbol for a member of `types.ModuleType`. -pub(crate) fn module_type_symbol<'db>(db: &'db dyn Db, name: &str) -> Symbol<'db> { - if module_type_symbols(db) - .iter() - .any(|module_type_member| &**module_type_member == name) - { - KnownClass::ModuleType.to_instance(db).member(db, name) - } else { - Symbol::Unbound - } -} - -/// Infer the public type of a symbol (its type as seen from outside its scope) in the given -/// `scope`. -fn symbol<'db>(db: &'db dyn Db, scope: ScopeId<'db>, name: &str) -> Symbol<'db> { - symbol_impl(db, scope, name, RequiresExplicitReExport::No) -} - -/// Infers the public type of a module-global symbol as seen from within the same file. -/// -/// If it's not defined explicitly in the global scope, it will look it up in `types.ModuleType` -/// with a few very special exceptions. -/// -/// Use [`imported_symbol`] to perform the lookup as seen from outside the file (e.g. via imports). -pub(crate) fn global_symbol<'db>(db: &'db dyn Db, file: File, name: &str) -> Symbol<'db> { - symbol_impl( - db, - global_scope(db, file), - name, - RequiresExplicitReExport::No, - ) - .or_fall_back_to(db, || module_type_symbol(db, name)) -} - -/// Infers the public type of an imported symbol. -pub(crate) fn imported_symbol<'db>(db: &'db dyn Db, module: &Module, name: &str) -> Symbol<'db> { - // If it's not found in the global scope, check if it's present as an instance on - // `types.ModuleType` or `builtins.object`. - // - // We do a more limited version of this in `global_symbol`, but there are two crucial - // differences here: - // - If a member is looked up as an attribute, `__init__` is also available on the module, but - // it isn't available as a global from inside the module - // - If a member is looked up as an attribute, members on `builtins.object` are also available - // (because `types.ModuleType` inherits from `object`); these attributes are also not - // available as globals from inside the module. - // - // The same way as in `global_symbol`, however, we need to be careful to 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. - external_symbol_impl(db, module.file(), name).or_fall_back_to(db, || { - if name == "__getattr__" { - Symbol::Unbound - } else { - KnownClass::ModuleType.to_instance(db).member(db, name) - } - }) -} - -/// Lookup the type of `symbol` in the builtins namespace. -/// -/// Returns `Symbol::Unbound` if the `builtins` module isn't available for some reason. -/// -/// Note that this function is only intended for use in the context of the builtins *namespace* -/// and should not be used when a symbol is being explicitly imported from the `builtins` module -/// (e.g. `from builtins import int`). -pub(crate) fn builtins_symbol<'db>(db: &'db dyn Db, symbol: &str) -> Symbol<'db> { - resolve_module(db, &KnownModule::Builtins.name()) - .map(|module| { - external_symbol_impl(db, module.file(), symbol).or_fall_back_to(db, || { - // We're looking up in the builtins namespace and not the module, so we should - // do the normal lookup in `types.ModuleType` and not the special one as in - // `imported_symbol`. - module_type_symbol(db, symbol) - }) - }) - .unwrap_or(Symbol::Unbound) -} - -fn external_symbol_impl<'db>(db: &'db dyn Db, file: File, name: &str) -> Symbol<'db> { - symbol_impl( - db, - global_scope(db, file), - name, - if file.is_stub(db.upcast()) { - RequiresExplicitReExport::Yes - } else { - RequiresExplicitReExport::No - }, - ) -} - /// Infer the type of a binding. pub(crate) fn binding_type<'db>(db: &'db dyn Db, definition: Definition<'db>) -> Type<'db> { let inference = infer_definition_types(db, definition); @@ -369,7 +91,10 @@ pub(crate) fn binding_type<'db>(db: &'db dyn Db, definition: Definition<'db>) -> } /// Infer the type of a declaration. -fn declaration_type<'db>(db: &'db dyn Db, definition: Definition<'db>) -> TypeAndQualifiers<'db> { +pub(crate) fn declaration_type<'db>( + db: &'db dyn Db, + definition: Definition<'db>, +) -> TypeAndQualifiers<'db> { let inference = infer_definition_types(db, definition); inference.declaration_type(definition) } @@ -404,229 +129,6 @@ fn definition_expression_type<'db>( } } -/// Infer the combined type from an iterator of bindings, and return it -/// together with boundness information in a [`Symbol`]. -/// -/// The type will be a union if there are multiple bindings with different types. -fn symbol_from_bindings<'db>( - db: &'db dyn Db, - bindings_with_constraints: BindingWithConstraintsIterator<'_, 'db>, - requires_explicit_reexport: RequiresExplicitReExport, -) -> Symbol<'db> { - let visibility_constraints = bindings_with_constraints.visibility_constraints; - let mut bindings_with_constraints = bindings_with_constraints.peekable(); - - let is_non_exported = |binding: Definition<'db>| { - requires_explicit_reexport.is_yes() && !binding.is_reexported(db) - }; - - let unbound_visibility = match bindings_with_constraints.peek() { - Some(BindingWithConstraints { - binding, - visibility_constraint, - constraints: _, - }) if binding.map_or(true, is_non_exported) => { - visibility_constraints.evaluate(db, *visibility_constraint) - } - _ => Truthiness::AlwaysFalse, - }; - - let mut types = bindings_with_constraints.filter_map( - |BindingWithConstraints { - binding, - constraints, - visibility_constraint, - }| { - let binding = binding?; - - if is_non_exported(binding) { - return None; - } - - let static_visibility = visibility_constraints.evaluate(db, visibility_constraint); - - if static_visibility.is_always_false() { - return None; - } - - let mut constraint_tys = constraints - .filter_map(|constraint| narrowing_constraint(db, constraint, binding)) - .peekable(); - - let binding_ty = binding_type(db, binding); - if constraint_tys.peek().is_some() { - let intersection_ty = constraint_tys - .fold( - IntersectionBuilder::new(db).add_positive(binding_ty), - IntersectionBuilder::add_positive, - ) - .build(); - Some(intersection_ty) - } else { - Some(binding_ty) - } - }, - ); - - if let Some(first) = types.next() { - let boundness = match unbound_visibility { - Truthiness::AlwaysTrue => { - unreachable!("If we have at least one binding, the scope-start should not be definitely visible") - } - Truthiness::AlwaysFalse => Boundness::Bound, - Truthiness::Ambiguous => Boundness::PossiblyUnbound, - }; - - if let Some(second) = types.next() { - Symbol::Type( - UnionType::from_elements(db, [first, second].into_iter().chain(types)), - boundness, - ) - } else { - Symbol::Type(first, boundness) - } - } else { - Symbol::Unbound - } -} - -/// A type with declaredness information, and a set of type qualifiers. -/// -/// This is used to represent the result of looking up the declared type. Consider this -/// example: -/// ```py -/// class C: -/// if flag: -/// variable: ClassVar[int] -/// ``` -/// If we look up the declared type of `variable` in the scope of class `C`, we will get -/// the type `int`, a "declaredness" of [`Boundness::PossiblyUnbound`], and the information -/// that this comes with a [`TypeQualifiers::CLASS_VAR`] type qualifier. -#[derive(Debug)] -pub(crate) struct SymbolAndQualifiers<'db>(Symbol<'db>, TypeQualifiers); - -impl SymbolAndQualifiers<'_> { - /// Constructor that creates a [`SymbolAndQualifiers`] instance with a [`TodoType`] type - /// and no qualifiers. - fn todo(message: &'static str) -> Self { - Self(Symbol::todo(message), TypeQualifiers::empty()) - } - - fn is_class_var(&self) -> bool { - self.1.contains(TypeQualifiers::CLASS_VAR) - } - - fn is_final(&self) -> bool { - self.1.contains(TypeQualifiers::FINAL) - } -} - -impl<'db> From> for SymbolAndQualifiers<'db> { - fn from(symbol: Symbol<'db>) -> Self { - SymbolAndQualifiers(symbol, TypeQualifiers::empty()) - } -} - -/// The result of looking up a declared type from declarations; see [`symbol_from_declarations`]. -type SymbolFromDeclarationsResult<'db> = - Result, (TypeAndQualifiers<'db>, Box<[Type<'db>]>)>; - -/// Build a declared type from a [`DeclarationsIterator`]. -/// -/// If there is only one declaration, or all declarations declare the same type, returns -/// `Ok(..)`. If there are conflicting declarations, returns an `Err(..)` variant with -/// a union of the declared types as well as a list of all conflicting types. -/// -/// This function also returns declaredness information (see [`Symbol`]) and a set of -/// [`TypeQualifiers`] that have been specified on the declaration(s). -fn symbol_from_declarations<'db>( - db: &'db dyn Db, - declarations: DeclarationsIterator<'_, 'db>, - requires_explicit_reexport: RequiresExplicitReExport, -) -> SymbolFromDeclarationsResult<'db> { - let visibility_constraints = declarations.visibility_constraints; - let mut declarations = declarations.peekable(); - - let is_non_exported = |declaration: Definition<'db>| { - requires_explicit_reexport.is_yes() && !declaration.is_reexported(db) - }; - - let undeclared_visibility = match declarations.peek() { - Some(DeclarationWithConstraint { - declaration, - visibility_constraint, - }) if declaration.map_or(true, is_non_exported) => { - visibility_constraints.evaluate(db, *visibility_constraint) - } - _ => Truthiness::AlwaysFalse, - }; - - let mut types = declarations.filter_map( - |DeclarationWithConstraint { - declaration, - visibility_constraint, - }| { - let declaration = declaration?; - - if is_non_exported(declaration) { - return None; - } - - let static_visibility = visibility_constraints.evaluate(db, visibility_constraint); - - if static_visibility.is_always_false() { - None - } else { - Some(declaration_type(db, declaration)) - } - }, - ); - - if let Some(first) = types.next() { - let mut conflicting: Vec> = vec![]; - let declared_ty = if let Some(second) = types.next() { - let ty_first = first.inner_type(); - let mut qualifiers = first.qualifiers(); - - let mut builder = UnionBuilder::new(db).add(ty_first); - for other in std::iter::once(second).chain(types) { - let other_ty = other.inner_type(); - if !ty_first.is_equivalent_to(db, other_ty) { - conflicting.push(other_ty); - } - builder = builder.add(other_ty); - qualifiers = qualifiers.union(other.qualifiers()); - } - TypeAndQualifiers::new(builder.build(), qualifiers) - } else { - first - }; - if conflicting.is_empty() { - let boundness = match undeclared_visibility { - Truthiness::AlwaysTrue => { - unreachable!("If we have at least one declaration, the scope-start should not be definitely visible") - } - Truthiness::AlwaysFalse => Boundness::Bound, - Truthiness::Ambiguous => Boundness::PossiblyUnbound, - }; - - Ok(SymbolAndQualifiers( - Symbol::Type(declared_ty.inner_type(), boundness), - declared_ty.qualifiers(), - )) - } else { - Err(( - declared_ty, - std::iter::once(first.inner_type()) - .chain(conflicting) - .collect(), - )) - } - } else { - Ok(Symbol::Unbound.into()) - } -} - /// Meta data for `Type::Todo`, which represents a known limitation in red-knot. #[cfg(debug_assertions)] #[derive(Copy, Clone, Debug, PartialEq, Eq, Hash)] @@ -4476,7 +3978,7 @@ impl<'db> Class<'db> { let declarations = use_def.public_declarations(symbol_id); - match symbol_from_declarations(db, declarations, RequiresExplicitReExport::No) { + match symbol_from_declarations(db, declarations) { Ok(SymbolAndQualifiers(Symbol::Type(declared_ty, _), qualifiers)) => { // The attribute is declared in the class body. @@ -4498,7 +4000,7 @@ impl<'db> Class<'db> { // in a method, and it could also be *bound* in the class body (and/or in a method). let bindings = use_def.public_bindings(symbol_id); - let inferred = symbol_from_bindings(db, bindings, RequiresExplicitReExport::No); + let inferred = symbol_from_bindings(db, bindings); let inferred_ty = inferred.ignore_possibly_unbound(); Self::implicit_instance_attribute(db, body_scope, name, inferred_ty).into() @@ -4601,6 +4103,10 @@ pub struct ClassLiteralType<'db> { } impl<'db> ClassLiteralType<'db> { + pub(crate) fn body_scope(self, db: &'db dyn Db) -> ScopeId<'db> { + self.class.body_scope(db) + } + fn member(self, db: &'db dyn Db, name: &str) -> Symbol<'db> { self.class.class_member(db, name) } @@ -5039,12 +4545,11 @@ static_assertions::assert_eq_size!(Type, [u8; 16]); pub(crate) mod tests { use super::*; use crate::db::tests::{setup_db, TestDbBuilder}; - use crate::stdlib::typing_symbol; + use crate::symbol::{typing_extensions_symbol, typing_symbol}; use ruff_db::files::system_path_to_file; use ruff_db::parsed::parsed_module; use ruff_db::system::DbWithTestSystem; use ruff_db::testing::assert_function_query_was_not_run; - use ruff_python_ast as ast; use ruff_python_ast::python_version::PythonVersion; use test_case::test_case; @@ -5081,18 +4586,6 @@ pub(crate) mod tests { ); } - #[test] - fn module_type_symbols_includes_declared_types_but_not_referenced_types() { - let db = setup_db(); - let symbol_names = module_type_symbols(&db); - - let dunder_name_symbol_name = ast::name::Name::new_static("__name__"); - assert!(symbol_names.contains(&dunder_name_symbol_name)); - - let property_symbol_name = ast::name::Name::new_static("property"); - assert!(!symbol_names.contains(&property_symbol_name)); - } - /// Inferring the result of a call-expression shouldn't need to re-run after /// a trivial change to the function's file (e.g. by adding a docstring to the function). #[test] diff --git a/crates/red_knot_python_semantic/src/types/infer.rs b/crates/red_knot_python_semantic/src/types/infer.rs index a7b4e7c8100cc0..0b9f6d21ea6ac4 100644 --- a/crates/red_knot_python_semantic/src/types/infer.rs +++ b/crates/red_knot_python_semantic/src/types/infer.rs @@ -48,8 +48,10 @@ use crate::semantic_index::expression::{Expression, ExpressionKind}; use crate::semantic_index::semantic_index; use crate::semantic_index::symbol::{NodeWithScopeKind, NodeWithScopeRef, ScopeId}; use crate::semantic_index::SemanticIndex; -use crate::stdlib::builtins_module_scope; -use crate::symbol::LookupError; +use crate::symbol::{ + builtins_module_scope, builtins_symbol, symbol, symbol_from_bindings, symbol_from_declarations, + typing_extensions_symbol, LookupError, +}; use crate::types::call::{Argument, CallArguments}; use crate::types::diagnostic::{ report_invalid_arguments_to_annotated, report_invalid_assignment, @@ -64,13 +66,12 @@ use crate::types::diagnostic::{ use crate::types::mro::MroErrorKind; use crate::types::unpacker::{UnpackResult, Unpacker}; use crate::types::{ - builtins_symbol, symbol, symbol_from_bindings, symbol_from_declarations, todo_type, - typing_extensions_symbol, Boundness, CallDunderResult, Class, ClassLiteralType, DynamicType, - FunctionType, InstanceType, IntersectionBuilder, IntersectionType, IterationOutcome, - KnownClass, KnownFunction, KnownInstanceType, MetaclassCandidate, MetaclassErrorKind, - RequiresExplicitReExport, SliceLiteralType, SubclassOfType, Symbol, SymbolAndQualifiers, - Truthiness, TupleType, Type, TypeAliasType, TypeAndQualifiers, TypeArrayDisplay, - TypeQualifiers, TypeVarBoundOrConstraints, TypeVarInstance, UnionBuilder, UnionType, + todo_type, Boundness, CallDunderResult, Class, ClassLiteralType, DynamicType, FunctionType, + InstanceType, IntersectionBuilder, IntersectionType, IterationOutcome, KnownClass, + KnownFunction, KnownInstanceType, MetaclassCandidate, MetaclassErrorKind, SliceLiteralType, + SubclassOfType, Symbol, SymbolAndQualifiers, Truthiness, TupleType, Type, TypeAliasType, + TypeAndQualifiers, TypeArrayDisplay, TypeQualifiers, TypeVarBoundOrConstraints, + TypeVarInstance, UnionBuilder, UnionType, }; use crate::unpack::Unpack; use crate::util::subscript::{PyIndex, PySlice}; @@ -872,25 +873,22 @@ impl<'db> TypeInferenceBuilder<'db> { let use_def = self.index.use_def_map(binding.file_scope(self.db())); let declarations = use_def.declarations_at_binding(binding); let mut bound_ty = ty; - let declared_ty = - symbol_from_declarations(self.db(), declarations, RequiresExplicitReExport::No) - .map(|SymbolAndQualifiers(s, _)| { - s.ignore_possibly_unbound().unwrap_or(Type::unknown()) - }) - .unwrap_or_else(|(ty, conflicting)| { - // TODO point out the conflicting declarations in the diagnostic? - let symbol_table = self.index.symbol_table(binding.file_scope(self.db())); - let symbol_name = symbol_table.symbol(binding.symbol(self.db())).name(); - self.context.report_lint( - &CONFLICTING_DECLARATIONS, - node, - format_args!( - "Conflicting declared types for `{symbol_name}`: {}", - conflicting.display(self.db()) - ), - ); - ty.inner_type() - }); + let declared_ty = symbol_from_declarations(self.db(), declarations) + .map(|SymbolAndQualifiers(s, _)| s.ignore_possibly_unbound().unwrap_or(Type::unknown())) + .unwrap_or_else(|(ty, conflicting)| { + // TODO point out the conflicting declarations in the diagnostic? + let symbol_table = self.index.symbol_table(binding.file_scope(self.db())); + let symbol_name = symbol_table.symbol(binding.symbol(self.db())).name(); + self.context.report_lint( + &CONFLICTING_DECLARATIONS, + node, + format_args!( + "Conflicting declared types for `{symbol_name}`: {}", + conflicting.display(self.db()) + ), + ); + ty.inner_type() + }); if !bound_ty.is_assignable_to(self.db(), declared_ty) { report_invalid_assignment(&self.context, node, declared_ty, bound_ty); // allow declarations to override inference in case of invalid assignment @@ -910,10 +908,9 @@ impl<'db> TypeInferenceBuilder<'db> { let use_def = self.index.use_def_map(declaration.file_scope(self.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, RequiresExplicitReExport::No) - .ignore_possibly_unbound() - .unwrap_or(Type::Never); + let inferred_ty = symbol_from_bindings(self.db(), prior_bindings) + .ignore_possibly_unbound() + .unwrap_or(Type::Never); let ty = if inferred_ty.is_assignable_to(self.db(), ty.inner_type()) { ty } else { @@ -3308,11 +3305,7 @@ impl<'db> TypeInferenceBuilder<'db> { // If we're inferring types of deferred expressions, always treat them as public symbols let local_scope_symbol = if self.is_deferred() { if let Some(symbol_id) = symbol_table.symbol_id_by_name(symbol_name) { - symbol_from_bindings( - db, - use_def.public_bindings(symbol_id), - RequiresExplicitReExport::No, - ) + symbol_from_bindings(db, use_def.public_bindings(symbol_id)) } else { assert!( self.deferred_state.in_string_annotation(), @@ -3322,11 +3315,7 @@ impl<'db> TypeInferenceBuilder<'db> { } } else { let use_id = name_node.scoped_use_id(db, scope); - symbol_from_bindings( - db, - use_def.bindings_at_use(use_id), - RequiresExplicitReExport::No, - ) + symbol_from_bindings(db, use_def.bindings_at_use(use_id)) }; let symbol = local_scope_symbol.or_fall_back_to(db, || { diff --git a/crates/red_knot_python_semantic/src/types/property_tests.rs b/crates/red_knot_python_semantic/src/types/property_tests.rs index 9d3db01a76f8b8..c1606443f4b6b6 100644 --- a/crates/red_knot_python_semantic/src/types/property_tests.rs +++ b/crates/red_knot_python_semantic/src/types/property_tests.rs @@ -27,9 +27,9 @@ use std::sync::{Arc, Mutex, MutexGuard, OnceLock}; use crate::db::tests::{setup_db, TestDb}; +use crate::symbol::{builtins_symbol, known_module_symbol}; use crate::types::{ - builtins_symbol, known_module_symbol, IntersectionBuilder, KnownClass, KnownInstanceType, - SubclassOfType, TupleType, Type, UnionType, + IntersectionBuilder, KnownClass, KnownInstanceType, SubclassOfType, TupleType, Type, UnionType, }; use crate::KnownModule; use quickcheck::{Arbitrary, Gen};