mirror of https://github.com/astral-sh/ruff
Implement nested-if detection (#1649)
This commit is contained in:
parent
9041423b92
commit
30d6688c26
|
|
@ -964,6 +964,7 @@ For more, see [flake8-simplify](https://pypi.org/project/flake8-simplify/0.19.3/
|
||||||
|
|
||||||
| Code | Name | Message | Fix |
|
| Code | Name | Message | Fix |
|
||||||
| ---- | ---- | ------- | --- |
|
| ---- | ---- | ------- | --- |
|
||||||
|
| SIM102 | NestedIfStatements | Use a single if-statement instead of nested if-statements | |
|
||||||
| SIM105 | UseContextlibSuppress | Use `contextlib.suppress(...)` instead of try-except-pass | |
|
| SIM105 | UseContextlibSuppress | Use `contextlib.suppress(...)` instead of try-except-pass | |
|
||||||
| SIM118 | KeyInDict | Use `key in dict` instead of `key in dict.keys()` | 🛠 |
|
| SIM118 | KeyInDict | Use `key in dict` instead of `key in dict.keys()` | 🛠 |
|
||||||
| SIM220 | AAndNotA | Use `False` instead of `... and not ...` | 🛠 |
|
| SIM220 | AAndNotA | Use `False` instead of `... and not ...` | 🛠 |
|
||||||
|
|
|
||||||
|
|
@ -0,0 +1,25 @@
|
||||||
|
if a: # SIM102
|
||||||
|
if b:
|
||||||
|
c
|
||||||
|
|
||||||
|
|
||||||
|
if a:
|
||||||
|
pass
|
||||||
|
elif b: # SIM102
|
||||||
|
if c:
|
||||||
|
d
|
||||||
|
|
||||||
|
if a:
|
||||||
|
if b:
|
||||||
|
c
|
||||||
|
else:
|
||||||
|
d
|
||||||
|
|
||||||
|
if __name__ == "__main__":
|
||||||
|
if foo():
|
||||||
|
...
|
||||||
|
|
||||||
|
if a:
|
||||||
|
d
|
||||||
|
if b:
|
||||||
|
c
|
||||||
|
|
@ -1177,6 +1177,9 @@ where
|
||||||
if self.settings.enabled.contains(&CheckCode::F634) {
|
if self.settings.enabled.contains(&CheckCode::F634) {
|
||||||
pyflakes::plugins::if_tuple(self, stmt, test);
|
pyflakes::plugins::if_tuple(self, stmt, test);
|
||||||
}
|
}
|
||||||
|
if self.settings.enabled.contains(&CheckCode::SIM102) {
|
||||||
|
flake8_simplify::plugins::nested_if_statements(self, stmt);
|
||||||
|
}
|
||||||
}
|
}
|
||||||
StmtKind::Assert { test, msg } => {
|
StmtKind::Assert { test, msg } => {
|
||||||
if self.settings.enabled.contains(&CheckCode::F631) {
|
if self.settings.enabled.contains(&CheckCode::F631) {
|
||||||
|
|
|
||||||
|
|
@ -19,6 +19,7 @@ mod tests {
|
||||||
#[test_case(CheckCode::SIM300, Path::new("SIM300.py"); "SIM300")]
|
#[test_case(CheckCode::SIM300, Path::new("SIM300.py"); "SIM300")]
|
||||||
#[test_case(CheckCode::SIM221, Path::new("SIM221.py"); "SIM221")]
|
#[test_case(CheckCode::SIM221, Path::new("SIM221.py"); "SIM221")]
|
||||||
#[test_case(CheckCode::SIM220, Path::new("SIM220.py"); "SIM220")]
|
#[test_case(CheckCode::SIM220, Path::new("SIM220.py"); "SIM220")]
|
||||||
|
#[test_case(CheckCode::SIM102, Path::new("SIM102.py"); "SIM102")]
|
||||||
fn checks(check_code: CheckCode, path: &Path) -> Result<()> {
|
fn checks(check_code: CheckCode, path: &Path) -> Result<()> {
|
||||||
let snapshot = format!("{}_{}", check_code.as_ref(), path.to_string_lossy());
|
let snapshot = format!("{}_{}", check_code.as_ref(), path.to_string_lossy());
|
||||||
let checks = test_path(
|
let checks = test_path(
|
||||||
|
|
|
||||||
|
|
@ -0,0 +1,64 @@
|
||||||
|
use rustpython_ast::{Constant, Expr, ExprKind, Stmt, StmtKind};
|
||||||
|
|
||||||
|
use crate::ast::types::Range;
|
||||||
|
use crate::checkers::ast::Checker;
|
||||||
|
use crate::registry::{Check, CheckKind};
|
||||||
|
|
||||||
|
fn is_main_check(expr: &Expr) -> bool {
|
||||||
|
if let ExprKind::Compare {
|
||||||
|
left, comparators, ..
|
||||||
|
} = &expr.node
|
||||||
|
{
|
||||||
|
if let ExprKind::Name { id, .. } = &left.node {
|
||||||
|
if id == "__name__" {
|
||||||
|
if comparators.len() == 1 {
|
||||||
|
if let ExprKind::Constant {
|
||||||
|
value: Constant::Str(value),
|
||||||
|
..
|
||||||
|
} = &comparators[0].node
|
||||||
|
{
|
||||||
|
if value == "__main__" {
|
||||||
|
return true;
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
false
|
||||||
|
}
|
||||||
|
|
||||||
|
/// SIM102
|
||||||
|
pub fn nested_if_statements(checker: &mut Checker, stmt: &Stmt) {
|
||||||
|
let StmtKind::If { test, body, orelse } = &stmt.node else {
|
||||||
|
return;
|
||||||
|
};
|
||||||
|
|
||||||
|
// if a: <---
|
||||||
|
// if b: <---
|
||||||
|
// c
|
||||||
|
let is_nested_if = {
|
||||||
|
if orelse.is_empty() && body.len() == 1 {
|
||||||
|
if let StmtKind::If { orelse, .. } = &body[0].node {
|
||||||
|
orelse.is_empty()
|
||||||
|
} else {
|
||||||
|
false
|
||||||
|
}
|
||||||
|
} else {
|
||||||
|
false
|
||||||
|
}
|
||||||
|
};
|
||||||
|
|
||||||
|
if !is_nested_if {
|
||||||
|
return;
|
||||||
|
};
|
||||||
|
|
||||||
|
if is_main_check(test) {
|
||||||
|
return;
|
||||||
|
}
|
||||||
|
|
||||||
|
checker.add_check(Check::new(
|
||||||
|
CheckKind::NestedIfStatements,
|
||||||
|
Range::from_located(stmt),
|
||||||
|
));
|
||||||
|
}
|
||||||
|
|
@ -1,9 +1,11 @@
|
||||||
pub use bool_ops::{a_and_not_a, a_or_not_a, and_false, or_true};
|
pub use ast_bool_op::{a_and_not_a, a_or_not_a, and_false, or_true};
|
||||||
|
pub use ast_if::nested_if_statements;
|
||||||
pub use key_in_dict::{key_in_dict_compare, key_in_dict_for};
|
pub use key_in_dict::{key_in_dict_compare, key_in_dict_for};
|
||||||
pub use use_contextlib_suppress::use_contextlib_suppress;
|
pub use use_contextlib_suppress::use_contextlib_suppress;
|
||||||
pub use yoda_conditions::yoda_conditions;
|
pub use yoda_conditions::yoda_conditions;
|
||||||
|
|
||||||
mod bool_ops;
|
mod ast_bool_op;
|
||||||
|
mod ast_if;
|
||||||
mod key_in_dict;
|
mod key_in_dict;
|
||||||
mod use_contextlib_suppress;
|
mod use_contextlib_suppress;
|
||||||
mod yoda_conditions;
|
mod yoda_conditions;
|
||||||
|
|
|
||||||
|
|
@ -0,0 +1,23 @@
|
||||||
|
---
|
||||||
|
source: src/flake8_simplify/mod.rs
|
||||||
|
expression: checks
|
||||||
|
---
|
||||||
|
- kind: NestedIfStatements
|
||||||
|
location:
|
||||||
|
row: 1
|
||||||
|
column: 0
|
||||||
|
end_location:
|
||||||
|
row: 3
|
||||||
|
column: 9
|
||||||
|
fix: ~
|
||||||
|
parent: ~
|
||||||
|
- kind: NestedIfStatements
|
||||||
|
location:
|
||||||
|
row: 8
|
||||||
|
column: 0
|
||||||
|
end_location:
|
||||||
|
row: 10
|
||||||
|
column: 9
|
||||||
|
fix: ~
|
||||||
|
parent: ~
|
||||||
|
|
||||||
|
|
@ -217,6 +217,7 @@ pub enum CheckCode {
|
||||||
YTT302,
|
YTT302,
|
||||||
YTT303,
|
YTT303,
|
||||||
// flake8-simplify
|
// flake8-simplify
|
||||||
|
SIM102,
|
||||||
SIM105,
|
SIM105,
|
||||||
SIM118,
|
SIM118,
|
||||||
SIM220,
|
SIM220,
|
||||||
|
|
@ -951,6 +952,7 @@ pub enum CheckKind {
|
||||||
SysVersionCmpStr10,
|
SysVersionCmpStr10,
|
||||||
SysVersionSlice1Referenced,
|
SysVersionSlice1Referenced,
|
||||||
// flake8-simplify
|
// flake8-simplify
|
||||||
|
NestedIfStatements,
|
||||||
AAndNotA(String),
|
AAndNotA(String),
|
||||||
AOrNotA(String),
|
AOrNotA(String),
|
||||||
KeyInDict(String, String),
|
KeyInDict(String, String),
|
||||||
|
|
@ -1383,6 +1385,7 @@ impl CheckCode {
|
||||||
// flake8-blind-except
|
// flake8-blind-except
|
||||||
CheckCode::BLE001 => CheckKind::BlindExcept("Exception".to_string()),
|
CheckCode::BLE001 => CheckKind::BlindExcept("Exception".to_string()),
|
||||||
// flake8-simplify
|
// flake8-simplify
|
||||||
|
CheckCode::SIM102 => CheckKind::NestedIfStatements,
|
||||||
CheckCode::SIM105 => CheckKind::UseContextlibSuppress("...".to_string()),
|
CheckCode::SIM105 => CheckKind::UseContextlibSuppress("...".to_string()),
|
||||||
CheckCode::SIM118 => CheckKind::KeyInDict("key".to_string(), "dict".to_string()),
|
CheckCode::SIM118 => CheckKind::KeyInDict("key".to_string(), "dict".to_string()),
|
||||||
CheckCode::SIM220 => CheckKind::AAndNotA("...".to_string()),
|
CheckCode::SIM220 => CheckKind::AAndNotA("...".to_string()),
|
||||||
|
|
@ -1914,6 +1917,7 @@ impl CheckCode {
|
||||||
CheckCode::S106 => CheckCategory::Flake8Bandit,
|
CheckCode::S106 => CheckCategory::Flake8Bandit,
|
||||||
CheckCode::S107 => CheckCategory::Flake8Bandit,
|
CheckCode::S107 => CheckCategory::Flake8Bandit,
|
||||||
// flake8-simplify
|
// flake8-simplify
|
||||||
|
CheckCode::SIM102 => CheckCategory::Flake8Simplify,
|
||||||
CheckCode::SIM105 => CheckCategory::Flake8Simplify,
|
CheckCode::SIM105 => CheckCategory::Flake8Simplify,
|
||||||
CheckCode::SIM118 => CheckCategory::Flake8Simplify,
|
CheckCode::SIM118 => CheckCategory::Flake8Simplify,
|
||||||
CheckCode::SIM220 => CheckCategory::Flake8Simplify,
|
CheckCode::SIM220 => CheckCategory::Flake8Simplify,
|
||||||
|
|
@ -2167,6 +2171,7 @@ impl CheckKind {
|
||||||
CheckKind::SysVersionCmpStr10 => &CheckCode::YTT302,
|
CheckKind::SysVersionCmpStr10 => &CheckCode::YTT302,
|
||||||
CheckKind::SysVersionSlice1Referenced => &CheckCode::YTT303,
|
CheckKind::SysVersionSlice1Referenced => &CheckCode::YTT303,
|
||||||
// flake8-simplify
|
// flake8-simplify
|
||||||
|
CheckKind::NestedIfStatements => &CheckCode::SIM102,
|
||||||
CheckKind::UseContextlibSuppress(..) => &CheckCode::SIM105,
|
CheckKind::UseContextlibSuppress(..) => &CheckCode::SIM105,
|
||||||
CheckKind::KeyInDict(..) => &CheckCode::SIM118,
|
CheckKind::KeyInDict(..) => &CheckCode::SIM118,
|
||||||
CheckKind::AAndNotA(..) => &CheckCode::SIM220,
|
CheckKind::AAndNotA(..) => &CheckCode::SIM220,
|
||||||
|
|
@ -2930,6 +2935,9 @@ impl CheckKind {
|
||||||
"`sys.version[:1]` referenced (python10), use `sys.version_info`".to_string()
|
"`sys.version[:1]` referenced (python10), use `sys.version_info`".to_string()
|
||||||
}
|
}
|
||||||
// flake8-simplify
|
// flake8-simplify
|
||||||
|
CheckKind::NestedIfStatements => {
|
||||||
|
"Use a single if-statement instead of nested if-statements".to_string()
|
||||||
|
}
|
||||||
CheckKind::KeyInDict(key, dict) => {
|
CheckKind::KeyInDict(key, dict) => {
|
||||||
format!("Use `{key} in {dict}` instead of `{key} in {dict}.keys()`")
|
format!("Use `{key} in {dict}` instead of `{key} in {dict}.keys()`")
|
||||||
}
|
}
|
||||||
|
|
|
||||||
|
|
@ -517,6 +517,7 @@ pub enum CheckCodePrefix {
|
||||||
SIM,
|
SIM,
|
||||||
SIM1,
|
SIM1,
|
||||||
SIM10,
|
SIM10,
|
||||||
|
SIM102,
|
||||||
SIM105,
|
SIM105,
|
||||||
SIM11,
|
SIM11,
|
||||||
SIM118,
|
SIM118,
|
||||||
|
|
@ -805,6 +806,7 @@ impl CheckCodePrefix {
|
||||||
CheckCode::YTT301,
|
CheckCode::YTT301,
|
||||||
CheckCode::YTT302,
|
CheckCode::YTT302,
|
||||||
CheckCode::YTT303,
|
CheckCode::YTT303,
|
||||||
|
CheckCode::SIM102,
|
||||||
CheckCode::SIM105,
|
CheckCode::SIM105,
|
||||||
CheckCode::SIM118,
|
CheckCode::SIM118,
|
||||||
CheckCode::SIM220,
|
CheckCode::SIM220,
|
||||||
|
|
@ -2623,6 +2625,7 @@ impl CheckCodePrefix {
|
||||||
CheckCodePrefix::S106 => vec![CheckCode::S106],
|
CheckCodePrefix::S106 => vec![CheckCode::S106],
|
||||||
CheckCodePrefix::S107 => vec![CheckCode::S107],
|
CheckCodePrefix::S107 => vec![CheckCode::S107],
|
||||||
CheckCodePrefix::SIM => vec![
|
CheckCodePrefix::SIM => vec![
|
||||||
|
CheckCode::SIM102,
|
||||||
CheckCode::SIM105,
|
CheckCode::SIM105,
|
||||||
CheckCode::SIM118,
|
CheckCode::SIM118,
|
||||||
CheckCode::SIM220,
|
CheckCode::SIM220,
|
||||||
|
|
@ -2631,8 +2634,9 @@ impl CheckCodePrefix {
|
||||||
CheckCode::SIM223,
|
CheckCode::SIM223,
|
||||||
CheckCode::SIM300,
|
CheckCode::SIM300,
|
||||||
],
|
],
|
||||||
CheckCodePrefix::SIM1 => vec![CheckCode::SIM105, CheckCode::SIM118],
|
CheckCodePrefix::SIM1 => vec![CheckCode::SIM102, CheckCode::SIM105, CheckCode::SIM118],
|
||||||
CheckCodePrefix::SIM10 => vec![CheckCode::SIM105],
|
CheckCodePrefix::SIM10 => vec![CheckCode::SIM102, CheckCode::SIM105],
|
||||||
|
CheckCodePrefix::SIM102 => vec![CheckCode::SIM102],
|
||||||
CheckCodePrefix::SIM105 => vec![CheckCode::SIM105],
|
CheckCodePrefix::SIM105 => vec![CheckCode::SIM105],
|
||||||
CheckCodePrefix::SIM11 => vec![CheckCode::SIM118],
|
CheckCodePrefix::SIM11 => vec![CheckCode::SIM118],
|
||||||
CheckCodePrefix::SIM118 => vec![CheckCode::SIM118],
|
CheckCodePrefix::SIM118 => vec![CheckCode::SIM118],
|
||||||
|
|
@ -3605,6 +3609,7 @@ impl CheckCodePrefix {
|
||||||
CheckCodePrefix::SIM => SuffixLength::Zero,
|
CheckCodePrefix::SIM => SuffixLength::Zero,
|
||||||
CheckCodePrefix::SIM1 => SuffixLength::One,
|
CheckCodePrefix::SIM1 => SuffixLength::One,
|
||||||
CheckCodePrefix::SIM10 => SuffixLength::Two,
|
CheckCodePrefix::SIM10 => SuffixLength::Two,
|
||||||
|
CheckCodePrefix::SIM102 => SuffixLength::Three,
|
||||||
CheckCodePrefix::SIM105 => SuffixLength::Three,
|
CheckCodePrefix::SIM105 => SuffixLength::Three,
|
||||||
CheckCodePrefix::SIM11 => SuffixLength::Two,
|
CheckCodePrefix::SIM11 => SuffixLength::Two,
|
||||||
CheckCodePrefix::SIM118 => SuffixLength::Three,
|
CheckCodePrefix::SIM118 => SuffixLength::Three,
|
||||||
|
|
|
||||||
Loading…
Reference in New Issue