[red-knot] Use Unknown rather than Unbound for unresolved imports (…

…#12932)

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)
     }

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<ModuleName>) -> 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();

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),

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,
         );