Skip to content

Commit 4d50ee6

Browse files
authored
[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.
1 parent 06ffeb2 commit 4d50ee6

File tree

6 files changed

+79
-24
lines changed

6 files changed

+79
-24
lines changed

crates/red_knot_python_semantic/resources/mdtest/unreachable.md

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -423,14 +423,10 @@ if False:
423423
x = 1
424424

425425
def f():
426-
# TODO
427-
# error: [unresolved-reference]
428426
print(x)
429427

430428
class C:
431429
def __init__(self):
432-
# TODO
433-
# error: [unresolved-reference]
434430
print(x)
435431
```
436432

crates/red_knot_python_semantic/src/semantic_index.rs

Lines changed: 38 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@ use salsa::Update;
1111

1212
use crate::module_name::ModuleName;
1313
use crate::semantic_index::ast_ids::node_key::ExpressionNodeKey;
14-
use crate::semantic_index::ast_ids::AstIds;
14+
use crate::semantic_index::ast_ids::{AstIds, ScopedUseId};
1515
use crate::semantic_index::attribute_assignment::AttributeAssignments;
1616
use crate::semantic_index::builder::SemanticIndexBuilder;
1717
use crate::semantic_index::definition::{Definition, DefinitionNodeKey, Definitions};
@@ -240,6 +240,43 @@ impl<'db> SemanticIndex<'db> {
240240
Some(&self.scopes[self.parent_scope_id(scope_id)?])
241241
}
242242

243+
fn is_scope_reachable(&self, db: &'db dyn Db, scope_id: FileScopeId) -> bool {
244+
self.parent_scope_id(scope_id)
245+
.is_none_or(|parent_scope_id| {
246+
if !self.is_scope_reachable(db, parent_scope_id) {
247+
return false;
248+
}
249+
250+
let parent_use_def = self.use_def_map(parent_scope_id);
251+
let reachability = self.scope(scope_id).reachability();
252+
253+
parent_use_def.is_reachable(db, reachability)
254+
})
255+
}
256+
257+
/// Returns true if a given 'use' of a symbol is reachable from the start of the scope.
258+
/// For example, in the following code, use `2` is reachable, but `1` and `3` are not:
259+
/// ```py
260+
/// def f():
261+
/// x = 1
262+
/// if False:
263+
/// x # 1
264+
/// x # 2
265+
/// return
266+
/// x # 3
267+
/// ```
268+
pub(crate) fn is_symbol_use_reachable(
269+
&self,
270+
db: &'db dyn crate::Db,
271+
scope_id: FileScopeId,
272+
use_id: ScopedUseId,
273+
) -> bool {
274+
self.is_scope_reachable(db, scope_id)
275+
&& self
276+
.use_def_map(scope_id)
277+
.is_symbol_use_reachable(db, use_id)
278+
}
279+
243280
/// Returns an iterator over the descendent scopes of `scope`.
244281
#[allow(unused)]
245282
pub(crate) fn descendent_scopes(&self, scope: FileScopeId) -> DescendantsIter {

crates/red_knot_python_semantic/src/semantic_index/builder.rs

Lines changed: 19 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -129,7 +129,11 @@ impl<'db> SemanticIndexBuilder<'db> {
129129
eager_bindings: FxHashMap::default(),
130130
};
131131

132-
builder.push_scope_with_parent(NodeWithScopeRef::Module, None);
132+
builder.push_scope_with_parent(
133+
NodeWithScopeRef::Module,
134+
None,
135+
ScopedVisibilityConstraintId::ALWAYS_TRUE,
136+
);
133137

134138
builder
135139
}
@@ -191,17 +195,28 @@ impl<'db> SemanticIndexBuilder<'db> {
191195

192196
fn push_scope(&mut self, node: NodeWithScopeRef) {
193197
let parent = self.current_scope();
194-
self.push_scope_with_parent(node, Some(parent));
198+
let reachabililty = self.current_use_def_map().reachability;
199+
self.push_scope_with_parent(node, Some(parent), reachabililty);
195200
}
196201

197-
fn push_scope_with_parent(&mut self, node: NodeWithScopeRef, parent: Option<FileScopeId>) {
202+
fn push_scope_with_parent(
203+
&mut self,
204+
node: NodeWithScopeRef,
205+
parent: Option<FileScopeId>,
206+
reachability: ScopedVisibilityConstraintId,
207+
) {
198208
let children_start = self.scopes.next_index() + 1;
199209

200210
// SAFETY: `node` is guaranteed to be a child of `self.module`
201211
#[allow(unsafe_code)]
202212
let node_with_kind = unsafe { node.to_kind(self.module.clone()) };
203213

204-
let scope = Scope::new(parent, node_with_kind, children_start..children_start);
214+
let scope = Scope::new(
215+
parent,
216+
node_with_kind,
217+
children_start..children_start,
218+
reachability,
219+
);
205220
self.try_node_context_stack_manager.enter_nested_scope();
206221

207222
let file_scope_id = self.scopes.push(scope);

crates/red_knot_python_semantic/src/semantic_index/symbol.rs

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ use rustc_hash::FxHasher;
1212

1313
use crate::ast_node_ref::AstNodeRef;
1414
use crate::node_key::NodeKey;
15+
use crate::semantic_index::visibility_constraints::ScopedVisibilityConstraintId;
1516
use crate::semantic_index::{semantic_index, SymbolMap};
1617
use crate::Db;
1718

@@ -176,18 +177,21 @@ pub struct Scope {
176177
parent: Option<FileScopeId>,
177178
node: NodeWithScopeKind,
178179
descendants: Range<FileScopeId>,
180+
reachability: ScopedVisibilityConstraintId,
179181
}
180182

181183
impl Scope {
182184
pub(super) fn new(
183185
parent: Option<FileScopeId>,
184186
node: NodeWithScopeKind,
185187
descendants: Range<FileScopeId>,
188+
reachability: ScopedVisibilityConstraintId,
186189
) -> Self {
187190
Scope {
188191
parent,
189192
node,
190193
descendants,
194+
reachability,
191195
}
192196
}
193197

@@ -214,6 +218,10 @@ impl Scope {
214218
pub(crate) fn is_eager(&self) -> bool {
215219
self.kind().is_eager()
216220
}
221+
222+
pub(crate) fn reachability(&self) -> ScopedVisibilityConstraintId {
223+
self.reachability
224+
}
217225
}
218226

219227
#[derive(Copy, Clone, Debug, PartialEq, Eq)]

crates/red_knot_python_semantic/src/semantic_index/use_def.rs

Lines changed: 11 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -348,24 +348,21 @@ impl<'db> UseDefMap<'db> {
348348
self.bindings_iterator(&self.bindings_by_use[use_id])
349349
}
350350

351-
/// Returns true if a given 'use' of a symbol is reachable from the start of the scope.
352-
/// For example, in the following code, use `2` is reachable, but `1` and `3` are not:
353-
/// ```py
354-
/// def f():
355-
/// x = 1
356-
/// if False:
357-
/// x # 1
358-
/// x # 2
359-
/// return
360-
/// x # 3
361-
/// ```
362-
pub(crate) fn is_symbol_use_reachable(&self, db: &dyn crate::Db, use_id: ScopedUseId) -> bool {
351+
pub(super) fn is_reachable(
352+
&self,
353+
db: &dyn crate::Db,
354+
reachability: ScopedVisibilityConstraintId,
355+
) -> bool {
363356
!self
364357
.visibility_constraints
365-
.evaluate(db, &self.predicates, self.reachability_by_use[use_id])
358+
.evaluate(db, &self.predicates, reachability)
366359
.is_always_false()
367360
}
368361

362+
pub(super) fn is_symbol_use_reachable(&self, db: &dyn crate::Db, use_id: ScopedUseId) -> bool {
363+
self.is_reachable(db, self.reachability_by_use[use_id])
364+
}
365+
369366
pub(crate) fn public_bindings(
370367
&self,
371368
symbol: ScopedSymbolId,
@@ -618,7 +615,7 @@ pub(super) struct UseDefMapBuilder<'db> {
618615
/// ```
619616
/// Depending on the value of `test`, the `y = 1`, `y = 2`, or both bindings may be visible.
620617
/// The use of `x` is recorded with a reachability constraint of `[test]`.
621-
reachability: ScopedVisibilityConstraintId,
618+
pub(super) reachability: ScopedVisibilityConstraintId,
622619

623620
/// Tracks whether or not a given use of a symbol is reachable from the start of the scope.
624621
reachability_by_use: IndexVec<ScopedUseId, ScopedVisibilityConstraintId>,

crates/red_knot_python_semantic/src/types/infer.rs

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4302,7 +4302,9 @@ impl<'db> TypeInferenceBuilder<'db> {
43024302
} else {
43034303
let use_id = name_node.scoped_use_id(db, scope);
43044304
let symbol = symbol_from_bindings(db, use_def.bindings_at_use(use_id));
4305-
let report_unresolved_usage = use_def.is_symbol_use_reachable(db, use_id);
4305+
let report_unresolved_usage =
4306+
self.index
4307+
.is_symbol_use_reachable(db, file_scope_id, use_id);
43064308
(symbol, report_unresolved_usage)
43074309
};
43084310

0 commit comments

Comments
 (0)