diff --git a/crates/red_knot_python_semantic/resources/mdtest/unreachable.md b/crates/red_knot_python_semantic/resources/mdtest/unreachable.md index 6917a92b3a..d25e4f1dd5 100644 --- 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 ``` @@ -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. diff --git a/crates/red_knot_python_semantic/src/semantic_index.rs b/crates/red_knot_python_semantic/src/semantic_index.rs index f7e8979dc7..2da3bc6803 100644 --- 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 index af4c4b83c6..f7b169c90a 100644 --- 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,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); } _ => { 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 index 1cbf05b337..a95acb554f 100644 --- 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, @@ -297,8 +297,8 @@ pub(crate) struct UseDefMap<'db> { /// [`SymbolBindings`] reaching a [`ScopedUseId`]. bindings_by_use: IndexVec, - /// Tracks whether or not a given use of a symbol is reachable from the start of the scope. - reachability_by_use: IndexVec, + /// Tracks whether or not a given expression is reachable from the start of the scope. + expression_reachability: FxHashMap, /// 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,27 @@ 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 [`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 { + !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"), + ) + .is_always_false() } pub(crate) fn public_bindings( @@ -617,8 +636,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, + /// Tracks whether or not a given expression is reachable from the start of the scope. + expression_reachability: FxHashMap, /// Live declarations for each so-far-recorded binding. declarations_by_binding: FxHashMap, SymbolDeclarations>, @@ -644,7 +663,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,7 +818,12 @@ 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 @@ -807,8 +831,14 @@ impl<'db> UseDefMapBuilder<'db> { .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 +935,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.reachability_by_use.shrink_to_fit(); + self.expression_reachability.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 +946,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 index 8f8d9b2f46..ed64d04c5e 100644 --- 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() } @@ -4287,8 +4298,8 @@ impl<'db> TypeInferenceBuilder<'db> { let use_def = self.index.use_def_map(file_scope_id); // If we're inferring types of deferred expressions, always treat them as public symbols - let (local_scope_symbol, report_unresolved_usage) = if self.is_deferred() { - let symbol = if let Some(symbol_id) = symbol_table.symbol_id_by_name(symbol_name) { + let local_scope_symbol = if self.is_deferred() { + if let Some(symbol_id) = symbol_table.symbol_id_by_name(symbol_name) { symbol_from_bindings(db, use_def.public_bindings(symbol_id)) } else { assert!( @@ -4296,16 +4307,10 @@ impl<'db> TypeInferenceBuilder<'db> { "Expected the symbol table to create a symbol for every Name node" ); Symbol::Unbound - }; - - (symbol, true) + } } 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); - (symbol, report_unresolved_usage) + symbol_from_bindings(db, use_def.bindings_at_use(use_id)) }; let symbol = SymbolAndQualifiers::from(local_scope_symbol).or_fall_back_to(db, || { @@ -4452,16 +4457,24 @@ impl<'db> TypeInferenceBuilder<'db> { }) }); + let report_unresolved_usage = || { + self.index.is_expression_reachable( + db, + file_scope_id, + self.enclosing_expression_id(name_node), + ) + }; + symbol .unwrap_with_diagnostic(|lookup_error| match lookup_error { LookupError::Unbound(qualifiers) => { - if report_unresolved_usage { + if report_unresolved_usage() { report_unresolved_reference(&self.context, name_node); } TypeAndQualifiers::new(Type::unknown(), qualifiers) } LookupError::PossiblyUnbound(type_when_bound) => { - if report_unresolved_usage { + if report_unresolved_usage() { report_possibly_unresolved_reference(&self.context, name_node); } type_when_bound @@ -4494,43 +4507,53 @@ impl<'db> TypeInferenceBuilder<'db> { .member(db, &attr.id) .unwrap_with_diagnostic(|lookup_error| match lookup_error { LookupError::Unbound(_) => { - let bound_on_instance = match value_type { - Type::ClassLiteral(class) => { - !class.instance_member(db, None, attr).symbol.is_unbound() - } - Type::SubclassOf(subclass_of @ SubclassOfType { .. }) => { - match subclass_of.subclass_of() { - ClassBase::Class(class) => { - !class.instance_member(db, attr).symbol.is_unbound() - } - ClassBase::Dynamic(_) => unreachable!( - "Attribute lookup on a dynamic `SubclassOf` type should always return a bound symbol" - ), - } - } - _ => false, - }; + let report_unresolved_attribute = self + .index + .is_expression_reachable( + db, + self.scope().file_scope_id(db), + self.enclosing_expression_id(attribute), + ); - 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 - ), - ); + if report_unresolved_attribute { + let bound_on_instance = match value_type { + Type::ClassLiteral(class) => { + !class.instance_member(db, None, attr).symbol.is_unbound() + } + Type::SubclassOf(subclass_of @ SubclassOfType { .. }) => { + match subclass_of.subclass_of() { + ClassBase::Class(class) => { + !class.instance_member(db, attr).symbol.is_unbound() + } + ClassBase::Dynamic(_) => unreachable!( + "Attribute lookup on a dynamic `SubclassOf` type should always return a bound symbol" + ), + } + } + _ => 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 + ), + ); + } } 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,22 @@ enum DeferredExpressionState { /// /// The annotation of `a` is completely inside a string while for `b`, it's only partially /// stringified. - InStringAnnotation, + /// + /// This variant wraps a [`ScopedExpressionId`] that allows us to retrieve + /// the original [`ast::ExprStringLiteral`] node which created the string annotation + 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(_)) } }