mirror of
https://github.com/astral-sh/ruff.git
synced 2025-09-26 20:09:22 +00:00
[pylint
] Implement too-many-nested-blocks
(PLR1702
) (#9172)
## Summary Implement [`PLR1702`/`too-many-nested-blocks`](https://pylint.readthedocs.io/en/latest/user_guide/messages/refactor/too-many-nested-blocks.html) See: #970 ## Test Plan `cargo test`
This commit is contained in:
parent
45628a5883
commit
dba2cb79cb
10 changed files with 210 additions and 0 deletions
21
crates/ruff_linter/resources/test/fixtures/pylint/too_many_nested_blocks.py
vendored
Normal file
21
crates/ruff_linter/resources/test/fixtures/pylint/too_many_nested_blocks.py
vendored
Normal file
|
@ -0,0 +1,21 @@
|
||||||
|
def correct_fruits(fruits) -> bool:
|
||||||
|
if len(fruits) > 1: # PLR1702
|
||||||
|
if "apple" in fruits:
|
||||||
|
if "orange" in fruits:
|
||||||
|
count = fruits["orange"]
|
||||||
|
if count % 2:
|
||||||
|
if "kiwi" in fruits:
|
||||||
|
if count == 2:
|
||||||
|
return True
|
||||||
|
return False
|
||||||
|
|
||||||
|
# Ok
|
||||||
|
def correct_fruits(fruits) -> bool:
|
||||||
|
if len(fruits) > 1:
|
||||||
|
if "apple" in fruits:
|
||||||
|
if "orange" in fruits:
|
||||||
|
count = fruits["orange"]
|
||||||
|
if count % 2:
|
||||||
|
if "kiwi" in fruits:
|
||||||
|
return True
|
||||||
|
return False
|
|
@ -1073,6 +1073,9 @@ pub(crate) fn statement(stmt: &Stmt, checker: &mut Checker) {
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
Stmt::If(if_ @ ast::StmtIf { test, .. }) => {
|
Stmt::If(if_ @ ast::StmtIf { test, .. }) => {
|
||||||
|
if checker.enabled(Rule::TooManyNestedBlocks) {
|
||||||
|
pylint::rules::too_many_nested_blocks(checker, stmt);
|
||||||
|
}
|
||||||
if checker.enabled(Rule::EmptyTypeCheckingBlock) {
|
if checker.enabled(Rule::EmptyTypeCheckingBlock) {
|
||||||
if typing::is_type_checking_block(if_, &checker.semantic) {
|
if typing::is_type_checking_block(if_, &checker.semantic) {
|
||||||
flake8_type_checking::rules::empty_type_checking_block(checker, if_);
|
flake8_type_checking::rules::empty_type_checking_block(checker, if_);
|
||||||
|
@ -1205,6 +1208,9 @@ pub(crate) fn statement(stmt: &Stmt, checker: &mut Checker) {
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
Stmt::With(with_stmt @ ast::StmtWith { items, body, .. }) => {
|
Stmt::With(with_stmt @ ast::StmtWith { items, body, .. }) => {
|
||||||
|
if checker.enabled(Rule::TooManyNestedBlocks) {
|
||||||
|
pylint::rules::too_many_nested_blocks(checker, stmt);
|
||||||
|
}
|
||||||
if checker.enabled(Rule::AssertRaisesException) {
|
if checker.enabled(Rule::AssertRaisesException) {
|
||||||
flake8_bugbear::rules::assert_raises_exception(checker, items);
|
flake8_bugbear::rules::assert_raises_exception(checker, items);
|
||||||
}
|
}
|
||||||
|
@ -1232,6 +1238,9 @@ pub(crate) fn statement(stmt: &Stmt, checker: &mut Checker) {
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
Stmt::While(while_stmt @ ast::StmtWhile { body, orelse, .. }) => {
|
Stmt::While(while_stmt @ ast::StmtWhile { body, orelse, .. }) => {
|
||||||
|
if checker.enabled(Rule::TooManyNestedBlocks) {
|
||||||
|
pylint::rules::too_many_nested_blocks(checker, stmt);
|
||||||
|
}
|
||||||
if checker.enabled(Rule::FunctionUsesLoopVariable) {
|
if checker.enabled(Rule::FunctionUsesLoopVariable) {
|
||||||
flake8_bugbear::rules::function_uses_loop_variable(checker, &Node::Stmt(stmt));
|
flake8_bugbear::rules::function_uses_loop_variable(checker, &Node::Stmt(stmt));
|
||||||
}
|
}
|
||||||
|
@ -1255,6 +1264,9 @@ pub(crate) fn statement(stmt: &Stmt, checker: &mut Checker) {
|
||||||
range: _,
|
range: _,
|
||||||
},
|
},
|
||||||
) => {
|
) => {
|
||||||
|
if checker.enabled(Rule::TooManyNestedBlocks) {
|
||||||
|
pylint::rules::too_many_nested_blocks(checker, stmt);
|
||||||
|
}
|
||||||
if checker.any_enabled(&[
|
if checker.any_enabled(&[
|
||||||
Rule::EnumerateForLoop,
|
Rule::EnumerateForLoop,
|
||||||
Rule::IncorrectDictIterator,
|
Rule::IncorrectDictIterator,
|
||||||
|
@ -1319,6 +1331,9 @@ pub(crate) fn statement(stmt: &Stmt, checker: &mut Checker) {
|
||||||
finalbody,
|
finalbody,
|
||||||
..
|
..
|
||||||
}) => {
|
}) => {
|
||||||
|
if checker.enabled(Rule::TooManyNestedBlocks) {
|
||||||
|
pylint::rules::too_many_nested_blocks(checker, stmt);
|
||||||
|
}
|
||||||
if checker.enabled(Rule::JumpStatementInFinally) {
|
if checker.enabled(Rule::JumpStatementInFinally) {
|
||||||
flake8_bugbear::rules::jump_statement_in_finally(checker, finalbody);
|
flake8_bugbear::rules::jump_statement_in_finally(checker, finalbody);
|
||||||
}
|
}
|
||||||
|
|
|
@ -261,6 +261,7 @@ pub fn code_to_rule(linter: Linter, code: &str) -> Option<(RuleGroup, Rule)> {
|
||||||
(Pylint, "R0916") => (RuleGroup::Preview, rules::pylint::rules::TooManyBooleanExpressions),
|
(Pylint, "R0916") => (RuleGroup::Preview, rules::pylint::rules::TooManyBooleanExpressions),
|
||||||
(Pylint, "R0917") => (RuleGroup::Preview, rules::pylint::rules::TooManyPositional),
|
(Pylint, "R0917") => (RuleGroup::Preview, rules::pylint::rules::TooManyPositional),
|
||||||
(Pylint, "R1701") => (RuleGroup::Stable, rules::pylint::rules::RepeatedIsinstanceCalls),
|
(Pylint, "R1701") => (RuleGroup::Stable, rules::pylint::rules::RepeatedIsinstanceCalls),
|
||||||
|
(Pylint, "R1702") => (RuleGroup::Preview, rules::pylint::rules::TooManyNestedBlocks),
|
||||||
(Pylint, "R1704") => (RuleGroup::Preview, rules::pylint::rules::RedefinedArgumentFromLocal),
|
(Pylint, "R1704") => (RuleGroup::Preview, rules::pylint::rules::RedefinedArgumentFromLocal),
|
||||||
(Pylint, "R1711") => (RuleGroup::Stable, rules::pylint::rules::UselessReturn),
|
(Pylint, "R1711") => (RuleGroup::Stable, rules::pylint::rules::UselessReturn),
|
||||||
(Pylint, "R1714") => (RuleGroup::Stable, rules::pylint::rules::RepeatedEqualityComparison),
|
(Pylint, "R1714") => (RuleGroup::Stable, rules::pylint::rules::RepeatedEqualityComparison),
|
||||||
|
|
|
@ -171,6 +171,7 @@ mod tests {
|
||||||
#[test_case(Rule::NoStaticmethodDecorator, Path::new("no_method_decorator.py"))]
|
#[test_case(Rule::NoStaticmethodDecorator, Path::new("no_method_decorator.py"))]
|
||||||
#[test_case(Rule::PotentialIndexError, Path::new("potential_index_error.py"))]
|
#[test_case(Rule::PotentialIndexError, Path::new("potential_index_error.py"))]
|
||||||
#[test_case(Rule::SuperWithoutBrackets, Path::new("super_without_brackets.py"))]
|
#[test_case(Rule::SuperWithoutBrackets, Path::new("super_without_brackets.py"))]
|
||||||
|
#[test_case(Rule::TooManyNestedBlocks, Path::new("too_many_nested_blocks.py"))]
|
||||||
#[test_case(
|
#[test_case(
|
||||||
Rule::UnnecessaryDictIndexLookup,
|
Rule::UnnecessaryDictIndexLookup,
|
||||||
Path::new("unnecessary_dict_index_lookup.py")
|
Path::new("unnecessary_dict_index_lookup.py")
|
||||||
|
|
|
@ -61,6 +61,7 @@ pub(crate) use too_many_arguments::*;
|
||||||
pub(crate) use too_many_boolean_expressions::*;
|
pub(crate) use too_many_boolean_expressions::*;
|
||||||
pub(crate) use too_many_branches::*;
|
pub(crate) use too_many_branches::*;
|
||||||
pub(crate) use too_many_locals::*;
|
pub(crate) use too_many_locals::*;
|
||||||
|
pub(crate) use too_many_nested_blocks::*;
|
||||||
pub(crate) use too_many_positional::*;
|
pub(crate) use too_many_positional::*;
|
||||||
pub(crate) use too_many_public_methods::*;
|
pub(crate) use too_many_public_methods::*;
|
||||||
pub(crate) use too_many_return_statements::*;
|
pub(crate) use too_many_return_statements::*;
|
||||||
|
@ -145,6 +146,7 @@ mod too_many_arguments;
|
||||||
mod too_many_boolean_expressions;
|
mod too_many_boolean_expressions;
|
||||||
mod too_many_branches;
|
mod too_many_branches;
|
||||||
mod too_many_locals;
|
mod too_many_locals;
|
||||||
|
mod too_many_nested_blocks;
|
||||||
mod too_many_positional;
|
mod too_many_positional;
|
||||||
mod too_many_public_methods;
|
mod too_many_public_methods;
|
||||||
mod too_many_return_statements;
|
mod too_many_return_statements;
|
||||||
|
|
|
@ -0,0 +1,132 @@
|
||||||
|
use ast::ExceptHandler;
|
||||||
|
use ruff_diagnostics::{Diagnostic, Violation};
|
||||||
|
use ruff_macros::{derive_message_formats, violation};
|
||||||
|
use ruff_python_ast::{self as ast, Stmt};
|
||||||
|
use ruff_text_size::Ranged;
|
||||||
|
|
||||||
|
use crate::checkers::ast::Checker;
|
||||||
|
|
||||||
|
/// ## What it does
|
||||||
|
/// Checks for functions or methods with too many nested blocks.
|
||||||
|
///
|
||||||
|
/// By default, this rule allows up to five nested blocks.
|
||||||
|
/// This can be configured using the [`pylint.max-nested-blocks`] option.
|
||||||
|
///
|
||||||
|
/// ## Why is this bad?
|
||||||
|
/// Functions or methods with too many nested blocks are harder to understand
|
||||||
|
/// and maintain.
|
||||||
|
///
|
||||||
|
/// ## Options
|
||||||
|
/// - `pylint.max-nested-blocks`
|
||||||
|
#[violation]
|
||||||
|
pub struct TooManyNestedBlocks {
|
||||||
|
nested_blocks: usize,
|
||||||
|
max_nested_blocks: usize,
|
||||||
|
}
|
||||||
|
|
||||||
|
impl Violation for TooManyNestedBlocks {
|
||||||
|
#[derive_message_formats]
|
||||||
|
fn message(&self) -> String {
|
||||||
|
let TooManyNestedBlocks {
|
||||||
|
nested_blocks,
|
||||||
|
max_nested_blocks,
|
||||||
|
} = self;
|
||||||
|
format!("Too many nested blocks ({nested_blocks} > {max_nested_blocks})")
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
/// PLR1702
|
||||||
|
pub(crate) fn too_many_nested_blocks(checker: &mut Checker, stmt: &Stmt) {
|
||||||
|
// Only enforce nesting within functions or methods.
|
||||||
|
if !checker.semantic().current_scope().kind.is_function() {
|
||||||
|
return;
|
||||||
|
}
|
||||||
|
|
||||||
|
// If the statement isn't a leaf node, we don't want to emit a diagnostic, since the diagnostic
|
||||||
|
// will be emitted on the leaves.
|
||||||
|
if has_nested_block(stmt) {
|
||||||
|
return;
|
||||||
|
}
|
||||||
|
|
||||||
|
let max_nested_blocks = checker.settings.pylint.max_nested_blocks;
|
||||||
|
|
||||||
|
// Traverse up the hierarchy, identifying the root node and counting the number of nested
|
||||||
|
// blocks between the root and this leaf.
|
||||||
|
let (count, root_id) =
|
||||||
|
checker
|
||||||
|
.semantic()
|
||||||
|
.current_statement_ids()
|
||||||
|
.fold((0, None), |(count, ancestor_id), id| {
|
||||||
|
let stmt = checker.semantic().statement(id);
|
||||||
|
if is_nested_block(stmt) {
|
||||||
|
(count + 1, Some(id))
|
||||||
|
} else {
|
||||||
|
(count, ancestor_id)
|
||||||
|
}
|
||||||
|
});
|
||||||
|
|
||||||
|
let Some(root_id) = root_id else {
|
||||||
|
return;
|
||||||
|
};
|
||||||
|
|
||||||
|
// If the number of nested blocks is less than the maximum, we don't want to emit a diagnostic.
|
||||||
|
if count <= max_nested_blocks {
|
||||||
|
return;
|
||||||
|
}
|
||||||
|
|
||||||
|
checker.diagnostics.push(Diagnostic::new(
|
||||||
|
TooManyNestedBlocks {
|
||||||
|
nested_blocks: count,
|
||||||
|
max_nested_blocks,
|
||||||
|
},
|
||||||
|
checker.semantic().statement(root_id).range(),
|
||||||
|
));
|
||||||
|
}
|
||||||
|
|
||||||
|
/// Returns `true` if the given statement is a nested block.
|
||||||
|
fn is_nested_block(stmt: &Stmt) -> bool {
|
||||||
|
matches!(
|
||||||
|
stmt,
|
||||||
|
Stmt::If(_) | Stmt::While(_) | Stmt::For(_) | Stmt::Try(_) | Stmt::With(_)
|
||||||
|
)
|
||||||
|
}
|
||||||
|
|
||||||
|
/// Returns `true` if the given statement is a leaf node.
|
||||||
|
fn has_nested_block(stmt: &Stmt) -> bool {
|
||||||
|
match stmt {
|
||||||
|
Stmt::If(ast::StmtIf {
|
||||||
|
body,
|
||||||
|
elif_else_clauses,
|
||||||
|
..
|
||||||
|
}) => {
|
||||||
|
body.iter().any(is_nested_block)
|
||||||
|
|| elif_else_clauses
|
||||||
|
.iter()
|
||||||
|
.any(|elif_else| elif_else.body.iter().any(is_nested_block))
|
||||||
|
}
|
||||||
|
Stmt::While(ast::StmtWhile { body, orelse, .. }) => {
|
||||||
|
body.iter().any(is_nested_block) || orelse.iter().any(is_nested_block)
|
||||||
|
}
|
||||||
|
Stmt::For(ast::StmtFor { body, orelse, .. }) => {
|
||||||
|
body.iter().any(is_nested_block) || orelse.iter().any(is_nested_block)
|
||||||
|
}
|
||||||
|
Stmt::Try(ast::StmtTry {
|
||||||
|
body,
|
||||||
|
handlers,
|
||||||
|
orelse,
|
||||||
|
finalbody,
|
||||||
|
..
|
||||||
|
}) => {
|
||||||
|
body.iter().any(is_nested_block)
|
||||||
|
|| handlers.iter().any(|handler| match handler {
|
||||||
|
ExceptHandler::ExceptHandler(ast::ExceptHandlerExceptHandler {
|
||||||
|
body, ..
|
||||||
|
}) => body.iter().any(is_nested_block),
|
||||||
|
})
|
||||||
|
|| orelse.iter().any(is_nested_block)
|
||||||
|
|| finalbody.iter().any(is_nested_block)
|
||||||
|
}
|
||||||
|
Stmt::With(ast::StmtWith { body, .. }) => body.iter().any(is_nested_block),
|
||||||
|
_ => false,
|
||||||
|
}
|
||||||
|
}
|
|
@ -60,6 +60,7 @@ pub struct Settings {
|
||||||
pub max_statements: usize,
|
pub max_statements: usize,
|
||||||
pub max_public_methods: usize,
|
pub max_public_methods: usize,
|
||||||
pub max_locals: usize,
|
pub max_locals: usize,
|
||||||
|
pub max_nested_blocks: usize,
|
||||||
}
|
}
|
||||||
|
|
||||||
impl Default for Settings {
|
impl Default for Settings {
|
||||||
|
@ -75,6 +76,7 @@ impl Default for Settings {
|
||||||
max_statements: 50,
|
max_statements: 50,
|
||||||
max_public_methods: 20,
|
max_public_methods: 20,
|
||||||
max_locals: 15,
|
max_locals: 15,
|
||||||
|
max_nested_blocks: 5,
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
|
@ -0,0 +1,20 @@
|
||||||
|
---
|
||||||
|
source: crates/ruff_linter/src/rules/pylint/mod.rs
|
||||||
|
---
|
||||||
|
too_many_nested_blocks.py:2:5: PLR1702 Too many nested blocks (6 > 5)
|
||||||
|
|
|
||||||
|
1 | def correct_fruits(fruits) -> bool:
|
||||||
|
2 | if len(fruits) > 1: # PLR1702
|
||||||
|
| _____^
|
||||||
|
3 | | if "apple" in fruits:
|
||||||
|
4 | | if "orange" in fruits:
|
||||||
|
5 | | count = fruits["orange"]
|
||||||
|
6 | | if count % 2:
|
||||||
|
7 | | if "kiwi" in fruits:
|
||||||
|
8 | | if count == 2:
|
||||||
|
9 | | return True
|
||||||
|
| |_______________________________________^ PLR1702
|
||||||
|
10 | return False
|
||||||
|
|
|
||||||
|
|
||||||
|
|
|
@ -2728,6 +2728,11 @@ pub struct PylintOptions {
|
||||||
/// (see: `PLR0916`).
|
/// (see: `PLR0916`).
|
||||||
#[option(default = r"5", value_type = "int", example = r"max-bool-expr = 5")]
|
#[option(default = r"5", value_type = "int", example = r"max-bool-expr = 5")]
|
||||||
pub max_bool_expr: Option<usize>,
|
pub max_bool_expr: Option<usize>,
|
||||||
|
|
||||||
|
/// Maximum number of nested blocks allowed within a function or method body
|
||||||
|
/// (see: `PLR1702`).
|
||||||
|
#[option(default = r"5", value_type = "int", example = r"max-nested-blocks = 5")]
|
||||||
|
pub max_nested_blocks: Option<usize>,
|
||||||
}
|
}
|
||||||
|
|
||||||
impl PylintOptions {
|
impl PylintOptions {
|
||||||
|
@ -2751,6 +2756,7 @@ impl PylintOptions {
|
||||||
.max_public_methods
|
.max_public_methods
|
||||||
.unwrap_or(defaults.max_public_methods),
|
.unwrap_or(defaults.max_public_methods),
|
||||||
max_locals: self.max_locals.unwrap_or(defaults.max_locals),
|
max_locals: self.max_locals.unwrap_or(defaults.max_locals),
|
||||||
|
max_nested_blocks: self.max_nested_blocks.unwrap_or(defaults.max_nested_blocks),
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
10
ruff.schema.json
generated
10
ruff.schema.json
generated
|
@ -2433,6 +2433,15 @@
|
||||||
"format": "uint",
|
"format": "uint",
|
||||||
"minimum": 0.0
|
"minimum": 0.0
|
||||||
},
|
},
|
||||||
|
"max-nested-blocks": {
|
||||||
|
"description": "Maximum number of nested blocks allowed within a function or method body (see: `PLR1702`).",
|
||||||
|
"type": [
|
||||||
|
"integer",
|
||||||
|
"null"
|
||||||
|
],
|
||||||
|
"format": "uint",
|
||||||
|
"minimum": 0.0
|
||||||
|
},
|
||||||
"max-positional-args": {
|
"max-positional-args": {
|
||||||
"description": "Maximum number of positional arguments allowed for a function or method definition (see: `PLR0917`).\n\nIf not specified, defaults to the value of `max-args`.",
|
"description": "Maximum number of positional arguments allowed for a function or method definition (see: `PLR0917`).\n\nIf not specified, defaults to the value of `max-args`.",
|
||||||
"type": [
|
"type": [
|
||||||
|
@ -3212,6 +3221,7 @@
|
||||||
"PLR17",
|
"PLR17",
|
||||||
"PLR170",
|
"PLR170",
|
||||||
"PLR1701",
|
"PLR1701",
|
||||||
|
"PLR1702",
|
||||||
"PLR1704",
|
"PLR1704",
|
||||||
"PLR1706",
|
"PLR1706",
|
||||||
"PLR171",
|
"PLR171",
|
||||||
|
|
Loading…
Add table
Add a link
Reference in a new issue