[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.
This commit is contained in:
David Peter 2025-04-10 13:56:40 +02:00 committed by GitHub
parent 06ffeb2e09
commit 4d50ee6f52
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
6 changed files with 79 additions and 24 deletions

View file

@ -423,14 +423,10 @@ if False:
x = 1 x = 1
def f(): def f():
# TODO
# error: [unresolved-reference]
print(x) print(x)
class C: class C:
def __init__(self): def __init__(self):
# TODO
# error: [unresolved-reference]
print(x) print(x)
``` ```

View file

@ -11,7 +11,7 @@ use salsa::Update;
use crate::module_name::ModuleName; use crate::module_name::ModuleName;
use crate::semantic_index::ast_ids::node_key::ExpressionNodeKey; 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::attribute_assignment::AttributeAssignments;
use crate::semantic_index::builder::SemanticIndexBuilder; use crate::semantic_index::builder::SemanticIndexBuilder;
use crate::semantic_index::definition::{Definition, DefinitionNodeKey, Definitions}; 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)?]) 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`. /// Returns an iterator over the descendent scopes of `scope`.
#[allow(unused)] #[allow(unused)]
pub(crate) fn descendent_scopes(&self, scope: FileScopeId) -> DescendantsIter { pub(crate) fn descendent_scopes(&self, scope: FileScopeId) -> DescendantsIter {

View file

@ -129,7 +129,11 @@ impl<'db> SemanticIndexBuilder<'db> {
eager_bindings: FxHashMap::default(), eager_bindings: FxHashMap::default(),
}; };
builder.push_scope_with_parent(NodeWithScopeRef::Module, None); builder.push_scope_with_parent(
NodeWithScopeRef::Module,
None,
ScopedVisibilityConstraintId::ALWAYS_TRUE,
);
builder builder
} }
@ -191,17 +195,28 @@ impl<'db> SemanticIndexBuilder<'db> {
fn push_scope(&mut self, node: NodeWithScopeRef) { fn push_scope(&mut self, node: NodeWithScopeRef) {
let parent = self.current_scope(); 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<FileScopeId>) { fn push_scope_with_parent(
&mut self,
node: NodeWithScopeRef,
parent: Option<FileScopeId>,
reachability: ScopedVisibilityConstraintId,
) {
let children_start = self.scopes.next_index() + 1; let children_start = self.scopes.next_index() + 1;
// SAFETY: `node` is guaranteed to be a child of `self.module` // SAFETY: `node` is guaranteed to be a child of `self.module`
#[allow(unsafe_code)] #[allow(unsafe_code)]
let node_with_kind = unsafe { node.to_kind(self.module.clone()) }; 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(); self.try_node_context_stack_manager.enter_nested_scope();
let file_scope_id = self.scopes.push(scope); let file_scope_id = self.scopes.push(scope);

View file

@ -12,6 +12,7 @@ use rustc_hash::FxHasher;
use crate::ast_node_ref::AstNodeRef; use crate::ast_node_ref::AstNodeRef;
use crate::node_key::NodeKey; use crate::node_key::NodeKey;
use crate::semantic_index::visibility_constraints::ScopedVisibilityConstraintId;
use crate::semantic_index::{semantic_index, SymbolMap}; use crate::semantic_index::{semantic_index, SymbolMap};
use crate::Db; use crate::Db;
@ -176,6 +177,7 @@ pub struct Scope {
parent: Option<FileScopeId>, parent: Option<FileScopeId>,
node: NodeWithScopeKind, node: NodeWithScopeKind,
descendants: Range<FileScopeId>, descendants: Range<FileScopeId>,
reachability: ScopedVisibilityConstraintId,
} }
impl Scope { impl Scope {
@ -183,11 +185,13 @@ impl Scope {
parent: Option<FileScopeId>, parent: Option<FileScopeId>,
node: NodeWithScopeKind, node: NodeWithScopeKind,
descendants: Range<FileScopeId>, descendants: Range<FileScopeId>,
reachability: ScopedVisibilityConstraintId,
) -> Self { ) -> Self {
Scope { Scope {
parent, parent,
node, node,
descendants, descendants,
reachability,
} }
} }
@ -214,6 +218,10 @@ impl Scope {
pub(crate) fn is_eager(&self) -> bool { pub(crate) fn is_eager(&self) -> bool {
self.kind().is_eager() self.kind().is_eager()
} }
pub(crate) fn reachability(&self) -> ScopedVisibilityConstraintId {
self.reachability
}
} }
#[derive(Copy, Clone, Debug, PartialEq, Eq)] #[derive(Copy, Clone, Debug, PartialEq, Eq)]

View file

@ -348,24 +348,21 @@ impl<'db> UseDefMap<'db> {
self.bindings_iterator(&self.bindings_by_use[use_id]) 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. pub(super) fn is_reachable(
/// For example, in the following code, use `2` is reachable, but `1` and `3` are not: &self,
/// ```py db: &dyn crate::Db,
/// def f(): reachability: ScopedVisibilityConstraintId,
/// x = 1 ) -> bool {
/// 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 {
!self !self
.visibility_constraints .visibility_constraints
.evaluate(db, &self.predicates, self.reachability_by_use[use_id]) .evaluate(db, &self.predicates, reachability)
.is_always_false() .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( pub(crate) fn public_bindings(
&self, &self,
symbol: ScopedSymbolId, 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. /// 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]`. /// 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. /// Tracks whether or not a given use of a symbol is reachable from the start of the scope.
reachability_by_use: IndexVec<ScopedUseId, ScopedVisibilityConstraintId>, reachability_by_use: IndexVec<ScopedUseId, ScopedVisibilityConstraintId>,

View file

@ -4302,7 +4302,9 @@ impl<'db> TypeInferenceBuilder<'db> {
} else { } else {
let use_id = name_node.scoped_use_id(db, scope); let use_id = name_node.scoped_use_id(db, scope);
let symbol = symbol_from_bindings(db, use_def.bindings_at_use(use_id)); 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) (symbol, report_unresolved_usage)
}; };