From a876090715a18aa44afb4447ccf3e70a767f319a Mon Sep 17 00:00:00 2001 From: Dylan <53534755+dylwil3@users.noreply.github.com> Date: Tue, 7 Jan 2025 11:26:04 -0600 Subject: [PATCH] [`internal`] Return statements in finally block point to end block for `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 ``` --- .../try-finally-nested-if-while.py | 9 +++ ...ts__try-finally-nested-if-while.py.md.snap | 63 +++++++++++++++++++ .../src/rules/pylint/rules/unreachable.rs | 5 ++ 3 files changed, 77 insertions(+) create mode 100644 crates/ruff_linter/resources/test/fixtures/control-flow-graph/try-finally-nested-if-while.py create mode 100644 crates/ruff_linter/src/rules/pylint/rules/snapshots/ruff_linter__rules__pylint__rules__unreachable__tests__try-finally-nested-if-while.py.md.snap diff --git a/crates/ruff_linter/resources/test/fixtures/control-flow-graph/try-finally-nested-if-while.py b/crates/ruff_linter/resources/test/fixtures/control-flow-graph/try-finally-nested-if-while.py new file mode 100644 index 0000000000..2f0bca473a --- /dev/null +++ b/crates/ruff_linter/resources/test/fixtures/control-flow-graph/try-finally-nested-if-while.py @@ -0,0 +1,9 @@ +def l(): + while T: + try: + while (): + if 3: + break + finally: + return + diff --git a/crates/ruff_linter/src/rules/pylint/rules/snapshots/ruff_linter__rules__pylint__rules__unreachable__tests__try-finally-nested-if-while.py.md.snap b/crates/ruff_linter/src/rules/pylint/rules/snapshots/ruff_linter__rules__pylint__rules__unreachable__tests__try-finally-nested-if-while.py.md.snap new file mode 100644 index 0000000000..73ff72848b --- /dev/null +++ b/crates/ruff_linter/src/rules/pylint/rules/snapshots/ruff_linter__rules__pylint__rules__unreachable__tests__try-finally-nested-if-while.py.md.snap @@ -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 +``` diff --git a/crates/ruff_linter/src/rules/pylint/rules/unreachable.rs b/crates/ruff_linter/src/rules/pylint/rules/unreachable.rs index 4e489f4e22..7f6d498293 100644 --- a/crates/ruff_linter/src/rules/pylint/rules/unreachable.rs +++ b/crates/ruff_linter/src/rules/pylint/rules/unreachable.rs @@ -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); @@ -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");