mirror of https://github.com/astral-sh/ruff
[`perflint`] Implement `try-except-in-loop` (`PERF203`) (#5166)
## Summary Implements PERF203 from #4789, which throws if a `try/except` block is inside of a loop. Not sure if we want to extend the diagnostic to the `except` as well, but I thought that that may get a little messy. We may also want to just throw on the word `try` - open to suggestions though. ## Test Plan `cargo test`
This commit is contained in:
parent
d53b986fd4
commit
190bed124f
|
|
@ -0,0 +1,28 @@
|
||||||
|
for i in range(10):
|
||||||
|
try: # PERF203
|
||||||
|
print(f"{i}")
|
||||||
|
except:
|
||||||
|
print("error")
|
||||||
|
|
||||||
|
try:
|
||||||
|
for i in range(10):
|
||||||
|
print(f"{i}")
|
||||||
|
except:
|
||||||
|
print("error")
|
||||||
|
|
||||||
|
i = 0
|
||||||
|
while i < 10: # PERF203
|
||||||
|
try:
|
||||||
|
print(f"{i}")
|
||||||
|
except:
|
||||||
|
print("error")
|
||||||
|
|
||||||
|
i += 1
|
||||||
|
|
||||||
|
try:
|
||||||
|
i = 0
|
||||||
|
while i < 10:
|
||||||
|
print(f"{i}")
|
||||||
|
i += 1
|
||||||
|
except:
|
||||||
|
print("error")
|
||||||
|
|
@ -1430,6 +1430,9 @@ where
|
||||||
if self.enabled(Rule::UselessElseOnLoop) {
|
if self.enabled(Rule::UselessElseOnLoop) {
|
||||||
pylint::rules::useless_else_on_loop(self, stmt, body, orelse);
|
pylint::rules::useless_else_on_loop(self, stmt, body, orelse);
|
||||||
}
|
}
|
||||||
|
if self.enabled(Rule::TryExceptInLoop) {
|
||||||
|
perflint::rules::try_except_in_loop(self, body);
|
||||||
|
}
|
||||||
}
|
}
|
||||||
Stmt::For(ast::StmtFor {
|
Stmt::For(ast::StmtFor {
|
||||||
target,
|
target,
|
||||||
|
|
@ -1477,6 +1480,9 @@ where
|
||||||
if self.enabled(Rule::InDictKeys) {
|
if self.enabled(Rule::InDictKeys) {
|
||||||
flake8_simplify::rules::key_in_dict_for(self, target, iter);
|
flake8_simplify::rules::key_in_dict_for(self, target, iter);
|
||||||
}
|
}
|
||||||
|
if self.enabled(Rule::TryExceptInLoop) {
|
||||||
|
perflint::rules::try_except_in_loop(self, body);
|
||||||
|
}
|
||||||
}
|
}
|
||||||
if self.enabled(Rule::IncorrectDictIterator) {
|
if self.enabled(Rule::IncorrectDictIterator) {
|
||||||
perflint::rules::incorrect_dict_iterator(self, target, iter);
|
perflint::rules::incorrect_dict_iterator(self, target, iter);
|
||||||
|
|
|
||||||
|
|
@ -786,6 +786,7 @@ pub fn code_to_rule(linter: Linter, code: &str) -> Option<(RuleGroup, Rule)> {
|
||||||
// perflint
|
// perflint
|
||||||
(Perflint, "101") => (RuleGroup::Unspecified, rules::perflint::rules::UnnecessaryListCast),
|
(Perflint, "101") => (RuleGroup::Unspecified, rules::perflint::rules::UnnecessaryListCast),
|
||||||
(Perflint, "102") => (RuleGroup::Unspecified, rules::perflint::rules::IncorrectDictIterator),
|
(Perflint, "102") => (RuleGroup::Unspecified, rules::perflint::rules::IncorrectDictIterator),
|
||||||
|
(Perflint, "203") => (RuleGroup::Unspecified, rules::perflint::rules::TryExceptInLoop),
|
||||||
|
|
||||||
// flake8-fixme
|
// flake8-fixme
|
||||||
(Flake8Fixme, "001") => (RuleGroup::Unspecified, rules::flake8_fixme::rules::LineContainsFixme),
|
(Flake8Fixme, "001") => (RuleGroup::Unspecified, rules::flake8_fixme::rules::LineContainsFixme),
|
||||||
|
|
|
||||||
|
|
@ -15,6 +15,7 @@ mod tests {
|
||||||
|
|
||||||
#[test_case(Rule::UnnecessaryListCast, Path::new("PERF101.py"))]
|
#[test_case(Rule::UnnecessaryListCast, Path::new("PERF101.py"))]
|
||||||
#[test_case(Rule::IncorrectDictIterator, Path::new("PERF102.py"))]
|
#[test_case(Rule::IncorrectDictIterator, Path::new("PERF102.py"))]
|
||||||
|
#[test_case(Rule::TryExceptInLoop, Path::new("PERF203.py"))]
|
||||||
fn rules(rule_code: Rule, path: &Path) -> Result<()> {
|
fn rules(rule_code: Rule, path: &Path) -> Result<()> {
|
||||||
let snapshot = format!("{}_{}", rule_code.noqa_code(), path.to_string_lossy());
|
let snapshot = format!("{}_{}", rule_code.noqa_code(), path.to_string_lossy());
|
||||||
let diagnostics = test_path(
|
let diagnostics = test_path(
|
||||||
|
|
|
||||||
|
|
@ -1,5 +1,7 @@
|
||||||
pub(crate) use incorrect_dict_iterator::*;
|
pub(crate) use incorrect_dict_iterator::*;
|
||||||
|
pub(crate) use try_except_in_loop::*;
|
||||||
pub(crate) use unnecessary_list_cast::*;
|
pub(crate) use unnecessary_list_cast::*;
|
||||||
|
|
||||||
mod incorrect_dict_iterator;
|
mod incorrect_dict_iterator;
|
||||||
|
mod try_except_in_loop;
|
||||||
mod unnecessary_list_cast;
|
mod unnecessary_list_cast;
|
||||||
|
|
|
||||||
|
|
@ -0,0 +1,74 @@
|
||||||
|
use rustpython_parser::ast::{self, Ranged, Stmt};
|
||||||
|
|
||||||
|
use ruff_diagnostics::{Diagnostic, Violation};
|
||||||
|
use ruff_macros::{derive_message_formats, violation};
|
||||||
|
|
||||||
|
use crate::checkers::ast::Checker;
|
||||||
|
use crate::settings::types::PythonVersion;
|
||||||
|
|
||||||
|
/// ## What it does
|
||||||
|
/// Checks for uses of except handling via `try`-`except` within `for` and
|
||||||
|
/// `while` loops.
|
||||||
|
///
|
||||||
|
/// ## Why is this bad?
|
||||||
|
/// Exception handling via `try`-`except` blocks incurs some performance
|
||||||
|
/// overhead, regardless of whether an exception is raised.
|
||||||
|
///
|
||||||
|
/// When possible, refactor your code to put the entire loop into the
|
||||||
|
/// `try`-`except` block, rather than wrapping each iteration in a separate
|
||||||
|
/// `try`-`except` block.
|
||||||
|
///
|
||||||
|
/// This rule is only enforced for Python versions prior to 3.11, which
|
||||||
|
/// introduced "zero cost" exception handling.
|
||||||
|
///
|
||||||
|
/// Note that, as with all `perflint` rules, this is only intended as a
|
||||||
|
/// micro-optimization, and will have a negligible impact on performance in
|
||||||
|
/// most cases.
|
||||||
|
///
|
||||||
|
/// ## Example
|
||||||
|
/// ```python
|
||||||
|
/// for i in range(10):
|
||||||
|
/// try:
|
||||||
|
/// print(i * i)
|
||||||
|
/// except:
|
||||||
|
/// break
|
||||||
|
/// ```
|
||||||
|
///
|
||||||
|
/// Use instead:
|
||||||
|
/// ```python
|
||||||
|
/// try:
|
||||||
|
/// for i in range(10):
|
||||||
|
/// print(i * i)
|
||||||
|
/// except:
|
||||||
|
/// break
|
||||||
|
/// ```
|
||||||
|
///
|
||||||
|
/// ## Options
|
||||||
|
/// - `target-version`
|
||||||
|
#[violation]
|
||||||
|
pub struct TryExceptInLoop;
|
||||||
|
|
||||||
|
impl Violation for TryExceptInLoop {
|
||||||
|
#[derive_message_formats]
|
||||||
|
fn message(&self) -> String {
|
||||||
|
format!("`try`-`except` within a loop incurs performance overhead")
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
/// PERF203
|
||||||
|
pub(crate) fn try_except_in_loop(checker: &mut Checker, body: &[Stmt]) {
|
||||||
|
if checker.settings.target_version >= PythonVersion::Py311 {
|
||||||
|
return;
|
||||||
|
}
|
||||||
|
|
||||||
|
checker.diagnostics.extend(body.iter().filter_map(|stmt| {
|
||||||
|
if let Stmt::Try(ast::StmtTry { handlers, .. }) = stmt {
|
||||||
|
handlers
|
||||||
|
.iter()
|
||||||
|
.next()
|
||||||
|
.map(|handler| Diagnostic::new(TryExceptInLoop, handler.range()))
|
||||||
|
} else {
|
||||||
|
None
|
||||||
|
}
|
||||||
|
}));
|
||||||
|
}
|
||||||
|
|
@ -0,0 +1,28 @@
|
||||||
|
---
|
||||||
|
source: crates/ruff/src/rules/perflint/mod.rs
|
||||||
|
---
|
||||||
|
PERF203.py:4:5: PERF203 `try`-`except` within a loop incurs performance overhead
|
||||||
|
|
|
||||||
|
2 | try: # PERF203
|
||||||
|
3 | print(f"{i}")
|
||||||
|
4 | except:
|
||||||
|
| _____^
|
||||||
|
5 | | print("error")
|
||||||
|
| |______________________^ PERF203
|
||||||
|
6 |
|
||||||
|
7 | try:
|
||||||
|
|
|
||||||
|
|
||||||
|
PERF203.py:17:5: PERF203 `try`-`except` within a loop incurs performance overhead
|
||||||
|
|
|
||||||
|
15 | try:
|
||||||
|
16 | print(f"{i}")
|
||||||
|
17 | except:
|
||||||
|
| _____^
|
||||||
|
18 | | print("error")
|
||||||
|
| |______________________^ PERF203
|
||||||
|
19 |
|
||||||
|
20 | i += 1
|
||||||
|
|
|
||||||
|
|
||||||
|
|
||||||
|
|
@ -2065,6 +2065,9 @@
|
||||||
"PERF10",
|
"PERF10",
|
||||||
"PERF101",
|
"PERF101",
|
||||||
"PERF102",
|
"PERF102",
|
||||||
|
"PERF2",
|
||||||
|
"PERF20",
|
||||||
|
"PERF203",
|
||||||
"PGH",
|
"PGH",
|
||||||
"PGH0",
|
"PGH0",
|
||||||
"PGH00",
|
"PGH00",
|
||||||
|
|
|
||||||
|
|
@ -18,6 +18,7 @@ ignore = [
|
||||||
"G", # flake8-logging
|
"G", # flake8-logging
|
||||||
"T", # flake8-print
|
"T", # flake8-print
|
||||||
"FBT", # flake8-boolean-trap
|
"FBT", # flake8-boolean-trap
|
||||||
|
"PERF", # perflint
|
||||||
]
|
]
|
||||||
|
|
||||||
[tool.ruff.isort]
|
[tool.ruff.isort]
|
||||||
|
|
|
||||||
Loading…
Reference in New Issue