[red-knot] Silence unresolved-import in unreachable code

Author: sharkdp

crates/red_knot_python_semantic/resources/mdtest/unreachable.md

@@ -117,8 +117,6 @@ python-version = "3.10"
 import sys
 
 if sys.version_info >= (3, 11):
-    # TODO: we should not emit an error here
-    # error: [unresolved-import]
     from typing import Self
 ```
 
@@ -391,22 +389,14 @@ diagnostics:
 import sys
 
 if sys.version_info >= (3, 11):
-    # TODO
-    # error: [unresolved-import]
     from builtins import ExceptionGroup
 
-    # TODO
-    # error: [unresolved-import]
     import builtins.ExceptionGroup
 
     # See https://docs.python.org/3/whatsnew/3.11.html#new-modules
 
-    # TODO
-    # error: [unresolved-import]
     import tomllib
 
-    # TODO
-    # error: [unresolved-import]
     import wsgiref.types
 ```
 
@@ -435,8 +425,6 @@ import sys
 import typing
 
 if sys.version_info >= (3, 11):
-    # TODO (silence diagnostics for imports, see above)
-    # error: [unresolved-import]
     from typing import Self
 
     class C:

crates/red_knot_python_semantic/src/semantic_index.rs

@@ -10,8 +10,9 @@ use salsa::plumbing::AsId;
 use salsa::Update;
 
 use crate::module_name::ModuleName;
+use crate::node_key::NodeKey;
 use crate::semantic_index::ast_ids::node_key::ExpressionNodeKey;
-use crate::semantic_index::ast_ids::{AstIds, ScopedExpressionId};
+use crate::semantic_index::ast_ids::AstIds;
 use crate::semantic_index::attribute_assignment::AttributeAssignments;
 use crate::semantic_index::builder::SemanticIndexBuilder;
 use crate::semantic_index::definition::{Definition, DefinitionNodeKey, Definitions};
@@ -254,7 +255,7 @@ impl<'db> SemanticIndex<'db> {
             })
     }
 
-    /// Returns true if a given expression is reachable from the start of the scope. For example,
+    /// Returns true if a given AST node is reachable from the start of the scope. For example,
     /// in the following code, expression `2` is reachable, but expressions `1` and `3` are not:
     /// ```py
     /// def f():
@@ -265,16 +266,14 @@ impl<'db> SemanticIndex<'db> {
     ///     return
     ///     x  # 3
     /// ```
-    pub(crate) fn is_expression_reachable(
+    pub(crate) fn is_node_reachable(
         &self,
         db: &'db dyn crate::Db,
         scope_id: FileScopeId,
-        expression_id: ScopedExpressionId,
+        node_key: NodeKey,
     ) -> bool {
         self.is_scope_reachable(db, scope_id)
-            && self
-                .use_def_map(scope_id)
-                .is_expression_reachable(db, expression_id)
+            && self.use_def_map(scope_id).is_node_reachable(db, node_key)
     }
 
     /// Returns an iterator over the descendent scopes of `scope`.

crates/red_knot_python_semantic/src/semantic_index/builder.rs

@@ -13,6 +13,7 @@ use ruff_python_ast::{self as ast};
 use crate::ast_node_ref::AstNodeRef;
 use crate::module_name::ModuleName;
 use crate::module_resolver::resolve_module;
+use crate::node_key::NodeKey;
 use crate::semantic_index::ast_ids::node_key::ExpressionNodeKey;
 use crate::semantic_index::ast_ids::AstIdsBuilder;
 use crate::semantic_index::attribute_assignment::{AttributeAssignment, AttributeAssignments};
@@ -1139,7 +1140,10 @@ where
                 );
             }
             ast::Stmt::Import(node) => {
-                for alias in &node.names {
+                self.current_use_def_map_mut()
+                    .record_node_reachability(NodeKey::from_node(node));
+
+                for (alias_index, alias) in node.names.iter().enumerate() {
                     // Mark the imported module, and all of its parents, as being imported in this
                     // file.
                     if let Some(module_name) = ModuleName::new(&alias.name) {
@@ -1156,13 +1160,17 @@ where
                     self.add_definition(
                         symbol,
                         ImportDefinitionNodeRef {
-                            alias,
+                            node,
+                            alias_index,
                             is_reexported,
                         },
                     );
                 }
             }
             ast::Stmt::ImportFrom(node) => {
+                self.current_use_def_map_mut()
+                    .record_node_reachability(NodeKey::from_node(node));
+
                 let mut found_star = false;
                 for (alias_index, alias) in node.names.iter().enumerate() {
                     if &alias.name == "*" {
@@ -1753,6 +1761,8 @@ where
             .insert(expr.into(), self.current_scope());
         let expression_id = self.current_ast_ids().record_expression(expr);
 
+        let node_key = NodeKey::from_node(expr);
+
         match expr {
             ast::Expr::Name(name_node @ ast::ExprName { id, ctx, .. }) => {
                 let (is_use, is_definition) = match (ctx, self.current_assignment()) {
@@ -1771,7 +1781,7 @@ where
                     self.mark_symbol_used(symbol);
                     let use_id = self.current_ast_ids().record_use(expr);
                     self.current_use_def_map_mut()
-                        .record_use(symbol, use_id, expression_id);
+                        .record_use(symbol, use_id, node_key);
                 }
 
                 if is_definition {
@@ -2035,15 +2045,15 @@ where
                 // Track reachability of attribute expressions to silence `unresolved-attribute`
                 // diagnostics in unreachable code.
                 self.current_use_def_map_mut()
-                    .record_expression_reachability(expression_id);
+                    .record_node_reachability(node_key);
 
                 walk_expr(self, expr);
             }
             ast::Expr::StringLiteral(_) => {
                 // Track reachability of string literals, as they could be a stringified annotation
                 // with child expressions whose reachability we are interested in.
                 self.current_use_def_map_mut()
-                    .record_expression_reachability(expression_id);
+                    .record_node_reachability(node_key);
 
                 walk_expr(self, expr);
             }

crates/red_knot_python_semantic/src/semantic_index/definition.rs

@@ -224,7 +224,8 @@ impl<'a> From<StarImportDefinitionNodeRef<'a>> for DefinitionNodeRef<'a> {
 
 #[derive(Copy, Clone, Debug)]
 pub(crate) struct ImportDefinitionNodeRef<'a> {
-    pub(crate) alias: &'a ast::Alias,
+    pub(crate) node: &'a ast::StmtImport,
+    pub(crate) alias_index: usize,
     pub(crate) is_reexported: bool,
 }
 
@@ -294,10 +295,12 @@ impl<'db> DefinitionNodeRef<'db> {
     pub(super) unsafe fn into_owned(self, parsed: ParsedModule) -> DefinitionKind<'db> {
         match self {
             DefinitionNodeRef::Import(ImportDefinitionNodeRef {
-                alias,
+                node,
+                alias_index,
                 is_reexported,
             }) => DefinitionKind::Import(ImportDefinitionKind {
-                alias: AstNodeRef::new(parsed, alias),
+                node: AstNodeRef::new(parsed, node),
+                alias_index,
                 is_reexported,
             }),
 
@@ -417,9 +420,10 @@ impl<'db> DefinitionNodeRef<'db> {
     pub(super) fn key(self) -> DefinitionNodeKey {
         match self {
             Self::Import(ImportDefinitionNodeRef {
-                alias,
+                node,
+                alias_index,
                 is_reexported: _,
-            }) => alias.into(),
+            }) => (&node.names[alias_index]).into(),
             Self::ImportFrom(ImportFromDefinitionNodeRef {
                 node,
                 alias_index,
@@ -757,13 +761,18 @@ impl ComprehensionDefinitionKind {
 
 #[derive(Clone, Debug)]
 pub struct ImportDefinitionKind {
-    alias: AstNodeRef<ast::Alias>,
+    node: AstNodeRef<ast::StmtImport>,
+    alias_index: usize,
     is_reexported: bool,
 }
 
 impl ImportDefinitionKind {
+    pub(crate) fn import(&self) -> &ast::StmtImport {
+        self.node.node()
+    }
+
     pub(crate) fn alias(&self) -> &ast::Alias {
-        self.alias.node()
+        &self.node.node().names[self.alias_index]
     }
 
     pub(crate) fn is_reexported(&self) -> bool {

crates/red_knot_python_semantic/src/semantic_index/use_def.rs

@@ -263,7 +263,8 @@ use self::symbol_state::{
     LiveBindingsIterator, LiveDeclaration, LiveDeclarationsIterator, ScopedDefinitionId,
     SymbolBindings, SymbolDeclarations, SymbolState,
 };
-use crate::semantic_index::ast_ids::{ScopedExpressionId, ScopedUseId};
+use crate::node_key::NodeKey;
+use crate::semantic_index::ast_ids::ScopedUseId;
 use crate::semantic_index::definition::Definition;
 use crate::semantic_index::narrowing_constraints::{
     NarrowingConstraints, NarrowingConstraintsBuilder, NarrowingConstraintsIterator,
@@ -297,8 +298,8 @@ pub(crate) struct UseDefMap<'db> {
     /// [`SymbolBindings`] reaching a [`ScopedUseId`].
     bindings_by_use: IndexVec<ScopedUseId, SymbolBindings>,
 
-    /// Tracks whether or not a given expression is reachable from the start of the scope.
-    expression_reachability: FxHashMap<ScopedExpressionId, ScopedVisibilityConstraintId>,
+    /// Tracks whether or not a given AST node is reachable from the start of the scope.
+    node_reachability: FxHashMap<NodeKey, ScopedVisibilityConstraintId>,
 
     /// If the definition is a binding (only) -- `x = 1` for example -- then we need
     /// [`SymbolDeclarations`] to know whether this binding is permitted by the live declarations.
@@ -364,20 +365,16 @@ impl<'db> UseDefMap<'db> {
     /// be unreachable. Use [`super::SemanticIndex::is_expression_reachable`] for the global
     /// analysis.
     #[track_caller]
-    pub(super) fn is_expression_reachable(
-        &self,
-        db: &dyn crate::Db,
-        expression_id: ScopedExpressionId,
-    ) -> bool {
+    pub(super) fn is_node_reachable(&self, db: &dyn crate::Db, node_key: NodeKey) -> bool {
         !self
             .visibility_constraints
             .evaluate(
                 db,
                 &self.predicates,
                 *self
-                    .expression_reachability
-                    .get(&expression_id)
-                    .expect("`is_expression_reachable` should only be called on expressions with recorded reachability"),
+                    .node_reachability
+                    .get(&node_key)
+                    .expect("`is_node_reachable` should only be called on AST nodes with recorded reachability"),
             )
             .is_always_false()
     }
@@ -636,8 +633,8 @@ pub(super) struct UseDefMapBuilder<'db> {
     /// The use of `x` is recorded with a reachability constraint of `[test]`.
     pub(super) reachability: ScopedVisibilityConstraintId,
 
-    /// Tracks whether or not a given expression is reachable from the start of the scope.
-    expression_reachability: FxHashMap<ScopedExpressionId, ScopedVisibilityConstraintId>,
+    /// Tracks whether or not a given AST node is reachable from the start of the scope.
+    node_reachability: FxHashMap<NodeKey, ScopedVisibilityConstraintId>,
 
     /// Live declarations for each so-far-recorded binding.
     declarations_by_binding: FxHashMap<Definition<'db>, SymbolDeclarations>,
@@ -663,7 +660,7 @@ impl Default for UseDefMapBuilder<'_> {
             scope_start_visibility: ScopedVisibilityConstraintId::ALWAYS_TRUE,
             bindings_by_use: IndexVec::new(),
             reachability: ScopedVisibilityConstraintId::ALWAYS_TRUE,
-            expression_reachability: FxHashMap::default(),
+            node_reachability: FxHashMap::default(),
             declarations_by_binding: FxHashMap::default(),
             bindings_by_declaration: FxHashMap::default(),
             symbol_states: IndexVec::new(),
@@ -822,7 +819,7 @@ impl<'db> UseDefMapBuilder<'db> {
         &mut self,
         symbol: ScopedSymbolId,
         use_id: ScopedUseId,
-        expression_id: ScopedExpressionId,
+        node_key: NodeKey,
     ) {
         // We have a use of a symbol; clone the current bindings for that symbol, and record them
         // as the live bindings for this use.
@@ -833,12 +830,11 @@ impl<'db> UseDefMapBuilder<'db> {
 
         // Track reachability of all uses of symbols to silence `unresolved-reference`
         // diagnostics in unreachable code.
-        self.record_expression_reachability(expression_id);
+        self.record_node_reachability(node_key);
     }
 
-    pub(super) fn record_expression_reachability(&mut self, expression_id: ScopedExpressionId) {
-        self.expression_reachability
-            .insert(expression_id, self.reachability);
+    pub(super) fn record_node_reachability(&mut self, node_key: NodeKey) {
+        self.node_reachability.insert(node_key, self.reachability);
     }
 
     pub(super) fn snapshot_eager_bindings(
@@ -935,7 +931,7 @@ impl<'db> UseDefMapBuilder<'db> {
         self.all_definitions.shrink_to_fit();
         self.symbol_states.shrink_to_fit();
         self.bindings_by_use.shrink_to_fit();
-        self.expression_reachability.shrink_to_fit();
+        self.node_reachability.shrink_to_fit();
         self.declarations_by_binding.shrink_to_fit();
         self.bindings_by_declaration.shrink_to_fit();
         self.eager_bindings.shrink_to_fit();
@@ -946,7 +942,7 @@ impl<'db> UseDefMapBuilder<'db> {
             narrowing_constraints: self.narrowing_constraints.build(),
             visibility_constraints: self.visibility_constraints.build(),
             bindings_by_use: self.bindings_by_use,
-            expression_reachability: self.expression_reachability,
+            node_reachability: self.node_reachability,
             public_symbols: self.symbol_states,
             declarations_by_binding: self.declarations_by_binding,
             bindings_by_declaration: self.bindings_by_declaration,