Cyclic0007/ruff
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

WIP: defer walking function bodies to end-of-scope in SemanticIndexBuilder

Browse Source 
This is intended to fix the one failing test from the previous commit.
And it actually does fix it! But it also causes a huge number of other
tests to fail. The minimized repro seems to be this:

```
$ cat test.py
class Foo:
    pass
foo = Foo()
$ ty check test.py
error[panic]: Panicked at crates/ty_python_semantic/src/types.rs:156:38 when checking `/tmp/test.py`: `Failed to retrieve the inferred type for an `ast::Expr` node passed to `TypeInference::expression_type()`. The `TypeInferenceBuilder` should infer and store types for all `ast::Expr` nodes in any `TypeInference` region it analyzes.`
info: This indicates a bug in ty.
info: If you could open an issue at https://github.com/astral-sh/ty/issues/new?title=%5Bpanic%5D, we'd be very appreciative!
info: Platform: linux x86_64
info: Args: ["/home/jacko/astral/ruff/target-mold/debug/ty", "check", "test.py"]
info: run with `RUST_BACKTRACE=1` environment variable to show the full backtrace information
info: query stacktrace:
   0: FunctionType < 'db >::signature_(Id(5007))
             at crates/ty_python_semantic/src/types/function.rs:595
             cycle heads: infer_scope_types(Id(c62)) -> IterationCount(0), FunctionType < 'db >::signature_(Id(5007)) -> IterationCount(0), FunctionType < 'db >::signature_(Id(5000)) -> IterationCount(0)
   1: infer_expression_types(Id(1463))
             at crates/ty_python_semantic/src/types/infer.rs:235
   2: infer_definition_types(Id(11ab))
             at crates/ty_python_semantic/src/types/infer.rs:159
   3: infer_scope_types(Id(c62))
             at crates/ty_python_semantic/src/types/infer.rs:130
             cycle heads: infer_scope_types(Id(c62)) -> IterationCount(0)
   4: FunctionType < 'db >::signature_(Id(5000))
             at crates/ty_python_semantic/src/types/function.rs:595
   5: infer_expression_types(Id(1400))
             at crates/ty_python_semantic/src/types/infer.rs:235
   6: infer_definition_types(Id(1001))
             at crates/ty_python_semantic/src/types/infer.rs:159
   7: infer_scope_types(Id(c00))
             at crates/ty_python_semantic/src/types/infer.rs:130
   8: check_file_impl(Id(800))
             at crates/ty_project/src/lib.rs:474
```
This commit is contained in:
Jack O'Connor
2025-07-10 14:20:44 -07:00
parent 05cf7c3458
commit 3f22497bdd
1 changed files with 86 additions and 35 deletions
Download Patch File Download Diff File Expand all files Collapse all files

121
crates/ty_python_semantic/src/semantic_index/builder.rs
View File

@@ -83,6 +83,8 @@ pub(super) struct SemanticIndexBuilder<'db, 'ast> {
current_match_case: Option<CurrentMatchCase<'ast>>,
/// The name of the first function parameter of the innermost function that we're currently visiting.
current_first_parameter_name: Option<&'ast str>,
/// Functions defined in the current scope. We walk their bodies at the end of the scope.
deferred_function_bodies: Vec<&'ast ast::StmtFunctionDef>,
/// Per-scope contexts regarding nested `try`/`except` statements
try_node_context_stack_manager: TryNodeContextStackManager,
@@ -126,6 +128,7 @@ impl<'db, 'ast> SemanticIndexBuilder<'db, 'ast> {
current_assignments: vec![],
current_match_case: None,
current_first_parameter_name: None,
deferred_function_bodies: Vec::new(),
try_node_context_stack_manager: TryNodeContextStackManager::default(),
has_future_annotations: false,
@@ -1007,8 +1010,83 @@ impl<'db, 'ast> SemanticIndexBuilder<'db, 'ast> {
}
}
fn visit_function_body(&mut self, function_def: &'ast ast::StmtFunctionDef) {
let ast::StmtFunctionDef {
parameters,
type_params,
returns,
body,
..
} = function_def;
self.with_type_params(
NodeWithScopeRef::FunctionTypeParameters(function_def),
type_params.as_deref(),
|builder| {
builder.visit_parameters(parameters);
if let Some(returns) = returns {
builder.visit_annotation(returns);
}
builder.push_scope(NodeWithScopeRef::Function(function_def));
builder.declare_parameters(parameters);
let mut first_parameter_name = parameters
.iter_non_variadic_params()
.next()
.map(|first_param| first_param.parameter.name.id().as_str());
std::mem::swap(
&mut builder.current_first_parameter_name,
&mut first_parameter_name,
);
builder.visit_scoped_body(body);
builder.current_first_parameter_name = first_parameter_name;
builder.pop_scope()
},
);
}
/// Walk the body of a scope, either the global scope or a function scope.
///
/// When we encounter a (top-level or nested) function definition, we add the function's name
/// to the current scope, but we defer walking its body until the end. (See the `FunctionDef`
/// branch of `visit_stmt`.) This deferred approach is necessary to be able to check `nonlocal`
/// statements as we encounter them, for example:
///
/// ```py
/// def f():
/// def g():
/// nonlocal x # allowed
/// nonlocal y # SyntaxError: no binding for nonlocal 'y' found
/// x = 1
/// ```
///
/// See the comments in the `Nonlocal` branch of `visit_stmt`, which relies on this binding
/// information being present.
fn visit_scoped_body(&mut self, body: &'ast [ast::Stmt]) {
debug_assert!(
self.deferred_function_bodies.is_empty(),
"every function starts with a clean scope",
);
// If this scope contains function definitions, they'll be added to
// `self.deferred_function_bodies` as we walk each statement.
self.visit_body(body);
// Now that we've walked all the statements in this scope, walk any deferred function
// bodies. This is recursive, so we need to clear out the contents of
// `self.deferred_function_bodies` and give each function a fresh list (or else we'll fail
// the `debug_assert!` above).
let taken_deferred_function_bodies = std::mem::take(&mut self.deferred_function_bodies);
for function_def in taken_deferred_function_bodies {
self.visit_function_body(function_def);
}
}
pub(super) fn build(mut self) -> SemanticIndex<'db> {
self.visit_body(self.module.suite());
self.visit_scoped_body(self.module.suite());
// Pop the root scope
self.pop_scope();
@@ -1085,46 +1163,19 @@ impl<'ast> Visitor<'ast> for SemanticIndexBuilder<'_, 'ast> {
let ast::StmtFunctionDef {
decorator_list,
parameters,
type_params,
name,
returns,
body,
is_async: _,
range: _,
node_index: _,
..
} = function_def;
// Like Ruff, we don't walk the body of the function here. Instead, we defer it to
// the end of the current scope. See `visit_scoped_body`. See also the comments in
// the `Nonlocal` branch below about why this deferred visit order is necessary.
self.deferred_function_bodies.push(function_def);
for decorator in decorator_list {
self.visit_decorator(decorator);
}
self.with_type_params(
NodeWithScopeRef::FunctionTypeParameters(function_def),
type_params.as_deref(),
|builder| {
builder.visit_parameters(parameters);
if let Some(returns) = returns {
builder.visit_annotation(returns);
}
builder.push_scope(NodeWithScopeRef::Function(function_def));
builder.declare_parameters(parameters);
let mut first_parameter_name = parameters
.iter_non_variadic_params()
.next()
.map(|first_param| first_param.parameter.name.id().as_str());
std::mem::swap(
&mut builder.current_first_parameter_name,
&mut first_parameter_name,
);
builder.visit_body(body);
builder.current_first_parameter_name = first_parameter_name;
builder.pop_scope()
},
);
// The default value of the parameters needs to be evaluated in the
// enclosing scope.
for default in parameters