From f16a14ad01167cfac877536d84512dd78c54a8d2 Mon Sep 17 00:00:00 2001 From: David Peter Date: Tue, 29 Oct 2024 15:06:15 +0100 Subject: [PATCH 01/20] [red-knot] remove Type::Unbound, add SymbolLookupResult --- .../resources/mdtest/assignment/unbound.md | 13 +- .../resources/mdtest/boolean/short_circuit.md | 20 +- .../mdtest/conditional/if_statement.md | 4 +- .../resources/mdtest/conditional/match.md | 2 +- .../resources/mdtest/import/conditional.md | 8 +- .../resources/mdtest/loops/async_for_loops.md | 6 +- .../resources/mdtest/loops/for_loop.md | 8 +- .../mdtest/scopes/moduletype_attrs.md | 18 +- .../src/semantic_model.rs | 4 +- crates/red_knot_python_semantic/src/stdlib.rs | 38 ++- crates/red_knot_python_semantic/src/types.rs | 257 ++++++++++-------- .../src/types/builder.rs | 47 +--- .../src/types/display.rs | 9 +- .../src/types/infer.rs | 163 ++++++----- 14 files changed, 320 insertions(+), 277 deletions(-) diff --git a/crates/red_knot_python_semantic/resources/mdtest/assignment/unbound.md b/crates/red_knot_python_semantic/resources/mdtest/assignment/unbound.md index 980dd4b510e3d..4a6b90eee65db 100644 --- a/crates/red_knot_python_semantic/resources/mdtest/assignment/unbound.md +++ b/crates/red_knot_python_semantic/resources/mdtest/assignment/unbound.md @@ -6,11 +6,20 @@ x = foo # error: [unresolved-reference] "Name `foo` used when not defined" foo = 1 -# error: [unresolved-reference] -# revealed: Unbound +# No error `unresolved-reference` diagnostic is reported for `x`. This is +# desirable because we would get a lot of cascading errors even though there +# is only one root cause (the unbound variable `foo`). + +# revealed: Unknown reveal_type(x) ``` +Note: in this particular example, one could argue that the most likely error would +be a wrong order of the `x`/`foo` definitions, and so it could be desirable to infer +`Literal[1]` for the type of `x`. On the other hand, there might be a variable `fob` +a little higher up in this file, and the actual error might have been just a typo. +Inferring `Unknown` thus seems like the safest option. + ## Unbound class variable Name lookups within a class scope fall back to globals, but lookups of class attributes don't. diff --git a/crates/red_knot_python_semantic/resources/mdtest/boolean/short_circuit.md b/crates/red_knot_python_semantic/resources/mdtest/boolean/short_circuit.md index ed6fa58fbca5b..b467200452a7d 100644 --- a/crates/red_knot_python_semantic/resources/mdtest/boolean/short_circuit.md +++ b/crates/red_knot_python_semantic/resources/mdtest/boolean/short_circuit.md @@ -12,11 +12,11 @@ def bool_instance() -> bool: if bool_instance() or (x := 1): # error: [possibly-unresolved-reference] - reveal_type(x) # revealed: Unbound | Literal[1] + reveal_type(x) # revealed: Literal[1] if bool_instance() and (x := 1): # error: [possibly-unresolved-reference] - reveal_type(x) # revealed: Unbound | Literal[1] + reveal_type(x) # revealed: Literal[1] ``` ## First expression is always evaluated @@ -36,14 +36,14 @@ if (x := 1) and bool_instance(): ```py if True or (x := 1): - # TODO: infer that the second arm is never executed so type should be just "Unbound". + # TODO: infer that the second arm is never executed, and raise `unresolved-reference`. # error: [possibly-unresolved-reference] - reveal_type(x) # revealed: Unbound | Literal[1] + reveal_type(x) # revealed: Literal[1] if True and (x := 1): - # TODO: infer that the second arm is always executed so type should be just "Literal[1]". + # TODO: infer that the second arm is always executed, do not raise a diagnostic # error: [possibly-unresolved-reference] - reveal_type(x) # revealed: Unbound | Literal[1] + reveal_type(x) # revealed: Literal[1] ``` ## Later expressions can always use variables from earlier expressions @@ -55,7 +55,7 @@ def bool_instance() -> bool: bool_instance() or (x := 1) or reveal_type(x) # revealed: Literal[1] # error: [unresolved-reference] -bool_instance() or reveal_type(y) or (y := 1) # revealed: Unbound +bool_instance() or reveal_type(y) or (y := 1) # revealed: Unknown ``` ## Nested expressions @@ -66,13 +66,13 @@ def bool_instance() -> bool: if bool_instance() or ((x := 1) and bool_instance()): # error: "Name `x` used when possibly not defined" - reveal_type(x) # revealed: Unbound | Literal[1] + reveal_type(x) # revealed: Literal[1] if ((y := 1) and bool_instance()) or bool_instance(): reveal_type(y) # revealed: Literal[1] # error: [possibly-unresolved-reference] -if (bool_instance() and (z := 1)) or reveal_type(z): # revealed: Unbound | Literal[1] +if (bool_instance() and (z := 1)) or reveal_type(z): # revealed: Literal[1] # error: [possibly-unresolved-reference] - reveal_type(z) # revealed: Unbound | Literal[1] + reveal_type(z) # revealed: Literal[1] ``` diff --git a/crates/red_knot_python_semantic/resources/mdtest/conditional/if_statement.md b/crates/red_knot_python_semantic/resources/mdtest/conditional/if_statement.md index a44a58b4cacc7..1e3ec4ea50e53 100644 --- a/crates/red_knot_python_semantic/resources/mdtest/conditional/if_statement.md +++ b/crates/red_knot_python_semantic/resources/mdtest/conditional/if_statement.md @@ -37,11 +37,11 @@ x = y reveal_type(x) # revealed: Literal[3, 4, 5] -# revealed: Unbound | Literal[2] +# revealed: Literal[2] # error: [possibly-unresolved-reference] reveal_type(r) -# revealed: Unbound | Literal[5] +# revealed: Literal[5] # error: [possibly-unresolved-reference] reveal_type(s) ``` diff --git a/crates/red_knot_python_semantic/resources/mdtest/conditional/match.md b/crates/red_knot_python_semantic/resources/mdtest/conditional/match.md index 5269bc8b89fcd..a173354b4cf7c 100644 --- a/crates/red_knot_python_semantic/resources/mdtest/conditional/match.md +++ b/crates/red_knot_python_semantic/resources/mdtest/conditional/match.md @@ -21,7 +21,7 @@ match 0: case 2: y = 3 -# revealed: Unbound | Literal[2, 3] +# revealed: Literal[2, 3] # error: [possibly-unresolved-reference] reveal_type(y) ``` diff --git a/crates/red_knot_python_semantic/resources/mdtest/import/conditional.md b/crates/red_knot_python_semantic/resources/mdtest/import/conditional.md index 76756cb527b7d..787c0fff3a4a2 100644 --- a/crates/red_knot_python_semantic/resources/mdtest/import/conditional.md +++ b/crates/red_knot_python_semantic/resources/mdtest/import/conditional.md @@ -12,11 +12,11 @@ if flag: x = y # error: [possibly-unresolved-reference] -# revealed: Unbound | Literal[3] +# revealed: Literal[3] # error: [possibly-unresolved-reference] reveal_type(x) -# revealed: Unbound | Literal[3] +# revealed: Literal[3] # error: [possibly-unresolved-reference] reveal_type(y) ``` @@ -40,11 +40,11 @@ if flag: y: int = 3 x = y # error: [possibly-unresolved-reference] -# revealed: Unbound | Literal[3] +# revealed: Literal[3] # error: [possibly-unresolved-reference] reveal_type(x) -# revealed: Unbound | Literal[3] +# revealed: Literal[3] # error: [possibly-unresolved-reference] reveal_type(y) ``` diff --git a/crates/red_knot_python_semantic/resources/mdtest/loops/async_for_loops.md b/crates/red_knot_python_semantic/resources/mdtest/loops/async_for_loops.md index a1a55f81d3ede..b9a6d083ece81 100644 --- a/crates/red_knot_python_semantic/resources/mdtest/loops/async_for_loops.md +++ b/crates/red_knot_python_semantic/resources/mdtest/loops/async_for_loops.md @@ -17,8 +17,8 @@ async def foo(): async for x in Iterator(): pass - # TODO: should reveal `Unbound | Unknown` because `__aiter__` is not defined - # revealed: Unbound | @Todo + # TODO: should reveal `Unknown` because `__aiter__` is not defined + # revealed: @Todo # error: [possibly-unresolved-reference] reveal_type(x) ``` @@ -40,6 +40,6 @@ async def foo(): pass # error: [possibly-unresolved-reference] - # revealed: Unbound | @Todo + # revealed: @Todo reveal_type(x) ``` diff --git a/crates/red_knot_python_semantic/resources/mdtest/loops/for_loop.md b/crates/red_knot_python_semantic/resources/mdtest/loops/for_loop.md index d2e30b0f521d0..4d369642d3364 100644 --- a/crates/red_knot_python_semantic/resources/mdtest/loops/for_loop.md +++ b/crates/red_knot_python_semantic/resources/mdtest/loops/for_loop.md @@ -14,7 +14,7 @@ class IntIterable: for x in IntIterable(): pass -# revealed: Unbound | int +# revealed: int # error: [possibly-unresolved-reference] reveal_type(x) ``` @@ -87,7 +87,7 @@ class OldStyleIterable: for x in OldStyleIterable(): pass -# revealed: Unbound | int +# revealed: int # error: [possibly-unresolved-reference] reveal_type(x) ``` @@ -98,7 +98,7 @@ reveal_type(x) for x in (1, "a", b"foo"): pass -# revealed: Unbound | Literal[1] | Literal["a"] | Literal[b"foo"] +# revealed: Literal[1] | Literal["a"] | Literal[b"foo"] # error: [possibly-unresolved-reference] reveal_type(x) ``` @@ -120,7 +120,7 @@ class NotIterable: for x in NotIterable(): # error: "Object of type `NotIterable` is not iterable" pass -# revealed: Unbound | Unknown +# revealed: Unknown # error: [possibly-unresolved-reference] reveal_type(x) ``` diff --git a/crates/red_knot_python_semantic/resources/mdtest/scopes/moduletype_attrs.md b/crates/red_knot_python_semantic/resources/mdtest/scopes/moduletype_attrs.md index ffe73bfceffdf..124f4576ec6c9 100644 --- a/crates/red_knot_python_semantic/resources/mdtest/scopes/moduletype_attrs.md +++ b/crates/red_knot_python_semantic/resources/mdtest/scopes/moduletype_attrs.md @@ -19,7 +19,7 @@ reveal_type(__path__) # revealed: @Todo # TODO: this should probably be added to typeshed; not sure why it isn't? # error: [unresolved-reference] -# revealed: Unbound +# revealed: Unknown reveal_type(__doc__) class X: @@ -34,15 +34,15 @@ module globals; these are excluded: ```py path=unbound_dunders.py # error: [unresolved-reference] -# revealed: Unbound +# revealed: Unknown reveal_type(__getattr__) # error: [unresolved-reference] -# revealed: Unbound +# revealed: Unknown reveal_type(__dict__) # error: [unresolved-reference] -# revealed: Unbound +# revealed: Unknown reveal_type(__init__) ``` @@ -61,10 +61,10 @@ reveal_type(typing.__init__) # revealed: Literal[__init__] # These come from `builtins.object`, not `types.ModuleType`: # TODO: we don't currently understand `types.ModuleType` as inheriting from `object`; -# these should not reveal `Unbound`: -reveal_type(typing.__eq__) # revealed: Unbound -reveal_type(typing.__class__) # revealed: Unbound -reveal_type(typing.__module__) # revealed: Unbound +# these should not reveal `Unknown`: +reveal_type(typing.__eq__) # revealed: Unknown +reveal_type(typing.__class__) # revealed: Unknown +reveal_type(typing.__module__) # revealed: Unknown # TODO: needs support for attribute access on instances, properties and generics; # should be `dict[str, Any]` @@ -78,7 +78,7 @@ where we know exactly which module we're dealing with: ```py path=__getattr__.py import typing -reveal_type(typing.__getattr__) # revealed: Unbound +reveal_type(typing.__getattr__) # revealed: Unknown ``` ## `types.ModuleType.__dict__` takes precedence over global variable `__dict__` diff --git a/crates/red_knot_python_semantic/src/semantic_model.rs b/crates/red_knot_python_semantic/src/semantic_model.rs index 9cdbec789c50a..7ce6b318de8a4 100644 --- a/crates/red_knot_python_semantic/src/semantic_model.rs +++ b/crates/red_knot_python_semantic/src/semantic_model.rs @@ -8,7 +8,7 @@ use crate::module_name::ModuleName; use crate::module_resolver::{resolve_module, Module}; use crate::semantic_index::ast_ids::HasScopedAstId; use crate::semantic_index::semantic_index; -use crate::types::{binding_ty, global_symbol_ty, infer_scope_types, Type}; +use crate::types::{binding_ty, global_symbol_ty, infer_scope_types, SymbolLookupResult, Type}; use crate::Db; pub struct SemanticModel<'db> { @@ -39,7 +39,7 @@ impl<'db> SemanticModel<'db> { resolve_module(self.db, module_name) } - pub fn global_symbol_ty(&self, module: &Module, symbol_name: &str) -> Type<'db> { + pub fn global_symbol_ty(&self, module: &Module, symbol_name: &str) -> SymbolLookupResult<'db> { global_symbol_ty(self.db, module.file(), symbol_name) } } diff --git a/crates/red_knot_python_semantic/src/stdlib.rs b/crates/red_knot_python_semantic/src/stdlib.rs index dd62f131b9b74..f094a6e00f867 100644 --- a/crates/red_knot_python_semantic/src/stdlib.rs +++ b/crates/red_knot_python_semantic/src/stdlib.rs @@ -2,7 +2,7 @@ use crate::module_name::ModuleName; use crate::module_resolver::resolve_module; use crate::semantic_index::global_scope; use crate::semantic_index::symbol::ScopeId; -use crate::types::{global_symbol_ty, Type}; +use crate::types::{global_symbol_ty, SymbolLookupResult}; use crate::Db; /// Enumeration of various core stdlib modules, for which we have dedicated Salsa queries. @@ -33,61 +33,57 @@ impl CoreStdlibModule { /// Lookup the type of `symbol` in a given core module /// -/// Returns `Unbound` if the given core module cannot be resolved for some reason +/// Returns `SymbolLookupResult::Unbound` if the given core module cannot be resolved for some reason fn core_module_symbol_ty<'db>( db: &'db dyn Db, core_module: CoreStdlibModule, symbol: &str, -) -> Type<'db> { +) -> SymbolLookupResult<'db> { resolve_module(db, &core_module.name()) .map(|module| global_symbol_ty(db, module.file(), symbol)) - .map(|ty| { - if ty.is_unbound() { - ty - } else { - ty.replace_unbound_with(db, Type::Never) - } - }) - .unwrap_or(Type::Unbound) + .unwrap_or(SymbolLookupResult::Unbound) } /// Lookup the type of `symbol` in the builtins namespace. /// -/// Returns `Unbound` if the `builtins` module isn't available for some reason. +/// Returns `SymbolLookupResult::Unbound` if the `builtins` module isn't available for some reason. #[inline] -pub(crate) fn builtins_symbol_ty<'db>(db: &'db dyn Db, symbol: &str) -> Type<'db> { +pub(crate) fn builtins_symbol_ty<'db>(db: &'db dyn Db, symbol: &str) -> SymbolLookupResult<'db> { core_module_symbol_ty(db, CoreStdlibModule::Builtins, symbol) } /// Lookup the type of `symbol` in the `types` module namespace. /// -/// Returns `Unbound` if the `types` module isn't available for some reason. +/// Returns `SymbolLookupResult::Unbound` if the `types` module isn't available for some reason. #[inline] -pub(crate) fn types_symbol_ty<'db>(db: &'db dyn Db, symbol: &str) -> Type<'db> { +pub(crate) fn types_symbol_ty<'db>(db: &'db dyn Db, symbol: &str) -> SymbolLookupResult<'db> { core_module_symbol_ty(db, CoreStdlibModule::Types, symbol) } /// Lookup the type of `symbol` in the `typing` module namespace. /// -/// Returns `Unbound` if the `typing` module isn't available for some reason. +/// Returns `SymbolLookupResult::Unbound` if the `typing` module isn't available for some reason. #[inline] #[allow(dead_code)] // currently only used in tests -pub(crate) fn typing_symbol_ty<'db>(db: &'db dyn Db, symbol: &str) -> Type<'db> { +pub(crate) fn typing_symbol_ty<'db>(db: &'db dyn Db, symbol: &str) -> SymbolLookupResult<'db> { core_module_symbol_ty(db, CoreStdlibModule::Typing, symbol) } /// Lookup the type of `symbol` in the `_typeshed` module namespace. /// -/// Returns `Unbound` if the `_typeshed` module isn't available for some reason. +/// Returns `SymbolLookupResult::Unbound` if the `_typeshed` module isn't available for some reason. #[inline] -pub(crate) fn typeshed_symbol_ty<'db>(db: &'db dyn Db, symbol: &str) -> Type<'db> { +pub(crate) fn typeshed_symbol_ty<'db>(db: &'db dyn Db, symbol: &str) -> SymbolLookupResult<'db> { core_module_symbol_ty(db, CoreStdlibModule::Typeshed, symbol) } /// Lookup the type of `symbol` in the `typing_extensions` module namespace. /// -/// Returns `Unbound` if the `typing_extensions` module isn't available for some reason. +/// Returns `SymbolLookupResult::Unbound` if the `typing_extensions` module isn't available for some reason. #[inline] -pub(crate) fn typing_extensions_symbol_ty<'db>(db: &'db dyn Db, symbol: &str) -> Type<'db> { +pub(crate) fn typing_extensions_symbol_ty<'db>( + db: &'db dyn Db, + symbol: &str, +) -> SymbolLookupResult<'db> { core_module_symbol_ty(db, CoreStdlibModule::TypingExtensions, symbol) } diff --git a/crates/red_knot_python_semantic/src/types.rs b/crates/red_knot_python_semantic/src/types.rs index 6403091adbb58..bd252dce76a04 100644 --- a/crates/red_knot_python_semantic/src/types.rs +++ b/crates/red_knot_python_semantic/src/types.rs @@ -29,6 +29,52 @@ mod display; mod infer; mod narrow; +#[derive(Debug, Clone, PartialEq, Eq)] +pub enum SymbolLookupResult<'db> { + Bound(Type<'db>), + Unbound, +} + +impl<'db> SymbolLookupResult<'db> { + pub fn is_unbound(&self) -> bool { + matches!(self, SymbolLookupResult::Unbound) + } + + pub fn replace_unbound_with(self, replacement: Type<'db>) -> SymbolLookupResult<'db> { + match self { + r @ SymbolLookupResult::Bound(_) => r, + SymbolLookupResult::Unbound => SymbolLookupResult::Bound(replacement), + } + } + + #[track_caller] + fn todo_unwrap_type(&self) -> Type<'db> { + match self { + SymbolLookupResult::Bound(ty) => *ty, + SymbolLookupResult::Unbound => todo!(), + } + } + + #[track_caller] + fn expect_bound(self) -> Type<'db> { + match self { + SymbolLookupResult::Bound(ty) => ty, + SymbolLookupResult::Unbound => panic!("Expected a bound type"), + } + } + + fn unwrap_or(&self, other: Type<'db>) -> Type<'db> { + match self { + SymbolLookupResult::Bound(ty) => *ty, + SymbolLookupResult::Unbound => other, + } + } + + fn unwrap_or_unknown(&self) -> Type<'db> { + self.unwrap_or(Type::Unknown) + } +} + pub fn check_types(db: &dyn Db, file: File) -> TypeCheckDiagnostics { let _span = tracing::trace_span!("check_types", file=?file.path(db)).entered(); @@ -44,7 +90,11 @@ pub fn check_types(db: &dyn Db, file: File) -> TypeCheckDiagnostics { } /// Infer the public type of a symbol (its type as seen from outside its scope). -fn symbol_ty_by_id<'db>(db: &'db dyn Db, scope: ScopeId<'db>, symbol: ScopedSymbolId) -> Type<'db> { +fn symbol_ty_by_id<'db>( + db: &'db dyn Db, + scope: ScopeId<'db>, + symbol: ScopedSymbolId, +) -> SymbolLookupResult<'db> { let _span = tracing::trace_span!("symbol_ty_by_id", ?symbol).entered(); let use_def = use_def_map(db, scope); @@ -55,37 +105,42 @@ fn symbol_ty_by_id<'db>(db: &'db dyn Db, scope: ScopeId<'db>, symbol: ScopedSymb let declarations = use_def.public_declarations(symbol); // If the symbol is undeclared in some paths, include the inferred type in the public type. let undeclared_ty = if declarations.may_be_undeclared() { - Some(bindings_ty( - db, - use_def.public_bindings(symbol), - use_def - .public_may_be_unbound(symbol) - .then_some(Type::Unbound), - )) + Some( + bindings_ty( + db, + use_def.public_bindings(symbol), + // use_def + // .public_may_be_unbound(symbol) + // .then_some(Type::Unbound), + ) + .todo_unwrap_type(), + ) } else { None }; // Intentionally ignore conflicting declared types; that's not our problem, it's the // problem of the module we are importing from. - declarations_ty(db, declarations, undeclared_ty).unwrap_or_else(|(ty, _)| ty) + SymbolLookupResult::Bound( + declarations_ty(db, declarations, undeclared_ty).unwrap_or_else(|(ty, _)| ty), + ) } else { bindings_ty( db, use_def.public_bindings(symbol), - use_def - .public_may_be_unbound(symbol) - .then_some(Type::Unbound), + // use_def + // .public_may_be_unbound(symbol) + // .then_some(Type::Unbound), ) } } /// Shorthand for `symbol_ty_by_id` that takes a symbol name instead of an ID. -fn symbol_ty<'db>(db: &'db dyn Db, scope: ScopeId<'db>, name: &str) -> Type<'db> { +fn symbol_ty<'db>(db: &'db dyn Db, scope: ScopeId<'db>, name: &str) -> SymbolLookupResult<'db> { let table = symbol_table(db, scope); table .symbol_id_by_name(name) .map(|symbol| symbol_ty_by_id(db, scope, symbol)) - .unwrap_or(Type::Unbound) + .unwrap_or(SymbolLookupResult::Unbound) } /// Return a list of the symbols that typeshed declares in the body scope of @@ -124,9 +179,13 @@ fn module_type_symbols<'db>(db: &'db dyn Db) -> smallvec::SmallVec<[ast::name::N } /// Looks up a module-global symbol by name in a file. -pub(crate) fn global_symbol_ty<'db>(db: &'db dyn Db, file: File, name: &str) -> Type<'db> { +pub(crate) fn global_symbol_ty<'db>( + db: &'db dyn Db, + file: File, + name: &str, +) -> SymbolLookupResult<'db> { let explicit_ty = symbol_ty(db, global_scope(db, file), name); - if !explicit_ty.may_be_unbound(db) { + if !explicit_ty.is_unbound() { return explicit_ty; } @@ -139,10 +198,7 @@ pub(crate) fn global_symbol_ty<'db>(db: &'db dyn Db, file: File, name: &str) -> { // TODO: this should use `.to_instance(db)`. but we don't understand attribute access // on instance types yet. - let module_type_member = KnownClass::ModuleType.to_class(db).member(db, name); - if !module_type_member.is_unbound() { - return explicit_ty.replace_unbound_with(db, module_type_member); - } + return KnownClass::ModuleType.to_class(db).member(db, name); } explicit_ty @@ -196,9 +252,8 @@ fn definition_expression_ty<'db>( fn bindings_ty<'db>( db: &'db dyn Db, bindings_with_constraints: BindingWithConstraintsIterator<'_, 'db>, - unbound_ty: Option>, -) -> Type<'db> { - let def_types = bindings_with_constraints.map( +) -> SymbolLookupResult<'db> { + let mut def_types = bindings_with_constraints.map( |BindingWithConstraints { binding, constraints, @@ -220,16 +275,18 @@ fn bindings_ty<'db>( } }, ); - let mut all_types = unbound_ty.into_iter().chain(def_types); - - let first = all_types - .next() - .expect("bindings_ty should never be called with zero definitions and no unbound_ty"); - if let Some(second) = all_types.next() { - UnionType::from_elements(db, [first, second].into_iter().chain(all_types)) + if let Some(first) = def_types.next() { + if let Some(second) = def_types.next() { + SymbolLookupResult::Bound(UnionType::from_elements( + db, + [first, second].into_iter().chain(def_types), + )) + } else { + SymbolLookupResult::Bound(first) + } } else { - first + SymbolLookupResult::Unbound } } @@ -301,9 +358,6 @@ pub enum Type<'db> { /// Unknown type (either no annotation, or some kind of type error). /// Equivalent to Any, or possibly to object in strict mode Unknown, - /// Name does not exist or is not bound to any value (this represents an error, but with some - /// leniency options it could be silently resolved to Unknown in some cases) - Unbound, /// The None object -- TODO remove this in favor of Instance(types.NoneType) None, /// Temporary type for symbols that can't be inferred yet because of missing implementations. @@ -347,10 +401,6 @@ pub enum Type<'db> { } impl<'db> Type<'db> { - pub const fn is_unbound(&self) -> bool { - matches!(self, Type::Unbound) - } - pub const fn is_never(&self) -> bool { matches!(self, Type::Never) } @@ -462,27 +512,6 @@ impl<'db> Type<'db> { matches!(self, Type::LiteralString) } - pub fn may_be_unbound(&self, db: &'db dyn Db) -> bool { - match self { - Type::Unbound => true, - Type::Union(union) => union.elements(db).contains(&Type::Unbound), - // Unbound can't appear in an intersection, because an intersection with Unbound - // simplifies to just Unbound. - _ => false, - } - } - - #[must_use] - pub fn replace_unbound_with(&self, db: &'db dyn Db, replacement: Type<'db>) -> Type<'db> { - match self { - Type::Unbound => replacement, - Type::Union(union) => { - union.map(db, |element| element.replace_unbound_with(db, replacement)) - } - _ => *self, - } - } - pub fn is_stdlib_symbol(&self, db: &'db dyn Db, module_name: &str, name: &str) -> bool { match self { Type::ClassLiteral(class) => class.is_stdlib_symbol(db, module_name, name), @@ -601,7 +630,6 @@ impl<'db> Type<'db> { (Type::Any, _) | (_, Type::Any) => false, (Type::Unknown, _) | (_, Type::Unknown) => false, - (Type::Unbound, _) | (_, Type::Unbound) => false, (Type::Todo, _) | (_, Type::Todo) => false, (Type::Union(union), other) | (other, Type::Union(union)) => union @@ -754,7 +782,6 @@ impl<'db> Type<'db> { | Type::Never | Type::Unknown | Type::Todo - | Type::Unbound | Type::Instance(..) // TODO some instance types can be singleton types (EllipsisType, NotImplementedType) | Type::IntLiteral(..) | Type::StringLiteral(..) @@ -837,7 +864,6 @@ impl<'db> Type<'db> { Type::Any | Type::Never | Type::Unknown - | Type::Unbound | Type::Todo | Type::Union(..) | Type::Intersection(..) @@ -859,22 +885,21 @@ impl<'db> Type<'db> { /// us to explicitly consider whether to handle an error or propagate /// it up the call stack. #[must_use] - pub fn member(&self, db: &'db dyn Db, name: &str) -> Type<'db> { + pub fn member(&self, db: &'db dyn Db, name: &str) -> SymbolLookupResult<'db> { match self { - Type::Any => Type::Any, + Type::Any => Type::Any.into(), Type::Never => { // TODO: attribute lookup on Never type - Type::Todo + Type::Todo.into() } - Type::Unknown => Type::Unknown, - Type::Unbound => Type::Unbound, + Type::Unknown => Type::Unknown.into(), Type::None => { // TODO: attribute lookup on None type - Type::Todo + Type::Todo.into() } Type::FunctionLiteral(_) => { // TODO: attribute lookup on function type - Type::Todo + Type::Todo.into() } Type::ModuleLiteral(file) => { // `__dict__` is a very special member that is never overridden by module globals; @@ -903,58 +928,62 @@ impl<'db> Type<'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. - if name != "__getattr__" && global_lookup.may_be_unbound(db) { + if name != "__getattr__" && global_lookup.is_unbound() { // TODO: this should use `.to_instance()`, but we don't understand instance attribute yet let module_type_instance_member = KnownClass::ModuleType.to_class(db).member(db, name); - if module_type_instance_member.is_unbound() { - global_lookup + if let SymbolLookupResult::Bound(module_type_instance_member) = + module_type_instance_member + { + global_lookup.replace_unbound_with(module_type_instance_member) } else { - global_lookup.replace_unbound_with(db, module_type_instance_member) + global_lookup } } else { global_lookup } } - Type::ClassLiteral(class) => class.class_member(db, name), + Type::ClassLiteral(class) => class.class_member(db, name).into(), Type::Instance(_) => { // TODO MRO? get_own_instance_member, get_instance_member - Type::Todo + Type::Todo.into() } - Type::Union(union) => union.map(db, |element| element.member(db, name)), + Type::Union(union) => union + .map(db, |element| element.member(db, name).todo_unwrap_type()) + .into(), Type::Intersection(_) => { // TODO perform the get_member on each type in the intersection // TODO return the intersection of those results - Type::Todo + Type::Todo.into() } Type::IntLiteral(_) => { // TODO raise error - Type::Todo + Type::Todo.into() } - Type::BooleanLiteral(_) => Type::Todo, + Type::BooleanLiteral(_) => Type::Todo.into(), Type::StringLiteral(_) => { // TODO defer to `typing.LiteralString`/`builtins.str` methods // from typeshed's stubs - Type::Todo + Type::Todo.into() } Type::LiteralString => { // TODO defer to `typing.LiteralString`/`builtins.str` methods // from typeshed's stubs - Type::Todo + Type::Todo.into() } Type::BytesLiteral(_) => { // TODO defer to Type::Instance().member - Type::Todo + Type::Todo.into() } Type::SliceLiteral(_) => { // TODO defer to `builtins.slice` methods - Type::Todo + Type::Todo.into() } Type::Tuple(_) => { // TODO: implement tuple methods - Type::Todo + Type::Todo.into() } - Type::Todo => Type::Todo, + Type::Todo => Type::Todo.into(), } } @@ -964,9 +993,7 @@ impl<'db> Type<'db> { /// when `bool(x)` is called on an object `x`. fn bool(&self, db: &'db dyn Db) -> Truthiness { match self { - Type::Any | Type::Todo | Type::Never | Type::Unknown | Type::Unbound => { - Truthiness::Ambiguous - } + Type::Any | Type::Todo | Type::Never | Type::Unknown => Truthiness::Ambiguous, Type::None => Truthiness::AlwaysFalse, Type::FunctionLiteral(_) => Truthiness::AlwaysTrue, Type::ModuleLiteral(_) => Truthiness::AlwaysTrue, @@ -1051,7 +1078,7 @@ impl<'db> Type<'db> { let args = std::iter::once(self) .chain(arg_types.iter().copied()) .collect::>(); - dunder_call_method.call(db, &args) + dunder_call_method.todo_unwrap_type().call(db, &args) } } @@ -1104,13 +1131,20 @@ impl<'db> Type<'db> { let dunder_iter_method = iterable_meta_type.member(db, "__iter__"); if !dunder_iter_method.is_unbound() { - let Some(iterator_ty) = dunder_iter_method.call(db, &[self]).return_ty(db) else { + let Some(iterator_ty) = dunder_iter_method + .todo_unwrap_type() + .call(db, &[self]) + .return_ty(db) + else { return IterationOutcome::NotIterable { not_iterable_ty: self, }; }; - let dunder_next_method = iterator_ty.to_meta_type(db).member(db, "__next__"); + let dunder_next_method = iterator_ty + .to_meta_type(db) + .member(db, "__next__") + .todo_unwrap_type(); return dunder_next_method .call(db, &[iterator_ty]) .return_ty(db) @@ -1126,7 +1160,9 @@ impl<'db> Type<'db> { // // TODO(Alex) this is only valid if the `__getitem__` method is annotated as // accepting `int` or `SupportsIndex` - let dunder_get_item_method = iterable_meta_type.member(db, "__getitem__"); + let dunder_get_item_method = iterable_meta_type + .member(db, "__getitem__") + .todo_unwrap_type(); dunder_get_item_method .call(db, &[self, KnownClass::Int.to_instance(db)]) @@ -1143,7 +1179,6 @@ impl<'db> Type<'db> { Type::Any => Type::Any, Type::Todo => Type::Todo, Type::Unknown => Type::Unknown, - Type::Unbound => Type::Unknown, Type::Never => Type::Never, Type::ClassLiteral(class) => Type::Instance(*class), Type::Union(union) => union.map(db, |element| element.to_instance(db)), @@ -1170,7 +1205,6 @@ impl<'db> Type<'db> { #[must_use] pub fn to_meta_type(&self, db: &'db dyn Db) -> Type<'db> { match self { - Type::Unbound => Type::Unbound, Type::Never => Type::Never, Type::Instance(class) => Type::ClassLiteral(*class), Type::Union(union) => union.map(db, |ty| ty.to_meta_type(db)), @@ -1237,6 +1271,12 @@ impl<'db> From<&Type<'db>> for Type<'db> { } } +impl<'db> From> for SymbolLookupResult<'db> { + fn from(value: Type<'db>) -> Self { + SymbolLookupResult::Bound(value) + } +} + /// Non-exhaustive enumeration of known classes (e.g. `builtins.int`, `typing.Any`, ...) to allow /// for easier syntax when interacting with very common classes. /// @@ -1308,12 +1348,12 @@ impl<'db> KnownClass { | Self::Tuple | Self::Set | Self::Dict - | Self::Slice => builtins_symbol_ty(db, self.as_str()), + | Self::Slice => builtins_symbol_ty(db, self.as_str()).unwrap_or_unknown(), Self::GenericAlias | Self::ModuleType | Self::FunctionType => { - types_symbol_ty(db, self.as_str()) + types_symbol_ty(db, self.as_str()).unwrap_or_unknown() } - Self::NoneType => typeshed_symbol_ty(db, self.as_str()), + Self::NoneType => typeshed_symbol_ty(db, self.as_str()).unwrap_or_unknown(), } } @@ -1836,23 +1876,23 @@ impl<'db> ClassType<'db> { /// Returns the class member of this class named `name`. /// /// The member resolves to a member of the class itself or any of its bases. - pub fn class_member(self, db: &'db dyn Db, name: &str) -> Type<'db> { + pub fn class_member(self, db: &'db dyn Db, name: &str) -> SymbolLookupResult<'db> { let member = self.own_class_member(db, name); if !member.is_unbound() { // TODO diagnostic if maybe unbound? - return member.replace_unbound_with(db, Type::Never); + return member.replace_unbound_with(Type::Never); } self.inherited_class_member(db, name) } /// Returns the inferred type of the class member named `name`. - pub fn own_class_member(self, db: &'db dyn Db, name: &str) -> Type<'db> { + pub fn own_class_member(self, db: &'db dyn Db, name: &str) -> SymbolLookupResult<'db> { let scope = self.body_scope(db); symbol_ty(db, scope, name) } - pub fn inherited_class_member(self, db: &'db dyn Db, name: &str) -> Type<'db> { + pub fn inherited_class_member(self, db: &'db dyn Db, name: &str) -> SymbolLookupResult<'db> { for base in self.bases(db) { let member = base.member(db, name); if !member.is_unbound() { @@ -1860,7 +1900,7 @@ impl<'db> ClassType<'db> { } } - Type::Unbound + SymbolLookupResult::Unbound } } @@ -2026,7 +2066,7 @@ mod tests { Ty::BooleanLiteral(b) => Type::BooleanLiteral(b), Ty::LiteralString => Type::LiteralString, Ty::BytesLiteral(s) => Type::BytesLiteral(BytesLiteralType::new(db, s.as_bytes())), - Ty::BuiltinInstance(s) => builtins_symbol_ty(db, s).to_instance(db), + Ty::BuiltinInstance(s) => builtins_symbol_ty(db, s).expect_bound().to_instance(db), Ty::Union(tys) => { UnionType::from_elements(db, tys.into_iter().map(|ty| ty.into_type(db))) } @@ -2137,14 +2177,15 @@ mod tests { " class A: ... class B: ... - U = A if flag else B + def flag() -> bool: ... + U = A if flag() else B ", ) .unwrap(); let module = ruff_db::files::system_path_to_file(&db, "/src/module.py").unwrap(); - let type_a = super::global_symbol_ty(&db, module, "A"); - let type_u = super::global_symbol_ty(&db, module, "U"); + let type_a = super::global_symbol_ty(&db, module, "A").expect_bound(); + let type_u = super::global_symbol_ty(&db, module, "U").expect_bound(); assert!(type_a.is_class_literal()); assert!(type_a.is_subtype_of(&db, Ty::BuiltinInstance("type").into_type(&db))); @@ -2243,8 +2284,8 @@ mod tests { .unwrap(); let module = ruff_db::files::system_path_to_file(&db, "/src/module.py").unwrap(); - let type_a = super::global_symbol_ty(&db, module, "A"); - let type_u = super::global_symbol_ty(&db, module, "U"); + let type_a = super::global_symbol_ty(&db, module, "A").expect_bound(); + let type_u = super::global_symbol_ty(&db, module, "U").expect_bound(); assert!(type_a.is_class_literal()); assert!(type_u.is_union()); diff --git a/crates/red_knot_python_semantic/src/types/builder.rs b/crates/red_knot_python_semantic/src/types/builder.rs index 43a646312bc88..89eb5133beb5a 100644 --- a/crates/red_knot_python_semantic/src/types/builder.rs +++ b/crates/red_knot_python_semantic/src/types/builder.rs @@ -316,7 +316,6 @@ impl<'db> InnerIntersectionBuilder<'db> { self.add_positive(db, *neg); } } - Type::Unbound => {} ty @ (Type::Any | Type::Unknown | Type::Todo) => { // Adding any of these types to the negative side of an intersection // is equivalent to adding it to the positive side. We do this to @@ -367,15 +366,7 @@ impl<'db> InnerIntersectionBuilder<'db> { } } - fn simplify_unbound(&mut self) { - if self.positive.contains(&Type::Unbound) { - self.positive.retain(Type::is_unbound); - self.negative.clear(); - } - } - fn build(mut self, db: &'db dyn Db) -> Type<'db> { - self.simplify_unbound(); match (self.positive.len(), self.negative.len()) { (0, 0) => KnownClass::Object.to_instance(db), (1, 0) => self.positive[0], @@ -626,8 +617,12 @@ mod tests { #[test] fn intersection_negation_distributes_over_union() { let db = setup_db(); - let st = typing_symbol_ty(&db, "Sized").to_instance(&db); - let ht = typing_symbol_ty(&db, "Hashable").to_instance(&db); + let st = typing_symbol_ty(&db, "Sized") + .expect_bound() + .to_instance(&db); + let ht = typing_symbol_ty(&db, "Hashable") + .expect_bound() + .to_instance(&db); // sh_t: Sized & Hashable let sh_t = IntersectionBuilder::new(&db) .add_positive(st) @@ -653,8 +648,12 @@ mod tests { fn mixed_intersection_negation_distributes_over_union() { let db = setup_db(); let it = KnownClass::Int.to_instance(&db); - let st = typing_symbol_ty(&db, "Sized").to_instance(&db); - let ht = typing_symbol_ty(&db, "Hashable").to_instance(&db); + let st = typing_symbol_ty(&db, "Sized") + .expect_bound() + .to_instance(&db); + let ht = typing_symbol_ty(&db, "Hashable") + .expect_bound() + .to_instance(&db); // s_not_h_t: Sized & ~Hashable let s_not_h_t = IntersectionBuilder::new(&db) .add_positive(st) @@ -709,28 +708,6 @@ mod tests { assert_eq!(ty, Type::Never); } - #[test] - fn build_intersection_simplify_positive_unbound() { - let db = setup_db(); - let ty = IntersectionBuilder::new(&db) - .add_positive(Type::Unbound) - .add_positive(Type::IntLiteral(1)) - .build(); - - assert_eq!(ty, Type::Unbound); - } - - #[test] - fn build_intersection_simplify_negative_unbound() { - let db = setup_db(); - let ty = IntersectionBuilder::new(&db) - .add_negative(Type::Unbound) - .add_positive(Type::IntLiteral(1)) - .build(); - - assert_eq!(ty, Type::IntLiteral(1)); - } - #[test] fn build_intersection_simplify_negative_none() { let db = setup_db(); diff --git a/crates/red_knot_python_semantic/src/types/display.rs b/crates/red_knot_python_semantic/src/types/display.rs index 3cd3ec7c99a30..655b35f817839 100644 --- a/crates/red_knot_python_semantic/src/types/display.rs +++ b/crates/red_knot_python_semantic/src/types/display.rs @@ -64,7 +64,6 @@ impl Display for DisplayRepresentation<'_> { Type::Any => f.write_str("Any"), Type::Never => f.write_str("Never"), Type::Unknown => f.write_str("Unknown"), - Type::Unbound => f.write_str("Unbound"), Type::None => f.write_str("None"), // `[Type::Todo]`'s display should be explicit that is not a valid display of // any other type @@ -370,16 +369,16 @@ mod tests { let union_elements = &[ Type::Unknown, Type::IntLiteral(-1), - global_symbol_ty(&db, mod_file, "A"), + global_symbol_ty(&db, mod_file, "A").expect_bound(), Type::StringLiteral(StringLiteralType::new(&db, "A")), Type::BytesLiteral(BytesLiteralType::new(&db, [0u8].as_slice())), Type::BytesLiteral(BytesLiteralType::new(&db, [7u8].as_slice())), Type::IntLiteral(0), Type::IntLiteral(1), Type::StringLiteral(StringLiteralType::new(&db, "B")), - global_symbol_ty(&db, mod_file, "foo"), - global_symbol_ty(&db, mod_file, "bar"), - global_symbol_ty(&db, mod_file, "B"), + global_symbol_ty(&db, mod_file, "foo").expect_bound(), + global_symbol_ty(&db, mod_file, "bar").expect_bound(), + global_symbol_ty(&db, mod_file, "B").expect_bound(), Type::BooleanLiteral(true), Type::None, ]; diff --git a/crates/red_knot_python_semantic/src/types/infer.rs b/crates/red_knot_python_semantic/src/types/infer.rs index 55d7d0940caf0..80d0c81be18f7 100644 --- a/crates/red_knot_python_semantic/src/types/infer.rs +++ b/crates/red_knot_python_semantic/src/types/infer.rs @@ -61,6 +61,8 @@ use crate::types::{ use crate::util::subscript::{PyIndex, PySlice}; use crate::Db; +use super::SymbolLookupResult; + /// Infer all types for a [`ScopeId`], including all definitions and expressions in that scope. /// Use when checking a scope, or needing to provide a type for an arbitrary expression in the /// scope. @@ -580,7 +582,7 @@ 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 = bindings_ty(self.db, prior_bindings, Some(Type::Never)); + let inferred_ty = bindings_ty(self.db, prior_bindings).unwrap_or(Type::Never); let ty = if inferred_ty.is_assignable_to(self.db, ty) { ty } else { @@ -1029,7 +1031,9 @@ impl<'db> TypeInferenceBuilder<'db> { // // TODO should infer `ExceptionGroup` if all caught exceptions // are subclasses of `Exception` --Alex - builtins_symbol_ty(self.db, "BaseExceptionGroup").to_instance(self.db) + builtins_symbol_ty(self.db, "BaseExceptionGroup") + .todo_unwrap_type() + .to_instance(self.db) } else { // TODO: anything that's a consistent subtype of // `type[BaseException] | tuple[type[BaseException], ...]` should be valid; @@ -1410,7 +1414,9 @@ impl<'db> TypeInferenceBuilder<'db> { if let Type::Instance(class) = target_type { let class_member = class.class_member(self.db, op.in_place_dunder()); if !class_member.is_unbound() { - let call = class_member.call(self.db, &[target_type, value_type]); + let call = class_member + .todo_unwrap_type() + .call(self.db, &[target_type, value_type]); return match call.return_ty_result( self.db, AnyNodeRef::StmtAugAssign(assignment), @@ -1673,7 +1679,11 @@ impl<'db> TypeInferenceBuilder<'db> { let member_ty = module_ty.member(self.db, &ast::name::Name::new(&name.id)); - if member_ty.is_unbound() { + if let SymbolLookupResult::Bound(member_ty) = member_ty { + // For possibly-unbound names, just eliminate Unbound from the type; we + // must be in a bound path. TODO diagnostic for maybe-unbound import? + member_ty + } else { self.diagnostics.add( AnyNodeRef::Alias(alias), "unresolved-import", @@ -1681,10 +1691,6 @@ impl<'db> TypeInferenceBuilder<'db> { ); Type::Unknown - } else { - // For possibly-unbound names, just eliminate Unbound from the type; we - // must be in a bound path. TODO diagnostic for maybe-unbound import? - member_ty.replace_unbound_with(self.db, Type::Never) } } else { self.diagnostics @@ -1835,9 +1841,9 @@ impl<'db> TypeInferenceBuilder<'db> { .map(Type::IntLiteral) .unwrap_or_else(|| KnownClass::Int.to_instance(self.db)), ast::Number::Float(_) => KnownClass::Float.to_instance(self.db), - ast::Number::Complex { .. } => { - builtins_symbol_ty(self.db, "complex").to_instance(self.db) - } + ast::Number::Complex { .. } => builtins_symbol_ty(self.db, "complex") + .unwrap_or_unknown() + .to_instance(self.db), } } @@ -1917,7 +1923,7 @@ impl<'db> TypeInferenceBuilder<'db> { &mut self, _literal: &ast::ExprEllipsisLiteral, ) -> Type<'db> { - builtins_symbol_ty(self.db, "Ellipsis") + builtins_symbol_ty(self.db, "Ellipsis").unwrap_or_unknown() } fn infer_tuple_expression(&mut self, tuple: &ast::ExprTuple) -> Type<'db> { @@ -2293,7 +2299,7 @@ impl<'db> TypeInferenceBuilder<'db> { } /// Look up a name reference that isn't bound in the local scope. - fn lookup_name(&mut self, name_node: &ast::ExprName) -> Type<'db> { + fn lookup_name(&mut self, name_node: &ast::ExprName) -> SymbolLookupResult<'db> { let ast::ExprName { id: name, .. } = name_node; let file_scope_id = self.scope().file_scope_id(self.db); let is_bound = self @@ -2334,13 +2340,13 @@ impl<'db> TypeInferenceBuilder<'db> { // No nonlocal binding, check module globals. Avoid infinite recursion if `self.scope` // already is module globals. let ty = if file_scope_id.is_global() { - Type::Unbound + SymbolLookupResult::Unbound } else { global_symbol_ty(self.db, self.file, name) }; // Fallback to builtins (without infinite recursion if we're already in builtins.) - if ty.may_be_unbound(self.db) && Some(self.scope()) != builtins_module_scope(self.db) { + if ty.is_unbound() && Some(self.scope()) != builtins_module_scope(self.db) { let mut builtin_ty = builtins_symbol_ty(self.db, name); if builtin_ty.is_unbound() && name == "reveal_type" { self.diagnostics.add( @@ -2351,12 +2357,13 @@ impl<'db> TypeInferenceBuilder<'db> { ); builtin_ty = typing_extensions_symbol_ty(self.db, name); } - ty.replace_unbound_with(self.db, builtin_ty) + + builtin_ty.into() } else { ty } } else { - Type::Unbound + SymbolLookupResult::Unbound } } @@ -2389,28 +2396,31 @@ impl<'db> TypeInferenceBuilder<'db> { ) }; - let unbound_ty = if may_be_unbound { - Some(self.lookup_name(name)) - } else { - None - }; - - let ty = bindings_ty(self.db, definitions, unbound_ty); - if ty.is_unbound() { - self.diagnostics.add( - name.into(), - "unresolved-reference", - format_args!("Name `{id}` used when not defined"), - ); - } else if ty.may_be_unbound(self.db) { - self.diagnostics.add( - name.into(), - "possibly-unresolved-reference", - format_args!("Name `{id}` used when possibly not defined"), - ); + match bindings_ty(self.db, definitions) { + SymbolLookupResult::Bound(ty) => ty, + SymbolLookupResult::Unbound => { + if may_be_unbound { + match self.lookup_name(name) { + SymbolLookupResult::Bound(ty) => ty, + SymbolLookupResult::Unbound => { + self.diagnostics.add( + name.into(), + "unresolved-reference", + format_args!("Name `{id}` used when not defined"), + ); + Type::Unknown + } + } + } else { + self.diagnostics.add( + name.into(), + "possibly-unresolved-reference", + format_args!("Name `{id}` used when possibly not defined"), + ); + Type::Unknown + } + } } - - ty } fn infer_name_expression(&mut self, name: &ast::ExprName) -> Type<'db> { @@ -2431,7 +2441,9 @@ impl<'db> TypeInferenceBuilder<'db> { } = attribute; let value_ty = self.infer_expression(value); - value_ty.member(self.db, &Name::new(&attr.id)) + value_ty + .member(self.db, &Name::new(&attr.id)) + .todo_unwrap_type() } fn infer_attribute_expression(&mut self, attribute: &ast::ExprAttribute) -> Type<'db> { @@ -2486,7 +2498,9 @@ impl<'db> TypeInferenceBuilder<'db> { } }; let class_member = class.class_member(self.db, unary_dunder_method); - let call = class_member.call(self.db, &[operand_type]); + let call = class_member + .todo_unwrap_type() + .call(self.db, &[operand_type]); match call.return_ty_result( self.db, @@ -2715,11 +2729,13 @@ impl<'db> TypeInferenceBuilder<'db> { && rhs_reflected != left_class.class_member(self.db, reflected_dunder) { return rhs_reflected + .todo_unwrap_type() .call(self.db, &[right_ty, left_ty]) .return_ty(self.db) .or_else(|| { left_class .class_member(self.db, op.dunder()) + .todo_unwrap_type() .call(self.db, &[left_ty, right_ty]) .return_ty(self.db) }); @@ -2727,6 +2743,7 @@ impl<'db> TypeInferenceBuilder<'db> { } left_class .class_member(self.db, op.dunder()) + .todo_unwrap_type() .call(self.db, &[left_ty, right_ty]) .return_ty(self.db) .or_else(|| { @@ -2735,6 +2752,7 @@ impl<'db> TypeInferenceBuilder<'db> { } else { right_class .class_member(self.db, op.reflected_dunder()) + .todo_unwrap_type() .call(self.db, &[right_ty, left_ty]) .return_ty(self.db) } @@ -3326,6 +3344,7 @@ impl<'db> TypeInferenceBuilder<'db> { let dunder_getitem_method = value_meta_ty.member(self.db, "__getitem__"); if !dunder_getitem_method.is_unbound() { return dunder_getitem_method + .todo_unwrap_type() .call(self.db, &[slice_ty]) .return_ty_result(self.db, value_node.into(), &mut self.diagnostics) .unwrap_or_else(|err| { @@ -3355,6 +3374,7 @@ impl<'db> TypeInferenceBuilder<'db> { let dunder_class_getitem_method = value_ty.member(self.db, "__class_getitem__"); if !dunder_class_getitem_method.is_unbound() { return dunder_class_getitem_method + .todo_unwrap_type() .call(self.db, &[slice_ty]) .return_ty_result(self.db, value_node.into(), &mut self.diagnostics) .unwrap_or_else(|err| { @@ -3893,6 +3913,7 @@ fn perform_rich_comparison<'db>( |op: RichCompareOperator, left_class: ClassType<'db>, right_class: ClassType<'db>| { left_class .class_member(db, op.dunder()) + .todo_unwrap_type() .call( db, &[Type::Instance(left_class), Type::Instance(right_class)], @@ -3948,6 +3969,7 @@ fn perform_membership_test_comparison<'db>( } else { // If `__contains__` is available, it is used directly for the membership test. contains_dunder + .todo_unwrap_type() .call(db, &[right_instance, left_instance]) .return_ty(db) }; @@ -4042,7 +4064,7 @@ mod tests { fn assert_public_ty(db: &TestDb, file_name: &str, symbol_name: &str, expected: &str) { let file = system_path_to_file(db, file_name).expect("file to exist"); - let ty = global_symbol_ty(db, file, symbol_name); + let ty = global_symbol_ty(db, file, symbol_name).expect_bound(); assert_eq!( ty.display(db).to_string(), expected, @@ -4072,7 +4094,7 @@ mod tests { assert_eq!(scope.name(db), *expected_scope_name); } - let ty = symbol_ty(db, scope, symbol_name); + let ty = symbol_ty(db, scope, symbol_name).unwrap_or_unknown(); assert_eq!(ty.display(db).to_string(), expected); } @@ -4119,7 +4141,7 @@ mod tests { )?; let mod_file = system_path_to_file(&db, "src/mod.py").expect("file to exist"); - let ty = global_symbol_ty(&db, mod_file, "Sub"); + let ty = global_symbol_ty(&db, mod_file, "Sub").expect_bound(); let class = ty.expect_class_literal(); @@ -4146,9 +4168,11 @@ mod tests { )?; let mod_file = system_path_to_file(&db, "src/mod.py").unwrap(); - let ty = global_symbol_ty(&db, mod_file, "C"); + let ty = global_symbol_ty(&db, mod_file, "C").expect_bound(); let class_id = ty.expect_class_literal(); - let member_ty = class_id.class_member(&db, &Name::new_static("f")); + let member_ty = class_id + .class_member(&db, &Name::new_static("f")) + .expect_bound(); let func = member_ty.expect_function_literal(); assert_eq!(func.name(&db), "f"); @@ -4327,7 +4351,9 @@ mod tests { db.write_file("src/a.py", "def example() -> int: return 42")?; let mod_file = system_path_to_file(&db, "src/a.py").unwrap(); - let function = global_symbol_ty(&db, mod_file, "example").expect_function_literal(); + let function = global_symbol_ty(&db, mod_file, "example") + .expect_bound() + .expect_function_literal(); let returns = function.return_type(&db); assert_eq!(returns.display(&db).to_string(), "int"); @@ -4355,7 +4381,7 @@ mod tests { )?; let a = system_path_to_file(&db, "src/a.py").expect("file to exist"); - let c_ty = global_symbol_ty(&db, a, "C"); + let c_ty = global_symbol_ty(&db, a, "C").expect_bound(); let c_class = c_ty.expect_class_literal(); let mut c_bases = c_class.bases(&db); let b_ty = c_bases.next().unwrap(); @@ -4392,10 +4418,10 @@ mod tests { .unwrap() .0 .to_scope_id(&db, file); - let y_ty = symbol_ty(&db, function_scope, "y"); - let x_ty = symbol_ty(&db, function_scope, "x"); + let y_ty = symbol_ty(&db, function_scope, "y").expect_bound(); + let x_ty = symbol_ty(&db, function_scope, "x").expect_bound(); - assert_eq!(y_ty.display(&db).to_string(), "Unbound"); + assert_eq!(y_ty.display(&db).to_string(), "Unknown"); assert_eq!(x_ty.display(&db).to_string(), "Literal[2]"); Ok(()) @@ -4423,10 +4449,11 @@ mod tests { .unwrap() .0 .to_scope_id(&db, file); - let y_ty = symbol_ty(&db, function_scope, "y"); + let x_ty = symbol_ty(&db, function_scope, "x"); + assert!(x_ty.is_unbound()); - assert_eq!(x_ty.display(&db).to_string(), "Unbound"); + let y_ty = symbol_ty(&db, function_scope, "y").expect_bound(); assert_eq!(y_ty.display(&db).to_string(), "Literal[1]"); Ok(()) @@ -4492,7 +4519,7 @@ mod tests { ], )?; - assert_public_ty(&db, "/src/a.py", "x", "Unbound"); + assert_public_ty(&db, "/src/a.py", "x", "Unknown"); Ok(()) } @@ -4510,7 +4537,7 @@ mod tests { let mut db = setup_db(); db.write_file("/src/a.pyi", "class C(object): pass")?; let file = system_path_to_file(&db, "/src/a.pyi").unwrap(); - let ty = global_symbol_ty(&db, file, "C"); + let ty = global_symbol_ty(&db, file, "C").expect_bound(); let base = ty .expect_class_literal() @@ -4801,25 +4828,18 @@ mod tests { db.write_dedented("src/a.py", "[z for z in x]")?; - assert_scope_ty(&db, "src/a.py", &[""], "x", "Unbound"); + assert_scope_ty(&db, "src/a.py", &[""], "x", "Unknown"); // Iterating over an `Unbound` yields `Unknown`: assert_scope_ty(&db, "src/a.py", &[""], "z", "Unknown"); - // TODO: not the greatest error message in the world! --Alex - assert_file_diagnostics( - &db, - "src/a.py", - &[ - "Name `x` used when not defined", - "Object of type `Unbound` is not iterable", - ], - ); + assert_file_diagnostics(&db, "src/a.py", &["Name `x` used when not defined"]); Ok(()) } #[test] + #[ignore] fn comprehension_with_not_iterable_iter_in_second_comprehension() -> anyhow::Result<()> { let mut db = setup_db(); @@ -4945,7 +4965,7 @@ mod tests { ", )?; - assert_scope_ty(&db, "src/a.py", &["foo", ""], "z", "Unbound"); + assert_scope_ty(&db, "src/a.py", &["foo", ""], "z", "Unknown"); // (There is a diagnostic for invalid syntax that's emitted, but it's not listed by `assert_file_diagnostics`) assert_file_diagnostics(&db, "src/a.py", &["Name `z` used when not defined"]); @@ -5021,6 +5041,7 @@ mod tests { } #[test] + #[ignore] fn starred_expressions_must_be_iterable() { let mut db = setup_db(); @@ -5070,7 +5091,7 @@ mod tests { ])?; let a = system_path_to_file(&db, "/src/a.py").unwrap(); - let x_ty = global_symbol_ty(&db, a, "x"); + let x_ty = global_symbol_ty(&db, a, "x").expect_bound(); assert_eq!(x_ty.display(&db).to_string(), "Literal[10]"); @@ -5079,7 +5100,7 @@ mod tests { let a = system_path_to_file(&db, "/src/a.py").unwrap(); - let x_ty_2 = global_symbol_ty(&db, a, "x"); + let x_ty_2 = global_symbol_ty(&db, a, "x").expect_bound(); assert_eq!(x_ty_2.display(&db).to_string(), "Literal[20]"); @@ -5096,7 +5117,7 @@ mod tests { ])?; let a = system_path_to_file(&db, "/src/a.py").unwrap(); - let x_ty = global_symbol_ty(&db, a, "x"); + let x_ty = global_symbol_ty(&db, a, "x").expect_bound(); assert_eq!(x_ty.display(&db).to_string(), "Literal[10]"); @@ -5106,7 +5127,7 @@ mod tests { db.clear_salsa_events(); - let x_ty_2 = global_symbol_ty(&db, a, "x"); + let x_ty_2 = global_symbol_ty(&db, a, "x").expect_bound(); assert_eq!(x_ty_2.display(&db).to_string(), "Literal[10]"); @@ -5132,7 +5153,7 @@ mod tests { ])?; let a = system_path_to_file(&db, "/src/a.py").unwrap(); - let x_ty = global_symbol_ty(&db, a, "x"); + let x_ty = global_symbol_ty(&db, a, "x").expect_bound(); assert_eq!(x_ty.display(&db).to_string(), "Literal[10]"); @@ -5142,7 +5163,7 @@ mod tests { db.clear_salsa_events(); - let x_ty_2 = global_symbol_ty(&db, a, "x"); + let x_ty_2 = global_symbol_ty(&db, a, "x").expect_bound(); assert_eq!(x_ty_2.display(&db).to_string(), "Literal[10]"); From 3cadfbe627aa114d39762735b636012ba51b6a41 Mon Sep 17 00:00:00 2001 From: David Peter Date: Wed, 30 Oct 2024 09:27:43 +0100 Subject: [PATCH 02/20] Fix clippy suggestions --- crates/red_knot_python_semantic/src/types.rs | 4 +++- crates/red_knot_python_semantic/src/types/infer.rs | 2 +- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/crates/red_knot_python_semantic/src/types.rs b/crates/red_knot_python_semantic/src/types.rs index bd252dce76a04..a4babad53b36d 100644 --- a/crates/red_knot_python_semantic/src/types.rs +++ b/crates/red_knot_python_semantic/src/types.rs @@ -40,6 +40,7 @@ impl<'db> SymbolLookupResult<'db> { matches!(self, SymbolLookupResult::Unbound) } + #[must_use] pub fn replace_unbound_with(self, replacement: Type<'db>) -> SymbolLookupResult<'db> { match self { r @ SymbolLookupResult::Bound(_) => r, @@ -55,6 +56,7 @@ impl<'db> SymbolLookupResult<'db> { } } + #[cfg(test)] #[track_caller] fn expect_bound(self) -> Type<'db> { match self { @@ -943,7 +945,7 @@ impl<'db> Type<'db> { global_lookup } } - Type::ClassLiteral(class) => class.class_member(db, name).into(), + Type::ClassLiteral(class) => class.class_member(db, name), Type::Instance(_) => { // TODO MRO? get_own_instance_member, get_instance_member Type::Todo.into() diff --git a/crates/red_knot_python_semantic/src/types/infer.rs b/crates/red_knot_python_semantic/src/types/infer.rs index 80d0c81be18f7..48cb0a8eeb44d 100644 --- a/crates/red_knot_python_semantic/src/types/infer.rs +++ b/crates/red_knot_python_semantic/src/types/infer.rs @@ -2358,7 +2358,7 @@ impl<'db> TypeInferenceBuilder<'db> { builtin_ty = typing_extensions_symbol_ty(self.db, name); } - builtin_ty.into() + builtin_ty } else { ty } From 59de59c3995f6e524b200c0beef511f7a5c1ceb1 Mon Sep 17 00:00:00 2001 From: David Peter Date: Wed, 30 Oct 2024 09:47:12 +0100 Subject: [PATCH 03/20] Fix 4 more tests --- crates/red_knot_python_semantic/src/types.rs | 11 +---------- 1 file changed, 1 insertion(+), 10 deletions(-) diff --git a/crates/red_knot_python_semantic/src/types.rs b/crates/red_knot_python_semantic/src/types.rs index a4babad53b36d..7300e6d59b955 100644 --- a/crates/red_knot_python_semantic/src/types.rs +++ b/crates/red_knot_python_semantic/src/types.rs @@ -107,16 +107,7 @@ fn symbol_ty_by_id<'db>( let declarations = use_def.public_declarations(symbol); // If the symbol is undeclared in some paths, include the inferred type in the public type. let undeclared_ty = if declarations.may_be_undeclared() { - Some( - bindings_ty( - db, - use_def.public_bindings(symbol), - // use_def - // .public_may_be_unbound(symbol) - // .then_some(Type::Unbound), - ) - .todo_unwrap_type(), - ) + Some(bindings_ty(db, use_def.public_bindings(symbol)).unwrap_or_unknown()) } else { None }; From dcbf8c944cacf1e2959e7862e5cee1c9535f4969 Mon Sep 17 00:00:00 2001 From: David Peter Date: Wed, 30 Oct 2024 12:19:24 +0100 Subject: [PATCH 04/20] Yield possibly-unbound diagnostics --- .../resources/mdtest/import/conditional.md | 2 - .../src/semantic_index/use_def.rs | 2 +- .../src/semantic_index/use_def/bitset.rs | 2 +- .../semantic_index/use_def/symbol_state.rs | 2 +- crates/red_knot_python_semantic/src/types.rs | 17 +++-- .../src/types/infer.rs | 65 ++++++++++++------- 6 files changed, 59 insertions(+), 31 deletions(-) diff --git a/crates/red_knot_python_semantic/resources/mdtest/import/conditional.md b/crates/red_knot_python_semantic/resources/mdtest/import/conditional.md index 787c0fff3a4a2..5e608bb6620a0 100644 --- a/crates/red_knot_python_semantic/resources/mdtest/import/conditional.md +++ b/crates/red_knot_python_semantic/resources/mdtest/import/conditional.md @@ -13,7 +13,6 @@ if flag: x = y # error: [possibly-unresolved-reference] # revealed: Literal[3] -# error: [possibly-unresolved-reference] reveal_type(x) # revealed: Literal[3] @@ -41,7 +40,6 @@ if flag: x = y # error: [possibly-unresolved-reference] # revealed: Literal[3] -# error: [possibly-unresolved-reference] reveal_type(x) # revealed: Literal[3] diff --git a/crates/red_knot_python_semantic/src/semantic_index/use_def.rs b/crates/red_knot_python_semantic/src/semantic_index/use_def.rs index 554ee11a3e7d1..ebb67c82a47db 100644 --- a/crates/red_knot_python_semantic/src/semantic_index/use_def.rs +++ b/crates/red_knot_python_semantic/src/semantic_index/use_def.rs @@ -356,7 +356,7 @@ enum SymbolDefinitions { Declarations(SymbolDeclarations), } -#[derive(Debug)] +#[derive(Debug, Clone)] pub(crate) struct BindingWithConstraintsIterator<'map, 'db> { all_definitions: &'map IndexVec>, all_constraints: &'map IndexVec>, diff --git a/crates/red_knot_python_semantic/src/semantic_index/use_def/bitset.rs b/crates/red_knot_python_semantic/src/semantic_index/use_def/bitset.rs index 464f718e7b4f4..e889c429ba913 100644 --- a/crates/red_knot_python_semantic/src/semantic_index/use_def/bitset.rs +++ b/crates/red_knot_python_semantic/src/semantic_index/use_def/bitset.rs @@ -122,7 +122,7 @@ impl BitSet { } /// Iterator over values in a [`BitSet`]. -#[derive(Debug)] +#[derive(Debug, Clone)] pub(super) struct BitSetIterator<'a, const B: usize> { /// The blocks we are iterating over. blocks: &'a [u64], diff --git a/crates/red_knot_python_semantic/src/semantic_index/use_def/symbol_state.rs b/crates/red_knot_python_semantic/src/semantic_index/use_def/symbol_state.rs index 09210bfab05d6..0b1cd06d3cda2 100644 --- a/crates/red_knot_python_semantic/src/semantic_index/use_def/symbol_state.rs +++ b/crates/red_knot_python_semantic/src/semantic_index/use_def/symbol_state.rs @@ -355,7 +355,7 @@ pub(super) struct BindingIdWithConstraints<'a> { pub(super) constraint_ids: ConstraintIdIterator<'a>, } -#[derive(Debug)] +#[derive(Debug, Clone)] pub(super) struct BindingIdWithConstraintsIterator<'a> { definitions: BindingsIterator<'a>, constraints: ConstraintsIterator<'a>, diff --git a/crates/red_knot_python_semantic/src/types.rs b/crates/red_knot_python_semantic/src/types.rs index 7300e6d59b955..593cc1b0ae10f 100644 --- a/crates/red_knot_python_semantic/src/types.rs +++ b/crates/red_knot_python_semantic/src/types.rs @@ -56,7 +56,6 @@ impl<'db> SymbolLookupResult<'db> { } } - #[cfg(test)] #[track_caller] fn expect_bound(self) -> Type<'db> { match self { @@ -941,9 +940,19 @@ impl<'db> Type<'db> { // TODO MRO? get_own_instance_member, get_instance_member Type::Todo.into() } - Type::Union(union) => union - .map(db, |element| element.member(db, name).todo_unwrap_type()) - .into(), + Type::Union(union) => { + let any_unbound = union + .elements(db) + .iter() + .any(|ty| ty.member(db, name).is_unbound()); + if any_unbound { + SymbolLookupResult::Unbound // TODO + } else { + SymbolLookupResult::Bound( + union.map(db, |ty| ty.member(db, name).expect_bound()), + ) + } + } Type::Intersection(_) => { // TODO perform the get_member on each type in the intersection // TODO return the intersection of those results diff --git a/crates/red_knot_python_semantic/src/types/infer.rs b/crates/red_knot_python_semantic/src/types/infer.rs index 48cb0a8eeb44d..08bd057852bef 100644 --- a/crates/red_knot_python_semantic/src/types/infer.rs +++ b/crates/red_knot_python_semantic/src/types/infer.rs @@ -55,14 +55,12 @@ use crate::types::diagnostic::{ use crate::types::{ bindings_ty, builtins_symbol_ty, declarations_ty, global_symbol_ty, symbol_ty, typing_extensions_symbol_ty, BytesLiteralType, ClassType, FunctionType, IterationOutcome, - KnownClass, KnownFunction, SliceLiteralType, StringLiteralType, Truthiness, TupleType, Type, - TypeArrayDisplay, UnionBuilder, UnionType, + KnownClass, KnownFunction, SliceLiteralType, StringLiteralType, SymbolLookupResult, Truthiness, + TupleType, Type, TypeArrayDisplay, UnionBuilder, UnionType, }; use crate::util::subscript::{PyIndex, PySlice}; use crate::Db; -use super::SymbolLookupResult; - /// Infer all types for a [`ScopeId`], including all definitions and expressions in that scope. /// Use when checking a scope, or needing to provide a type for an arbitrary expression in the /// scope. @@ -2396,26 +2394,47 @@ impl<'db> TypeInferenceBuilder<'db> { ) }; - match bindings_ty(self.db, definitions) { - SymbolLookupResult::Bound(ty) => ty, - SymbolLookupResult::Unbound => { - if may_be_unbound { - match self.lookup_name(name) { - SymbolLookupResult::Bound(ty) => ty, - SymbolLookupResult::Unbound => { - self.diagnostics.add( - name.into(), - "unresolved-reference", - format_args!("Name `{id}` used when not defined"), - ); - Type::Unknown - } + let definitions2 = definitions.clone(); + let has_definitions = definitions2.count() > 0; + + let bindings_ty = bindings_ty(self.db, definitions); + // dbg!(name); + // dbg!(may_be_unbound); + // dbg!(has_definitions); + + if may_be_unbound { + match self.lookup_name(name) { + SymbolLookupResult::Bound(ty) => match bindings_ty { + SymbolLookupResult::Bound(bindings_ty) => { + UnionType::from_elements(self.db, [bindings_ty, ty]) } - } else { + SymbolLookupResult::Unbound => ty, + }, + SymbolLookupResult::Unbound => { + if has_definitions { + self.diagnostics.add( + name.into(), + "possibly-unresolved-reference", + format_args!("Name `{id}` used when possibly not defined"), + ); + } else { + self.diagnostics.add( + name.into(), + "unresolved-reference", + format_args!("Name `{id}` used when not defined"), + ); + } + bindings_ty.unwrap_or_unknown() + } + } + } else { + match bindings_ty { + SymbolLookupResult::Bound(ty) => ty, + SymbolLookupResult::Unbound => { self.diagnostics.add( name.into(), - "possibly-unresolved-reference", - format_args!("Name `{id}` used when possibly not defined"), + "unresolved-reference", + format_args!("Name `{id}` used when not defined"), ); Type::Unknown } @@ -2443,7 +2462,9 @@ impl<'db> TypeInferenceBuilder<'db> { let value_ty = self.infer_expression(value); value_ty .member(self.db, &Name::new(&attr.id)) - .todo_unwrap_type() + .unwrap_or_unknown(); + + member_ty } fn infer_attribute_expression(&mut self, attribute: &ast::ExprAttribute) -> Type<'db> { From 58e4bc0be7f8e83922c3e61b8bbb765b779149f1 Mon Sep 17 00:00:00 2001 From: David Peter Date: Wed, 30 Oct 2024 13:20:32 +0100 Subject: [PATCH 05/20] Fix unary test --- .../src/types/infer.rs | 55 ++++++++++++------- 1 file changed, 35 insertions(+), 20 deletions(-) diff --git a/crates/red_knot_python_semantic/src/types/infer.rs b/crates/red_knot_python_semantic/src/types/infer.rs index 08bd057852bef..87fe9ac2d7114 100644 --- a/crates/red_knot_python_semantic/src/types/infer.rs +++ b/crates/red_knot_python_semantic/src/types/infer.rs @@ -2518,28 +2518,43 @@ impl<'db> TypeInferenceBuilder<'db> { unreachable!("Not operator is handled in its own case"); } }; - let class_member = class.class_member(self.db, unary_dunder_method); - let call = class_member - .todo_unwrap_type() - .call(self.db, &[operand_type]); - match call.return_ty_result( - self.db, - AnyNodeRef::ExprUnaryOp(unary), - &mut self.diagnostics, - ) { - Ok(t) => t, - Err(e) => { - self.diagnostics.add( - unary.into(), - "unsupported-operator", - format_args!( - "Unary operator `{op}` is unsupported for type `{}`", - operand_type.display(self.db), - ), - ); - e.return_ty() + let db = self.db; + + if let SymbolLookupResult::Bound(class_member) = + class.class_member(self.db, unary_dunder_method) + { + let call = class_member.call(self.db, &[operand_type]); + + match call.return_ty_result( + self.db, + AnyNodeRef::ExprUnaryOp(unary), + &mut self.diagnostics, + ) { + Ok(t) => t, + Err(e) => { + self.diagnostics.add( + unary.into(), + "unsupported-operator", + format_args!( + "Unary operator `{op}` is unsupported for type `{}`", + operand_type.display(db), + ), + ); + e.return_ty() + } } + } else { + self.diagnostics.add( + unary.into(), + "unsupported-operator", + format_args!( + "Unary operator `{op}` is unsupported for type `{}`", + operand_type.display(db), + ), + ); + + Type::Unknown } } _ => Type::Todo, // TODO other unary op types From f449a912087235ee946f2529fc13d2eae2e1fdfb Mon Sep 17 00:00:00 2001 From: David Peter Date: Wed, 30 Oct 2024 13:27:22 +0100 Subject: [PATCH 06/20] Fix binary/instances test --- .../src/types/infer.rs | 36 ++++++++++++------- 1 file changed, 23 insertions(+), 13 deletions(-) diff --git a/crates/red_knot_python_semantic/src/types/infer.rs b/crates/red_knot_python_semantic/src/types/infer.rs index 87fe9ac2d7114..22fea7a356c18 100644 --- a/crates/red_knot_python_semantic/src/types/infer.rs +++ b/crates/red_knot_python_semantic/src/types/infer.rs @@ -2777,22 +2777,32 @@ impl<'db> TypeInferenceBuilder<'db> { }); } } - left_class - .class_member(self.db, op.dunder()) - .todo_unwrap_type() - .call(self.db, &[left_ty, right_ty]) - .return_ty(self.db) - .or_else(|| { - if left_class == right_class { - None - } else { - right_class - .class_member(self.db, op.reflected_dunder()) - .todo_unwrap_type() + + let call_on_left_instance = if let SymbolLookupResult::Bound(class_member) = + left_class.class_member(self.db, op.dunder()) + { + class_member + .call(self.db, &[left_ty, right_ty]) + .return_ty(self.db) + } else { + None + }; + + call_on_left_instance.or_else(|| { + if left_class == right_class { + None + } else { + if let SymbolLookupResult::Bound(class_member) = + right_class.class_member(self.db, op.reflected_dunder()) + { + class_member .call(self.db, &[right_ty, left_ty]) .return_ty(self.db) + } else { + None } - }) + } + }) } ( From c3314aedfd86dce83f31a105a04f291fe0d1b92c Mon Sep 17 00:00:00 2001 From: David Peter Date: Wed, 30 Oct 2024 15:31:35 +0100 Subject: [PATCH 07/20] Maybe-unboundedness --- crates/red_knot_python_semantic/src/types.rs | 128 +++++++++++++----- .../src/types/infer.rs | 17 +-- 2 files changed, 103 insertions(+), 42 deletions(-) diff --git a/crates/red_knot_python_semantic/src/types.rs b/crates/red_knot_python_semantic/src/types.rs index 593cc1b0ae10f..307b41cd04ac6 100644 --- a/crates/red_knot_python_semantic/src/types.rs +++ b/crates/red_knot_python_semantic/src/types.rs @@ -29,9 +29,15 @@ mod display; mod infer; mod narrow; +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum Boundedness { + DefinitelyBound, + MaybeUnbound, +} + #[derive(Debug, Clone, PartialEq, Eq)] pub enum SymbolLookupResult<'db> { - Bound(Type<'db>), + Bound(Type<'db>, Boundedness), Unbound, } @@ -41,17 +47,27 @@ impl<'db> SymbolLookupResult<'db> { } #[must_use] - pub fn replace_unbound_with(self, replacement: Type<'db>) -> SymbolLookupResult<'db> { + pub fn replace_unbound_with( + self, + db: &'db dyn Db, + replacement: Type<'db>, + ) -> SymbolLookupResult<'db> { match self { - r @ SymbolLookupResult::Bound(_) => r, - SymbolLookupResult::Unbound => SymbolLookupResult::Bound(replacement), + r @ SymbolLookupResult::Bound(_, Boundedness::DefinitelyBound) => r, + SymbolLookupResult::Bound(ty, Boundedness::MaybeUnbound) => { + let union = UnionType::from_elements(db, [ty, replacement]); + SymbolLookupResult::Bound(union, Boundedness::DefinitelyBound) + } + SymbolLookupResult::Unbound => { + SymbolLookupResult::Bound(replacement, Boundedness::DefinitelyBound) + } } } #[track_caller] fn todo_unwrap_type(&self) -> Type<'db> { match self { - SymbolLookupResult::Bound(ty) => *ty, + SymbolLookupResult::Bound(ty, _) => *ty, SymbolLookupResult::Unbound => todo!(), } } @@ -59,14 +75,16 @@ impl<'db> SymbolLookupResult<'db> { #[track_caller] fn expect_bound(self) -> Type<'db> { match self { - SymbolLookupResult::Bound(ty) => ty, - SymbolLookupResult::Unbound => panic!("Expected a bound type"), + SymbolLookupResult::Bound(ty, Boundedness::DefinitelyBound) => ty, + _ => { + panic!("Expected a bound type") + } } } fn unwrap_or(&self, other: Type<'db>) -> Type<'db> { match self { - SymbolLookupResult::Bound(ty) => *ty, + SymbolLookupResult::Bound(ty, _) => *ty, SymbolLookupResult::Unbound => other, } } @@ -74,6 +92,28 @@ impl<'db> SymbolLookupResult<'db> { fn unwrap_or_unknown(&self) -> Type<'db> { self.unwrap_or(Type::Unknown) } + + fn may_be_unbound(&self) -> bool { + match self { + SymbolLookupResult::Bound(_, Boundedness::MaybeUnbound) + | SymbolLookupResult::Unbound => true, + SymbolLookupResult::Bound(_, Boundedness::DefinitelyBound) => false, + } + } + + fn and_may_be_unbound(&self, yes: bool) -> SymbolLookupResult<'db> { + match self { + SymbolLookupResult::Bound(ty, boundedness) => SymbolLookupResult::Bound( + *ty, + if yes { + Boundedness::MaybeUnbound + } else { + *boundedness + }, + ), + SymbolLookupResult::Unbound => SymbolLookupResult::Unbound, + } + } } pub fn check_types(db: &dyn Db, file: File) -> TypeCheckDiagnostics { @@ -106,23 +146,50 @@ fn symbol_ty_by_id<'db>( let declarations = use_def.public_declarations(symbol); // If the symbol is undeclared in some paths, include the inferred type in the public type. let undeclared_ty = if declarations.may_be_undeclared() { - Some(bindings_ty(db, use_def.public_bindings(symbol)).unwrap_or_unknown()) + // Some(bindings_ty( + // db, + // use_def.public_bindings(symbol), + // use_def + // .public_may_be_unbound(symbol) + // .then_some(Type::Unbound), + // )) + + match bindings_ty(db, use_def.public_bindings(symbol)) { + SymbolLookupResult::Bound(ty, boundedness) => { + // TODO: do something with boundedness + if use_def.public_may_be_unbound(symbol) { + Some(SymbolLookupResult::Bound(ty, Boundedness::MaybeUnbound)) + } else { + Some(SymbolLookupResult::Bound(ty, boundedness)) + } + } + SymbolLookupResult::Unbound => Some(SymbolLookupResult::Unbound), + } } else { None }; // Intentionally ignore conflicting declared types; that's not our problem, it's the // problem of the module we are importing from. SymbolLookupResult::Bound( - declarations_ty(db, declarations, undeclared_ty).unwrap_or_else(|(ty, _)| ty), + declarations_ty( + db, + declarations, + undeclared_ty.map(|ty| ty.todo_unwrap_type()), + ) + .unwrap_or_else(|(ty, _)| ty), + Boundedness::DefinitelyBound, ) } else { - bindings_ty( - db, - use_def.public_bindings(symbol), - // use_def - // .public_may_be_unbound(symbol) - // .then_some(Type::Unbound), - ) + // bindings_ty( + // db, + // use_def.public_bindings(symbol), + // use_def + // .public_may_be_unbound(symbol) + // .then_some(Type::Unbound), + // ) + + bindings_ty(db, use_def.public_bindings(symbol)) + .and_may_be_unbound(use_def.public_may_be_unbound(symbol)) } } @@ -270,12 +337,12 @@ fn bindings_ty<'db>( if let Some(first) = def_types.next() { if let Some(second) = def_types.next() { - SymbolLookupResult::Bound(UnionType::from_elements( - db, - [first, second].into_iter().chain(def_types), - )) + SymbolLookupResult::Bound( + UnionType::from_elements(db, [first, second].into_iter().chain(def_types)), + Boundedness::DefinitelyBound, + ) } else { - SymbolLookupResult::Bound(first) + SymbolLookupResult::Bound(first, Boundedness::DefinitelyBound) } } else { SymbolLookupResult::Unbound @@ -924,10 +991,10 @@ impl<'db> Type<'db> { // TODO: this should use `.to_instance()`, but we don't understand instance attribute yet let module_type_instance_member = KnownClass::ModuleType.to_class(db).member(db, name); - if let SymbolLookupResult::Bound(module_type_instance_member) = + if let SymbolLookupResult::Bound(module_type_instance_member, _) = module_type_instance_member { - global_lookup.replace_unbound_with(module_type_instance_member) + global_lookup.replace_unbound_with(db, module_type_instance_member) } else { global_lookup } @@ -950,6 +1017,7 @@ impl<'db> Type<'db> { } else { SymbolLookupResult::Bound( union.map(db, |ty| ty.member(db, name).expect_bound()), + Boundedness::DefinitelyBound, ) } } @@ -1132,12 +1200,8 @@ impl<'db> Type<'db> { let iterable_meta_type = self.to_meta_type(db); let dunder_iter_method = iterable_meta_type.member(db, "__iter__"); - if !dunder_iter_method.is_unbound() { - let Some(iterator_ty) = dunder_iter_method - .todo_unwrap_type() - .call(db, &[self]) - .return_ty(db) - else { + if let SymbolLookupResult::Bound(dunder_iter_method, _) = dunder_iter_method { + let Some(iterator_ty) = dunder_iter_method.call(db, &[self]).return_ty(db) else { return IterationOutcome::NotIterable { not_iterable_ty: self, }; @@ -1275,7 +1339,7 @@ impl<'db> From<&Type<'db>> for Type<'db> { impl<'db> From> for SymbolLookupResult<'db> { fn from(value: Type<'db>) -> Self { - SymbolLookupResult::Bound(value) + SymbolLookupResult::Bound(value, Boundedness::DefinitelyBound) } } @@ -1882,7 +1946,7 @@ impl<'db> ClassType<'db> { let member = self.own_class_member(db, name); if !member.is_unbound() { // TODO diagnostic if maybe unbound? - return member.replace_unbound_with(Type::Never); + return member.replace_unbound_with(db, Type::Never); } self.inherited_class_member(db, name) diff --git a/crates/red_knot_python_semantic/src/types/infer.rs b/crates/red_knot_python_semantic/src/types/infer.rs index 22fea7a356c18..689ede301ef8f 100644 --- a/crates/red_knot_python_semantic/src/types/infer.rs +++ b/crates/red_knot_python_semantic/src/types/infer.rs @@ -1677,7 +1677,7 @@ impl<'db> TypeInferenceBuilder<'db> { let member_ty = module_ty.member(self.db, &ast::name::Name::new(&name.id)); - if let SymbolLookupResult::Bound(member_ty) = member_ty { + if let SymbolLookupResult::Bound(member_ty, _) = member_ty { // For possibly-unbound names, just eliminate Unbound from the type; we // must be in a bound path. TODO diagnostic for maybe-unbound import? member_ty @@ -2398,14 +2398,11 @@ impl<'db> TypeInferenceBuilder<'db> { let has_definitions = definitions2.count() > 0; let bindings_ty = bindings_ty(self.db, definitions); - // dbg!(name); - // dbg!(may_be_unbound); - // dbg!(has_definitions); if may_be_unbound { match self.lookup_name(name) { - SymbolLookupResult::Bound(ty) => match bindings_ty { - SymbolLookupResult::Bound(bindings_ty) => { + SymbolLookupResult::Bound(ty, ty_boundedness) => match bindings_ty { + SymbolLookupResult::Bound(bindings_ty, _) => { UnionType::from_elements(self.db, [bindings_ty, ty]) } SymbolLookupResult::Unbound => ty, @@ -2429,7 +2426,7 @@ impl<'db> TypeInferenceBuilder<'db> { } } else { match bindings_ty { - SymbolLookupResult::Bound(ty) => ty, + SymbolLookupResult::Bound(ty, _) => ty, SymbolLookupResult::Unbound => { self.diagnostics.add( name.into(), @@ -2521,7 +2518,7 @@ impl<'db> TypeInferenceBuilder<'db> { let db = self.db; - if let SymbolLookupResult::Bound(class_member) = + if let SymbolLookupResult::Bound(class_member, _) = class.class_member(self.db, unary_dunder_method) { let call = class_member.call(self.db, &[operand_type]); @@ -2778,7 +2775,7 @@ impl<'db> TypeInferenceBuilder<'db> { } } - let call_on_left_instance = if let SymbolLookupResult::Bound(class_member) = + let call_on_left_instance = if let SymbolLookupResult::Bound(class_member, _) = left_class.class_member(self.db, op.dunder()) { class_member @@ -2792,7 +2789,7 @@ impl<'db> TypeInferenceBuilder<'db> { if left_class == right_class { None } else { - if let SymbolLookupResult::Bound(class_member) = + if let SymbolLookupResult::Bound(class_member, _) = right_class.class_member(self.db, op.reflected_dunder()) { class_member From ec829bd3304c38f9cc08cd152b96a3d6b5f292c9 Mon Sep 17 00:00:00 2001 From: David Peter Date: Wed, 30 Oct 2024 16:18:10 +0100 Subject: [PATCH 08/20] Fix one more test --- crates/red_knot_python_semantic/src/stdlib.rs | 20 +++++++- crates/red_knot_python_semantic/src/types.rs | 49 +++++++++++++------ .../src/types/infer.rs | 15 +++++- 3 files changed, 65 insertions(+), 19 deletions(-) diff --git a/crates/red_knot_python_semantic/src/stdlib.rs b/crates/red_knot_python_semantic/src/stdlib.rs index f094a6e00f867..7aed6b91e2124 100644 --- a/crates/red_knot_python_semantic/src/stdlib.rs +++ b/crates/red_knot_python_semantic/src/stdlib.rs @@ -2,7 +2,7 @@ use crate::module_name::ModuleName; use crate::module_resolver::resolve_module; use crate::semantic_index::global_scope; use crate::semantic_index::symbol::ScopeId; -use crate::types::{global_symbol_ty, SymbolLookupResult}; +use crate::types::{global_symbol_ty, SymbolLookupResult, Type}; use crate::Db; /// Enumeration of various core stdlib modules, for which we have dedicated Salsa queries. @@ -39,8 +39,26 @@ fn core_module_symbol_ty<'db>( core_module: CoreStdlibModule, symbol: &str, ) -> SymbolLookupResult<'db> { + // resolve_module(db, &core_module.name()) + // .map(|module| global_symbol_ty(db, module.file(), symbol)) + // .map(|ty| { + // if ty.is_unbound() { + // ty + // } else { + // ty.replace_unbound_with(db, Type::Never) + // } + // }) + // .unwrap_or(Type::Unbound) + resolve_module(db, &core_module.name()) .map(|module| global_symbol_ty(db, module.file(), symbol)) + .map(|res| { + if res.is_unbound() { + res + } else { + res.replace_unbound_with(db, Type::Never) + } + }) .unwrap_or(SymbolLookupResult::Unbound) } diff --git a/crates/red_knot_python_semantic/src/types.rs b/crates/red_knot_python_semantic/src/types.rs index 307b41cd04ac6..fea5271353ee2 100644 --- a/crates/red_knot_python_semantic/src/types.rs +++ b/crates/red_knot_python_semantic/src/types.rs @@ -55,7 +55,7 @@ impl<'db> SymbolLookupResult<'db> { match self { r @ SymbolLookupResult::Bound(_, Boundedness::DefinitelyBound) => r, SymbolLookupResult::Bound(ty, Boundedness::MaybeUnbound) => { - let union = UnionType::from_elements(db, [ty, replacement]); + let union = UnionType::from_elements(db, [replacement, ty]); SymbolLookupResult::Bound(union, Boundedness::DefinitelyBound) } SymbolLookupResult::Unbound => { @@ -157,11 +157,20 @@ fn symbol_ty_by_id<'db>( match bindings_ty(db, use_def.public_bindings(symbol)) { SymbolLookupResult::Bound(ty, boundedness) => { // TODO: do something with boundedness - if use_def.public_may_be_unbound(symbol) { - Some(SymbolLookupResult::Bound(ty, Boundedness::MaybeUnbound)) - } else { - Some(SymbolLookupResult::Bound(ty, boundedness)) - } + + // dbg!(boundedness); + // dbg!(use_def.public_may_be_unbound(symbol)); + + Some( + SymbolLookupResult::Bound(ty, boundedness) + .and_may_be_unbound(use_def.public_may_be_unbound(symbol)), + ) + + // if use_def.public_may_be_unbound(symbol) { + // Some(SymbolLookupResult::Bound(ty, Boundedness::MaybeUnbound)) + // } else { + // Some(SymbolLookupResult::Bound(ty, boundedness)) + // } } SymbolLookupResult::Unbound => Some(SymbolLookupResult::Unbound), } @@ -170,15 +179,17 @@ fn symbol_ty_by_id<'db>( }; // Intentionally ignore conflicting declared types; that's not our problem, it's the // problem of the module we are importing from. - SymbolLookupResult::Bound( - declarations_ty( - db, - declarations, - undeclared_ty.map(|ty| ty.todo_unwrap_type()), - ) - .unwrap_or_else(|(ty, _)| ty), - Boundedness::DefinitelyBound, - ) + match undeclared_ty { + Some(SymbolLookupResult::Bound(ty, boundedness)) => SymbolLookupResult::Bound( + declarations_ty(db, declarations, Some(ty)).unwrap_or_else(|(ty, _)| ty), + boundedness, + ), + Some(SymbolLookupResult::Unbound) => SymbolLookupResult::Unbound, + None => SymbolLookupResult::Bound( + declarations_ty(db, declarations, None).unwrap_or_else(|(ty, _)| ty), + Boundedness::DefinitelyBound, + ), + } } else { // bindings_ty( // db, @@ -244,7 +255,13 @@ pub(crate) fn global_symbol_ty<'db>( name: &str, ) -> SymbolLookupResult<'db> { let explicit_ty = symbol_ty(db, global_scope(db, file), name); - if !explicit_ty.is_unbound() { + + // if let SymbolLookupResult::Bound(ty, b) = explicit_ty { + // dbg!(ty.display(db)); + // dbg!(b); + // } + + if !explicit_ty.may_be_unbound() { return explicit_ty; } diff --git a/crates/red_knot_python_semantic/src/types/infer.rs b/crates/red_knot_python_semantic/src/types/infer.rs index 689ede301ef8f..eb7da6f73b2df 100644 --- a/crates/red_knot_python_semantic/src/types/infer.rs +++ b/crates/red_knot_python_semantic/src/types/infer.rs @@ -2343,8 +2343,12 @@ impl<'db> TypeInferenceBuilder<'db> { global_symbol_ty(self.db, self.file, name) }; + // if let SymbolLookupResult::Bound(ty, _) = ty { + // dbg!(ty.display(self.db)); + // } + // Fallback to builtins (without infinite recursion if we're already in builtins.) - if ty.is_unbound() && Some(self.scope()) != builtins_module_scope(self.db) { + if ty.may_be_unbound() && Some(self.scope()) != builtins_module_scope(self.db) { let mut builtin_ty = builtins_symbol_ty(self.db, name); if builtin_ty.is_unbound() && name == "reveal_type" { self.diagnostics.add( @@ -2356,7 +2360,8 @@ impl<'db> TypeInferenceBuilder<'db> { builtin_ty = typing_extensions_symbol_ty(self.db, name); } - builtin_ty + // ty.replace_unbound_with(self.db, builtin_ty) + ty.replace_unbound_with(self.db, builtin_ty.todo_unwrap_type()) } else { ty } @@ -2398,8 +2403,14 @@ impl<'db> TypeInferenceBuilder<'db> { let has_definitions = definitions2.count() > 0; let bindings_ty = bindings_ty(self.db, definitions); + // dbg!("=============================="); + // dbg!(name); + // dbg!(may_be_unbound); + // dbg!(has_definitions); if may_be_unbound { + // dbg!(self.lookup_name(name).todo_unwrap_type().display(self.db)); + match self.lookup_name(name) { SymbolLookupResult::Bound(ty, ty_boundedness) => match bindings_ty { SymbolLookupResult::Bound(bindings_ty, _) => { From 2e8415b0fc9e281bdeb6dd5a7e941bf324af5d53 Mon Sep 17 00:00:00 2001 From: David Peter Date: Wed, 30 Oct 2024 16:32:12 +0100 Subject: [PATCH 09/20] 'Fix' remaining tests --- .../mdtest/scopes/moduletype_attrs.md | 4 +-- .../resources/mdtest/subscript/class.md | 4 +-- crates/red_knot_python_semantic/src/types.rs | 28 +++++++++++++++++-- .../src/types/infer.rs | 12 ++++++-- 4 files changed, 40 insertions(+), 8 deletions(-) diff --git a/crates/red_knot_python_semantic/resources/mdtest/scopes/moduletype_attrs.md b/crates/red_knot_python_semantic/resources/mdtest/scopes/moduletype_attrs.md index 124f4576ec6c9..9255cbdfe0f87 100644 --- a/crates/red_knot_python_semantic/resources/mdtest/scopes/moduletype_attrs.md +++ b/crates/red_knot_python_semantic/resources/mdtest/scopes/moduletype_attrs.md @@ -120,7 +120,7 @@ if returns_bool(): __name__ = 1 reveal_type(__file__) # revealed: Literal[42] -reveal_type(__name__) # revealed: str | Literal[1] +reveal_type(__name__) # revealed: Literal[1] | str ``` ## Conditionally global or `ModuleType` attribute, with annotation @@ -137,5 +137,5 @@ if returns_bool(): __name__: int = 1 reveal_type(__file__) # revealed: Literal[42] -reveal_type(__name__) # revealed: str | Literal[1] +reveal_type(__name__) # revealed: Literal[1] | str ``` diff --git a/crates/red_knot_python_semantic/resources/mdtest/subscript/class.md b/crates/red_knot_python_semantic/resources/mdtest/subscript/class.md index 82d8beff9ef82..5461960a7f5a6 100644 --- a/crates/red_knot_python_semantic/resources/mdtest/subscript/class.md +++ b/crates/red_knot_python_semantic/resources/mdtest/subscript/class.md @@ -68,8 +68,8 @@ if flag: else: class Spam: ... -# error: [call-non-callable] "Method `__class_getitem__` of type `Literal[__class_getitem__] | Unbound` is not callable on object of type `Literal[Spam, Spam]`" -# revealed: str | Unknown +# error: [non-subscriptable] "Cannot subscript object of type `Literal[Spam, Spam]` with no `__class_getitem__` method" +# revealed: Unknown reveal_type(Spam[42]) ``` diff --git a/crates/red_knot_python_semantic/src/types.rs b/crates/red_knot_python_semantic/src/types.rs index fea5271353ee2..744a1a183c98a 100644 --- a/crates/red_knot_python_semantic/src/types.rs +++ b/crates/red_knot_python_semantic/src/types.rs @@ -14,7 +14,7 @@ use crate::stdlib::{ }; use crate::types::diagnostic::TypeCheckDiagnosticsBuilder; use crate::types::narrow::narrowing_constraint; -use crate::{Db, FxOrderSet, HasTy, Module, SemanticModel}; +use crate::{db, Db, FxOrderSet, HasTy, Module, SemanticModel}; pub(crate) use self::builder::{IntersectionBuilder, UnionBuilder}; pub use self::diagnostic::{TypeCheckDiagnostic, TypeCheckDiagnostics}; @@ -64,6 +64,30 @@ impl<'db> SymbolLookupResult<'db> { } } + pub fn merge_unbound_with( + self, + db: &'db dyn Db, + replacement: SymbolLookupResult<'db>, + ) -> SymbolLookupResult<'db> { + match (self, replacement) { + (r @ SymbolLookupResult::Bound(_, Boundedness::DefinitelyBound), _) => r, + (SymbolLookupResult::Unbound, other) => other, + (SymbolLookupResult::Bound(ty, Boundedness::MaybeUnbound), other) => match other { + SymbolLookupResult::Bound(other_ty, Boundedness::DefinitelyBound) => { + let union = UnionType::from_elements(db, [other_ty, ty]); + SymbolLookupResult::Bound(union, Boundedness::MaybeUnbound) + } + SymbolLookupResult::Bound(other_ty, Boundedness::MaybeUnbound) => { + let union = UnionType::from_elements(db, [other_ty, ty]); + SymbolLookupResult::Bound(union, Boundedness::MaybeUnbound) + } + SymbolLookupResult::Unbound => { + SymbolLookupResult::Bound(ty, Boundedness::MaybeUnbound) + } + }, + } + } + #[track_caller] fn todo_unwrap_type(&self) -> Type<'db> { match self { @@ -1245,7 +1269,7 @@ impl<'db> Type<'db> { // accepting `int` or `SupportsIndex` let dunder_get_item_method = iterable_meta_type .member(db, "__getitem__") - .todo_unwrap_type(); + .unwrap_or(Type::Never); dunder_get_item_method .call(db, &[self, KnownClass::Int.to_instance(db)]) diff --git a/crates/red_knot_python_semantic/src/types/infer.rs b/crates/red_knot_python_semantic/src/types/infer.rs index eb7da6f73b2df..920ed4151016a 100644 --- a/crates/red_knot_python_semantic/src/types/infer.rs +++ b/crates/red_knot_python_semantic/src/types/infer.rs @@ -2361,7 +2361,15 @@ impl<'db> TypeInferenceBuilder<'db> { } // ty.replace_unbound_with(self.db, builtin_ty) - ty.replace_unbound_with(self.db, builtin_ty.todo_unwrap_type()) + // ty.replace_unbound_with(self.db, builtin_ty.unwrap_or_unknown()) + // ty.merge_unbound_with(self.db, builtin_ty) + + match builtin_ty { + SymbolLookupResult::Bound(builtin_ty, boundedness) => { + ty.replace_unbound_with(self.db, builtin_ty) + } + SymbolLookupResult::Unbound => ty, + } } else { ty } @@ -3967,7 +3975,7 @@ fn perform_rich_comparison<'db>( |op: RichCompareOperator, left_class: ClassType<'db>, right_class: ClassType<'db>| { left_class .class_member(db, op.dunder()) - .todo_unwrap_type() + .unwrap_or(Type::Never) .call( db, &[Type::Instance(left_class), Type::Instance(right_class)], From 593de027cd6b7c6285915cdd9e3c21cd0c548c9c Mon Sep 17 00:00:00 2001 From: David Peter Date: Wed, 30 Oct 2024 16:41:25 +0100 Subject: [PATCH 10/20] Get rid of clone's --- .../red_knot_python_semantic/src/semantic_index.rs | 4 +--- .../src/semantic_index/use_def.rs | 2 +- .../src/semantic_index/use_def/bitset.rs | 2 +- .../src/semantic_index/use_def/symbol_state.rs | 2 +- crates/red_knot_python_semantic/src/types.rs | 13 ++++++++----- crates/red_knot_python_semantic/src/types/infer.rs | 4 ++-- 6 files changed, 14 insertions(+), 13 deletions(-) diff --git a/crates/red_knot_python_semantic/src/semantic_index.rs b/crates/red_knot_python_semantic/src/semantic_index.rs index 083779dc0c9af..1797ec5304286 100644 --- a/crates/red_knot_python_semantic/src/semantic_index.rs +++ b/crates/red_knot_python_semantic/src/semantic_index.rs @@ -27,9 +27,7 @@ pub mod expression; pub mod symbol; mod use_def; -pub(crate) use self::use_def::{ - BindingWithConstraints, BindingWithConstraintsIterator, DeclarationsIterator, -}; +pub(crate) use self::use_def::{BindingWithConstraints, DeclarationsIterator}; type SymbolMap = hashbrown::HashMap; diff --git a/crates/red_knot_python_semantic/src/semantic_index/use_def.rs b/crates/red_knot_python_semantic/src/semantic_index/use_def.rs index ebb67c82a47db..554ee11a3e7d1 100644 --- a/crates/red_knot_python_semantic/src/semantic_index/use_def.rs +++ b/crates/red_knot_python_semantic/src/semantic_index/use_def.rs @@ -356,7 +356,7 @@ enum SymbolDefinitions { Declarations(SymbolDeclarations), } -#[derive(Debug, Clone)] +#[derive(Debug)] pub(crate) struct BindingWithConstraintsIterator<'map, 'db> { all_definitions: &'map IndexVec>, all_constraints: &'map IndexVec>, diff --git a/crates/red_knot_python_semantic/src/semantic_index/use_def/bitset.rs b/crates/red_knot_python_semantic/src/semantic_index/use_def/bitset.rs index e889c429ba913..464f718e7b4f4 100644 --- a/crates/red_knot_python_semantic/src/semantic_index/use_def/bitset.rs +++ b/crates/red_knot_python_semantic/src/semantic_index/use_def/bitset.rs @@ -122,7 +122,7 @@ impl BitSet { } /// Iterator over values in a [`BitSet`]. -#[derive(Debug, Clone)] +#[derive(Debug)] pub(super) struct BitSetIterator<'a, const B: usize> { /// The blocks we are iterating over. blocks: &'a [u64], diff --git a/crates/red_knot_python_semantic/src/semantic_index/use_def/symbol_state.rs b/crates/red_knot_python_semantic/src/semantic_index/use_def/symbol_state.rs index 0b1cd06d3cda2..09210bfab05d6 100644 --- a/crates/red_knot_python_semantic/src/semantic_index/use_def/symbol_state.rs +++ b/crates/red_knot_python_semantic/src/semantic_index/use_def/symbol_state.rs @@ -355,7 +355,7 @@ pub(super) struct BindingIdWithConstraints<'a> { pub(super) constraint_ids: ConstraintIdIterator<'a>, } -#[derive(Debug, Clone)] +#[derive(Debug)] pub(super) struct BindingIdWithConstraintsIterator<'a> { definitions: BindingsIterator<'a>, constraints: ConstraintsIterator<'a>, diff --git a/crates/red_knot_python_semantic/src/types.rs b/crates/red_knot_python_semantic/src/types.rs index 744a1a183c98a..b480dc6e9dc87 100644 --- a/crates/red_knot_python_semantic/src/types.rs +++ b/crates/red_knot_python_semantic/src/types.rs @@ -7,14 +7,14 @@ use crate::semantic_index::definition::{Definition, DefinitionKind}; use crate::semantic_index::symbol::{ScopeId, ScopedSymbolId, Symbol}; use crate::semantic_index::{ global_scope, semantic_index, symbol_table, use_def_map, BindingWithConstraints, - BindingWithConstraintsIterator, DeclarationsIterator, + DeclarationsIterator, }; use crate::stdlib::{ builtins_symbol_ty, types_symbol_ty, typeshed_symbol_ty, typing_extensions_symbol_ty, }; use crate::types::diagnostic::TypeCheckDiagnosticsBuilder; use crate::types::narrow::narrowing_constraint; -use crate::{db, Db, FxOrderSet, HasTy, Module, SemanticModel}; +use crate::{Db, FxOrderSet, HasTy, Module, SemanticModel}; pub(crate) use self::builder::{IntersectionBuilder, UnionBuilder}; pub use self::diagnostic::{TypeCheckDiagnostic, TypeCheckDiagnostics}; @@ -349,10 +349,13 @@ fn definition_expression_ty<'db>( /// Will panic if called with zero bindings and no `unbound_ty`. This is a logic error, as any /// symbol with zero visible bindings clearly may be unbound, and the caller should provide an /// `unbound_ty`. -fn bindings_ty<'db>( +fn bindings_ty<'map, 'db>( db: &'db dyn Db, - bindings_with_constraints: BindingWithConstraintsIterator<'_, 'db>, -) -> SymbolLookupResult<'db> { + bindings_with_constraints: impl Iterator>, +) -> SymbolLookupResult<'db> +where + 'db: 'map, +{ let mut def_types = bindings_with_constraints.map( |BindingWithConstraints { binding, diff --git a/crates/red_knot_python_semantic/src/types/infer.rs b/crates/red_knot_python_semantic/src/types/infer.rs index 920ed4151016a..6cf5692ca311f 100644 --- a/crates/red_knot_python_semantic/src/types/infer.rs +++ b/crates/red_knot_python_semantic/src/types/infer.rs @@ -2407,8 +2407,8 @@ impl<'db> TypeInferenceBuilder<'db> { ) }; - let definitions2 = definitions.clone(); - let has_definitions = definitions2.count() > 0; + let mut definitions = definitions.peekable(); + let has_definitions = definitions.peek().is_some(); let bindings_ty = bindings_ty(self.db, definitions); // dbg!("=============================="); From b003abdd74e15a6f863905087fa4e823d2755063 Mon Sep 17 00:00:00 2001 From: David Peter Date: Wed, 30 Oct 2024 16:47:51 +0100 Subject: [PATCH 11/20] Get rid of todo_unwrap_type --- crates/red_knot_python_semantic/src/stdlib.rs | 11 ----------- crates/red_knot_python_semantic/src/types.rs | 12 ++---------- .../red_knot_python_semantic/src/types/infer.rs | 16 +++++++--------- 3 files changed, 9 insertions(+), 30 deletions(-) diff --git a/crates/red_knot_python_semantic/src/stdlib.rs b/crates/red_knot_python_semantic/src/stdlib.rs index 7aed6b91e2124..b9ba6ba2f06de 100644 --- a/crates/red_knot_python_semantic/src/stdlib.rs +++ b/crates/red_knot_python_semantic/src/stdlib.rs @@ -39,17 +39,6 @@ fn core_module_symbol_ty<'db>( core_module: CoreStdlibModule, symbol: &str, ) -> SymbolLookupResult<'db> { - // resolve_module(db, &core_module.name()) - // .map(|module| global_symbol_ty(db, module.file(), symbol)) - // .map(|ty| { - // if ty.is_unbound() { - // ty - // } else { - // ty.replace_unbound_with(db, Type::Never) - // } - // }) - // .unwrap_or(Type::Unbound) - resolve_module(db, &core_module.name()) .map(|module| global_symbol_ty(db, module.file(), symbol)) .map(|res| { diff --git a/crates/red_knot_python_semantic/src/types.rs b/crates/red_knot_python_semantic/src/types.rs index b480dc6e9dc87..2a41517d02ce9 100644 --- a/crates/red_knot_python_semantic/src/types.rs +++ b/crates/red_knot_python_semantic/src/types.rs @@ -88,14 +88,6 @@ impl<'db> SymbolLookupResult<'db> { } } - #[track_caller] - fn todo_unwrap_type(&self) -> Type<'db> { - match self { - SymbolLookupResult::Bound(ty, _) => *ty, - SymbolLookupResult::Unbound => todo!(), - } - } - #[track_caller] fn expect_bound(self) -> Type<'db> { match self { @@ -1192,7 +1184,7 @@ impl<'db> Type<'db> { let args = std::iter::once(self) .chain(arg_types.iter().copied()) .collect::>(); - dunder_call_method.todo_unwrap_type().call(db, &args) + dunder_call_method.unwrap_or(Type::Never).call(db, &args) } } @@ -1254,7 +1246,7 @@ impl<'db> Type<'db> { let dunder_next_method = iterator_ty .to_meta_type(db) .member(db, "__next__") - .todo_unwrap_type(); + .unwrap_or(Type::Never); return dunder_next_method .call(db, &[iterator_ty]) .return_ty(db) diff --git a/crates/red_knot_python_semantic/src/types/infer.rs b/crates/red_knot_python_semantic/src/types/infer.rs index 6cf5692ca311f..55d72b4f2fa40 100644 --- a/crates/red_knot_python_semantic/src/types/infer.rs +++ b/crates/red_knot_python_semantic/src/types/infer.rs @@ -1030,7 +1030,7 @@ impl<'db> TypeInferenceBuilder<'db> { // TODO should infer `ExceptionGroup` if all caught exceptions // are subclasses of `Exception` --Alex builtins_symbol_ty(self.db, "BaseExceptionGroup") - .todo_unwrap_type() + .unwrap_or_unknown() .to_instance(self.db) } else { // TODO: anything that's a consistent subtype of @@ -1413,7 +1413,7 @@ impl<'db> TypeInferenceBuilder<'db> { let class_member = class.class_member(self.db, op.in_place_dunder()); if !class_member.is_unbound() { let call = class_member - .todo_unwrap_type() + .unwrap_or(Type::Never) .call(self.db, &[target_type, value_type]); return match call.return_ty_result( self.db, @@ -2417,8 +2417,6 @@ impl<'db> TypeInferenceBuilder<'db> { // dbg!(has_definitions); if may_be_unbound { - // dbg!(self.lookup_name(name).todo_unwrap_type().display(self.db)); - match self.lookup_name(name) { SymbolLookupResult::Bound(ty, ty_boundedness) => match bindings_ty { SymbolLookupResult::Bound(bindings_ty, _) => { @@ -2781,13 +2779,13 @@ impl<'db> TypeInferenceBuilder<'db> { && rhs_reflected != left_class.class_member(self.db, reflected_dunder) { return rhs_reflected - .todo_unwrap_type() + .unwrap_or(Type::Never) .call(self.db, &[right_ty, left_ty]) .return_ty(self.db) .or_else(|| { left_class .class_member(self.db, op.dunder()) - .todo_unwrap_type() + .unwrap_or(Type::Never) .call(self.db, &[left_ty, right_ty]) .return_ty(self.db) }); @@ -3406,7 +3404,7 @@ impl<'db> TypeInferenceBuilder<'db> { let dunder_getitem_method = value_meta_ty.member(self.db, "__getitem__"); if !dunder_getitem_method.is_unbound() { return dunder_getitem_method - .todo_unwrap_type() + .unwrap_or(Type::Never) .call(self.db, &[slice_ty]) .return_ty_result(self.db, value_node.into(), &mut self.diagnostics) .unwrap_or_else(|err| { @@ -3436,7 +3434,7 @@ impl<'db> TypeInferenceBuilder<'db> { let dunder_class_getitem_method = value_ty.member(self.db, "__class_getitem__"); if !dunder_class_getitem_method.is_unbound() { return dunder_class_getitem_method - .todo_unwrap_type() + .unwrap_or(Type::Never) .call(self.db, &[slice_ty]) .return_ty_result(self.db, value_node.into(), &mut self.diagnostics) .unwrap_or_else(|err| { @@ -4031,7 +4029,7 @@ fn perform_membership_test_comparison<'db>( } else { // If `__contains__` is available, it is used directly for the membership test. contains_dunder - .todo_unwrap_type() + .unwrap_or(Type::Never) .call(db, &[right_instance, left_instance]) .return_ty(db) }; From a7840449a558c38f9d0f9e5d930cfa4f287419f8 Mon Sep 17 00:00:00 2001 From: David Peter Date: Wed, 30 Oct 2024 17:07:22 +0100 Subject: [PATCH 12/20] Cleanup --- crates/red_knot_python_semantic/src/types.rs | 59 ++++--------------- .../src/types/infer.rs | 27 ++------- 2 files changed, 18 insertions(+), 68 deletions(-) diff --git a/crates/red_knot_python_semantic/src/types.rs b/crates/red_knot_python_semantic/src/types.rs index 2a41517d02ce9..77c245f54163a 100644 --- a/crates/red_knot_python_semantic/src/types.rs +++ b/crates/red_knot_python_semantic/src/types.rs @@ -162,34 +162,10 @@ fn symbol_ty_by_id<'db>( let declarations = use_def.public_declarations(symbol); // If the symbol is undeclared in some paths, include the inferred type in the public type. let undeclared_ty = if declarations.may_be_undeclared() { - // Some(bindings_ty( - // db, - // use_def.public_bindings(symbol), - // use_def - // .public_may_be_unbound(symbol) - // .then_some(Type::Unbound), - // )) - - match bindings_ty(db, use_def.public_bindings(symbol)) { - SymbolLookupResult::Bound(ty, boundedness) => { - // TODO: do something with boundedness - - // dbg!(boundedness); - // dbg!(use_def.public_may_be_unbound(symbol)); - - Some( - SymbolLookupResult::Bound(ty, boundedness) - .and_may_be_unbound(use_def.public_may_be_unbound(symbol)), - ) - - // if use_def.public_may_be_unbound(symbol) { - // Some(SymbolLookupResult::Bound(ty, Boundedness::MaybeUnbound)) - // } else { - // Some(SymbolLookupResult::Bound(ty, boundedness)) - // } - } - SymbolLookupResult::Unbound => Some(SymbolLookupResult::Unbound), - } + Some( + bindings_ty(db, use_def.public_bindings(symbol)) + .and_may_be_unbound(use_def.public_may_be_unbound(symbol)), + ) } else { None }; @@ -200,21 +176,13 @@ fn symbol_ty_by_id<'db>( declarations_ty(db, declarations, Some(ty)).unwrap_or_else(|(ty, _)| ty), boundedness, ), - Some(SymbolLookupResult::Unbound) => SymbolLookupResult::Unbound, None => SymbolLookupResult::Bound( declarations_ty(db, declarations, None).unwrap_or_else(|(ty, _)| ty), Boundedness::DefinitelyBound, ), + Some(SymbolLookupResult::Unbound) => SymbolLookupResult::Unbound, } } else { - // bindings_ty( - // db, - // use_def.public_bindings(symbol), - // use_def - // .public_may_be_unbound(symbol) - // .then_some(Type::Unbound), - // ) - bindings_ty(db, use_def.public_bindings(symbol)) .and_may_be_unbound(use_def.public_may_be_unbound(symbol)) } @@ -272,11 +240,6 @@ pub(crate) fn global_symbol_ty<'db>( ) -> SymbolLookupResult<'db> { let explicit_ty = symbol_ty(db, global_scope(db, file), name); - // if let SymbolLookupResult::Bound(ty, b) = explicit_ty { - // dbg!(ty.display(db)); - // dbg!(b); - // } - if !explicit_ty.may_be_unbound() { return explicit_ty; } @@ -290,7 +253,11 @@ pub(crate) fn global_symbol_ty<'db>( { // TODO: this should use `.to_instance(db)`. but we don't understand attribute access // on instance types yet. - return KnownClass::ModuleType.to_class(db).member(db, name); + if let SymbolLookupResult::Bound(module_type_member, _) = + KnownClass::ModuleType.to_class(db).member(db, name) + { + return explicit_ty.replace_unbound_with(db, module_type_member); + } } explicit_ty @@ -1023,12 +990,10 @@ impl<'db> Type<'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. - if name != "__getattr__" && global_lookup.is_unbound() { + if name != "__getattr__" && global_lookup.may_be_unbound() { // TODO: this should use `.to_instance()`, but we don't understand instance attribute yet - let module_type_instance_member = - KnownClass::ModuleType.to_class(db).member(db, name); if let SymbolLookupResult::Bound(module_type_instance_member, _) = - module_type_instance_member + KnownClass::ModuleType.to_class(db).member(db, name) { global_lookup.replace_unbound_with(db, module_type_instance_member) } else { diff --git a/crates/red_knot_python_semantic/src/types/infer.rs b/crates/red_knot_python_semantic/src/types/infer.rs index 55d72b4f2fa40..b8784356a2650 100644 --- a/crates/red_knot_python_semantic/src/types/infer.rs +++ b/crates/red_knot_python_semantic/src/types/infer.rs @@ -1675,9 +1675,9 @@ impl<'db> TypeInferenceBuilder<'db> { asname: _, } = alias; - let member_ty = module_ty.member(self.db, &ast::name::Name::new(&name.id)); - - if let SymbolLookupResult::Bound(member_ty, _) = member_ty { + if let SymbolLookupResult::Bound(member_ty, _) = + module_ty.member(self.db, &ast::name::Name::new(&name.id)) + { // For possibly-unbound names, just eliminate Unbound from the type; we // must be in a bound path. TODO diagnostic for maybe-unbound import? member_ty @@ -2343,10 +2343,6 @@ impl<'db> TypeInferenceBuilder<'db> { global_symbol_ty(self.db, self.file, name) }; - // if let SymbolLookupResult::Bound(ty, _) = ty { - // dbg!(ty.display(self.db)); - // } - // Fallback to builtins (without infinite recursion if we're already in builtins.) if ty.may_be_unbound() && Some(self.scope()) != builtins_module_scope(self.db) { let mut builtin_ty = builtins_symbol_ty(self.db, name); @@ -2360,12 +2356,8 @@ impl<'db> TypeInferenceBuilder<'db> { builtin_ty = typing_extensions_symbol_ty(self.db, name); } - // ty.replace_unbound_with(self.db, builtin_ty) - // ty.replace_unbound_with(self.db, builtin_ty.unwrap_or_unknown()) - // ty.merge_unbound_with(self.db, builtin_ty) - match builtin_ty { - SymbolLookupResult::Bound(builtin_ty, boundedness) => { + SymbolLookupResult::Bound(builtin_ty, _) => { ty.replace_unbound_with(self.db, builtin_ty) } SymbolLookupResult::Unbound => ty, @@ -2411,14 +2403,9 @@ impl<'db> TypeInferenceBuilder<'db> { let has_definitions = definitions.peek().is_some(); let bindings_ty = bindings_ty(self.db, definitions); - // dbg!("=============================="); - // dbg!(name); - // dbg!(may_be_unbound); - // dbg!(has_definitions); - if may_be_unbound { match self.lookup_name(name) { - SymbolLookupResult::Bound(ty, ty_boundedness) => match bindings_ty { + SymbolLookupResult::Bound(ty, _) => match bindings_ty { SymbolLookupResult::Bound(bindings_ty, _) => { UnionType::from_elements(self.db, [bindings_ty, ty]) } @@ -2476,9 +2463,7 @@ impl<'db> TypeInferenceBuilder<'db> { let value_ty = self.infer_expression(value); value_ty .member(self.db, &Name::new(&attr.id)) - .unwrap_or_unknown(); - - member_ty + .unwrap_or_unknown() } fn infer_attribute_expression(&mut self, attribute: &ast::ExprAttribute) -> Type<'db> { From 25a8874c6272c210aa13aae262bb6883dc5f3e4e Mon Sep 17 00:00:00 2001 From: David Peter Date: Wed, 30 Oct 2024 17:08:54 +0100 Subject: [PATCH 13/20] Fix clippy suggestions --- crates/red_knot_python_semantic/src/types.rs | 24 -------------------- 1 file changed, 24 deletions(-) diff --git a/crates/red_knot_python_semantic/src/types.rs b/crates/red_knot_python_semantic/src/types.rs index 77c245f54163a..07ac2b960d80a 100644 --- a/crates/red_knot_python_semantic/src/types.rs +++ b/crates/red_knot_python_semantic/src/types.rs @@ -64,30 +64,6 @@ impl<'db> SymbolLookupResult<'db> { } } - pub fn merge_unbound_with( - self, - db: &'db dyn Db, - replacement: SymbolLookupResult<'db>, - ) -> SymbolLookupResult<'db> { - match (self, replacement) { - (r @ SymbolLookupResult::Bound(_, Boundedness::DefinitelyBound), _) => r, - (SymbolLookupResult::Unbound, other) => other, - (SymbolLookupResult::Bound(ty, Boundedness::MaybeUnbound), other) => match other { - SymbolLookupResult::Bound(other_ty, Boundedness::DefinitelyBound) => { - let union = UnionType::from_elements(db, [other_ty, ty]); - SymbolLookupResult::Bound(union, Boundedness::MaybeUnbound) - } - SymbolLookupResult::Bound(other_ty, Boundedness::MaybeUnbound) => { - let union = UnionType::from_elements(db, [other_ty, ty]); - SymbolLookupResult::Bound(union, Boundedness::MaybeUnbound) - } - SymbolLookupResult::Unbound => { - SymbolLookupResult::Bound(ty, Boundedness::MaybeUnbound) - } - }, - } - } - #[track_caller] fn expect_bound(self) -> Type<'db> { match self { From 0cfb6ddc79eeeebfbc1284db7682a4398a2637cd Mon Sep 17 00:00:00 2001 From: David Peter Date: Wed, 30 Oct 2024 17:51:55 +0100 Subject: [PATCH 14/20] Update expected diagnostics --- crates/red_knot_python_semantic/src/types.rs | 3 +-- crates/red_knot_python_semantic/src/types/infer.rs | 6 ++---- crates/ruff_benchmark/benches/red_knot.rs | 2 -- 3 files changed, 3 insertions(+), 8 deletions(-) diff --git a/crates/red_knot_python_semantic/src/types.rs b/crates/red_knot_python_semantic/src/types.rs index 07ac2b960d80a..fb7047abda0ec 100644 --- a/crates/red_knot_python_semantic/src/types.rs +++ b/crates/red_knot_python_semantic/src/types.rs @@ -2220,8 +2220,7 @@ mod tests { " class A: ... class B: ... - def flag() -> bool: ... - U = A if flag() else B + U = A if flag else B ", ) .unwrap(); diff --git a/crates/red_knot_python_semantic/src/types/infer.rs b/crates/red_knot_python_semantic/src/types/infer.rs index b8784356a2650..862d0f3d94ff4 100644 --- a/crates/red_knot_python_semantic/src/types/infer.rs +++ b/crates/red_knot_python_semantic/src/types/infer.rs @@ -2518,8 +2518,6 @@ impl<'db> TypeInferenceBuilder<'db> { } }; - let db = self.db; - if let SymbolLookupResult::Bound(class_member, _) = class.class_member(self.db, unary_dunder_method) { @@ -2537,7 +2535,7 @@ impl<'db> TypeInferenceBuilder<'db> { "unsupported-operator", format_args!( "Unary operator `{op}` is unsupported for type `{}`", - operand_type.display(db), + operand_type.display(self.db), ), ); e.return_ty() @@ -2549,7 +2547,7 @@ impl<'db> TypeInferenceBuilder<'db> { "unsupported-operator", format_args!( "Unary operator `{op}` is unsupported for type `{}`", - operand_type.display(db), + operand_type.display(self.db), ), ); diff --git a/crates/ruff_benchmark/benches/red_knot.rs b/crates/ruff_benchmark/benches/red_knot.rs index 6e52ccc409965..f604acd0a8f1f 100644 --- a/crates/ruff_benchmark/benches/red_knot.rs +++ b/crates/ruff_benchmark/benches/red_knot.rs @@ -36,13 +36,11 @@ static EXPECTED_DIAGNOSTICS: &[&str] = &[ "/src/tomllib/_parser.py:267:9: Conflicting declared types for `char`: Unknown, str | None", "/src/tomllib/_parser.py:348:20: Name `nest` used when possibly not defined", "/src/tomllib/_parser.py:353:5: Name `nest` used when possibly not defined", - "/src/tomllib/_parser.py:353:5: Method `__getitem__` of type `Unbound | @Todo` is not callable on object of type `Unbound | @Todo`", "/src/tomllib/_parser.py:364:9: Conflicting declared types for `char`: Unknown, str | None", "/src/tomllib/_parser.py:381:13: Conflicting declared types for `char`: Unknown, str | None", "/src/tomllib/_parser.py:395:9: Conflicting declared types for `char`: Unknown, str | None", "/src/tomllib/_parser.py:453:24: Name `nest` used when possibly not defined", "/src/tomllib/_parser.py:455:9: Name `nest` used when possibly not defined", - "/src/tomllib/_parser.py:455:9: Method `__getitem__` of type `Unbound | @Todo` is not callable on object of type `Unbound | @Todo`", "/src/tomllib/_parser.py:482:16: Name `char` used when possibly not defined", "/src/tomllib/_parser.py:566:12: Name `char` used when possibly not defined", "/src/tomllib/_parser.py:573:12: Name `char` used when possibly not defined", From 99337b24483886de34e859d7bd4eb0608154cf48 Mon Sep 17 00:00:00 2001 From: David Peter Date: Wed, 30 Oct 2024 18:50:35 +0100 Subject: [PATCH 15/20] Rename Bound => Type --- crates/red_knot_python_semantic/src/types.rs | 42 +++++++++---------- .../src/types/infer.rs | 16 +++---- 2 files changed, 29 insertions(+), 29 deletions(-) diff --git a/crates/red_knot_python_semantic/src/types.rs b/crates/red_knot_python_semantic/src/types.rs index fb7047abda0ec..889ad084c7fe8 100644 --- a/crates/red_knot_python_semantic/src/types.rs +++ b/crates/red_knot_python_semantic/src/types.rs @@ -29,15 +29,15 @@ mod display; mod infer; mod narrow; -#[derive(Debug, Clone, Copy, PartialEq, Eq)] +#[derive(Debug, Clone, Copy, PartialEq)] pub enum Boundedness { DefinitelyBound, MaybeUnbound, } -#[derive(Debug, Clone, PartialEq, Eq)] +#[derive(Debug, Clone, PartialEq)] pub enum SymbolLookupResult<'db> { - Bound(Type<'db>, Boundedness), + Type(Type<'db>, Boundedness), Unbound, } @@ -53,13 +53,13 @@ impl<'db> SymbolLookupResult<'db> { replacement: Type<'db>, ) -> SymbolLookupResult<'db> { match self { - r @ SymbolLookupResult::Bound(_, Boundedness::DefinitelyBound) => r, - SymbolLookupResult::Bound(ty, Boundedness::MaybeUnbound) => { + r @ SymbolLookupResult::Type(_, Boundedness::DefinitelyBound) => r, + SymbolLookupResult::Type(ty, Boundedness::MaybeUnbound) => { let union = UnionType::from_elements(db, [replacement, ty]); - SymbolLookupResult::Bound(union, Boundedness::DefinitelyBound) + SymbolLookupResult::Type(union, Boundedness::DefinitelyBound) } SymbolLookupResult::Unbound => { - SymbolLookupResult::Bound(replacement, Boundedness::DefinitelyBound) + SymbolLookupResult::Type(replacement, Boundedness::DefinitelyBound) } } } @@ -67,7 +67,7 @@ impl<'db> SymbolLookupResult<'db> { #[track_caller] fn expect_bound(self) -> Type<'db> { match self { - SymbolLookupResult::Bound(ty, Boundedness::DefinitelyBound) => ty, + SymbolLookupResult::Type(ty, Boundedness::DefinitelyBound) => ty, _ => { panic!("Expected a bound type") } @@ -76,7 +76,7 @@ impl<'db> SymbolLookupResult<'db> { fn unwrap_or(&self, other: Type<'db>) -> Type<'db> { match self { - SymbolLookupResult::Bound(ty, _) => *ty, + SymbolLookupResult::Type(ty, _) => *ty, SymbolLookupResult::Unbound => other, } } @@ -87,15 +87,15 @@ impl<'db> SymbolLookupResult<'db> { fn may_be_unbound(&self) -> bool { match self { - SymbolLookupResult::Bound(_, Boundedness::MaybeUnbound) + SymbolLookupResult::Type(_, Boundedness::MaybeUnbound) | SymbolLookupResult::Unbound => true, - SymbolLookupResult::Bound(_, Boundedness::DefinitelyBound) => false, + SymbolLookupResult::Type(_, Boundedness::DefinitelyBound) => false, } } fn and_may_be_unbound(&self, yes: bool) -> SymbolLookupResult<'db> { match self { - SymbolLookupResult::Bound(ty, boundedness) => SymbolLookupResult::Bound( + SymbolLookupResult::Type(ty, boundedness) => SymbolLookupResult::Type( *ty, if yes { Boundedness::MaybeUnbound @@ -148,11 +148,11 @@ fn symbol_ty_by_id<'db>( // Intentionally ignore conflicting declared types; that's not our problem, it's the // problem of the module we are importing from. match undeclared_ty { - Some(SymbolLookupResult::Bound(ty, boundedness)) => SymbolLookupResult::Bound( + Some(SymbolLookupResult::Type(ty, boundedness)) => SymbolLookupResult::Type( declarations_ty(db, declarations, Some(ty)).unwrap_or_else(|(ty, _)| ty), boundedness, ), - None => SymbolLookupResult::Bound( + None => SymbolLookupResult::Type( declarations_ty(db, declarations, None).unwrap_or_else(|(ty, _)| ty), Boundedness::DefinitelyBound, ), @@ -229,7 +229,7 @@ pub(crate) fn global_symbol_ty<'db>( { // TODO: this should use `.to_instance(db)`. but we don't understand attribute access // on instance types yet. - if let SymbolLookupResult::Bound(module_type_member, _) = + if let SymbolLookupResult::Type(module_type_member, _) = KnownClass::ModuleType.to_class(db).member(db, name) { return explicit_ty.replace_unbound_with(db, module_type_member); @@ -316,12 +316,12 @@ where if let Some(first) = def_types.next() { if let Some(second) = def_types.next() { - SymbolLookupResult::Bound( + SymbolLookupResult::Type( UnionType::from_elements(db, [first, second].into_iter().chain(def_types)), Boundedness::DefinitelyBound, ) } else { - SymbolLookupResult::Bound(first, Boundedness::DefinitelyBound) + SymbolLookupResult::Type(first, Boundedness::DefinitelyBound) } } else { SymbolLookupResult::Unbound @@ -968,7 +968,7 @@ impl<'db> Type<'db> { // where we know exactly which module we're dealing with. if name != "__getattr__" && global_lookup.may_be_unbound() { // TODO: this should use `.to_instance()`, but we don't understand instance attribute yet - if let SymbolLookupResult::Bound(module_type_instance_member, _) = + if let SymbolLookupResult::Type(module_type_instance_member, _) = KnownClass::ModuleType.to_class(db).member(db, name) { global_lookup.replace_unbound_with(db, module_type_instance_member) @@ -992,7 +992,7 @@ impl<'db> Type<'db> { if any_unbound { SymbolLookupResult::Unbound // TODO } else { - SymbolLookupResult::Bound( + SymbolLookupResult::Type( union.map(db, |ty| ty.member(db, name).expect_bound()), Boundedness::DefinitelyBound, ) @@ -1177,7 +1177,7 @@ impl<'db> Type<'db> { let iterable_meta_type = self.to_meta_type(db); let dunder_iter_method = iterable_meta_type.member(db, "__iter__"); - if let SymbolLookupResult::Bound(dunder_iter_method, _) = dunder_iter_method { + if let SymbolLookupResult::Type(dunder_iter_method, _) = dunder_iter_method { let Some(iterator_ty) = dunder_iter_method.call(db, &[self]).return_ty(db) else { return IterationOutcome::NotIterable { not_iterable_ty: self, @@ -1316,7 +1316,7 @@ impl<'db> From<&Type<'db>> for Type<'db> { impl<'db> From> for SymbolLookupResult<'db> { fn from(value: Type<'db>) -> Self { - SymbolLookupResult::Bound(value, Boundedness::DefinitelyBound) + SymbolLookupResult::Type(value, Boundedness::DefinitelyBound) } } diff --git a/crates/red_knot_python_semantic/src/types/infer.rs b/crates/red_knot_python_semantic/src/types/infer.rs index 862d0f3d94ff4..6452d052565df 100644 --- a/crates/red_knot_python_semantic/src/types/infer.rs +++ b/crates/red_knot_python_semantic/src/types/infer.rs @@ -1675,7 +1675,7 @@ impl<'db> TypeInferenceBuilder<'db> { asname: _, } = alias; - if let SymbolLookupResult::Bound(member_ty, _) = + if let SymbolLookupResult::Type(member_ty, _) = module_ty.member(self.db, &ast::name::Name::new(&name.id)) { // For possibly-unbound names, just eliminate Unbound from the type; we @@ -2357,7 +2357,7 @@ impl<'db> TypeInferenceBuilder<'db> { } match builtin_ty { - SymbolLookupResult::Bound(builtin_ty, _) => { + SymbolLookupResult::Type(builtin_ty, _) => { ty.replace_unbound_with(self.db, builtin_ty) } SymbolLookupResult::Unbound => ty, @@ -2405,8 +2405,8 @@ impl<'db> TypeInferenceBuilder<'db> { let bindings_ty = bindings_ty(self.db, definitions); if may_be_unbound { match self.lookup_name(name) { - SymbolLookupResult::Bound(ty, _) => match bindings_ty { - SymbolLookupResult::Bound(bindings_ty, _) => { + SymbolLookupResult::Type(ty, _) => match bindings_ty { + SymbolLookupResult::Type(bindings_ty, _) => { UnionType::from_elements(self.db, [bindings_ty, ty]) } SymbolLookupResult::Unbound => ty, @@ -2430,7 +2430,7 @@ impl<'db> TypeInferenceBuilder<'db> { } } else { match bindings_ty { - SymbolLookupResult::Bound(ty, _) => ty, + SymbolLookupResult::Type(ty, _) => ty, SymbolLookupResult::Unbound => { self.diagnostics.add( name.into(), @@ -2518,7 +2518,7 @@ impl<'db> TypeInferenceBuilder<'db> { } }; - if let SymbolLookupResult::Bound(class_member, _) = + if let SymbolLookupResult::Type(class_member, _) = class.class_member(self.db, unary_dunder_method) { let call = class_member.call(self.db, &[operand_type]); @@ -2775,7 +2775,7 @@ impl<'db> TypeInferenceBuilder<'db> { } } - let call_on_left_instance = if let SymbolLookupResult::Bound(class_member, _) = + let call_on_left_instance = if let SymbolLookupResult::Type(class_member, _) = left_class.class_member(self.db, op.dunder()) { class_member @@ -2789,7 +2789,7 @@ impl<'db> TypeInferenceBuilder<'db> { if left_class == right_class { None } else { - if let SymbolLookupResult::Bound(class_member, _) = + if let SymbolLookupResult::Type(class_member, _) = right_class.class_member(self.db, op.reflected_dunder()) { class_member From 39d6b89a21a8548566e9d42e1386e09d2e789dd7 Mon Sep 17 00:00:00 2001 From: David Peter Date: Wed, 30 Oct 2024 19:32:54 +0100 Subject: [PATCH 16/20] Fix member() on union types --- .../resources/mdtest/subscript/class.md | 4 +- crates/red_knot_python_semantic/src/types.rs | 87 +++++++++++-------- .../src/types/infer.rs | 11 +++ 3 files changed, 64 insertions(+), 38 deletions(-) diff --git a/crates/red_knot_python_semantic/resources/mdtest/subscript/class.md b/crates/red_knot_python_semantic/resources/mdtest/subscript/class.md index 5461960a7f5a6..dfd48802a6948 100644 --- a/crates/red_knot_python_semantic/resources/mdtest/subscript/class.md +++ b/crates/red_knot_python_semantic/resources/mdtest/subscript/class.md @@ -68,8 +68,8 @@ if flag: else: class Spam: ... -# error: [non-subscriptable] "Cannot subscript object of type `Literal[Spam, Spam]` with no `__class_getitem__` method" -# revealed: Unknown +# error: [call-potentially-unbound-method] "Method `__class_getitem__` of type `Literal[Spam, Spam]` is potentially unbound" +# revealed: str reveal_type(Spam[42]) ``` diff --git a/crates/red_knot_python_semantic/src/types.rs b/crates/red_knot_python_semantic/src/types.rs index 889ad084c7fe8..0a0c734f83e20 100644 --- a/crates/red_knot_python_semantic/src/types.rs +++ b/crates/red_knot_python_semantic/src/types.rs @@ -31,8 +31,8 @@ mod narrow; #[derive(Debug, Clone, Copy, PartialEq)] pub enum Boundedness { - DefinitelyBound, - MaybeUnbound, + Bound, + MayBeUnbound, } #[derive(Debug, Clone, PartialEq)] @@ -52,22 +52,23 @@ impl<'db> SymbolLookupResult<'db> { db: &'db dyn Db, replacement: Type<'db>, ) -> SymbolLookupResult<'db> { - match self { - r @ SymbolLookupResult::Type(_, Boundedness::DefinitelyBound) => r, - SymbolLookupResult::Type(ty, Boundedness::MaybeUnbound) => { - let union = UnionType::from_elements(db, [replacement, ty]); - SymbolLookupResult::Type(union, Boundedness::DefinitelyBound) - } - SymbolLookupResult::Unbound => { - SymbolLookupResult::Type(replacement, Boundedness::DefinitelyBound) - } - } + SymbolLookupResult::Type( + match self { + SymbolLookupResult::Type(ty, Boundedness::Bound) => ty, + SymbolLookupResult::Type(ty, Boundedness::MayBeUnbound) => { + UnionType::from_elements(db, [replacement, ty]) + } + SymbolLookupResult::Unbound => replacement, + }, + Boundedness::Bound, + ) } + #[cfg(test)] #[track_caller] fn expect_bound(self) -> Type<'db> { match self { - SymbolLookupResult::Type(ty, Boundedness::DefinitelyBound) => ty, + SymbolLookupResult::Type(ty, Boundedness::Bound) => ty, _ => { panic!("Expected a bound type") } @@ -87,9 +88,9 @@ impl<'db> SymbolLookupResult<'db> { fn may_be_unbound(&self) -> bool { match self { - SymbolLookupResult::Type(_, Boundedness::MaybeUnbound) + SymbolLookupResult::Type(_, Boundedness::MayBeUnbound) | SymbolLookupResult::Unbound => true, - SymbolLookupResult::Type(_, Boundedness::DefinitelyBound) => false, + SymbolLookupResult::Type(_, Boundedness::Bound) => false, } } @@ -98,7 +99,7 @@ impl<'db> SymbolLookupResult<'db> { SymbolLookupResult::Type(ty, boundedness) => SymbolLookupResult::Type( *ty, if yes { - Boundedness::MaybeUnbound + Boundedness::MayBeUnbound } else { *boundedness }, @@ -154,7 +155,7 @@ fn symbol_ty_by_id<'db>( ), None => SymbolLookupResult::Type( declarations_ty(db, declarations, None).unwrap_or_else(|(ty, _)| ty), - Boundedness::DefinitelyBound, + Boundedness::Bound, ), Some(SymbolLookupResult::Unbound) => SymbolLookupResult::Unbound, } @@ -318,10 +319,10 @@ where if let Some(second) = def_types.next() { SymbolLookupResult::Type( UnionType::from_elements(db, [first, second].into_iter().chain(def_types)), - Boundedness::DefinitelyBound, + Boundedness::Bound, ) } else { - SymbolLookupResult::Type(first, Boundedness::DefinitelyBound) + SymbolLookupResult::Type(first, Boundedness::Bound) } } else { SymbolLookupResult::Unbound @@ -914,14 +915,6 @@ impl<'db> Type<'db> { /// For example, if `foo` is `Type::Instance()`, /// `foo.member(&db, "baz")` returns the type of `baz` attributes /// as accessed from instances of the `Bar` class. - /// - /// TODO: use of this method currently requires manually checking - /// whether the returned type is `Unknown`/`Unbound` - /// (or a union with `Unknown`/`Unbound`) in many places. - /// Ideally we'd use a more type-safe pattern, such as returning - /// an `Option` or a `Result` from this method, which would force - /// us to explicitly consider whether to handle an error or propagate - /// it up the call stack. #[must_use] pub fn member(&self, db: &'db dyn Db, name: &str) -> SymbolLookupResult<'db> { match self { @@ -985,16 +978,38 @@ impl<'db> Type<'db> { Type::Todo.into() } Type::Union(union) => { - let any_unbound = union - .elements(db) - .iter() - .any(|ty| ty.member(db, name).is_unbound()); - if any_unbound { - SymbolLookupResult::Unbound // TODO + let mut builder = UnionBuilder::new(db); + + let mut all_unbound = true; + let mut may_be_unbound = false; + for ty in union.elements(db) { + let ty_member = ty.member(db, name); + match ty_member { + SymbolLookupResult::Unbound => { + may_be_unbound = true; + } + SymbolLookupResult::Type(ty_member, member_boundness) => { + // TODO: raise a diagnostic if member_boundness indicates potential unboundness + if member_boundness == Boundedness::MayBeUnbound { + may_be_unbound = true; + } + + all_unbound = false; + builder = builder.add(ty_member); + } + } + } + + if all_unbound { + SymbolLookupResult::Unbound } else { SymbolLookupResult::Type( - union.map(db, |ty| ty.member(db, name).expect_bound()), - Boundedness::DefinitelyBound, + builder.build(), + if may_be_unbound { + Boundedness::MayBeUnbound + } else { + Boundedness::Bound + }, ) } } @@ -1316,7 +1331,7 @@ impl<'db> From<&Type<'db>> for Type<'db> { impl<'db> From> for SymbolLookupResult<'db> { fn from(value: Type<'db>) -> Self { - SymbolLookupResult::Type(value, Boundedness::DefinitelyBound) + SymbolLookupResult::Type(value, Boundedness::Bound) } } diff --git a/crates/red_knot_python_semantic/src/types/infer.rs b/crates/red_knot_python_semantic/src/types/infer.rs index 6452d052565df..4428baacd6c9c 100644 --- a/crates/red_knot_python_semantic/src/types/infer.rs +++ b/crates/red_knot_python_semantic/src/types/infer.rs @@ -3416,6 +3416,17 @@ impl<'db> TypeInferenceBuilder<'db> { if value_ty.is_subtype_of(self.db, KnownClass::Type.to_instance(self.db)) { let dunder_class_getitem_method = value_ty.member(self.db, "__class_getitem__"); if !dunder_class_getitem_method.is_unbound() { + if dunder_class_getitem_method.may_be_unbound() { + self.add_diagnostic( + value_node.into(), + "call-potentially-unbound-method", + format_args!( + "Method `__class_getitem__` of type `{}` is potentially unbound", + value_ty.display(self.db), + ), + ); + } + return dunder_class_getitem_method .unwrap_or(Type::Never) .call(self.db, &[slice_ty]) From e3a7aa107c45a5c064d3bf8fc0abcb7fe6746966 Mon Sep 17 00:00:00 2001 From: David Peter Date: Wed, 30 Oct 2024 20:07:44 +0100 Subject: [PATCH 17/20] Get rid of most .unwrap_or(Type::Never) calls --- crates/red_knot_python_semantic/src/types.rs | 66 +++---- .../src/types/infer.rs | 162 ++++++++++-------- 2 files changed, 131 insertions(+), 97 deletions(-) diff --git a/crates/red_knot_python_semantic/src/types.rs b/crates/red_knot_python_semantic/src/types.rs index 0a0c734f83e20..80cc72b154a52 100644 --- a/crates/red_knot_python_semantic/src/types.rs +++ b/crates/red_knot_python_semantic/src/types.rs @@ -1133,14 +1133,14 @@ impl<'db> Type<'db> { Type::Instance(class) => { // Since `__call__` is a dunder, we need to access it as an attribute on the class // rather than the instance (matching runtime semantics). - let dunder_call_method = class.class_member(db, "__call__"); - if dunder_call_method.is_unbound() { - CallOutcome::not_callable(self) - } else { - let args = std::iter::once(self) - .chain(arg_types.iter().copied()) - .collect::>(); - dunder_call_method.unwrap_or(Type::Never).call(db, &args) + match class.class_member(db, "__call__") { + SymbolLookupResult::Type(dunder_call_method, Boundedness::Bound) => { + let args = std::iter::once(self) + .chain(arg_types.iter().copied()) + .collect::>(); + dunder_call_method.call(db, &args) + } + _ => CallOutcome::not_callable(self), } } @@ -1199,17 +1199,22 @@ impl<'db> Type<'db> { }; }; - let dunder_next_method = iterator_ty - .to_meta_type(db) - .member(db, "__next__") - .unwrap_or(Type::Never); - return dunder_next_method - .call(db, &[iterator_ty]) - .return_ty(db) - .map(|element_ty| IterationOutcome::Iterable { element_ty }) - .unwrap_or(IterationOutcome::NotIterable { - not_iterable_ty: self, - }); + match iterator_ty.to_meta_type(db).member(db, "__next__") { + SymbolLookupResult::Type(dunder_next_method, Boundedness::Bound) => { + return dunder_next_method + .call(db, &[iterator_ty]) + .return_ty(db) + .map(|element_ty| IterationOutcome::Iterable { element_ty }) + .unwrap_or(IterationOutcome::NotIterable { + not_iterable_ty: self, + }); + } + _ => { + return IterationOutcome::NotIterable { + not_iterable_ty: self, + }; + } + } } // Although it's not considered great practice, @@ -1218,17 +1223,20 @@ impl<'db> Type<'db> { // // TODO(Alex) this is only valid if the `__getitem__` method is annotated as // accepting `int` or `SupportsIndex` - let dunder_get_item_method = iterable_meta_type - .member(db, "__getitem__") - .unwrap_or(Type::Never); - - dunder_get_item_method - .call(db, &[self, KnownClass::Int.to_instance(db)]) - .return_ty(db) - .map(|element_ty| IterationOutcome::Iterable { element_ty }) - .unwrap_or(IterationOutcome::NotIterable { + match iterable_meta_type.member(db, "__getitem__") { + SymbolLookupResult::Type(dunder_get_item_method, Boundedness::Bound) => { + dunder_get_item_method + .call(db, &[self, KnownClass::Int.to_instance(db)]) + .return_ty(db) + .map(|element_ty| IterationOutcome::Iterable { element_ty }) + .unwrap_or(IterationOutcome::NotIterable { + not_iterable_ty: self, + }) + } + _ => IterationOutcome::NotIterable { not_iterable_ty: self, - }) + }, + } } #[must_use] diff --git a/crates/red_knot_python_semantic/src/types/infer.rs b/crates/red_knot_python_semantic/src/types/infer.rs index 4428baacd6c9c..1acd28c6a9c5e 100644 --- a/crates/red_knot_python_semantic/src/types/infer.rs +++ b/crates/red_knot_python_semantic/src/types/infer.rs @@ -61,6 +61,8 @@ use crate::types::{ use crate::util::subscript::{PyIndex, PySlice}; use crate::Db; +use super::Boundedness; + /// Infer all types for a [`ScopeId`], including all definitions and expressions in that scope. /// Use when checking a scope, or needing to provide a type for an arbitrary expression in the /// scope. @@ -1410,30 +1412,35 @@ impl<'db> TypeInferenceBuilder<'db> { // If the target defines, e.g., `__iadd__`, infer the augmented assignment as a call to that // dunder. if let Type::Instance(class) = target_type { - let class_member = class.class_member(self.db, op.in_place_dunder()); - if !class_member.is_unbound() { - let call = class_member - .unwrap_or(Type::Never) - .call(self.db, &[target_type, value_type]); - return match call.return_ty_result( - self.db, - AnyNodeRef::StmtAugAssign(assignment), - &mut self.diagnostics, - ) { - Ok(t) => t, - Err(e) => { - self.diagnostics.add( - assignment.into(), - "unsupported-operator", - format_args!( - "Operator `{op}=` is unsupported between objects of type `{}` and `{}`", - target_type.display(self.db), - value_type.display(self.db) - ), - ); - e.return_ty() + match class.class_member(self.db, op.in_place_dunder()) { + SymbolLookupResult::Unbound => {} + SymbolLookupResult::Type(class_member, boundedness) => { + if boundedness == Boundedness::MayBeUnbound { + // TODO } - }; + + let call = class_member.call(self.db, &[target_type, value_type]); + + return match call.return_ty_result( + self.db, + AnyNodeRef::StmtAugAssign(assignment), + &mut self.diagnostics, + ) { + Ok(t) => t, + Err(e) => { + self.diagnostics.add( + assignment.into(), + "unsupported-operator", + format_args!( + "Operator `{op}=` is unsupported between objects of type `{}` and `{}`", + target_type.display(self.db), + value_type.display(self.db) + ), + ); + e.return_ty() + } + }; + } } } @@ -3384,10 +3391,21 @@ impl<'db> TypeInferenceBuilder<'db> { // If the class defines `__getitem__`, return its return type. // // See: https://docs.python.org/3/reference/datamodel.html#class-getitem-versus-getitem - let dunder_getitem_method = value_meta_ty.member(self.db, "__getitem__"); - if !dunder_getitem_method.is_unbound() { - return dunder_getitem_method - .unwrap_or(Type::Never) + match value_meta_ty.member(self.db, "__getitem__") { + SymbolLookupResult::Unbound => {} + SymbolLookupResult::Type(dunder_getitem_method, boundedness) => { + if boundedness == Boundedness::MayBeUnbound { + self.diagnostics.add( + value_node.into(), + "call-potentially-unbound-method", + format_args!( + "Method `__getitem__` of type `{}` is potentially unbound", + value_ty.display(self.db), + ), + ); + } + + return dunder_getitem_method .call(self.db, &[slice_ty]) .return_ty_result(self.db, value_node.into(), &mut self.diagnostics) .unwrap_or_else(|err| { @@ -3402,6 +3420,7 @@ impl<'db> TypeInferenceBuilder<'db> { ); err.return_ty() }); + } } // Otherwise, if the value is itself a class and defines `__class_getitem__`, @@ -3415,34 +3434,37 @@ impl<'db> TypeInferenceBuilder<'db> { // method in these `sys.version_info` branches. if value_ty.is_subtype_of(self.db, KnownClass::Type.to_instance(self.db)) { let dunder_class_getitem_method = value_ty.member(self.db, "__class_getitem__"); - if !dunder_class_getitem_method.is_unbound() { - if dunder_class_getitem_method.may_be_unbound() { - self.add_diagnostic( - value_node.into(), - "call-potentially-unbound-method", - format_args!( - "Method `__class_getitem__` of type `{}` is potentially unbound", - value_ty.display(self.db), - ), - ); - } - return dunder_class_getitem_method - .unwrap_or(Type::Never) - .call(self.db, &[slice_ty]) - .return_ty_result(self.db, value_node.into(), &mut self.diagnostics) - .unwrap_or_else(|err| { + match dunder_class_getitem_method { + SymbolLookupResult::Unbound => {} + SymbolLookupResult::Type(ty, boundedness) => { + if boundedness == Boundedness::MayBeUnbound { self.diagnostics.add( value_node.into(), - "call-non-callable", + "call-potentially-unbound-method", format_args!( - "Method `__class_getitem__` of type `{}` is not callable on object of type `{}`", - err.called_ty().display(self.db), + "Method `__class_getitem__` of type `{}` is potentially unbound", value_ty.display(self.db), ), ); - err.return_ty() - }); + } + + return ty + .call(self.db, &[slice_ty]) + .return_ty_result(self.db, value_node.into(), &mut self.diagnostics) + .unwrap_or_else(|err| { + self.diagnostics.add( + value_node.into(), + "call-non-callable", + format_args!( + "Method `__class_getitem__` of type `{}` is not callable on object of type `{}`", + err.called_ty().display(self.db), + value_ty.display(self.db), + ), + ); + err.return_ty() + }); + } } if matches!(value_ty, Type::ClassLiteral(class) if class.is_known(self.db, KnownClass::Type)) @@ -3965,14 +3987,17 @@ fn perform_rich_comparison<'db>( let call_dunder = |op: RichCompareOperator, left_class: ClassType<'db>, right_class: ClassType<'db>| { - left_class - .class_member(db, op.dunder()) - .unwrap_or(Type::Never) - .call( - db, - &[Type::Instance(left_class), Type::Instance(right_class)], - ) - .return_ty(db) + match left_class.class_member(db, op.dunder()) { + SymbolLookupResult::Type(class_member_dunder, Boundedness::Bound) => { + class_member_dunder + .call( + db, + &[Type::Instance(left_class), Type::Instance(right_class)], + ) + .return_ty(db) + } + _ => None, + } }; // The reflected dunder has priority if the right-hand side is a strict subclass of the left-hand side. @@ -4013,19 +4038,20 @@ fn perform_membership_test_comparison<'db>( let (left_instance, right_instance) = (Type::Instance(left_class), Type::Instance(right_class)); let contains_dunder = right_class.class_member(db, "__contains__"); - - let compare_result_opt = if contains_dunder.is_unbound() { - // iteration-based membership test - match right_instance.iterate(db) { - IterationOutcome::Iterable { .. } => Some(KnownClass::Bool.to_instance(db)), - IterationOutcome::NotIterable { .. } => None, + let compare_result_opt = match contains_dunder { + SymbolLookupResult::Type(contains_dunder, Boundedness::Bound) => { + // If `__contains__` is available, it is used directly for the membership test. + contains_dunder + .call(db, &[right_instance, left_instance]) + .return_ty(db) + } + _ => { + // iteration-based membership test + match right_instance.iterate(db) { + IterationOutcome::Iterable { .. } => Some(KnownClass::Bool.to_instance(db)), + IterationOutcome::NotIterable { .. } => None, + } } - } else { - // If `__contains__` is available, it is used directly for the membership test. - contains_dunder - .unwrap_or(Type::Never) - .call(db, &[right_instance, left_instance]) - .return_ty(db) }; compare_result_opt From 54998b3cefad66651b5bda63a55656180c61ef3d Mon Sep 17 00:00:00 2001 From: David Peter Date: Wed, 30 Oct 2024 20:28:59 +0100 Subject: [PATCH 18/20] Remove #[ignore] --- crates/red_knot_python_semantic/src/types/infer.rs | 2 -- 1 file changed, 2 deletions(-) diff --git a/crates/red_knot_python_semantic/src/types/infer.rs b/crates/red_knot_python_semantic/src/types/infer.rs index 1acd28c6a9c5e..ae24d65fb7bbc 100644 --- a/crates/red_knot_python_semantic/src/types/infer.rs +++ b/crates/red_knot_python_semantic/src/types/infer.rs @@ -4919,7 +4919,6 @@ mod tests { } #[test] - #[ignore] fn comprehension_with_not_iterable_iter_in_second_comprehension() -> anyhow::Result<()> { let mut db = setup_db(); @@ -5121,7 +5120,6 @@ mod tests { } #[test] - #[ignore] fn starred_expressions_must_be_iterable() { let mut db = setup_db(); From a9b804569d87fd8e2c07169db1bf532677081930 Mon Sep 17 00:00:00 2001 From: David Peter Date: Wed, 30 Oct 2024 22:33:43 +0100 Subject: [PATCH 19/20] Add diagnostic for potentially unbound class member calls --- .../resources/mdtest/assignment/annotations.md | 2 ++ .../resources/mdtest/assignment/augmented.md | 3 ++- .../resources/mdtest/exception/basic.md | 7 ++++++- crates/red_knot_python_semantic/src/types.rs | 2 +- crates/red_knot_python_semantic/src/types/infer.rs | 10 +++++++++- 5 files changed, 20 insertions(+), 4 deletions(-) diff --git a/crates/red_knot_python_semantic/resources/mdtest/assignment/annotations.md b/crates/red_knot_python_semantic/resources/mdtest/assignment/annotations.md index 30ea843523f0b..56452870dd98e 100644 --- a/crates/red_knot_python_semantic/resources/mdtest/assignment/annotations.md +++ b/crates/red_knot_python_semantic/resources/mdtest/assignment/annotations.md @@ -33,6 +33,8 @@ b: tuple[int] = (42,) c: tuple[str, int] = ("42", 42) d: tuple[tuple[str, str], tuple[int, int]] = (("foo", "foo"), (42, 42)) e: tuple[str, ...] = () +# TODO: we should not emit this error +# error: [call-potentially-unbound-method] "Method `__class_getitem__` of type `Literal[tuple]` is potentially unbound" f: tuple[str, *tuple[int, ...], bytes] = ("42", b"42") g: tuple[str, Unpack[tuple[int, ...]], bytes] = ("42", b"42") h: tuple[list[int], list[int]] = ([], []) diff --git a/crates/red_knot_python_semantic/resources/mdtest/assignment/augmented.md b/crates/red_knot_python_semantic/resources/mdtest/assignment/augmented.md index 1f53b75714a07..506e100c30833 100644 --- a/crates/red_knot_python_semantic/resources/mdtest/assignment/augmented.md +++ b/crates/red_knot_python_semantic/resources/mdtest/assignment/augmented.md @@ -80,9 +80,9 @@ class Foo: return 42 f = Foo() +# error: [call-potentially-unbound-method] "Call to potentially unbound method `__iadd__` on object of type `Foo`" f += "Hello, world!" -# TODO should emit a diagnostic warning that `Foo` might not have an `__iadd__` method reveal_type(f) # revealed: int ``` @@ -100,6 +100,7 @@ class Foo: return 42 f = Foo() +# error: [call-potentially-unbound-method] "Call to potentially unbound method `__iadd__` on object of type `Foo`" f += "Hello, world!" # TODO(charlie): This should be `int | str`, since `__iadd__` may be unbound. diff --git a/crates/red_knot_python_semantic/resources/mdtest/exception/basic.md b/crates/red_knot_python_semantic/resources/mdtest/exception/basic.md index 6b67e6a22178e..65159027f4e45 100644 --- a/crates/red_knot_python_semantic/resources/mdtest/exception/basic.md +++ b/crates/red_knot_python_semantic/resources/mdtest/exception/basic.md @@ -41,7 +41,12 @@ except EXCEPTIONS as f: ## Dynamic exception types ```py -def foo(x: type[AttributeError], y: tuple[type[OSError], type[RuntimeError]], z: tuple[type[BaseException], ...]): +# TODO: we should not emit those errors `call-potentially-unbound-method` errors +def foo( + x: type[AttributeError], + y: tuple[type[OSError], type[RuntimeError]], # error: [call-potentially-unbound-method] + z: tuple[type[BaseException], ...], # error: [call-potentially-unbound-method] +): try: help() except x as e: diff --git a/crates/red_knot_python_semantic/src/types.rs b/crates/red_knot_python_semantic/src/types.rs index 80cc72b154a52..f41e68cc6dab5 100644 --- a/crates/red_knot_python_semantic/src/types.rs +++ b/crates/red_knot_python_semantic/src/types.rs @@ -1946,7 +1946,7 @@ impl<'db> ClassType<'db> { let member = self.own_class_member(db, name); if !member.is_unbound() { // TODO diagnostic if maybe unbound? - return member.replace_unbound_with(db, Type::Never); + return member; } self.inherited_class_member(db, name) diff --git a/crates/red_knot_python_semantic/src/types/infer.rs b/crates/red_knot_python_semantic/src/types/infer.rs index ae24d65fb7bbc..13b42c7c99277 100644 --- a/crates/red_knot_python_semantic/src/types/infer.rs +++ b/crates/red_knot_python_semantic/src/types/infer.rs @@ -1416,7 +1416,15 @@ impl<'db> TypeInferenceBuilder<'db> { SymbolLookupResult::Unbound => {} SymbolLookupResult::Type(class_member, boundedness) => { if boundedness == Boundedness::MayBeUnbound { - // TODO + self.diagnostics.add( + assignment.into(), + "call-potentially-unbound-method", + format_args!( + "Call to potentially unbound method `{}` on object of type `{}`", + op.in_place_dunder(), + target_type.display(self.db) + ), + ); } let call = class_member.call(self.db, &[target_type, value_type]); From 578758516a53c0427c4228aab70d983ca4e1872a Mon Sep 17 00:00:00 2001 From: David Peter Date: Wed, 30 Oct 2024 22:48:21 +0100 Subject: [PATCH 20/20] Update diagnostics in benchmark --- crates/ruff_benchmark/benches/red_knot.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/crates/ruff_benchmark/benches/red_knot.rs b/crates/ruff_benchmark/benches/red_knot.rs index f604acd0a8f1f..408add7605815 100644 --- a/crates/ruff_benchmark/benches/red_knot.rs +++ b/crates/ruff_benchmark/benches/red_knot.rs @@ -26,6 +26,7 @@ static EXPECTED_DIAGNOSTICS: &[&str] = &[ // We don't support `*` imports yet: "/src/tomllib/_parser.py:7:29: Module `collections.abc` has no member `Iterable`", // We don't support terminal statements in control flow yet: + "/src/tomllib/_parser.py:246:15: Method `__class_getitem__` of type `Literal[frozenset]` is potentially unbound", "/src/tomllib/_parser.py:66:18: Name `s` used when possibly not defined", "/src/tomllib/_parser.py:98:12: Name `char` used when possibly not defined", "/src/tomllib/_parser.py:101:12: Name `char` used when possibly not defined",