Skip to content

Commit

Permalink
Implement nested with detection (SIM117) (#1651)
Browse files Browse the repository at this point in the history
  • Loading branch information
charliermarsh committed Jan 5, 2023
1 parent 2f71bdf commit fb8024a
Show file tree
Hide file tree
Showing 9 changed files with 88 additions and 17 deletions.
3 changes: 2 additions & 1 deletion README.md
Expand Up @@ -965,7 +965,8 @@ 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 | |
| SIM117 | MultipleWithStatements | Use a single `with` statement with multiple contexts instead of nested `with` statements | |
| 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
13 changes: 13 additions & 0 deletions resources/test/fixtures/flake8_simplify/SIM117.py
@@ -0,0 +1,13 @@
with A() as a: # SIM117
with B() as b:
print("hello")

with A() as a:
a()
with B() as b:
print("hello")

with A() as a:
with B() as b:
print("hello")
a()
3 changes: 3 additions & 0 deletions src/checkers/ast.rs
Expand Up @@ -1216,6 +1216,9 @@ where
if self.settings.enabled.contains(&CheckCode::PT012) {
flake8_pytest_style::plugins::complex_raises(self, stmt, items, body);
}
if self.settings.enabled.contains(&CheckCode::SIM117) {
flake8_simplify::plugins::multiple_with_statements(self, stmt);
}
}
StmtKind::While { body, orelse, .. } => {
if self.settings.enabled.contains(&CheckCode::B023) {
Expand Down
7 changes: 4 additions & 3 deletions src/flake8_simplify/mod.rs
Expand Up @@ -12,14 +12,15 @@ mod tests {
use crate::registry::CheckCode;
use crate::settings;

#[test_case(CheckCode::SIM102, Path::new("SIM102.py"); "SIM102")]
#[test_case(CheckCode::SIM105, Path::new("SIM105.py"); "SIM105")]
#[test_case(CheckCode::SIM117, Path::new("SIM117.py"); "SIM117")]
#[test_case(CheckCode::SIM118, Path::new("SIM118.py"); "SIM118")]
#[test_case(CheckCode::SIM220, Path::new("SIM220.py"); "SIM220")]
#[test_case(CheckCode::SIM221, Path::new("SIM221.py"); "SIM221")]
#[test_case(CheckCode::SIM222, Path::new("SIM222.py"); "SIM222")]
#[test_case(CheckCode::SIM223, Path::new("SIM223.py"); "SIM223")]
#[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
21 changes: 21 additions & 0 deletions src/flake8_simplify/plugins/ast_with.rs
@@ -0,0 +1,21 @@
use rustpython_ast::{Stmt, StmtKind};

use crate::ast::types::Range;
use crate::checkers::ast::Checker;
use crate::registry::{Check, CheckKind};

/// SIM117
pub fn multiple_with_statements(checker: &mut Checker, stmt: &Stmt) {
let StmtKind::With { body, .. } = &stmt.node else {
return;
};
if body.len() != 1 {
return;
}
if matches!(body[0].node, StmtKind::With { .. }) {
checker.add_check(Check::new(
CheckKind::MultipleWithStatements,
Range::from_located(stmt),
));
}
}
3 changes: 2 additions & 1 deletion src/flake8_simplify/plugins/mod.rs
@@ -1,11 +1,12 @@
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 ast_with::multiple_with_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 ast_bool_op;
mod ast_if;
mod ast_with;
mod key_in_dict;
mod use_contextlib_suppress;
mod yoda_conditions;
@@ -0,0 +1,14 @@
---
source: src/flake8_simplify/mod.rs
expression: checks
---
- kind: MultipleWithStatements
location:
row: 1
column: 0
end_location:
row: 3
column: 22
fix: ~
parent: ~

27 changes: 17 additions & 10 deletions src/registry.rs
Expand Up @@ -217,6 +217,7 @@ pub enum CheckCode {
YTT302,
YTT303,
// flake8-simplify
SIM117,
SIM102,
SIM105,
SIM118,
Expand Down Expand Up @@ -953,6 +954,7 @@ pub enum CheckKind {
SysVersionCmpStr10,
SysVersionSlice1Referenced,
// flake8-simplify
MultipleWithStatements,
NestedIfStatements,
AAndNotA(String),
AOrNotA(String),
Expand Down Expand Up @@ -1389,6 +1391,7 @@ impl CheckCode {
// flake8-simplify
CheckCode::SIM102 => CheckKind::NestedIfStatements,
CheckCode::SIM105 => CheckKind::UseContextlibSuppress("...".to_string()),
CheckCode::SIM117 => CheckKind::MultipleWithStatements,
CheckCode::SIM118 => CheckKind::KeyInDict("key".to_string(), "dict".to_string()),
CheckCode::SIM220 => CheckKind::AAndNotA("...".to_string()),
CheckCode::SIM221 => CheckKind::AOrNotA("...".to_string()),
Expand Down Expand Up @@ -1923,6 +1926,7 @@ impl CheckCode {
// flake8-simplify
CheckCode::SIM102 => CheckCategory::Flake8Simplify,
CheckCode::SIM105 => CheckCategory::Flake8Simplify,
CheckCode::SIM117 => CheckCategory::Flake8Simplify,
CheckCode::SIM118 => CheckCategory::Flake8Simplify,
CheckCode::SIM220 => CheckCategory::Flake8Simplify,
CheckCode::SIM221 => CheckCategory::Flake8Simplify,
Expand Down Expand Up @@ -2175,13 +2179,14 @@ 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,
CheckKind::AOrNotA(..) => &CheckCode::SIM221,
CheckKind::OrTrue => &CheckCode::SIM222,
CheckKind::AndFalse => &CheckCode::SIM223,
CheckKind::KeyInDict(..) => &CheckCode::SIM118,
CheckKind::MultipleWithStatements => &CheckCode::SIM117,
CheckKind::NestedIfStatements => &CheckCode::SIM102,
CheckKind::OrTrue => &CheckCode::SIM222,
CheckKind::UseContextlibSuppress(..) => &CheckCode::SIM105,
CheckKind::YodaConditions(..) => &CheckCode::SIM300,
// pyupgrade
CheckKind::ConvertNamedTupleFunctionalToClass(..) => &CheckCode::UP014,
Expand Down Expand Up @@ -2940,16 +2945,18 @@ 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::AAndNotA(name) => format!("Use `False` instead of `{name} and not {name}`"),
CheckKind::AOrNotA(name) => format!("Use `True` instead of `{name} or not {name}`"),
CheckKind::AndFalse => "Use `False` instead of `... and False`".to_string(),
CheckKind::KeyInDict(key, dict) => {
format!("Use `{key} in {dict}` instead of `{key} in {dict}.keys()`")
}
CheckKind::AAndNotA(name) => format!("Use `False` instead of `{name} and not {name}`"),
CheckKind::AOrNotA(name) => format!("Use `True` instead of `{name} or not {name}`"),
CheckKind::MultipleWithStatements => "Use a single `with` statement with multiple contexts instead of nested `with` \
statements".to_string(),
CheckKind::NestedIfStatements => {
"Use a single `if` statement instead of nested `if` statements".to_string()
}
CheckKind::OrTrue => "Use `True` instead of `... or True`".to_string(),
CheckKind::AndFalse => "Use `False` instead of `... and False`".to_string(),
CheckKind::YodaConditions(left, right) => {
format!("Yoda conditions are discouraged, use `{left} == {right}` instead")
}
Expand Down
14 changes: 12 additions & 2 deletions src/registry_gen.rs
Expand Up @@ -521,6 +521,7 @@ pub enum CheckCodePrefix {
SIM102,
SIM105,
SIM11,
SIM117,
SIM118,
SIM2,
SIM22,
Expand Down Expand Up @@ -807,6 +808,7 @@ impl CheckCodePrefix {
CheckCode::YTT301,
CheckCode::YTT302,
CheckCode::YTT303,
CheckCode::SIM117,
CheckCode::SIM102,
CheckCode::SIM105,
CheckCode::SIM118,
Expand Down Expand Up @@ -2631,6 +2633,7 @@ impl CheckCodePrefix {
CheckCodePrefix::S107 => vec![CheckCode::S107],
CheckCodePrefix::S108 => vec![CheckCode::S108],
CheckCodePrefix::SIM => vec![
CheckCode::SIM117,
CheckCode::SIM102,
CheckCode::SIM105,
CheckCode::SIM118,
Expand All @@ -2640,11 +2643,17 @@ impl CheckCodePrefix {
CheckCode::SIM223,
CheckCode::SIM300,
],
CheckCodePrefix::SIM1 => vec![CheckCode::SIM102, CheckCode::SIM105, CheckCode::SIM118],
CheckCodePrefix::SIM1 => vec![
CheckCode::SIM117,
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::SIM11 => vec![CheckCode::SIM117, CheckCode::SIM118],
CheckCodePrefix::SIM117 => vec![CheckCode::SIM117],
CheckCodePrefix::SIM118 => vec![CheckCode::SIM118],
CheckCodePrefix::SIM2 => vec![
CheckCode::SIM220,
Expand Down Expand Up @@ -3619,6 +3628,7 @@ impl CheckCodePrefix {
CheckCodePrefix::SIM102 => SuffixLength::Three,
CheckCodePrefix::SIM105 => SuffixLength::Three,
CheckCodePrefix::SIM11 => SuffixLength::Two,
CheckCodePrefix::SIM117 => SuffixLength::Three,
CheckCodePrefix::SIM118 => SuffixLength::Three,
CheckCodePrefix::SIM2 => SuffixLength::One,
CheckCodePrefix::SIM22 => SuffixLength::Two,
Expand Down

0 comments on commit fb8024a

Please sign in to comment.