Skip to content

Commit

Permalink
PLR1702
Browse files Browse the repository at this point in the history
  • Loading branch information
diceroll123 committed Jan 20, 2024
1 parent 866bea6 commit 99431c8
Show file tree
Hide file tree
Showing 10 changed files with 207 additions and 0 deletions.
Original file line number Diff line number Diff line change
@@ -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
15 changes: 15 additions & 0 deletions crates/ruff_linter/src/checkers/ast/analyze/statement.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1076,6 +1076,9 @@ pub(crate) fn statement(stmt: &Stmt, checker: &mut Checker) {
..
},
) => {
if checker.enabled(Rule::TooManyNestedBlocks) {
pylint::rules::too_many_nested_blocks(checker, stmt);
}
if checker.enabled(Rule::EmptyTypeCheckingBlock) {
if typing::is_type_checking_block(if_, &checker.semantic) {
flake8_type_checking::rules::empty_type_checking_block(checker, if_);
Expand Down Expand Up @@ -1210,6 +1213,9 @@ pub(crate) fn statement(stmt: &Stmt, checker: &mut Checker) {
}
}
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) {
flake8_bugbear::rules::assert_raises_exception(checker, items);
}
Expand Down Expand Up @@ -1237,6 +1243,9 @@ pub(crate) fn statement(stmt: &Stmt, checker: &mut Checker) {
}
}
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) {
flake8_bugbear::rules::function_uses_loop_variable(checker, &Node::Stmt(stmt));
}
Expand All @@ -1260,6 +1269,9 @@ pub(crate) fn statement(stmt: &Stmt, checker: &mut Checker) {
range: _,
},
) => {
if checker.enabled(Rule::TooManyNestedBlocks) {
pylint::rules::too_many_nested_blocks(checker, stmt);
}
if checker.any_enabled(&[
Rule::EnumerateForLoop,
Rule::IncorrectDictIterator,
Expand Down Expand Up @@ -1324,6 +1336,9 @@ pub(crate) fn statement(stmt: &Stmt, checker: &mut Checker) {
finalbody,
..
}) => {
if checker.enabled(Rule::TooManyNestedBlocks) {
pylint::rules::too_many_nested_blocks(checker, stmt);
}
if checker.enabled(Rule::JumpStatementInFinally) {
flake8_bugbear::rules::jump_statement_in_finally(checker, finalbody);
}
Expand Down
1 change: 1 addition & 0 deletions crates/ruff_linter/src/codes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -259,6 +259,7 @@ pub fn code_to_rule(linter: Linter, code: &str) -> Option<(RuleGroup, Rule)> {
(Pylint, "R0916") => (RuleGroup::Preview, rules::pylint::rules::TooManyBooleanExpressions),
(Pylint, "R0917") => (RuleGroup::Preview, rules::pylint::rules::TooManyPositional),
(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, "R1711") => (RuleGroup::Stable, rules::pylint::rules::UselessReturn),
(Pylint, "R1714") => (RuleGroup::Stable, rules::pylint::rules::RepeatedEqualityComparison),
Expand Down
1 change: 1 addition & 0 deletions crates/ruff_linter/src/rules/pylint/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -168,6 +168,7 @@ mod tests {
#[test_case(Rule::UnnecessaryDunderCall, Path::new("unnecessary_dunder_call.py"))]
#[test_case(Rule::NoStaticmethodDecorator, Path::new("no_method_decorator.py"))]
#[test_case(Rule::SuperWithoutBrackets, Path::new("super_without_brackets.py"))]
#[test_case(Rule::TooManyNestedBlocks, Path::new("too_many_nested_blocks.py"))]
#[test_case(
Rule::UnnecessaryDictIndexLookup,
Path::new("unnecessary_dict_index_lookup.py")
Expand Down
2 changes: 2 additions & 0 deletions crates/ruff_linter/src/rules/pylint/rules/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,7 @@ pub(crate) use too_many_arguments::*;
pub(crate) use too_many_boolean_expressions::*;
pub(crate) use too_many_branches::*;
pub(crate) use too_many_locals::*;
pub(crate) use too_many_nested_blocks::*;
pub(crate) use too_many_positional::*;
pub(crate) use too_many_public_methods::*;
pub(crate) use too_many_return_statements::*;
Expand Down Expand Up @@ -141,6 +142,7 @@ mod too_many_arguments;
mod too_many_boolean_expressions;
mod too_many_branches;
mod too_many_locals;
mod too_many_nested_blocks;
mod too_many_positional;
mod too_many_public_methods;
mod too_many_return_statements;
Expand Down
129 changes: 129 additions & 0 deletions crates/ruff_linter/src/rules/pylint/rules/too_many_nested_blocks.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,129 @@
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) {
// check that we're in a function
// if not, return
if !checker.semantic().current_scope().kind.is_function() {
return;
}

// check if this statement has any more branching statements
// if so, return
if stmt_has_more_stmts(stmt) {
return;
}

let max_nested_blocks = checker.settings.pylint.max_nested_blocks;

let (count, oldest_ancestor_id) =
checker
.semantic()
.current_statement_ids()
.fold((0, None), |(count, previous_id), id| {
let stmt = checker.semantic().statement(id);
if stmt.is_with_stmt()
|| stmt.is_if_stmt()
|| stmt.is_try_stmt()
|| stmt.is_while_stmt()
|| stmt.is_for_stmt()
{
// we want to emit the diagnostic on the
// oldest nested statement
return (count + 1, Some(id));
}
(count, previous_id)
});

let Some(oldest_ancestor_id) = oldest_ancestor_id else {
return;
};

if count <= max_nested_blocks {
return;
}

let oldest_ancestor = checker.semantic().statement(oldest_ancestor_id);

checker.diagnostics.push(Diagnostic::new(
TooManyNestedBlocks {
nested_blocks: count,
max_nested_blocks,
},
oldest_ancestor.range(),
));
}

fn stmt_has_more_stmts(stmt: &Stmt) -> bool {
match stmt {
Stmt::If(ast::StmtIf {
body,
elif_else_clauses,
..
}) => {
body.iter().any(stmt_has_more_stmts)
|| elif_else_clauses
.iter()
.any(|elif_else| elif_else.body.iter().any(stmt_has_more_stmts))
}
Stmt::While(ast::StmtWhile { body, orelse, .. }) => {
body.iter().any(stmt_has_more_stmts) || orelse.iter().any(stmt_has_more_stmts)
}
Stmt::For(ast::StmtFor { body, orelse, .. }) => {
body.iter().any(stmt_has_more_stmts) || orelse.iter().any(stmt_has_more_stmts)
}
Stmt::Try(ast::StmtTry {
body,
handlers,
orelse,
finalbody,
..
}) => {
body.iter().any(stmt_has_more_stmts)
|| handlers.iter().any(|handler| match handler {
ExceptHandler::ExceptHandler(ast::ExceptHandlerExceptHandler {
body, ..
}) => body.iter().any(stmt_has_more_stmts),
})
|| orelse.iter().any(stmt_has_more_stmts)
|| finalbody.iter().any(stmt_has_more_stmts)
}
Stmt::With(ast::StmtWith { body, .. }) => body.iter().any(stmt_has_more_stmts),
_ => false,
}
}
2 changes: 2 additions & 0 deletions crates/ruff_linter/src/rules/pylint/settings.rs
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,7 @@ pub struct Settings {
pub max_statements: usize,
pub max_public_methods: usize,
pub max_locals: usize,
pub max_nested_blocks: usize,
}

impl Default for Settings {
Expand All @@ -75,6 +76,7 @@ impl Default for Settings {
max_statements: 50,
max_public_methods: 20,
max_locals: 15,
max_nested_blocks: 5,
}
}
}
Expand Down
Original file line number Diff line number Diff line change
@@ -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
|


6 changes: 6 additions & 0 deletions crates/ruff_workspace/src/options.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2728,6 +2728,11 @@ pub struct PylintOptions {
/// (see: `PLR0916`).
#[option(default = r"5", value_type = "int", example = r"max-bool-expr = 5")]
pub max_bool_expr: Option<usize>,

/// Maximum number of nested blocks allowed within a function / 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 {
Expand All @@ -2751,6 +2756,7 @@ impl PylintOptions {
.max_public_methods
.unwrap_or(defaults.max_public_methods),
max_locals: self.max_locals.unwrap_or(defaults.max_locals),
max_nested_blocks: self.max_nested_blocks.unwrap_or(defaults.max_nested_blocks),
}
}
}
Expand Down
10 changes: 10 additions & 0 deletions ruff.schema.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

0 comments on commit 99431c8

Please sign in to comment.