Skip to content

Commit

Permalink
[pylint] Implement PLR0916 (too-many-boolean-expressions) (#7975)
Browse files Browse the repository at this point in the history
## Summary

Add
[R0916](https://pylint.readthedocs.io/en/latest/user_guide/messages/refactor/too-many-boolean-expressions.html),
no autofix available.

See: #970 

## Test Plan

`cargo test` and manually.
  • Loading branch information
diceroll123 authored Oct 17, 2023
1 parent 5da0f91 commit 21ea290
Show file tree
Hide file tree
Showing 11 changed files with 398 additions and 0 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,54 @@
if a:
...
elif (a and b):
...
elif (a and b) and c:
...
elif (a and b) and c and d:
...
elif (a and b) and c and d and e:
...
elif (a and b) and c and d and e and f:
...
elif (a and b) and c and d and e and f and g:
...
elif (a and b) and c and d and e and f and g and h:
...
elif (a and b) and c and d and e and f and g and h and i:
...
elif (a and b) and c and d and e and f and g and h and i and j:
...
elif (a and b) and c and d and e and f and g and h and i and j and k:
...
elif (a and b) and c and d and e and f and g and h and i and j and k and l:
...
elif (a and b) and c and d and e and f and g and h and i and j and k and l and m:
...
elif (a and b) and c and d and e and f and g and h and i and j and k and l and m and n:
...
elif (a and b) and c and d and e and f and g and h and i and j and k and l and m and n and o:
...
elif (a and b) and c and d and e and f and g and h and i and j and k and l and m and n and o and p:
...
elif (a and b) and c and d and e and f and g and h and i and j and k and l and m and n and o and p and q:
...
elif (a and b) and c and d and e and f and g and h and i and j and k and l and m and n and o and p and q and r:
...
elif (a and b) and c and d and e and f and g and h and i and j and k and l and m and n and o and p and q and r and s:
...
elif (a and b) and c and d and e and f and g and h and i and j and k and l and m and n and o and p and q and r and s and t:
...
elif (a and b) and c and d and e and f and g and h and i and j and k and l and m and n and o and p and q and r and s and t and u:
...
elif (a and b) and c and d and e and f and g and h and i and j and k and l and m and n and o and p and q and r and s and t and u and v:
...
elif (a and b) and c and d and e and f and g and h and i and j and k and l and m and n and o and p and q and r and s and t and u and v and w:
...
elif (a and b) and c and d and e and f and g and h and i and j and k and l and m and n and o and p and q and r and s and t and u and v and w and x:
...
elif (a and b) and c and d and e and f and g and h and i and j and k and l and m and n and o and p and q and r and s and t and u and v and w and x and y:
...
elif (a and b) and c and d and e and f and g and h and i and j and k and l and m and n and o and p and q and r and s and t and u and v and w and x and y and z:
...
else:
...
3 changes: 3 additions & 0 deletions crates/ruff_linter/src/checkers/ast/analyze/statement.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1070,6 +1070,9 @@ pub(crate) fn statement(stmt: &Stmt, checker: &mut Checker) {
if checker.enabled(Rule::CheckAndRemoveFromSet) {
refurb::rules::check_and_remove_from_set(checker, if_);
}
if checker.enabled(Rule::TooManyBooleanExpressions) {
pylint::rules::too_many_boolean_expressions(checker, if_);
}
if checker.source_type.is_stub() {
if checker.any_enabled(&[
Rule::UnrecognizedVersionInfoCheck,
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 @@ -247,6 +247,7 @@ pub fn code_to_rule(linter: Linter, code: &str) -> Option<(RuleGroup, Rule)> {
(Pylint, "R0912") => (RuleGroup::Stable, rules::pylint::rules::TooManyBranches),
(Pylint, "R0913") => (RuleGroup::Stable, rules::pylint::rules::TooManyArguments),
(Pylint, "R0915") => (RuleGroup::Stable, rules::pylint::rules::TooManyStatements),
(Pylint, "R0916") => (RuleGroup::Preview, rules::pylint::rules::TooManyBooleanExpressions),
(Pylint, "R1701") => (RuleGroup::Stable, rules::pylint::rules::RepeatedIsinstanceCalls),
(Pylint, "R1711") => (RuleGroup::Stable, rules::pylint::rules::UselessReturn),
(Pylint, "R1714") => (RuleGroup::Stable, rules::pylint::rules::RepeatedEqualityComparison),
Expand Down
16 changes: 16 additions & 0 deletions crates/ruff_linter/src/rules/pylint/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -231,6 +231,22 @@ mod tests {
Ok(())
}

#[test]
fn max_boolean_expressions() -> Result<()> {
let diagnostics = test_path(
Path::new("pylint/too_many_boolean_expressions.py"),
&LinterSettings {
pylint: pylint::settings::Settings {
max_bool_expr: 5,
..pylint::settings::Settings::default()
},
..LinterSettings::for_rule(Rule::TooManyBooleanExpressions)
},
)?;
assert_messages!(diagnostics);
Ok(())
}

#[test]
fn max_statements() -> Result<()> {
let diagnostics = test_path(
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 @@ -45,6 +45,7 @@ pub(crate) use subprocess_popen_preexec_fn::*;
pub(crate) use subprocess_run_without_check::*;
pub(crate) use sys_exit_alias::*;
pub(crate) use too_many_arguments::*;
pub(crate) use too_many_boolean_expressions::*;
pub(crate) use too_many_branches::*;
pub(crate) use too_many_public_methods::*;
pub(crate) use too_many_return_statements::*;
Expand Down Expand Up @@ -108,6 +109,7 @@ mod subprocess_popen_preexec_fn;
mod subprocess_run_without_check;
mod sys_exit_alias;
mod too_many_arguments;
mod too_many_boolean_expressions;
mod too_many_branches;
mod too_many_public_methods;
mod too_many_return_statements;
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,89 @@
use ast::{Expr, StmtIf};
use ruff_diagnostics::{Diagnostic, Violation};
use ruff_macros::{derive_message_formats, violation};
use ruff_python_ast as ast;
use ruff_text_size::Ranged;

use crate::checkers::ast::Checker;

/// ## What it does
/// Checks for too many Boolean expressions in an `if` statement.
///
/// By default, this rule allows up to 5 expressions. This can be configured
/// using the [`pylint.max-bool-expr`] option.
///
/// ## Why is this bad?
/// `if` statements with many Boolean expressions are harder to understand
/// and maintain. Consider assigning the result of the Boolean expression,
/// or any of its sub-expressions, to a variable.
///
/// ## Example
/// ```python
/// if a and b and c and d and e and f and g and h:
/// ...
/// ```
///
/// ## Options
/// - `pylint.max-bool-expr`
#[violation]
pub struct TooManyBooleanExpressions {
expressions: usize,
max_expressions: usize,
}

impl Violation for TooManyBooleanExpressions {
#[derive_message_formats]
fn message(&self) -> String {
let TooManyBooleanExpressions {
expressions,
max_expressions,
} = self;
format!("Too many Boolean expressions ({expressions} > {max_expressions})")
}
}

/// PLR0916
pub(crate) fn too_many_boolean_expressions(checker: &mut Checker, stmt: &StmtIf) {
if let Some(bool_op) = stmt.test.as_bool_op_expr() {
let expressions = count_bools(bool_op);
if expressions > checker.settings.pylint.max_bool_expr {
checker.diagnostics.push(Diagnostic::new(
TooManyBooleanExpressions {
expressions,
max_expressions: checker.settings.pylint.max_bool_expr,
},
bool_op.range(),
));
}
}

for elif in &stmt.elif_else_clauses {
if let Some(bool_op) = elif.test.as_ref().and_then(Expr::as_bool_op_expr) {
let expressions = count_bools(bool_op);
if expressions > checker.settings.pylint.max_bool_expr {
checker.diagnostics.push(Diagnostic::new(
TooManyBooleanExpressions {
expressions,
max_expressions: checker.settings.pylint.max_bool_expr,
},
bool_op.range(),
));
}
}
}
}

/// Count the number of Boolean expressions in a `bool_op` expression.
fn count_bools(bool_op: &ast::ExprBoolOp) -> usize {
bool_op
.values
.iter()
.map(|expr| {
if let Expr::BoolOp(bool_op) = expr {
count_bools(bool_op)
} else {
1
}
})
.sum::<usize>()
}
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 @@ -40,6 +40,7 @@ pub struct Settings {
pub allow_magic_value_types: Vec<ConstantType>,
pub max_args: usize,
pub max_returns: usize,
pub max_bool_expr: usize,
pub max_branches: usize,
pub max_statements: usize,
pub max_public_methods: usize,
Expand All @@ -51,6 +52,7 @@ impl Default for Settings {
allow_magic_value_types: vec![ConstantType::Str, ConstantType::Bytes],
max_args: 5,
max_returns: 6,
max_bool_expr: 5,
max_branches: 12,
max_statements: 50,
max_public_methods: 20,
Expand Down
Loading

0 comments on commit 21ea290

Please sign in to comment.