mirror of
https://github.com/astral-sh/ruff.git
synced 2025-11-02 04:48:07 +00:00
[pylint] Implement stop-iteration-return (PLR1708) (#20733)
Some checks are pending
CI / cargo test (${{ github.repository == 'astral-sh/ruff' && 'depot-windows-2022-16' || 'windows-latest' }}) (push) Blocked by required conditions
CI / Determine changes (push) Waiting to run
CI / cargo fmt (push) Waiting to run
CI / cargo clippy (push) Blocked by required conditions
CI / cargo test (linux) (push) Blocked by required conditions
CI / cargo test (linux, release) (push) Blocked by required conditions
CI / cargo test (macos-latest) (push) Blocked by required conditions
CI / cargo test (wasm) (push) Blocked by required conditions
CI / cargo build (msrv) (push) Blocked by required conditions
CI / cargo fuzz build (push) Blocked by required conditions
CI / cargo shear (push) Blocked by required conditions
CI / fuzz parser (push) Blocked by required conditions
CI / test scripts (push) Blocked by required conditions
CI / ecosystem (push) Blocked by required conditions
CI / Fuzz for new ty panics (push) Blocked by required conditions
CI / ty completion evaluation (push) Blocked by required conditions
CI / python package (push) Waiting to run
CI / pre-commit (push) Waiting to run
CI / mkdocs (push) Waiting to run
CI / formatter instabilities and black similarity (push) Blocked by required conditions
CI / test ruff-lsp (push) Blocked by required conditions
CI / check playground (push) Blocked by required conditions
CI / benchmarks instrumented (ruff) (push) Blocked by required conditions
CI / benchmarks instrumented (ty) (push) Blocked by required conditions
CI / benchmarks walltime (medium|multithreaded) (push) Blocked by required conditions
CI / benchmarks walltime (small|large) (push) Blocked by required conditions
Some checks are pending
CI / cargo test (${{ github.repository == 'astral-sh/ruff' && 'depot-windows-2022-16' || 'windows-latest' }}) (push) Blocked by required conditions
CI / Determine changes (push) Waiting to run
CI / cargo fmt (push) Waiting to run
CI / cargo clippy (push) Blocked by required conditions
CI / cargo test (linux) (push) Blocked by required conditions
CI / cargo test (linux, release) (push) Blocked by required conditions
CI / cargo test (macos-latest) (push) Blocked by required conditions
CI / cargo test (wasm) (push) Blocked by required conditions
CI / cargo build (msrv) (push) Blocked by required conditions
CI / cargo fuzz build (push) Blocked by required conditions
CI / cargo shear (push) Blocked by required conditions
CI / fuzz parser (push) Blocked by required conditions
CI / test scripts (push) Blocked by required conditions
CI / ecosystem (push) Blocked by required conditions
CI / Fuzz for new ty panics (push) Blocked by required conditions
CI / ty completion evaluation (push) Blocked by required conditions
CI / python package (push) Waiting to run
CI / pre-commit (push) Waiting to run
CI / mkdocs (push) Waiting to run
CI / formatter instabilities and black similarity (push) Blocked by required conditions
CI / test ruff-lsp (push) Blocked by required conditions
CI / check playground (push) Blocked by required conditions
CI / benchmarks instrumented (ruff) (push) Blocked by required conditions
CI / benchmarks instrumented (ty) (push) Blocked by required conditions
CI / benchmarks walltime (medium|multithreaded) (push) Blocked by required conditions
CI / benchmarks walltime (small|large) (push) Blocked by required conditions
## Summary implement pylint rule stop-iteration-return / R1708 ## Test Plan <!-- How was it tested? --> --------- Co-authored-by: Brent Westbrook <brentrwestbrook@gmail.com>
This commit is contained in:
parent
05cde8bd19
commit
28aed61a22
8 changed files with 362 additions and 0 deletions
131
crates/ruff_linter/resources/test/fixtures/pylint/stop_iteration_return.py
vendored
Normal file
131
crates/ruff_linter/resources/test/fixtures/pylint/stop_iteration_return.py
vendored
Normal file
|
|
@ -0,0 +1,131 @@
|
|||
"""Test cases for PLR1708 stop-iteration-return."""
|
||||
|
||||
|
||||
# Valid cases - should not trigger the rule
|
||||
def normal_function():
|
||||
raise StopIteration # Not a generator, should not trigger
|
||||
|
||||
|
||||
def normal_function_with_value():
|
||||
raise StopIteration("value") # Not a generator, should not trigger
|
||||
|
||||
|
||||
def generator_with_return():
|
||||
yield 1
|
||||
yield 2
|
||||
return "finished" # This is the correct way
|
||||
|
||||
|
||||
def generator_with_yield_from():
|
||||
yield from [1, 2, 3]
|
||||
|
||||
|
||||
def generator_without_stop_iteration():
|
||||
yield 1
|
||||
yield 2
|
||||
# No explicit termination
|
||||
|
||||
|
||||
def generator_with_other_exception():
|
||||
yield 1
|
||||
raise ValueError("something else") # Different exception
|
||||
|
||||
|
||||
# Invalid cases - should trigger the rule
|
||||
def generator_with_stop_iteration():
|
||||
yield 1
|
||||
yield 2
|
||||
raise StopIteration # Should trigger
|
||||
|
||||
|
||||
def generator_with_stop_iteration_value():
|
||||
yield 1
|
||||
yield 2
|
||||
raise StopIteration("finished") # Should trigger
|
||||
|
||||
|
||||
def generator_with_stop_iteration_expr():
|
||||
yield 1
|
||||
yield 2
|
||||
raise StopIteration(1 + 2) # Should trigger
|
||||
|
||||
|
||||
def async_generator_with_stop_iteration():
|
||||
yield 1
|
||||
yield 2
|
||||
raise StopIteration("async") # Should trigger
|
||||
|
||||
|
||||
def nested_generator():
|
||||
def inner_gen():
|
||||
yield 1
|
||||
raise StopIteration("inner") # Should trigger
|
||||
|
||||
yield from inner_gen()
|
||||
|
||||
|
||||
def generator_in_class():
|
||||
class MyClass:
|
||||
def generator_method(self):
|
||||
yield 1
|
||||
raise StopIteration("method") # Should trigger
|
||||
|
||||
return MyClass
|
||||
|
||||
|
||||
# Complex cases
|
||||
def complex_generator():
|
||||
try:
|
||||
yield 1
|
||||
yield 2
|
||||
raise StopIteration("complex") # Should trigger
|
||||
except ValueError:
|
||||
yield 3
|
||||
finally:
|
||||
pass
|
||||
|
||||
|
||||
def generator_with_conditional_stop_iteration(condition):
|
||||
yield 1
|
||||
if condition:
|
||||
raise StopIteration("conditional") # Should trigger
|
||||
yield 2
|
||||
|
||||
|
||||
# Edge cases
|
||||
def generator_with_bare_stop_iteration():
|
||||
yield 1
|
||||
raise StopIteration # Should trigger (no arguments)
|
||||
|
||||
|
||||
def generator_with_stop_iteration_in_loop():
|
||||
for i in range(5):
|
||||
yield i
|
||||
if i == 3:
|
||||
raise StopIteration("loop") # Should trigger
|
||||
|
||||
|
||||
# Should not trigger - different exceptions
|
||||
def generator_with_runtime_error():
|
||||
yield 1
|
||||
raise RuntimeError("not StopIteration") # Should not trigger
|
||||
|
||||
|
||||
def generator_with_custom_exception():
|
||||
yield 1
|
||||
raise CustomException("custom") # Should not trigger
|
||||
|
||||
|
||||
class CustomException(Exception):
|
||||
pass
|
||||
|
||||
|
||||
# Generator comprehensions should not be affected
|
||||
list_comp = [x for x in range(10)] # Should not trigger
|
||||
|
||||
|
||||
# Lambda in generator context
|
||||
def generator_with_lambda():
|
||||
yield 1
|
||||
func = lambda x: x # Just a regular lambda
|
||||
yield 2
|
||||
|
|
@ -951,6 +951,9 @@ pub(crate) fn statement(stmt: &Stmt, checker: &mut Checker) {
|
|||
if checker.is_rule_enabled(Rule::MisplacedBareRaise) {
|
||||
pylint::rules::misplaced_bare_raise(checker, raise);
|
||||
}
|
||||
if checker.is_rule_enabled(Rule::StopIterationReturn) {
|
||||
pylint::rules::stop_iteration_return(checker, raise);
|
||||
}
|
||||
}
|
||||
Stmt::AugAssign(aug_assign @ ast::StmtAugAssign { target, .. }) => {
|
||||
if checker.is_rule_enabled(Rule::GlobalStatement) {
|
||||
|
|
|
|||
|
|
@ -286,6 +286,7 @@ pub fn code_to_rule(linter: Linter, code: &str) -> Option<(RuleGroup, Rule)> {
|
|||
(Pylint, "R1702") => rules::pylint::rules::TooManyNestedBlocks,
|
||||
(Pylint, "R1704") => rules::pylint::rules::RedefinedArgumentFromLocal,
|
||||
(Pylint, "R1706") => rules::pylint::rules::AndOrTernary,
|
||||
(Pylint, "R1708") => rules::pylint::rules::StopIterationReturn,
|
||||
(Pylint, "R1711") => rules::pylint::rules::UselessReturn,
|
||||
(Pylint, "R1714") => rules::pylint::rules::RepeatedEqualityComparison,
|
||||
(Pylint, "R1722") => rules::pylint::rules::SysExitAlias,
|
||||
|
|
|
|||
|
|
@ -52,6 +52,7 @@ mod tests {
|
|||
#[test_case(Rule::ManualFromImport, Path::new("import_aliasing.py"))]
|
||||
#[test_case(Rule::IfStmtMinMax, Path::new("if_stmt_min_max.py"))]
|
||||
#[test_case(Rule::SingleStringSlots, Path::new("single_string_slots.py"))]
|
||||
#[test_case(Rule::StopIterationReturn, Path::new("stop_iteration_return.py"))]
|
||||
#[test_case(Rule::SysExitAlias, Path::new("sys_exit_alias_0.py"))]
|
||||
#[test_case(Rule::SysExitAlias, Path::new("sys_exit_alias_1.py"))]
|
||||
#[test_case(Rule::SysExitAlias, Path::new("sys_exit_alias_2.py"))]
|
||||
|
|
|
|||
|
|
@ -75,6 +75,7 @@ pub(crate) use shallow_copy_environ::*;
|
|||
pub(crate) use single_string_slots::*;
|
||||
pub(crate) use singledispatch_method::*;
|
||||
pub(crate) use singledispatchmethod_function::*;
|
||||
pub(crate) use stop_iteration_return::*;
|
||||
pub(crate) use subprocess_popen_preexec_fn::*;
|
||||
pub(crate) use subprocess_run_without_check::*;
|
||||
pub(crate) use super_without_brackets::*;
|
||||
|
|
@ -185,6 +186,7 @@ mod shallow_copy_environ;
|
|||
mod single_string_slots;
|
||||
mod singledispatch_method;
|
||||
mod singledispatchmethod_function;
|
||||
mod stop_iteration_return;
|
||||
mod subprocess_popen_preexec_fn;
|
||||
mod subprocess_run_without_check;
|
||||
mod super_without_brackets;
|
||||
|
|
|
|||
|
|
@ -0,0 +1,114 @@
|
|||
use ruff_macros::{ViolationMetadata, derive_message_formats};
|
||||
use ruff_python_ast as ast;
|
||||
use ruff_python_ast::visitor::{Visitor, walk_expr, walk_stmt};
|
||||
use ruff_text_size::Ranged;
|
||||
|
||||
use crate::Violation;
|
||||
use crate::checkers::ast::Checker;
|
||||
|
||||
/// ## What it does
|
||||
/// Checks for explicit `raise StopIteration` in generator functions.
|
||||
///
|
||||
/// ## Why is this bad?
|
||||
/// Raising `StopIteration` in a generator function causes a `RuntimeError`
|
||||
/// when the generator is iterated over.
|
||||
///
|
||||
/// Instead of `raise StopIteration`, use `return` in generator functions.
|
||||
///
|
||||
/// ## Example
|
||||
/// ```python
|
||||
/// def my_generator():
|
||||
/// yield 1
|
||||
/// yield 2
|
||||
/// raise StopIteration # This causes RuntimeError at runtime
|
||||
/// ```
|
||||
///
|
||||
/// Use instead:
|
||||
/// ```python
|
||||
/// def my_generator():
|
||||
/// yield 1
|
||||
/// yield 2
|
||||
/// return # Use return instead
|
||||
/// ```
|
||||
///
|
||||
/// ## References
|
||||
/// - [PEP 479](https://peps.python.org/pep-0479/)
|
||||
/// - [Python documentation](https://docs.python.org/3/library/exceptions.html#StopIteration)
|
||||
#[derive(ViolationMetadata)]
|
||||
#[violation_metadata(preview_since = "0.14.3")]
|
||||
pub(crate) struct StopIterationReturn;
|
||||
|
||||
impl Violation for StopIterationReturn {
|
||||
#[derive_message_formats]
|
||||
fn message(&self) -> String {
|
||||
"Explicit `raise StopIteration` in generator".to_string()
|
||||
}
|
||||
|
||||
fn fix_title(&self) -> Option<String> {
|
||||
Some("Use `return` instead".to_string())
|
||||
}
|
||||
}
|
||||
|
||||
/// PLR1708
|
||||
pub(crate) fn stop_iteration_return(checker: &Checker, raise_stmt: &ast::StmtRaise) {
|
||||
// Fast-path: only continue if this is `raise StopIteration` (with or without args)
|
||||
let Some(exc) = &raise_stmt.exc else {
|
||||
return;
|
||||
};
|
||||
|
||||
let is_stop_iteration = match exc.as_ref() {
|
||||
ast::Expr::Call(ast::ExprCall { func, .. }) => {
|
||||
checker.semantic().match_builtin_expr(func, "StopIteration")
|
||||
}
|
||||
expr => checker.semantic().match_builtin_expr(expr, "StopIteration"),
|
||||
};
|
||||
|
||||
if !is_stop_iteration {
|
||||
return;
|
||||
}
|
||||
|
||||
// Now check the (more expensive) generator context
|
||||
if !in_generator_context(checker) {
|
||||
return;
|
||||
}
|
||||
|
||||
checker.report_diagnostic(StopIterationReturn, raise_stmt.range());
|
||||
}
|
||||
|
||||
/// Returns true if we're inside a function that contains any `yield`/`yield from`.
|
||||
fn in_generator_context(checker: &Checker) -> bool {
|
||||
for scope in checker.semantic().current_scopes() {
|
||||
if let ruff_python_semantic::ScopeKind::Function(function_def) = scope.kind {
|
||||
if contains_yield_statement(&function_def.body) {
|
||||
return true;
|
||||
}
|
||||
}
|
||||
}
|
||||
false
|
||||
}
|
||||
|
||||
/// Check if a statement list contains any yield statements
|
||||
fn contains_yield_statement(body: &[ast::Stmt]) -> bool {
|
||||
struct YieldFinder {
|
||||
found: bool,
|
||||
}
|
||||
|
||||
impl Visitor<'_> for YieldFinder {
|
||||
fn visit_expr(&mut self, expr: &ast::Expr) {
|
||||
if matches!(expr, ast::Expr::Yield(_) | ast::Expr::YieldFrom(_)) {
|
||||
self.found = true;
|
||||
} else {
|
||||
walk_expr(self, expr);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
let mut finder = YieldFinder { found: false };
|
||||
for stmt in body {
|
||||
walk_stmt(&mut finder, stmt);
|
||||
if finder.found {
|
||||
return true;
|
||||
}
|
||||
}
|
||||
false
|
||||
}
|
||||
|
|
@ -0,0 +1,109 @@
|
|||
---
|
||||
source: crates/ruff_linter/src/rules/pylint/mod.rs
|
||||
---
|
||||
PLR1708 Explicit `raise StopIteration` in generator
|
||||
--> stop_iteration_return.py:38:5
|
||||
|
|
||||
36 | yield 1
|
||||
37 | yield 2
|
||||
38 | raise StopIteration # Should trigger
|
||||
| ^^^^^^^^^^^^^^^^^^^
|
||||
|
|
||||
help: Use `return` instead
|
||||
|
||||
PLR1708 Explicit `raise StopIteration` in generator
|
||||
--> stop_iteration_return.py:44:5
|
||||
|
|
||||
42 | yield 1
|
||||
43 | yield 2
|
||||
44 | raise StopIteration("finished") # Should trigger
|
||||
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
||||
|
|
||||
help: Use `return` instead
|
||||
|
||||
PLR1708 Explicit `raise StopIteration` in generator
|
||||
--> stop_iteration_return.py:50:5
|
||||
|
|
||||
48 | yield 1
|
||||
49 | yield 2
|
||||
50 | raise StopIteration(1 + 2) # Should trigger
|
||||
| ^^^^^^^^^^^^^^^^^^^^^^^^^^
|
||||
|
|
||||
help: Use `return` instead
|
||||
|
||||
PLR1708 Explicit `raise StopIteration` in generator
|
||||
--> stop_iteration_return.py:56:5
|
||||
|
|
||||
54 | yield 1
|
||||
55 | yield 2
|
||||
56 | raise StopIteration("async") # Should trigger
|
||||
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
||||
|
|
||||
help: Use `return` instead
|
||||
|
||||
PLR1708 Explicit `raise StopIteration` in generator
|
||||
--> stop_iteration_return.py:62:9
|
||||
|
|
||||
60 | def inner_gen():
|
||||
61 | yield 1
|
||||
62 | raise StopIteration("inner") # Should trigger
|
||||
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
||||
63 |
|
||||
64 | yield from inner_gen()
|
||||
|
|
||||
help: Use `return` instead
|
||||
|
||||
PLR1708 Explicit `raise StopIteration` in generator
|
||||
--> stop_iteration_return.py:71:13
|
||||
|
|
||||
69 | def generator_method(self):
|
||||
70 | yield 1
|
||||
71 | raise StopIteration("method") # Should trigger
|
||||
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
||||
72 |
|
||||
73 | return MyClass
|
||||
|
|
||||
help: Use `return` instead
|
||||
|
||||
PLR1708 Explicit `raise StopIteration` in generator
|
||||
--> stop_iteration_return.py:81:9
|
||||
|
|
||||
79 | yield 1
|
||||
80 | yield 2
|
||||
81 | raise StopIteration("complex") # Should trigger
|
||||
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
||||
82 | except ValueError:
|
||||
83 | yield 3
|
||||
|
|
||||
help: Use `return` instead
|
||||
|
||||
PLR1708 Explicit `raise StopIteration` in generator
|
||||
--> stop_iteration_return.py:91:9
|
||||
|
|
||||
89 | yield 1
|
||||
90 | if condition:
|
||||
91 | raise StopIteration("conditional") # Should trigger
|
||||
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
||||
92 | yield 2
|
||||
|
|
||||
help: Use `return` instead
|
||||
|
||||
PLR1708 Explicit `raise StopIteration` in generator
|
||||
--> stop_iteration_return.py:98:5
|
||||
|
|
||||
96 | def generator_with_bare_stop_iteration():
|
||||
97 | yield 1
|
||||
98 | raise StopIteration # Should trigger (no arguments)
|
||||
| ^^^^^^^^^^^^^^^^^^^
|
||||
|
|
||||
help: Use `return` instead
|
||||
|
||||
PLR1708 Explicit `raise StopIteration` in generator
|
||||
--> stop_iteration_return.py:105:13
|
||||
|
|
||||
103 | yield i
|
||||
104 | if i == 3:
|
||||
105 | raise StopIteration("loop") # Should trigger
|
||||
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
||||
|
|
||||
help: Use `return` instead
|
||||
1
ruff.schema.json
generated
1
ruff.schema.json
generated
|
|
@ -3715,6 +3715,7 @@
|
|||
"PLR170",
|
||||
"PLR1702",
|
||||
"PLR1704",
|
||||
"PLR1708",
|
||||
"PLR171",
|
||||
"PLR1711",
|
||||
"PLR1714",
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue