diff --git a/crates/ruff_linter/src/rules/ruff/rules/snapshots/ruff_linter__rules__ruff__rules__unreachable__tests__while.py.md.snap b/crates/ruff_linter/src/rules/ruff/rules/snapshots/ruff_linter__rules__ruff__rules__unreachable__tests__while.py.md.snap index d285347e626367..7acb269a366ce5 100644 --- a/crates/ruff_linter/src/rules/ruff/rules/snapshots/ruff_linter__rules__ruff__rules__unreachable__tests__while.py.md.snap +++ b/crates/ruff_linter/src/rules/ruff/rules/snapshots/ruff_linter__rules__ruff__rules__unreachable__tests__while.py.md.snap @@ -733,7 +733,11 @@ flowchart TD return(("End")) block0["self.thread = threading.Thread(target=self._run_web_server)\n"] block1[["Loop continue"]] - block2["try: + block2["self.server = HTTPServer((host, port), HtmlOnlyHandler)\nself.host = host\nself.port = port\nbreak\n"] + block3[["Exception raised"]] + block4["log.debug(f#quot;port {port} is in use, trying to next one#quot;)\nport += 1\n"] + block5["log.debug(f#quot;port {port} is in use, trying to next one#quot;)\nport += 1\n"] + block6["try: self.server = HTTPServer((host, port), HtmlOnlyHandler) self.host = host self.port = port @@ -741,7 +745,7 @@ flowchart TD except OSError: log.debug(f#quot;port {port} is in use, trying to next one#quot;) port += 1\n"] - block3["self.stop_serving = False\nwhile True: + block7["self.stop_serving = False\nwhile True: try: self.server = HTTPServer((host, port), HtmlOnlyHandler) self.host = host @@ -751,10 +755,16 @@ flowchart TD log.debug(f#quot;port {port} is in use, trying to next one#quot;) port += 1\n"] - start --> block3 - block3 -- "True" --> block2 - block3 -- "else" --> block0 - block2 --> block1 - block1 --> block3 + start --> block7 + block7 -- "True" --> block6 + block7 -- "else" --> block0 + block6 -- "Exception raised" --> block5 + block6 -- "else" --> block2 + block5 -- "OSError" --> block4 + block5 -- "else" --> block3 + block4 --> block1 + block3 --> return + block2 --> block0 + block1 --> block7 block0 --> return ``` diff --git a/crates/ruff_linter/src/rules/ruff/rules/unreachable.rs b/crates/ruff_linter/src/rules/ruff/rules/unreachable.rs index 32a14bf797c5a2..26a4846adadada 100644 --- a/crates/ruff_linter/src/rules/ruff/rules/unreachable.rs +++ b/crates/ruff_linter/src/rules/ruff/rules/unreachable.rs @@ -2,8 +2,9 @@ use std::{fmt, iter, usize}; use log::error; use ruff_python_ast::{ - Expr, ExprBooleanLiteral, Identifier, MatchCase, Pattern, PatternMatchAs, PatternMatchOr, Stmt, - StmtContinue, StmtFor, StmtMatch, StmtReturn, StmtTry, StmtWhile, StmtWith, + self as ast, Expr, ExprBooleanLiteral, Identifier, MatchCase, Pattern, PatternMatchAs, + PatternMatchOr, Stmt, StmtContinue, StmtFor, StmtMatch, StmtReturn, StmtTry, StmtWhile, + StmtWith, }; use ruff_text_size::{Ranged, TextRange, TextSize}; @@ -195,6 +196,8 @@ fn taken(condition: &Condition) -> Option { }, Condition::Iterator(_) => None, Condition::Match { .. } => None, + Condition::ExceptionCaught(_) => None, + Condition::ExceptionRaised => None, } } @@ -306,12 +309,18 @@ enum Condition<'stmt> { /// `case $case`, include pattern, guard, etc. case: &'stmt MatchCase, }, + // Exception was raised and caught by `except` clause + ExceptionCaught(&'stmt Expr), + // Exception was raise in a `try` block + ExceptionRaised, } impl<'stmt> Ranged for Condition<'stmt> { fn range(&self) -> TextRange { match self { - Condition::Test(expr) | Condition::Iterator(expr) => expr.range(), + Condition::Test(expr) + | Condition::Iterator(expr) + | Condition::ExceptionCaught(expr) => expr.range(), // The case of the match statement, without the body. Condition::Match { subject: _, case } => TextRange::new( case.start(), @@ -319,6 +328,7 @@ impl<'stmt> Ranged for Condition<'stmt> { .as_ref() .map_or(case.pattern.end(), |guard| guard.end()), ), + Condition::ExceptionRaised => TextRange::new(TextSize::new(0), TextSize::new(0)), } } } @@ -547,15 +557,53 @@ impl<'stmt> BasicBlocksBuilder<'stmt> { finalbody, .. }) => { - // TODO: handle `try` statements. The `try` control flow is very - // complex, what blocks are and aren't taken and from which - // block the control flow is actually returns is **very** - // specific to the contents of the block. Read - // - // very carefully. - // For now we'll skip over it. - let _ = (body, handlers, orelse, finalbody); // Silence unused code warnings. - self.unconditional_next_block(after) + let after_block = self.maybe_next_block_index(after, || true); + let finally_block = self.append_blocks_if_not_empty(finalbody, after_block); + let else_block = self.append_blocks_if_not_empty(orelse, finally_block); + let try_block = self.append_blocks_if_not_empty(body, else_block); + + // If an exception is raised and not caught then terminate with exception + let mut next_branch = self.fake_exception_block_index(); + + for handler in handlers.iter().rev() { + let ast::ExceptHandler::ExceptHandler(ast::ExceptHandlerExceptHandler { + body, + type_, + .. + }) = handler; + let condition = match type_ { + Some(type_) => Condition::ExceptionCaught(type_.as_ref()), + None => { + let exception = Box::leak(Box::new(Expr::Name(ast::ExprName { + range: TextRange::new(TextSize::new(0), TextSize::new(0)), + id: "BaseException".to_string(), + ctx: ast::ExprContext::Store, + }))); + + Condition::ExceptionCaught(exception) + } + }; + let except_block = self.append_blocks_if_not_empty(body, finally_block); + + let next = NextBlock::If { + condition, + next: except_block, + orelse: next_branch, + exit: after, + }; + let block = BasicBlock { stmts: body, next }; + next_branch = self.blocks.push(block) + } + + // We cannot know if the try statment will raise an exception (apart from explicit raise statements) + // We therefore assume that any statement can raise an exception. + // i.e. both paths may execute + NextBlock::If { + condition: Condition::ExceptionRaised, + next: next_branch, // If exception raised go to except -> except -> .. -> finally + orelse: try_block, // Otherwise try -> else -> finally + exit: after, + } } Stmt::With(StmtWith { items, body, .. }) => { // TODO: handle `with` statements, see @@ -1061,7 +1109,10 @@ impl<'stmt, 'source> fmt::Display for MermaidGraph<'stmt, 'source> { orelse, .. } => { - let condition_code = &self.source[condition.range()].trim(); + let condition_code = match condition { + Condition::ExceptionRaised => "Exception raised", + _ => &self.source[condition.range()].trim(), + }; writeln!( f, " block{i} -- \"{condition_code}\" --> block{next}",