Skip to content

Commit

Permalink
Implement nested-if detection (#1649)
Browse files Browse the repository at this point in the history
  • Loading branch information
charliermarsh committed Jan 5, 2023
1 parent 9041423 commit 30d6688
Show file tree
Hide file tree
Showing 10 changed files with 136 additions and 4 deletions.
1 change: 1 addition & 0 deletions README.md
Expand Up @@ -964,6 +964,7 @@ For more, see [flake8-simplify](https://pypi.org/project/flake8-simplify/0.19.3/
| 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 | |
| SIM118 | KeyInDict | Use `key in dict` instead of `key in dict.keys()` | 🛠 |
| SIM220 | AAndNotA | Use `False` instead of `... and not ...` | 🛠 |
Expand Down
25 changes: 25 additions & 0 deletions resources/test/fixtures/flake8_simplify/SIM102.py
@@ -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
3 changes: 3 additions & 0 deletions src/checkers/ast.rs
Expand Up @@ -1177,6 +1177,9 @@ where
if self.settings.enabled.contains(&CheckCode::F634) {
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 } => {
if self.settings.enabled.contains(&CheckCode::F631) {
Expand Down
1 change: 1 addition & 0 deletions src/flake8_simplify/mod.rs
Expand Up @@ -19,6 +19,7 @@ mod tests {
#[test_case(CheckCode::SIM300, Path::new("SIM300.py"); "SIM300")]
#[test_case(CheckCode::SIM221, Path::new("SIM221.py"); "SIM221")]
#[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<()> {
let snapshot = format!("{}_{}", check_code.as_ref(), path.to_string_lossy());
let checks = test_path(
Expand Down
File renamed without changes.
64 changes: 64 additions & 0 deletions src/flake8_simplify/plugins/ast_if.rs
@@ -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),
));
}
6 changes: 4 additions & 2 deletions src/flake8_simplify/plugins/mod.rs
@@ -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 use_contextlib_suppress::use_contextlib_suppress;
pub use yoda_conditions::yoda_conditions;

mod bool_ops;
mod ast_bool_op;
mod ast_if;
mod key_in_dict;
mod use_contextlib_suppress;
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: ~

8 changes: 8 additions & 0 deletions src/registry.rs
Expand Up @@ -217,6 +217,7 @@ pub enum CheckCode {
YTT302,
YTT303,
// flake8-simplify
SIM102,
SIM105,
SIM118,
SIM220,
Expand Down Expand Up @@ -951,6 +952,7 @@ pub enum CheckKind {
SysVersionCmpStr10,
SysVersionSlice1Referenced,
// flake8-simplify
NestedIfStatements,
AAndNotA(String),
AOrNotA(String),
KeyInDict(String, String),
Expand Down Expand Up @@ -1383,6 +1385,7 @@ impl CheckCode {
// flake8-blind-except
CheckCode::BLE001 => CheckKind::BlindExcept("Exception".to_string()),
// flake8-simplify
CheckCode::SIM102 => CheckKind::NestedIfStatements,
CheckCode::SIM105 => CheckKind::UseContextlibSuppress("...".to_string()),
CheckCode::SIM118 => CheckKind::KeyInDict("key".to_string(), "dict".to_string()),
CheckCode::SIM220 => CheckKind::AAndNotA("...".to_string()),
Expand Down Expand Up @@ -1914,6 +1917,7 @@ impl CheckCode {
CheckCode::S106 => CheckCategory::Flake8Bandit,
CheckCode::S107 => CheckCategory::Flake8Bandit,
// flake8-simplify
CheckCode::SIM102 => CheckCategory::Flake8Simplify,
CheckCode::SIM105 => CheckCategory::Flake8Simplify,
CheckCode::SIM118 => CheckCategory::Flake8Simplify,
CheckCode::SIM220 => CheckCategory::Flake8Simplify,
Expand Down Expand Up @@ -2167,6 +2171,7 @@ impl CheckKind {
CheckKind::SysVersionCmpStr10 => &CheckCode::YTT302,
CheckKind::SysVersionSlice1Referenced => &CheckCode::YTT303,
// flake8-simplify
CheckKind::NestedIfStatements => &CheckCode::SIM102,
CheckKind::UseContextlibSuppress(..) => &CheckCode::SIM105,
CheckKind::KeyInDict(..) => &CheckCode::SIM118,
CheckKind::AAndNotA(..) => &CheckCode::SIM220,
Expand Down Expand Up @@ -2930,6 +2935,9 @@ impl CheckKind {
"`sys.version[:1]` referenced (python10), use `sys.version_info`".to_string()
}
// flake8-simplify
CheckKind::NestedIfStatements => {
"Use a single if-statement instead of nested if-statements".to_string()
}
CheckKind::KeyInDict(key, dict) => {
format!("Use `{key} in {dict}` instead of `{key} in {dict}.keys()`")
}
Expand Down
9 changes: 7 additions & 2 deletions src/registry_gen.rs
Expand Up @@ -517,6 +517,7 @@ pub enum CheckCodePrefix {
SIM,
SIM1,
SIM10,
SIM102,
SIM105,
SIM11,
SIM118,
Expand Down Expand Up @@ -805,6 +806,7 @@ impl CheckCodePrefix {
CheckCode::YTT301,
CheckCode::YTT302,
CheckCode::YTT303,
CheckCode::SIM102,
CheckCode::SIM105,
CheckCode::SIM118,
CheckCode::SIM220,
Expand Down Expand Up @@ -2623,6 +2625,7 @@ impl CheckCodePrefix {
CheckCodePrefix::S106 => vec![CheckCode::S106],
CheckCodePrefix::S107 => vec![CheckCode::S107],
CheckCodePrefix::SIM => vec![
CheckCode::SIM102,
CheckCode::SIM105,
CheckCode::SIM118,
CheckCode::SIM220,
Expand All @@ -2631,8 +2634,9 @@ impl CheckCodePrefix {
CheckCode::SIM223,
CheckCode::SIM300,
],
CheckCodePrefix::SIM1 => vec![CheckCode::SIM105, CheckCode::SIM118],
CheckCodePrefix::SIM10 => vec![CheckCode::SIM105],
CheckCodePrefix::SIM1 => vec![CheckCode::SIM102, CheckCode::SIM105, CheckCode::SIM118],
CheckCodePrefix::SIM10 => vec![CheckCode::SIM102, CheckCode::SIM105],
CheckCodePrefix::SIM102 => vec![CheckCode::SIM102],
CheckCodePrefix::SIM105 => vec![CheckCode::SIM105],
CheckCodePrefix::SIM11 => vec![CheckCode::SIM118],
CheckCodePrefix::SIM118 => vec![CheckCode::SIM118],
Expand Down Expand Up @@ -3605,6 +3609,7 @@ impl CheckCodePrefix {
CheckCodePrefix::SIM => SuffixLength::Zero,
CheckCodePrefix::SIM1 => SuffixLength::One,
CheckCodePrefix::SIM10 => SuffixLength::Two,
CheckCodePrefix::SIM102 => SuffixLength::Three,
CheckCodePrefix::SIM105 => SuffixLength::Three,
CheckCodePrefix::SIM11 => SuffixLength::Two,
CheckCodePrefix::SIM118 => SuffixLength::Three,
Expand Down

0 comments on commit 30d6688

Please sign in to comment.