[red-knot] Silence unresolved-attribute diagnostics

sharkdp · sharkdp · commit 05454c1e77df · 2025-04-10T15:35:35.000+02:00
diff --git a/crates/red_knot_python_semantic/resources/mdtest/unreachable.md b/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
 ```
 
diff --git a/crates/red_knot_python_semantic/src/semantic_index.rs b/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`.
diff --git a/crates/red_knot_python_semantic/src/semantic_index/builder.rs b/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,35 @@ 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,
+                            },
+                        );
+                    }
                 }
 
+                self.current_use_def_map_mut()
+                    .record_expression_reachability(expression_id);
+
+                walk_expr(self, expr);
+            }
+            ast::Expr::StringLiteral(_) => {
+                self.current_use_def_map_mut()
+                    .record_expression_reachability(expression_id);
+
                 walk_expr(self, expr);
             }
             _ => {
diff --git a/crates/red_knot_python_semantic/src/semantic_index/use_def.rs b/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,
@@ -298,7 +298,7 @@ pub(crate) struct UseDefMap<'db> {
     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>,
+    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,23 @@ 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])
+    #[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(
@@ -618,7 +633,7 @@ pub(super) struct UseDefMapBuilder<'db> {
     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>,
+    expression_reachability: FxHashMap<ScopedExpressionId, ScopedVisibilityConstraintId>,
 
     /// Live declarations for each so-far-recorded binding.
     declarations_by_binding: FxHashMap<Definition<'db>, SymbolDeclarations>,
@@ -644,7 +659,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 +814,25 @@ 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);
+        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 +929,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 +939,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,
diff --git a/crates/red_knot_python_semantic/src/types/infer.rs b/crates/red_knot_python_semantic/src/types/infer.rs
@@ -590,6 +590,17 @@ impl<'db> TypeInferenceBuilder<'db> {
         matches!(self.region, InferenceRegion::Deferred(_)) || self.deferred_state.is_deferred()
     }
 
+    /// Return the ID of the given expression, or the ID of the outermost enclosing string literal,
+    /// if the expression originates from a stringified annotation.
+    fn enclosing_expression_id(&self, expr: &impl HasScopedExpressionId) -> ScopedExpressionId {
+        match self.deferred_state {
+            DeferredExpressionState::InStringAnnotation(enclosing_string_expression) => {
+                enclosing_string_expression
+            }
+            _ => expr.scoped_expression_id(self.db(), self.scope()),
+        }
+    }
+
     fn in_stub(&self) -> bool {
         self.context.in_stub()
     }
@@ -4302,9 +4313,11 @@ impl<'db> TypeInferenceBuilder<'db> {
         } else {
             let use_id = name_node.scoped_use_id(db, scope);
             let symbol = symbol_from_bindings(db, use_def.bindings_at_use(use_id));
-            let report_unresolved_usage =
-                self.index
-                    .is_symbol_use_reachable(db, file_scope_id, use_id);
+            let report_unresolved_usage = self.index.is_expression_reachable(
+                db,
+                file_scope_id,
+                name_node.scoped_expression_id(db, scope),
+            );
             (symbol, report_unresolved_usage)
         };
 
@@ -4511,26 +4524,36 @@ impl<'db> TypeInferenceBuilder<'db> {
                         _ => false,
                     };
 
-                    if bound_on_instance {
-                        self.context.report_lint(
-                            &UNRESOLVED_ATTRIBUTE,
-                            attribute,
-                            format_args!(
-                                "Attribute `{}` can only be accessed on instances, not on the class object `{}` itself.",
-                                attr.id,
-                                value_type.display(db)
-                            ),
-                        );
-                    } else {
-                        self.context.report_lint(
-                            &UNRESOLVED_ATTRIBUTE,
-                            attribute,
-                            format_args!(
-                                "Type `{}` has no attribute `{}`",
-                                value_type.display(db),
-                                attr.id
-                            ),
+                    let report_diagnostic = self
+                        .index
+                        .is_expression_reachable(
+                            db,
+                            self.scope().file_scope_id(db),
+                            self.enclosing_expression_id(attribute),
                         );
+
+                    if report_diagnostic {
+                        if bound_on_instance {
+                            self.context.report_lint(
+                                &UNRESOLVED_ATTRIBUTE,
+                                attribute,
+                                format_args!(
+                                    "Attribute `{}` can only be accessed on instances, not on the class object `{}` itself.",
+                                    attr.id,
+                                    value_type.display(db)
+                                ),
+                            );
+                        } else {
+                            self.context.report_lint(
+                                &UNRESOLVED_ATTRIBUTE,
+                                attribute,
+                                format_args!(
+                                    "Type `{}` has no attribute `{}`",
+                                    value_type.display(db),
+                                    attr.id
+                                ),
+                            );
+                        }
                     }
 
                     Type::unknown().into()
@@ -6368,7 +6391,9 @@ impl<'db> TypeInferenceBuilder<'db> {
                 // String annotations are always evaluated in the deferred context.
                 self.infer_annotation_expression(
                     parsed.expr(),
-                    DeferredExpressionState::InStringAnnotation,
+                    DeferredExpressionState::InStringAnnotation(
+                        self.enclosing_expression_id(string),
+                    ),
                 )
             }
             None => TypeAndQualifiers::unknown(),
@@ -6732,7 +6757,9 @@ impl<'db> TypeInferenceBuilder<'db> {
                 // String annotations are always evaluated in the deferred context.
                 self.infer_type_expression_with_state(
                     parsed.expr(),
-                    DeferredExpressionState::InStringAnnotation,
+                    DeferredExpressionState::InStringAnnotation(
+                        self.enclosing_expression_id(string),
+                    ),
                 )
             }
             None => Type::unknown(),
@@ -7449,19 +7476,19 @@ enum DeferredExpressionState {
     ///
     /// The annotation of `a` is completely inside a string while for `b`, it's only partially
     /// stringified.
-    InStringAnnotation,
+    InStringAnnotation(ScopedExpressionId),
 }
 
 impl DeferredExpressionState {
     const fn is_deferred(self) -> bool {
         matches!(
             self,
-            DeferredExpressionState::Deferred | DeferredExpressionState::InStringAnnotation
+            DeferredExpressionState::Deferred | DeferredExpressionState::InStringAnnotation(_)
         )
     }
 
     const fn in_string_annotation(self) -> bool {
-        matches!(self, DeferredExpressionState::InStringAnnotation)
+        matches!(self, DeferredExpressionState::InStringAnnotation(_))
     }
 }
 
