[red-knot] consider imports to be declarations (#13398)

I noticed that this pattern sometimes occurs in typeshed:
```
if ...:
    from foo import bar
else:
    def bar(): ...
```

If we have the rule that symbols with declarations only use declarations
for the public type, then this ends up resolving as `Unknown |
Literal[bar]`, because we didn't consider the import to be a
declaration.

I think the most straightforward thing here is to also consider imports
as declarations. The same rationale applies as for function and class
definitions: if you shadow an import, you should have to explicitly
shadow with an annotation, rather than just doing it
implicitly/accidentally.

We may also ultimately need to re-evaluate the rule that public type
considers only declarations, if there are declarations.

Author: carljm

crates/red_knot_python_semantic/src/semantic_index/definition.rs

@@ -330,7 +330,7 @@ impl DefinitionCategory {
     /// If so, any assignments reached by this definition are in error if they assign a value of a
     /// type not assignable to the declared type.
     ///
-    /// Annotations establish a declared type. So do function and class definition.
+    /// Annotations establish a declared type. So do function and class definitions, and imports.
     pub(crate) fn is_declaration(self) -> bool {
         matches!(
             self,
@@ -371,10 +371,11 @@ pub enum DefinitionKind {
 impl DefinitionKind {
     pub(crate) fn category(&self) -> DefinitionCategory {
         match self {
-            // functions and classes always bind a value, and we always consider them declarations
-            DefinitionKind::Function(_) | DefinitionKind::Class(_) => {
-                DefinitionCategory::DeclarationAndBinding
-            }
+            // functions, classes, and imports always bind, and we consider them declarations
+            DefinitionKind::Function(_)
+            | DefinitionKind::Class(_)
+            | DefinitionKind::Import(_)
+            | DefinitionKind::ImportFrom(_) => DefinitionCategory::DeclarationAndBinding,
             // a parameter always binds a value, but is only a declaration if annotated
             DefinitionKind::Parameter(parameter) => {
                 if parameter.annotation.is_some() {
@@ -400,9 +401,7 @@ impl DefinitionKind {
                 }
             }
             // all of these bind values without declaring a type
-            DefinitionKind::Import(_)
-            | DefinitionKind::ImportFrom(_)
-            | DefinitionKind::NamedExpression(_)
+            DefinitionKind::NamedExpression(_)
             | DefinitionKind::Assignment(_)
             | DefinitionKind::AugmentedAssignment(_)
             | DefinitionKind::For(_)

crates/red_knot_python_semantic/src/types/infer.rs

@@ -1316,7 +1316,7 @@ impl<'db> TypeInferenceBuilder<'db> {
             Type::Unknown
         };
 
-        self.add_binding(alias.into(), definition, module_ty);
+        self.add_declaration_with_binding(alias.into(), definition, module_ty, module_ty);
     }
 
     fn infer_import_from_statement(&mut self, import: &ast::StmtImportFrom) {
@@ -1500,11 +1500,9 @@ impl<'db> TypeInferenceBuilder<'db> {
         // 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`
-        self.add_binding(
-            alias.into(),
-            definition,
-            member_ty.replace_unbound_with(self.db, Type::Unknown),
-        );
+        let ty = member_ty.replace_unbound_with(self.db, Type::Unknown);
+
+        self.add_declaration_with_binding(alias.into(), definition, ty, ty);
     }
 
     fn infer_return_statement(&mut self, ret: &ast::StmtReturn) {
@@ -5697,6 +5695,50 @@ mod tests {
         assert_file_diagnostics(&db, "/src/a.py", &[]);
     }
 
+    #[test]
+    fn no_implicit_shadow_import() {
+        let mut db = setup_db();
+
+        db.write_dedented(
+            "/src/a.py",
+            "
+            from b import x
+
+            x = 'foo'
+            ",
+        )
+        .unwrap();
+
+        db.write_file("/src/b.py", "x: int").unwrap();
+
+        assert_file_diagnostics(
+            &db,
+            "/src/a.py",
+            &[r#"Object of type 'Literal["foo"]' is not assignable to 'int'."#],
+        );
+    }
+
+    #[test]
+    fn import_from_conditional_reimport() {
+        let mut db = setup_db();
+
+        db.write_file("/src/a.py", "from b import f").unwrap();
+        db.write_dedented(
+            "/src/b.py",
+            "
+            if flag:
+                from c import f
+            else:
+                def f(): ...
+            ",
+        )
+        .unwrap();
+        db.write_file("/src/c.py", "def f(): ...").unwrap();
+
+        // TODO we should really disambiguate in such cases: Literal[b.f, c.f]
+        assert_public_ty(&db, "/src/a.py", "f", "Literal[f, f]");
+    }
+
     // Incremental inference tests
 
     fn first_public_binding<'db>(db: &'db TestDb, file: File, name: &str) -> Definition<'db> {