Skip to content

Commit

Permalink
Use a visitor
Browse files Browse the repository at this point in the history
  • Loading branch information
charliermarsh committed Aug 14, 2023
1 parent 92eea2a commit 14f222f
Show file tree
Hide file tree
Showing 2 changed files with 36 additions and 10 deletions.
9 changes: 9 additions & 0 deletions crates/ruff/resources/test/fixtures/perflint/PERF203.py
Original file line number Diff line number Diff line change
Expand Up @@ -38,3 +38,12 @@
except:
print("error")


# OK - no other way to write this
for i in range(10):
try:
print(f"{i}")
if i > 0:
break
except:
print("error")
37 changes: 27 additions & 10 deletions crates/ruff/src/rules/perflint/rules/try_except_in_loop.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
use ruff_python_ast::{self as ast, Ranged, Stmt};

use ruff_diagnostics::{Diagnostic, Violation};
use ruff_macros::{derive_message_formats, violation};
use ruff_python_ast::statement_visitor::{walk_stmt, StatementVisitor};
use ruff_python_ast::{self as ast, Ranged, Stmt};

use crate::checkers::ast::Checker;
use crate::settings::types::PythonVersion;
Expand Down Expand Up @@ -35,6 +35,7 @@ use crate::settings::types::PythonVersion;
/// int_numbers.append(int(num))
/// except ValueError as e:
/// print(f"Couldn't convert to integer: {e}")
/// break
/// ```
///
/// Use instead:
Expand Down Expand Up @@ -67,24 +68,40 @@ pub(crate) fn try_except_in_loop(checker: &mut Checker, body: &[Stmt]) {
return;
}

let [Stmt::Try(ast::StmtTry { handlers, body: try_body, .. })] = body else {
let [Stmt::Try(ast::StmtTry { handlers, body, .. })] = body else {
return;
};

let Some(handler) = handlers.first() else {
return;
};

// Parse all nodes underneath the `try` - if we see any loop control flow statements, we don't
// want to throw.
for stmt in try_body {
match stmt {
Stmt::Continue(_) | Stmt::Break(_) => return,
_ => (),
}
// Avoid flagging `try`-`except` blocks that contain `break` or `continue`,
// which rely on the exception handling mechanism.
let mut visitor = LoopControlFlowVisitor::default();
visitor.visit_body(body);
if visitor.has_break_or_continue {
return;
}

checker
.diagnostics
.push(Diagnostic::new(TryExceptInLoop, handler.range()));
}

#[derive(Debug, Default)]
struct LoopControlFlowVisitor {
has_break_or_continue: bool,
}

impl StatementVisitor<'_> for LoopControlFlowVisitor {
fn visit_stmt(&mut self, stmt: &Stmt) {
match stmt {
Stmt::Break(_) | Stmt::Continue(_) => self.has_break_or_continue = true,
Stmt::FunctionDef(_) | Stmt::ClassDef(_) => {
// Don't recurse.
}
_ => walk_stmt(self, stmt),
}
}
}

0 comments on commit 14f222f

Please sign in to comment.