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 b875f677ff..8005c6aee4 100644 --- a/crates/red_knot_python_semantic/resources/mdtest/assignment/unbound.md +++ b/crates/red_knot_python_semantic/resources/mdtest/assignment/unbound.md @@ -1,24 +1,16 @@ # Unbound -## Maybe unbound - -```py -if flag: - y = 3 -x = y -reveal_type(x) # revealed: Unbound | Literal[3] -``` - ## Unbound ```py -x = foo; foo = 1 +x = foo +foo = 1 reveal_type(x) # revealed: Unbound ``` ## Unbound class variable -Class variables can reference global variables unless overridden within the class scope. +Name lookups within a class scope fall back to globals, but lookups of class attributes don't. ```py x = 1 @@ -27,6 +19,6 @@ class C: if flag: x = 2 -reveal_type(C.x) # revealed: Unbound | Literal[2] +reveal_type(C.x) # revealed: Literal[2] reveal_type(C.y) # 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 6edf0fb855..fbfa4e7acd 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 @@ -28,6 +28,7 @@ else: y = 5 s = y x = y + reveal_type(x) # revealed: Literal[3, 4, 5] reveal_type(r) # revealed: Unbound | Literal[2] reveal_type(s) # revealed: Unbound | Literal[5] diff --git a/crates/red_knot_python_semantic/resources/mdtest/exception/except_star.md b/crates/red_knot_python_semantic/resources/mdtest/exception/except_star.md index e9b3dfbca4..a733c79e8a 100644 --- a/crates/red_knot_python_semantic/resources/mdtest/exception/except_star.md +++ b/crates/red_knot_python_semantic/resources/mdtest/exception/except_star.md @@ -1,14 +1,12 @@ # Except star -TODO(Alex): Once we support `sys.version_info` branches, we can set `--target-version=py311` in these tests and the inferred type will just be `BaseExceptionGroup` - ## Except\* with BaseException ```py try: x except* BaseException as e: - reveal_type(e) # revealed: Unknown | BaseExceptionGroup + reveal_type(e) # revealed: BaseExceptionGroup ``` ## Except\* with specific exception @@ -18,7 +16,7 @@ try: x except* OSError as e: # TODO(Alex): more precise would be `ExceptionGroup[OSError]` - reveal_type(e) # revealed: Unknown | BaseExceptionGroup + reveal_type(e) # revealed: BaseExceptionGroup ``` ## Except\* with multiple exceptions @@ -28,5 +26,5 @@ try: x except* (TypeError, AttributeError) as e: #TODO(Alex): more precise would be `ExceptionGroup[TypeError | AttributeError]`. - reveal_type(e) # revealed: Unknown | BaseExceptionGroup + reveal_type(e) # revealed: BaseExceptionGroup ``` 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 3ceb12c32a..199d4c1302 100644 --- a/crates/red_knot_python_semantic/resources/mdtest/import/conditional.md +++ b/crates/red_knot_python_semantic/resources/mdtest/import/conditional.md @@ -1,5 +1,39 @@ # Conditional imports +## Maybe unbound + +```py path=maybe_unbound.py +if flag: + y = 3 +x = y +reveal_type(x) # revealed: Unbound | Literal[3] +reveal_type(y) # revealed: Unbound | Literal[3] +``` + +```py +from maybe_unbound import x, y +reveal_type(x) # revealed: Literal[3] +reveal_type(y) # revealed: Literal[3] +``` + +## Maybe unbound annotated + +```py path=maybe_unbound_annotated.py +if flag: + y: int = 3 +x = y +reveal_type(x) # revealed: Unbound | Literal[3] +reveal_type(y) # revealed: Unbound | Literal[3] +``` + +Importing an annotated name prefers the declared type over the inferred type: + +```py +from maybe_unbound_annotated import x, y +reveal_type(x) # revealed: Literal[3] +reveal_type(y) # revealed: int +``` + ## Reimport ```py path=c.py @@ -14,8 +48,7 @@ else: ``` ```py -# TODO we should not emit this error -from b import f # error: [invalid-assignment] "Object of type `Literal[f, f]` is not assignable to `Literal[f, f]`" +from b import f # TODO: We should disambiguate in such cases, showing `Literal[b.f, c.f]`. reveal_type(f) # revealed: Literal[f, f] ``` diff --git a/crates/red_knot_python_semantic/resources/mdtest/import/errors.md b/crates/red_knot_python_semantic/resources/mdtest/import/errors.md index 9c3b8482c9..fac4acc60b 100644 --- a/crates/red_knot_python_semantic/resources/mdtest/import/errors.md +++ b/crates/red_knot_python_semantic/resources/mdtest/import/errors.md @@ -4,12 +4,14 @@ ```py import bar # error: "Cannot resolve import `bar`" +reveal_type(bar) # revealed: Unknown ``` ## Unresolved import from statement ```py from bar import baz # error: "Cannot resolve import `bar`" +reveal_type(baz) # revealed: Unknown ``` ## Unresolved import from resolved module @@ -19,12 +21,14 @@ from bar import baz # error: "Cannot resolve import `bar`" ```py from a import thing # error: "Module `a` has no member `thing`" +reveal_type(thing) # revealed: Unknown ``` ## Resolved import of symbol from unresolved import ```py path=a.py import foo as foo # error: "Cannot resolve import `foo`" +reveal_type(foo) # revealed: Unknown ``` Importing the unresolved import into a second file should not trigger an additional "unresolved @@ -32,9 +36,10 @@ import" violation: ```py from a import foo +reveal_type(foo) # revealed: Unknown ``` -## No implicit shadowing error +## No implicit shadowing ```py path=b.py x: int @@ -43,5 +48,5 @@ x: int ```py from b import x -x = 'foo' # error: "Object of type `Literal["foo"]" +x = 'foo' # error: [invalid-assignment] "Object of type `Literal["foo"]" ``` diff --git a/crates/red_knot_python_semantic/resources/mdtest/import/relative.md b/crates/red_knot_python_semantic/resources/mdtest/import/relative.md index c7d618cfca..fd1c4453db 100644 --- a/crates/red_knot_python_semantic/resources/mdtest/import/relative.md +++ b/crates/red_knot_python_semantic/resources/mdtest/import/relative.md @@ -126,7 +126,7 @@ reveal_type(y) # revealed: Unknown ``` ```py path=package/bar.py -# TODO: submodule imports possibly not supported right now? +# TODO: support submodule imports from . import foo # error: [unresolved-import] reveal_type(foo) # revealed: Unknown diff --git a/crates/red_knot_python_semantic/resources/mdtest/scopes/builtin.md b/crates/red_knot_python_semantic/resources/mdtest/scopes/builtin.md new file mode 100644 index 0000000000..7be6c7531b --- /dev/null +++ b/crates/red_knot_python_semantic/resources/mdtest/scopes/builtin.md @@ -0,0 +1,32 @@ +# Builtin scope + +## Conditionally global or builtin + +If a builtin name is conditionally defined as a global, a name lookup should union the builtin type +with the conditionally-defined type: + +```py +def returns_bool() -> bool: + return True + +if returns_bool(): + copyright = 1 + +def f(): + reveal_type(copyright) # revealed: Literal[copyright] | Literal[1] +``` + +## Conditionally global or builtin, with annotation + +Same is true if the name is annotated: + +```py +def returns_bool() -> bool: + return True + +if returns_bool(): + copyright: int = 1 + +def f(): + reveal_type(copyright) # revealed: Literal[copyright] | int +``` diff --git a/crates/red_knot_python_semantic/src/stdlib.rs b/crates/red_knot_python_semantic/src/stdlib.rs index 08de7bb968..4ba9d17a12 100644 --- a/crates/red_knot_python_semantic/src/stdlib.rs +++ b/crates/red_knot_python_semantic/src/stdlib.rs @@ -37,6 +37,13 @@ fn core_module_symbol_ty<'db>( ) -> Type<'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) } diff --git a/crates/red_knot_python_semantic/src/types.rs b/crates/red_knot_python_semantic/src/types.rs index bbcc055825..cf787e0462 100644 --- a/crates/red_knot_python_semantic/src/types.rs +++ b/crates/red_knot_python_semantic/src/types.rs @@ -60,7 +60,7 @@ fn symbol_ty_by_id<'db>(db: &'db dyn Db, scope: ScopeId<'db>, symbol: ScopedSymb use_def.public_bindings(symbol), use_def .public_may_be_unbound(symbol) - .then_some(Type::Unknown), + .then_some(Type::Unbound), )) } else { None @@ -79,7 +79,7 @@ fn symbol_ty_by_id<'db>(db: &'db dyn Db, scope: ScopeId<'db>, symbol: ScopedSymb } } -/// Shorthand for `symbol_ty` that takes a symbol name instead of an ID. +/// 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> { let table = symbol_table(db, scope); table @@ -381,7 +381,7 @@ impl<'db> Type<'db> { Type::Union(union) => { union.map(db, |element| element.replace_unbound_with(db, replacement)) } - ty => *ty, + _ => *self, } } @@ -444,6 +444,9 @@ impl<'db> Type<'db> { /// /// [assignable to]: https://typing.readthedocs.io/en/latest/spec/concepts.html#the-assignable-to-or-consistent-subtyping-relation pub(crate) fn is_assignable_to(self, db: &'db dyn Db, target: Type<'db>) -> bool { + if self.is_equivalent_to(db, target) { + return true; + } match (self, target) { (Type::Unknown | Type::Any | Type::Todo, _) => true, (_, Type::Unknown | Type::Any | Type::Todo) => true, @@ -1426,7 +1429,8 @@ impl<'db> ClassType<'db> { pub fn class_member(self, db: &'db dyn Db, name: &str) -> Type<'db> { let member = self.own_class_member(db, name); if !member.is_unbound() { - return member; + // TODO diagnostic if maybe unbound? + return member.replace_unbound_with(db, Type::Never); } self.inherited_class_member(db, name) @@ -1625,6 +1629,7 @@ mod tests { #[test_case(Ty::BytesLiteral("foo"), Ty::BuiltinInstance("bytes"))] #[test_case(Ty::IntLiteral(1), Ty::Union(vec![Ty::BuiltinInstance("int"), Ty::BuiltinInstance("str")]))] #[test_case(Ty::IntLiteral(1), Ty::Union(vec![Ty::Unknown, Ty::BuiltinInstance("str")]))] + #[test_case(Ty::Union(vec![Ty::IntLiteral(1), Ty::IntLiteral(2)]), Ty::Union(vec![Ty::IntLiteral(1), Ty::IntLiteral(2)]))] fn is_assignable_to(from: Ty, to: Ty) { let db = setup_db(); assert!(from.into_type(&db).is_assignable_to(&db, to.into_type(&db))); diff --git a/crates/red_knot_python_semantic/src/types/infer.rs b/crates/red_knot_python_semantic/src/types/infer.rs index 5f72c71684..c6a6cf4907 100644 --- a/crates/red_knot_python_semantic/src/types/infer.rs +++ b/crates/red_knot_python_semantic/src/types/infer.rs @@ -1699,10 +1699,30 @@ impl<'db> TypeInferenceBuilder<'db> { .ok_or(ModuleNameResolutionError::InvalidSyntax) }; - let module_ty = match module_name { - Ok(name) => { - if let Some(ty) = self.module_ty_from_name(&name) { - ty + let ty = match module_name { + Ok(module_name) => { + if let Some(module_ty) = self.module_ty_from_name(&module_name) { + let ast::Alias { + range: _, + name, + asname: _, + } = alias; + + let member_ty = module_ty.member(self.db, &ast::name::Name::new(&name.id)); + + if member_ty.is_unbound() { + self.add_diagnostic( + AnyNodeRef::Alias(alias), + "unresolved-import", + format_args!("Module `{module_name}` has no member `{name}`",), + ); + + 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.unresolved_module_diagnostic(import_from, *level, module); Type::Unknown @@ -1732,34 +1752,6 @@ impl<'db> TypeInferenceBuilder<'db> { } }; - let ast::Alias { - range: _, - name, - asname: _, - } = alias; - - let member_ty = module_ty.member(self.db, &ast::name::Name::new(&name.id)); - - // TODO: What if it's a union where one of the elements is `Unbound`? - if member_ty.is_unbound() { - self.add_diagnostic( - AnyNodeRef::Alias(alias), - "unresolved-import", - format_args!( - "Module `{}{}` has no member `{name}`", - ".".repeat(*level as usize), - module.unwrap_or_default() - ), - ); - } - - // If a symbol is unbound in the module the symbol was originally defined in, - // when we're trying to import the symbol from that module into "our" module, - // the runtime error will occur immediately (rather than when the symbol is *used*, - // as would be the case for a symbol with type `Unbound`), so it's appropriate to - // think of the type of the imported symbol as `Unknown` rather than `Unbound` - let ty = member_ty.replace_unbound_with(self.db, Type::Unknown); - self.add_declaration_with_binding(alias.into(), definition, ty, ty); } @@ -2368,6 +2360,7 @@ impl<'db> TypeInferenceBuilder<'db> { return symbol_ty(self.db, enclosing_scope_id, name); } } + // No nonlocal binding, check module globals. Avoid infinite recursion if `self.scope` // already is module globals. let ty = if file_scope_id.is_global() { @@ -2375,6 +2368,7 @@ impl<'db> TypeInferenceBuilder<'db> { } 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) { let mut builtin_ty = builtins_symbol_ty(self.db, name); @@ -3877,7 +3871,7 @@ mod tests { )?; // TODO: sys.version_info, and need to understand @final and @type_check_only - assert_public_ty(&db, "src/a.py", "x", "Unknown | EllipsisType"); + assert_public_ty(&db, "src/a.py", "x", "EllipsisType | Unknown"); Ok(()) } @@ -3994,38 +3988,6 @@ mod tests { Ok(()) } - #[test] - fn conditionally_global_or_builtin() -> anyhow::Result<()> { - let mut db = setup_db(); - - db.write_dedented( - "/src/a.py", - " - if flag: - copyright = 1 - def f(): - y = copyright - ", - )?; - - let file = system_path_to_file(&db, "src/a.py").expect("file to exist"); - let index = semantic_index(&db, file); - let function_scope = index - .child_scopes(FileScopeId::global()) - .next() - .unwrap() - .0 - .to_scope_id(&db, file); - let y_ty = symbol_ty(&db, function_scope, "y"); - - assert_eq!( - y_ty.display(&db).to_string(), - "Literal[copyright] | Literal[1]" - ); - - Ok(()) - } - #[test] fn local_inference() -> anyhow::Result<()> { let mut db = setup_db();