[red-knot] Silence unresolved-attribute diagnostics

Author: sharkdp

crates/red_knot_python_semantic/resources/mdtest/unreachable.md

@@ -166,8 +166,6 @@ python-platform = "linux"
 import sys
 
 if sys.platform == "win32":
-    # TODO: we should not emit an error here
-    # error: [unresolved-attribute]
     sys.getwindowsversion()
 ```
 
@@ -381,8 +379,6 @@ import sys
 import builtins
 
 if sys.version_info >= (3, 11):
-    # TODO
-    # error: [unresolved-attribute]
     builtins.ExceptionGroup
 ```
 
@@ -430,6 +426,33 @@ if False:
             print(x)
 ```
 
+### Type annotations
+
+Silencing of diagnostics also works for type annotations, even if they are stringified:
+
+```py
+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:
+        def name_expr(self) -> Self:
+            return self
+
+        def name_expr_stringified(self) -> "Self":
+            return self
+
+        def attribute_expr(self) -> typing.Self:
+            return self
+
+        def attribute_expr_stringified(self) -> "typing.Self":
+            return self
+```
+
 ### Use of unreachable symbols in type annotations, or as class bases
 
 We should not show any diagnostics in type annotations inside unreachable sections.

crates/red_knot_python_semantic/src/semantic_index.rs

@@ -11,7 +11,7 @@ use salsa::Update;
 
 use crate::module_name::ModuleName;
 use crate::semantic_index::ast_ids::node_key::ExpressionNodeKey;
-use crate::semantic_index::ast_ids::{AstIds, ScopedUseId};
+use crate::semantic_index::ast_ids::{AstIds, ScopedExpressionId};
 use crate::semantic_index::attribute_assignment::AttributeAssignments;
 use crate::semantic_index::builder::SemanticIndexBuilder;
 use crate::semantic_index::definition::{Definition, DefinitionNodeKey, Definitions};
@@ -254,8 +254,8 @@ impl<'db> SemanticIndex<'db> {
             })
     }
 
-    /// Returns true if a given 'use' of a symbol is reachable from the start of the scope.
-    /// For example, in the following code, use `2` is reachable, but `1` and `3` are not:
+    /// Returns true if a given expression 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():
     ///     x = 1
@@ -265,16 +265,16 @@ impl<'db> SemanticIndex<'db> {
     ///     return
     ///     x  # 3
     /// ```
-    pub(crate) fn is_symbol_use_reachable(
+    pub(crate) fn is_expression_reachable(
         &self,
         db: &'db dyn crate::Db,
         scope_id: FileScopeId,
-        use_id: ScopedUseId,
+        expression_id: ScopedExpressionId,
     ) -> bool {
         self.is_scope_reachable(db, scope_id)
             && self
                 .use_def_map(scope_id)
-                .is_symbol_use_reachable(db, use_id)
+                .is_expression_reachable(db, expression_id)
     }
 
     /// Returns an iterator over the descendent scopes of `scope`.

crates/red_knot_python_semantic/src/semantic_index/builder.rs

@@ -8,7 +8,7 @@ use ruff_db::parsed::ParsedModule;
 use ruff_index::IndexVec;
 use ruff_python_ast::name::Name;
 use ruff_python_ast::visitor::{walk_expr, walk_pattern, walk_stmt, Visitor};
-use ruff_python_ast::{self as ast, ExprContext};
+use ruff_python_ast::{self as ast};
 
 use crate::ast_node_ref::AstNodeRef;
 use crate::module_name::ModuleName;
@@ -1770,7 +1770,8 @@ where
                 if is_use {
                     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);
+                    self.current_use_def_map_mut()
+                        .record_use(symbol, use_id, expression_id);
                 }
 
                 if is_definition {
@@ -2011,24 +2012,39 @@ where
             ast::Expr::Attribute(ast::ExprAttribute {
                 value: object,
                 attr,
-                ctx: ExprContext::Store,
+                ctx,
                 range: _,
             }) => {
-                if let Some(unpack) = self
-                    .current_assignment()
-                    .as_ref()
-                    .and_then(CurrentAssignment::unpack)
-                {
-                    self.register_attribute_assignment(
-                        object,
-                        attr,
-                        AttributeAssignment::Unpack {
-                            attribute_expression_id: expression_id,
-                            unpack,
-                        },
-                    );
+                if ctx.is_store() {
+                    if let Some(unpack) = self
+                        .current_assignment()
+                        .as_ref()
+                        .and_then(CurrentAssignment::unpack)
+                    {
+                        self.register_attribute_assignment(
+                            object,
+                            attr,
+                            AttributeAssignment::Unpack {
+                                attribute_expression_id: expression_id,
+                                unpack,
+                            },
+                        );
+                    }
                 }
 
+                // Track reachability of attribute expressions to silence `unresolved-attribute`
+                // diagnostics in unreachable code.
+                self.current_use_def_map_mut()
+                    .record_expression_reachability(expression_id);
+
+                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);
+
                 walk_expr(self, expr);
             }
             _ => {

crates/red_knot_python_semantic/src/semantic_index/use_def.rs

@@ -263,7 +263,7 @@ use self::symbol_state::{
     LiveBindingsIterator, LiveDeclaration, LiveDeclarationsIterator, ScopedDefinitionId,
     SymbolBindings, SymbolDeclarations, SymbolState,
 };
-use crate::semantic_index::ast_ids::ScopedUseId;
+use crate::semantic_index::ast_ids::{ScopedExpressionId, ScopedUseId};
 use crate::semantic_index::definition::Definition;
 use crate::semantic_index::narrowing_constraints::{
     NarrowingConstraints, NarrowingConstraintsBuilder, NarrowingConstraintsIterator,
@@ -297,8 +297,8 @@ pub(crate) struct UseDefMap<'db> {
     /// [`SymbolBindings`] reaching a [`ScopedUseId`].
     bindings_by_use: IndexVec<ScopedUseId, SymbolBindings>,
 
-    /// Tracks whether or not a given use of a symbol is reachable from the start of the scope.
-    reachability_by_use: IndexVec<ScopedUseId, ScopedVisibilityConstraintId>,
+    /// Tracks whether or not a given expression is reachable from the start of the scope.
+    expression_reachability: FxHashMap<ScopedExpressionId, 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.
@@ -359,8 +359,26 @@ impl<'db> UseDefMap<'db> {
             .is_always_false()
     }
 
-    pub(super) fn is_symbol_use_reachable(&self, db: &dyn crate::Db, use_id: ScopedUseId) -> bool {
-        self.is_reachable(db, self.reachability_by_use[use_id])
+    /// Check whether or not a given expression is reachable from the start of the scope. This
+    /// is a local analysis which does not capture the possibility that the entire scope might
+    /// be unreachable. Use `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 {
+        !self
+            .visibility_constraints
+            .evaluate(
+                db,
+                &self.predicates,
+                *self
+                    .expression_reachability
+                    .get(&expression_id)
+                    .expect("only called on expressions with recorded reachability"),
+            )
+            .is_always_false()
     }
 
     pub(crate) fn public_bindings(
@@ -617,8 +635,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 use of a symbol is reachable from the start of the scope.
-    reachability_by_use: IndexVec<ScopedUseId, ScopedVisibilityConstraintId>,
+    /// Tracks whether or not a given expression is reachable from the start of the scope.
+    expression_reachability: FxHashMap<ScopedExpressionId, ScopedVisibilityConstraintId>,
 
     /// Live declarations for each so-far-recorded binding.
     declarations_by_binding: FxHashMap<Definition<'db>, SymbolDeclarations>,
@@ -644,7 +662,7 @@ impl Default for UseDefMapBuilder<'_> {
             scope_start_visibility: ScopedVisibilityConstraintId::ALWAYS_TRUE,
             bindings_by_use: IndexVec::new(),
             reachability: ScopedVisibilityConstraintId::ALWAYS_TRUE,
-            reachability_by_use: IndexVec::new(),
+            expression_reachability: FxHashMap::default(),
             declarations_by_binding: FxHashMap::default(),
             bindings_by_declaration: FxHashMap::default(),
             symbol_states: IndexVec::new(),
@@ -799,16 +817,27 @@ impl<'db> UseDefMapBuilder<'db> {
         symbol_state.record_binding(def_id, self.scope_start_visibility);
     }
 
-    pub(super) fn record_use(&mut self, symbol: ScopedSymbolId, use_id: ScopedUseId) {
+    pub(super) fn record_use(
+        &mut self,
+        symbol: ScopedSymbolId,
+        use_id: ScopedUseId,
+        expression_id: ScopedExpressionId,
+    ) {
         // We have a use of a symbol; clone the current bindings for that symbol, and record them
         // as the live bindings for this use.
         let new_use = self
             .bindings_by_use
             .push(self.symbol_states[symbol].bindings().clone());
         debug_assert_eq!(use_id, new_use);
 
-        let new_use = self.reachability_by_use.push(self.reachability);
-        debug_assert_eq!(use_id, new_use);
+        // Track reachability of all uses of symbols to silence `unresolved-reference`
+        // diagnostics in unreachable code.
+        self.record_expression_reachability(expression_id);
+    }
+
+    pub(super) fn record_expression_reachability(&mut self, expression_id: ScopedExpressionId) {
+        self.expression_reachability
+            .insert(expression_id, self.reachability);
     }
 
     pub(super) fn snapshot_eager_bindings(
@@ -905,7 +934,6 @@ impl<'db> UseDefMapBuilder<'db> {
         self.all_definitions.shrink_to_fit();
         self.symbol_states.shrink_to_fit();
         self.bindings_by_use.shrink_to_fit();
-        self.reachability_by_use.shrink_to_fit();
         self.declarations_by_binding.shrink_to_fit();
         self.bindings_by_declaration.shrink_to_fit();
         self.eager_bindings.shrink_to_fit();
@@ -916,7 +944,7 @@ impl<'db> UseDefMapBuilder<'db> {
             narrowing_constraints: self.narrowing_constraints.build(),
             visibility_constraints: self.visibility_constraints.build(),
             bindings_by_use: self.bindings_by_use,
-            reachability_by_use: self.reachability_by_use,
+            expression_reachability: self.expression_reachability,
             public_symbols: self.symbol_states,
             declarations_by_binding: self.declarations_by_binding,
             bindings_by_declaration: self.bindings_by_declaration,