From 4d50ee6f527e6628e6e788a90d95fb9285618679 Mon Sep 17 00:00:00 2001 From: David Peter Date: Thu, 10 Apr 2025 13:56:40 +0200 Subject: [PATCH] [red-knot] Track reachability of scopes (#17332) ## Summary Track the reachability of nested scopes within their parent scopes. We use this as an additional requirement for emitting `unresolved-reference` diagnostics (and in the future, `unresolved-attribute` and `unresolved-import`). This means that we only emit `unresolved-reference` for a given use of a symbol if the use itself is reachable (within its own scope), *and if the scope itself is reachable*. For example, no diagnostic should be emitted for the use of `x` here: ```py if False: x = 1 def f(): print(x) # this use of `x` is reachable inside the `f` scope, # but the whole `f` scope is not reachable. ``` There are probably more fine-grained ways of solving this problem, but they require a more sophisticated understanding of nested scopes (see #15777, in particular https://github.com/astral-sh/ruff/issues/15777#issuecomment-2788950267). But it doesn't seem completely unreasonable to silence *this specific kind of error* in unreachable scopes. ## Test Plan Observed changes in reachability tests and ecosystem. --- .../resources/mdtest/unreachable.md | 4 -- .../src/semantic_index.rs | 39 ++++++++++++++++++- .../src/semantic_index/builder.rs | 23 +++++++++-- .../src/semantic_index/symbol.rs | 8 ++++ .../src/semantic_index/use_def.rs | 25 ++++++------ .../src/types/infer.rs | 4 +- 6 files changed, 79 insertions(+), 24 deletions(-) diff --git a/crates/red_knot_python_semantic/resources/mdtest/unreachable.md b/crates/red_knot_python_semantic/resources/mdtest/unreachable.md index 9b185aee7a..6917a92b3a 100644 --- a/crates/red_knot_python_semantic/resources/mdtest/unreachable.md +++ b/crates/red_knot_python_semantic/resources/mdtest/unreachable.md @@ -423,14 +423,10 @@ if False: x = 1 def f(): - # TODO - # error: [unresolved-reference] print(x) class C: def __init__(self): - # TODO - # error: [unresolved-reference] print(x) ``` diff --git a/crates/red_knot_python_semantic/src/semantic_index.rs b/crates/red_knot_python_semantic/src/semantic_index.rs index e9e79d6b24..f7e8979dc7 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; +use crate::semantic_index::ast_ids::{AstIds, ScopedUseId}; use crate::semantic_index::attribute_assignment::AttributeAssignments; use crate::semantic_index::builder::SemanticIndexBuilder; use crate::semantic_index::definition::{Definition, DefinitionNodeKey, Definitions}; @@ -240,6 +240,43 @@ impl<'db> SemanticIndex<'db> { Some(&self.scopes[self.parent_scope_id(scope_id)?]) } + fn is_scope_reachable(&self, db: &'db dyn Db, scope_id: FileScopeId) -> bool { + self.parent_scope_id(scope_id) + .is_none_or(|parent_scope_id| { + if !self.is_scope_reachable(db, parent_scope_id) { + return false; + } + + let parent_use_def = self.use_def_map(parent_scope_id); + let reachability = self.scope(scope_id).reachability(); + + parent_use_def.is_reachable(db, reachability) + }) + } + + /// 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: + /// ```py + /// def f(): + /// x = 1 + /// if False: + /// x # 1 + /// x # 2 + /// return + /// x # 3 + /// ``` + pub(crate) fn is_symbol_use_reachable( + &self, + db: &'db dyn crate::Db, + scope_id: FileScopeId, + use_id: ScopedUseId, + ) -> bool { + self.is_scope_reachable(db, scope_id) + && self + .use_def_map(scope_id) + .is_symbol_use_reachable(db, use_id) + } + /// Returns an iterator over the descendent scopes of `scope`. #[allow(unused)] pub(crate) fn descendent_scopes(&self, scope: FileScopeId) -> DescendantsIter { 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 34e2755f25..af4c4b83c6 100644 --- a/crates/red_knot_python_semantic/src/semantic_index/builder.rs +++ b/crates/red_knot_python_semantic/src/semantic_index/builder.rs @@ -129,7 +129,11 @@ impl<'db> SemanticIndexBuilder<'db> { eager_bindings: FxHashMap::default(), }; - builder.push_scope_with_parent(NodeWithScopeRef::Module, None); + builder.push_scope_with_parent( + NodeWithScopeRef::Module, + None, + ScopedVisibilityConstraintId::ALWAYS_TRUE, + ); builder } @@ -191,17 +195,28 @@ impl<'db> SemanticIndexBuilder<'db> { fn push_scope(&mut self, node: NodeWithScopeRef) { let parent = self.current_scope(); - self.push_scope_with_parent(node, Some(parent)); + let reachabililty = self.current_use_def_map().reachability; + self.push_scope_with_parent(node, Some(parent), reachabililty); } - fn push_scope_with_parent(&mut self, node: NodeWithScopeRef, parent: Option) { + fn push_scope_with_parent( + &mut self, + node: NodeWithScopeRef, + parent: Option, + reachability: ScopedVisibilityConstraintId, + ) { let children_start = self.scopes.next_index() + 1; // SAFETY: `node` is guaranteed to be a child of `self.module` #[allow(unsafe_code)] let node_with_kind = unsafe { node.to_kind(self.module.clone()) }; - let scope = Scope::new(parent, node_with_kind, children_start..children_start); + let scope = Scope::new( + parent, + node_with_kind, + children_start..children_start, + reachability, + ); self.try_node_context_stack_manager.enter_nested_scope(); let file_scope_id = self.scopes.push(scope); diff --git a/crates/red_knot_python_semantic/src/semantic_index/symbol.rs b/crates/red_knot_python_semantic/src/semantic_index/symbol.rs index 07cfb1a61e..ec9ef3886e 100644 --- a/crates/red_knot_python_semantic/src/semantic_index/symbol.rs +++ b/crates/red_knot_python_semantic/src/semantic_index/symbol.rs @@ -12,6 +12,7 @@ use rustc_hash::FxHasher; use crate::ast_node_ref::AstNodeRef; use crate::node_key::NodeKey; +use crate::semantic_index::visibility_constraints::ScopedVisibilityConstraintId; use crate::semantic_index::{semantic_index, SymbolMap}; use crate::Db; @@ -176,6 +177,7 @@ pub struct Scope { parent: Option, node: NodeWithScopeKind, descendants: Range, + reachability: ScopedVisibilityConstraintId, } impl Scope { @@ -183,11 +185,13 @@ impl Scope { parent: Option, node: NodeWithScopeKind, descendants: Range, + reachability: ScopedVisibilityConstraintId, ) -> Self { Scope { parent, node, descendants, + reachability, } } @@ -214,6 +218,10 @@ impl Scope { pub(crate) fn is_eager(&self) -> bool { self.kind().is_eager() } + + pub(crate) fn reachability(&self) -> ScopedVisibilityConstraintId { + self.reachability + } } #[derive(Copy, Clone, Debug, PartialEq, Eq)] 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 057fb316c6..1cbf05b337 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 @@ -348,24 +348,21 @@ impl<'db> UseDefMap<'db> { self.bindings_iterator(&self.bindings_by_use[use_id]) } - /// 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: - /// ```py - /// def f(): - /// x = 1 - /// if False: - /// x # 1 - /// x # 2 - /// return - /// x # 3 - /// ``` - pub(crate) fn is_symbol_use_reachable(&self, db: &dyn crate::Db, use_id: ScopedUseId) -> bool { + pub(super) fn is_reachable( + &self, + db: &dyn crate::Db, + reachability: ScopedVisibilityConstraintId, + ) -> bool { !self .visibility_constraints - .evaluate(db, &self.predicates, self.reachability_by_use[use_id]) + .evaluate(db, &self.predicates, reachability) .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]) + } + pub(crate) fn public_bindings( &self, symbol: ScopedSymbolId, @@ -618,7 +615,7 @@ pub(super) struct UseDefMapBuilder<'db> { /// ``` /// Depending on the value of `test`, the `y = 1`, `y = 2`, or both bindings may be visible. /// The use of `x` is recorded with a reachability constraint of `[test]`. - reachability: ScopedVisibilityConstraintId, + 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, diff --git a/crates/red_knot_python_semantic/src/types/infer.rs b/crates/red_knot_python_semantic/src/types/infer.rs index 45f673e0fb..d6e8cc1ed2 100644 --- a/crates/red_knot_python_semantic/src/types/infer.rs +++ b/crates/red_knot_python_semantic/src/types/infer.rs @@ -4302,7 +4302,9 @@ 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 = use_def.is_symbol_use_reachable(db, use_id); + let report_unresolved_usage = + self.index + .is_symbol_use_reachable(db, file_scope_id, use_id); (symbol, report_unresolved_usage) };