diff --git a/crates/red_knot_python_semantic/src/types.rs b/crates/red_knot_python_semantic/src/types.rs index e6f739df3a009..b59d7a7f2513d 100644 --- a/crates/red_knot_python_semantic/src/types.rs +++ b/crates/red_knot_python_semantic/src/types.rs @@ -113,7 +113,7 @@ pub enum Type<'db> { Any, /// the empty set of values Never, - /// unknown type (no annotation) + /// 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 @@ -145,6 +145,10 @@ impl<'db> Type<'db> { matches!(self, Type::Unbound) } + pub const fn is_unknown(&self) -> bool { + matches!(self, Type::Unknown) + } + pub const fn is_never(&self) -> bool { matches!(self, Type::Never) } diff --git a/crates/red_knot_python_semantic/src/types/infer.rs b/crates/red_knot_python_semantic/src/types/infer.rs index 6f494f9c6bf96..8156dc6e73e71 100644 --- a/crates/red_knot_python_semantic/src/types/infer.rs +++ b/crates/red_knot_python_semantic/src/types/infer.rs @@ -930,7 +930,14 @@ impl<'db> TypeInferenceBuilder<'db> { asname: _, } = alias; - let ty = module_ty.member(self.db, &Name::new(&name.id)); + // 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 = module_ty + .member(self.db, &Name::new(&name.id)) + .replace_unbound_with(self.db, Type::Unknown); self.types.definitions.insert(definition, ty); } @@ -949,7 +956,7 @@ impl<'db> TypeInferenceBuilder<'db> { fn module_ty_from_name(&self, module_name: Option) -> Type<'db> { module_name .and_then(|module_name| resolve_module(self.db, module_name)) - .map_or(Type::Unbound, |module| Type::Module(module.file())) + .map_or(Type::Unknown, |module| Type::Module(module.file())) } fn infer_decorator(&mut self, decorator: &ast::Decorator) -> Type<'db> { @@ -1783,7 +1790,7 @@ mod tests { ("src/package/bar.py", "from .foo import X"), ])?; - assert_public_ty(&db, "src/package/bar.py", "X", "Unbound"); + assert_public_ty(&db, "src/package/bar.py", "X", "Unknown"); Ok(()) } @@ -1821,7 +1828,7 @@ mod tests { fn follow_nonexistent_relative_import_bare_to_package() -> anyhow::Result<()> { let mut db = setup_db(); db.write_files([("src/package/bar.py", "from . import X")])?; - assert_public_ty(&db, "src/package/bar.py", "X", "Unbound"); + assert_public_ty(&db, "src/package/bar.py", "X", "Unknown"); Ok(()) } @@ -1851,7 +1858,7 @@ mod tests { ("src/package/bar.py", "from . import foo"), ])?; - assert_public_ty(&db, "src/package/bar.py", "foo", "Unbound"); + assert_public_ty(&db, "src/package/bar.py", "foo", "Unknown"); Ok(()) } @@ -1874,7 +1881,7 @@ mod tests { fn follow_nonexistent_relative_import_from_dunder_init() -> anyhow::Result<()> { let mut db = setup_db(); db.write_files([("src/package/__init__.py", "from .foo import X")])?; - assert_public_ty(&db, "src/package/__init__.py", "X", "Unbound"); + assert_public_ty(&db, "src/package/__init__.py", "X", "Unknown"); Ok(()) } @@ -1901,6 +1908,24 @@ mod tests { Ok(()) } + #[test] + fn imported_unbound_symbol_is_unknown() -> anyhow::Result<()> { + let mut db = setup_db(); + + db.write_files([ + ("src/package/__init__.py", ""), + ("src/package/foo.py", "x"), + ("src/package/bar.py", "from package.foo import x"), + ])?; + + // the type as seen from external modules (`Unknown`) + // is different from the type inside the module itself (`Unbound`): + assert_public_ty(&db, "src/package/foo.py", "x", "Unbound"); + assert_public_ty(&db, "src/package/bar.py", "x", "Unknown"); + + Ok(()) + } + #[test] fn resolve_base_class_by_name() -> anyhow::Result<()> { let mut db = setup_db(); diff --git a/crates/red_knot_workspace/src/lint.rs b/crates/red_knot_workspace/src/lint.rs index ba8b3e5b19be1..79ae41ecd3a9c 100644 --- a/crates/red_knot_workspace/src/lint.rs +++ b/crates/red_knot_workspace/src/lint.rs @@ -124,12 +124,16 @@ fn format_diagnostic(context: &SemanticLintContext, message: &str, start: TextSi } fn lint_unresolved_imports(context: &SemanticLintContext, import: AnyImportRef) { + // TODO: this treats any symbol with `Type::Unknown` as an unresolved import, + // which isn't really correct: if it exists but has `Type::Unknown` in the + // module we're importing it from, we shouldn't really emit a diagnostic here, + // but currently do. match import { AnyImportRef::Import(import) => { for alias in &import.names { let ty = alias.ty(&context.semantic); - if ty.is_unbound() { + if ty.is_unknown() { context.push_diagnostic(format_diagnostic( context, &format!("Unresolved import '{}'", &alias.name), @@ -142,7 +146,7 @@ fn lint_unresolved_imports(context: &SemanticLintContext, import: AnyImportRef) for alias in &import.names { let ty = alias.ty(&context.semantic); - if ty.is_unbound() { + if ty.is_unknown() { context.push_diagnostic(format_diagnostic( context, &format!("Unresolved import '{}'", &alias.name), diff --git a/crates/ruff_benchmark/benches/red_knot.rs b/crates/ruff_benchmark/benches/red_knot.rs index f99a0fa06cc61..4126dda09ecf4 100644 --- a/crates/ruff_benchmark/benches/red_knot.rs +++ b/crates/ruff_benchmark/benches/red_knot.rs @@ -89,7 +89,7 @@ fn benchmark_incremental(criterion: &mut Criterion) { let Case { db, parser, .. } = case; let result = db.check_file(*parser).unwrap(); - assert_eq!(result.len(), 29); + assert_eq!(result.len(), 34); }, BatchSize::SmallInput, ); @@ -104,7 +104,7 @@ fn benchmark_cold(criterion: &mut Criterion) { let Case { db, parser, .. } = case; let result = db.check_file(*parser).unwrap(); - assert_eq!(result.len(), 29); + assert_eq!(result.len(), 34); }, BatchSize::SmallInput, );