Skip to content

Commit

Permalink
[internal] Return statements in finally block point to end block fo…
Browse files Browse the repository at this point in the history
…r `unreachable` (`PLW0101`) (#15276)

Note: `PLW0101` remains in testing rather than preview, so this PR does
not modify any public behavior (hence the title beginning with
`internal` rather than `pylint`, for the sake of the changelog.)

Fixes an error in the processing of `try` statements in the control flow
graph builder.

When processing a try statement, the block following a `return` was
forced to point to the `finally` block. However, if the return was _in_
the `finally` block, this caused the block to point to itself. In the
case where the whole `try-finally` statement was also included inside of
a loop, this caused an infinite loop in the builder for the control flow
graph as it attempted to resolve edges.

Closes #15248

## Test function
### Source
```python
def l():
    while T:
        try:
            while ():
                if 3:
                    break
        finally:
            return
```

### Control Flow Graph
```mermaid
flowchart TD
  start(("Start"))
  return(("End"))
  block0[["`*(empty)*`"]]
  block1[["Loop continue"]]
  block2["return\n"]
  block3[["Loop continue"]]
  block4["break\n"]
  block5["if 3:
                    break\n"]
  block6["while ():
                if 3:
                    break\n"]
  block7[["Exception raised"]]
  block8["try:
            while ():
                if 3:
                    break
        finally:
            return\n"]
  block9["while T:
        try:
            while ():
                if 3:
                    break
        finally:
            return\n"]
  start --> block9
  block9 -- "T" --> block8
  block9 -- "else" --> block0
  block8 -- "Exception raised" --> block7
  block8 -- "else" --> block6
  block7 --> block2
  block6 -- "()" --> block5
  block6 -- "else" --> block2
  block5 -- "3" --> block4
  block5 -- "else" --> block3
  block4 --> block2
  block3 --> block6
  block2 --> return
  block1 --> block9
  block0 --> return
```
  • Loading branch information
dylwil3 authored Jan 7, 2025
1 parent e413956 commit a876090
Show file tree
Hide file tree
Showing 3 changed files with 77 additions and 0 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
def l():
while T:
try:
while ():
if 3:
break
finally:
return

Original file line number Diff line number Diff line change
@@ -0,0 +1,63 @@
---
source: crates/ruff_linter/src/rules/pylint/rules/unreachable.rs
description: "This is a Mermaid graph. You can use https://mermaid.live to visualize it as a diagram."
---
## Function 0
### Source
```python
def l():
while T:
try:
while ():
if 3:
break
finally:
return
```

### Control Flow Graph
```mermaid
flowchart TD
start(("Start"))
return(("End"))
block0[["`*(empty)*`"]]
block1[["Loop continue"]]
block2["return\n"]
block3[["Loop continue"]]
block4["break\n"]
block5["if 3:
break\n"]
block6["while ():
if 3:
break\n"]
block7[["Exception raised"]]
block8["try:
while ():
if 3:
break
finally:
return\n"]
block9["while T:
try:
while ():
if 3:
break
finally:
return\n"]
start --> block9
block9 -- "T" --> block8
block9 -- "else" --> block0
block8 -- "Exception raised" --> block7
block8 -- "else" --> block6
block7 --> block2
block6 -- "()" --> block5
block6 -- "else" --> block2
block5 -- "3" --> block4
block5 -- "else" --> block3
block4 --> block2
block3 --> block6
block2 --> return
block1 --> block9
block0 --> return
```
5 changes: 5 additions & 0 deletions crates/ruff_linter/src/rules/pylint/rules/unreachable.rs
Original file line number Diff line number Diff line change
Expand Up @@ -665,6 +665,10 @@ fn post_process_try(
NextBlock::Terminate => {
match block.stmts.last() {
Some(Stmt::Return(_)) => {
// if we are already in a `finally` block, terminate
if Some(idx) == finally_index {
return;
}
// re-route to finally if present and not already re-routed
if let Some(finally_index) = finally_index {
blocks.blocks[idx].next = NextBlock::Always(finally_index);
Expand Down Expand Up @@ -1064,6 +1068,7 @@ mod tests {
#[test_case("raise.py")]
#[test_case("assert.py")]
#[test_case("match.py")]
#[test_case("try-finally-nested-if-while.py")]
fn control_flow_graph(filename: &str) {
let path = PathBuf::from_iter(["resources/test/fixtures/control-flow-graph", filename]);
let source = fs::read_to_string(path).expect("failed to read file");
Expand Down

0 comments on commit a876090

Please sign in to comment.