Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implement nested-if detection #1649

Merged
merged 1 commit into from Jan 5, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
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
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