mirror of
https://github.com/astral-sh/ruff.git
synced 2025-10-03 07:04:53 +00:00
[red-knot] Semantic index: handle invalid break
s (#14522)
## Summary This fix addresses panics related to invalid syntax like the following where a `break` statement is used in a nested definition inside a loop: ```py while True: def b(): x: int break ``` closes #14342 ## Test Plan * New corpus regression tests. * New unit test to make sure we handle nested while loops correctly. This test is passing on `main`, but can easily fail if the `is_inside_loop` state isn't properly saved/restored.
This commit is contained in:
parent
302fe76c2b
commit
f6b2cd5588
6 changed files with 90 additions and 4 deletions
|
@ -52,3 +52,29 @@ else:
|
||||||
reveal_type(x) # revealed: Literal[2, 3]
|
reveal_type(x) # revealed: Literal[2, 3]
|
||||||
reveal_type(y) # revealed: Literal[1, 2, 4]
|
reveal_type(y) # revealed: Literal[1, 2, 4]
|
||||||
```
|
```
|
||||||
|
|
||||||
|
## Nested while loops
|
||||||
|
|
||||||
|
```py
|
||||||
|
def flag() -> bool:
|
||||||
|
return True
|
||||||
|
|
||||||
|
x = 1
|
||||||
|
|
||||||
|
while flag():
|
||||||
|
x = 2
|
||||||
|
|
||||||
|
while flag():
|
||||||
|
x = 3
|
||||||
|
if flag():
|
||||||
|
break
|
||||||
|
else:
|
||||||
|
x = 4
|
||||||
|
|
||||||
|
if flag():
|
||||||
|
break
|
||||||
|
else:
|
||||||
|
x = 5
|
||||||
|
|
||||||
|
reveal_type(x) # revealed: Literal[3, 4, 5]
|
||||||
|
```
|
||||||
|
|
|
@ -36,12 +36,25 @@ use super::definition::{
|
||||||
|
|
||||||
mod except_handlers;
|
mod except_handlers;
|
||||||
|
|
||||||
|
/// Are we in a state where a `break` statement is allowed?
|
||||||
|
#[derive(Clone, Copy, Debug)]
|
||||||
|
enum LoopState {
|
||||||
|
InLoop,
|
||||||
|
NotInLoop,
|
||||||
|
}
|
||||||
|
|
||||||
|
impl LoopState {
|
||||||
|
fn is_inside(self) -> bool {
|
||||||
|
matches!(self, LoopState::InLoop)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
pub(super) struct SemanticIndexBuilder<'db> {
|
pub(super) struct SemanticIndexBuilder<'db> {
|
||||||
// Builder state
|
// Builder state
|
||||||
db: &'db dyn Db,
|
db: &'db dyn Db,
|
||||||
file: File,
|
file: File,
|
||||||
module: &'db ParsedModule,
|
module: &'db ParsedModule,
|
||||||
scope_stack: Vec<FileScopeId>,
|
scope_stack: Vec<(FileScopeId, LoopState)>,
|
||||||
/// The assignments we're currently visiting, with
|
/// The assignments we're currently visiting, with
|
||||||
/// the most recent visit at the end of the Vec
|
/// the most recent visit at the end of the Vec
|
||||||
current_assignments: Vec<CurrentAssignment<'db>>,
|
current_assignments: Vec<CurrentAssignment<'db>>,
|
||||||
|
@ -103,9 +116,24 @@ impl<'db> SemanticIndexBuilder<'db> {
|
||||||
*self
|
*self
|
||||||
.scope_stack
|
.scope_stack
|
||||||
.last()
|
.last()
|
||||||
|
.map(|(scope, _)| scope)
|
||||||
.expect("Always to have a root scope")
|
.expect("Always to have a root scope")
|
||||||
}
|
}
|
||||||
|
|
||||||
|
fn loop_state(&self) -> LoopState {
|
||||||
|
self.scope_stack
|
||||||
|
.last()
|
||||||
|
.expect("Always to have a root scope")
|
||||||
|
.1
|
||||||
|
}
|
||||||
|
|
||||||
|
fn set_inside_loop(&mut self, state: LoopState) {
|
||||||
|
self.scope_stack
|
||||||
|
.last_mut()
|
||||||
|
.expect("Always to have a root scope")
|
||||||
|
.1 = state;
|
||||||
|
}
|
||||||
|
|
||||||
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));
|
self.push_scope_with_parent(node, Some(parent));
|
||||||
|
@ -136,11 +164,11 @@ impl<'db> SemanticIndexBuilder<'db> {
|
||||||
|
|
||||||
debug_assert_eq!(ast_id_scope, file_scope_id);
|
debug_assert_eq!(ast_id_scope, file_scope_id);
|
||||||
|
|
||||||
self.scope_stack.push(file_scope_id);
|
self.scope_stack.push((file_scope_id, LoopState::NotInLoop));
|
||||||
}
|
}
|
||||||
|
|
||||||
fn pop_scope(&mut self) -> FileScopeId {
|
fn pop_scope(&mut self) -> FileScopeId {
|
||||||
let id = self.scope_stack.pop().expect("Root scope to be present");
|
let (id, _) = self.scope_stack.pop().expect("Root scope to be present");
|
||||||
let children_end = self.scopes.next_index();
|
let children_end = self.scopes.next_index();
|
||||||
let scope = &mut self.scopes[id];
|
let scope = &mut self.scopes[id];
|
||||||
scope.descendents = scope.descendents.start..children_end;
|
scope.descendents = scope.descendents.start..children_end;
|
||||||
|
@ -785,7 +813,10 @@ where
|
||||||
|
|
||||||
// TODO: definitions created inside the body should be fully visible
|
// TODO: definitions created inside the body should be fully visible
|
||||||
// to other statements/expressions inside the body --Alex/Carl
|
// to other statements/expressions inside the body --Alex/Carl
|
||||||
|
let outer_loop_state = self.loop_state();
|
||||||
|
self.set_inside_loop(LoopState::InLoop);
|
||||||
self.visit_body(body);
|
self.visit_body(body);
|
||||||
|
self.set_inside_loop(outer_loop_state);
|
||||||
|
|
||||||
// Get the break states from the body of this loop, and restore the saved outer
|
// Get the break states from the body of this loop, and restore the saved outer
|
||||||
// ones.
|
// ones.
|
||||||
|
@ -824,7 +855,9 @@ where
|
||||||
self.visit_body(body);
|
self.visit_body(body);
|
||||||
}
|
}
|
||||||
ast::Stmt::Break(_) => {
|
ast::Stmt::Break(_) => {
|
||||||
self.loop_break_states.push(self.flow_snapshot());
|
if self.loop_state().is_inside() {
|
||||||
|
self.loop_break_states.push(self.flow_snapshot());
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
ast::Stmt::For(
|
ast::Stmt::For(
|
||||||
|
@ -851,7 +884,10 @@ where
|
||||||
// TODO: Definitions created by loop variables
|
// TODO: Definitions created by loop variables
|
||||||
// (and definitions created inside the body)
|
// (and definitions created inside the body)
|
||||||
// are fully visible to other statements/expressions inside the body --Alex/Carl
|
// are fully visible to other statements/expressions inside the body --Alex/Carl
|
||||||
|
let outer_loop_state = self.loop_state();
|
||||||
|
self.set_inside_loop(LoopState::InLoop);
|
||||||
self.visit_body(body);
|
self.visit_body(body);
|
||||||
|
self.set_inside_loop(outer_loop_state);
|
||||||
|
|
||||||
let break_states =
|
let break_states =
|
||||||
std::mem::replace(&mut self.loop_break_states, saved_break_states);
|
std::mem::replace(&mut self.loop_break_states, saved_break_states);
|
||||||
|
|
|
@ -0,0 +1,6 @@
|
||||||
|
while True:
|
||||||
|
|
||||||
|
class A:
|
||||||
|
x: int
|
||||||
|
|
||||||
|
break
|
|
@ -0,0 +1,6 @@
|
||||||
|
while True:
|
||||||
|
|
||||||
|
def b():
|
||||||
|
x: int
|
||||||
|
|
||||||
|
break
|
|
@ -0,0 +1,6 @@
|
||||||
|
for _ in range(1):
|
||||||
|
|
||||||
|
class A:
|
||||||
|
x: int
|
||||||
|
|
||||||
|
break
|
|
@ -0,0 +1,6 @@
|
||||||
|
for _ in range(1):
|
||||||
|
|
||||||
|
def b():
|
||||||
|
x: int
|
||||||
|
|
||||||
|
break
|
Loading…
Add table
Add a link
Reference in a new issue