From 450d4e0e0c0fae3fed1286f7cc13f201efeba685 Mon Sep 17 00:00:00 2001 From: Auguste Lalande Date: Wed, 8 Jan 2025 10:45:04 -0500 Subject: [PATCH] [`pylint`] Fix `unreachable` infinite loop (`PLW0101`) (#15278) ## Summary Fix infinite loop issue reported here #15248. The issue was caused by the break inside the if block, which caused the flow to exit in an unforeseen way. This caused other issues, eventually leading to an infinite loop. Resolves #15248. Resolves #15336. ## Test Plan Added failing code to fixture. --------- Co-authored-by: Micha Reiser Co-authored-by: dylwil3 --- .../test/fixtures/control-flow-graph/try.py | 12 +++ .../test/fixtures/control-flow-graph/while.py | 9 +++ ..._rules__unreachable__tests__try.py.md.snap | 73 +++++++++++++++++++ ...ules__unreachable__tests__while.py.md.snap | 60 +++++++++++++++ .../src/rules/pylint/rules/unreachable.rs | 2 + 5 files changed, 156 insertions(+) diff --git a/crates/ruff_linter/resources/test/fixtures/control-flow-graph/try.py b/crates/ruff_linter/resources/test/fixtures/control-flow-graph/try.py index 24c43ef1ee..9eeaf1811d 100644 --- a/crates/ruff_linter/resources/test/fixtures/control-flow-graph/try.py +++ b/crates/ruff_linter/resources/test/fixtures/control-flow-graph/try.py @@ -116,3 +116,15 @@ def func(): finally: raise Exception("other") + +# previously caused infinite loop +# found by fuzzer +def func(): + for i in(): + try: + try: + while r: + if t:break + finally:() + return + except:l diff --git a/crates/ruff_linter/resources/test/fixtures/control-flow-graph/while.py b/crates/ruff_linter/resources/test/fixtures/control-flow-graph/while.py index 11d3f6164c..af509129c3 100644 --- a/crates/ruff_linter/resources/test/fixtures/control-flow-graph/while.py +++ b/crates/ruff_linter/resources/test/fixtures/control-flow-graph/while.py @@ -145,3 +145,12 @@ def bokeh2(self, host: str = DEFAULT_HOST, port: int = DEFAULT_PORT) -> None: port += 1 self.thread = threading.Thread(target=self._run_web_server) + +def func(): + 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.py.md.snap b/crates/ruff_linter/src/rules/pylint/rules/snapshots/ruff_linter__rules__pylint__rules__unreachable__tests__try.py.md.snap index 47720ae837..9d38653a79 100644 --- a/crates/ruff_linter/src/rules/pylint/rules/snapshots/ruff_linter__rules__pylint__rules__unreachable__tests__try.py.md.snap +++ b/crates/ruff_linter/src/rules/pylint/rules/snapshots/ruff_linter__rules__pylint__rules__unreachable__tests__try.py.md.snap @@ -708,3 +708,76 @@ flowchart TD block1 --> return block0 --> return ``` + +## Function 13 +### Source +```python +def func(): + for i in(): + try: + try: + while r: + if t:break + finally:() + return + except:l +``` + +### Control Flow Graph +```mermaid +flowchart TD + start(("Start")) + return(("End")) + block0[["`*(empty)*`"]] + block1[["Loop continue"]] + block2["return\n"] + block3["()\n"] + block4[["Loop continue"]] + block5["break\n"] + block6["if t:break\n"] + block7["while r: + if t:break\n"] + block8[["Exception raised"]] + block9["try: + while r: + if t:break + finally:()\n"] + block10[["Exception raised"]] + block11["l\n"] + block12["try: + try: + while r: + if t:break + finally:() + return + except:l\n"] + block13["for i in(): + try: + try: + while r: + if t:break + finally:() + return + except:l\n"] + + start --> block13 + block13 -- "()" --> block12 + block13 -- "else" --> block0 + block12 -- "Exception raised" --> block11 + block12 -- "else" --> block9 + block11 --> block1 + block10 --> return + block9 -- "Exception raised" --> block8 + block9 -- "else" --> block7 + block8 --> block3 + block7 -- "r" --> block6 + block7 -- "else" --> block3 + block6 -- "t" --> block5 + block6 -- "else" --> block4 + block5 --> block3 + block4 --> block7 + block3 --> block2 + block2 --> return + block1 --> block13 + block0 --> return +``` diff --git a/crates/ruff_linter/src/rules/pylint/rules/snapshots/ruff_linter__rules__pylint__rules__unreachable__tests__while.py.md.snap b/crates/ruff_linter/src/rules/pylint/rules/snapshots/ruff_linter__rules__pylint__rules__unreachable__tests__while.py.md.snap index 5e05666e8f..9ea1454a70 100644 --- a/crates/ruff_linter/src/rules/pylint/rules/snapshots/ruff_linter__rules__pylint__rules__unreachable__tests__while.py.md.snap +++ b/crates/ruff_linter/src/rules/pylint/rules/snapshots/ruff_linter__rules__pylint__rules__unreachable__tests__while.py.md.snap @@ -777,3 +777,63 @@ flowchart TD block1 --> block7 block0 --> return ``` + +## Function 23 +### Source +```python +def func(): + 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 7f6d498293..c6beb2e67a 100644 --- a/crates/ruff_linter/src/rules/pylint/rules/unreachable.rs +++ b/crates/ruff_linter/src/rules/pylint/rules/unreachable.rs @@ -462,6 +462,7 @@ fn post_process_loop( Some(Stmt::Continue(_)) => { block.next = NextBlock::Always(loop_start); } + Some(Stmt::Return(_)) => return, _ => {} }; idx = next; @@ -603,6 +604,7 @@ fn post_process_try( NextBlock::Always(next) => { next_index = *next; match block.stmts.last() { + Some(Stmt::Break(_)) => return, Some(Stmt::Continue(_)) => return, Some(Stmt::Raise(_)) => { // re-route to except if not already re-routed