Implement nested with detection (SIM117) (#1651)

2023-01-04 21:22:25 -05:00 · 2023-01-04 21:22:25 -05:00 · fb8024a6ac
parent 2f71bdfbfc
commit fb8024a6ac
9 changed files with 88 additions and 17 deletions
--- a/README.md
+++ b/README.md
@ -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 ...` | 🛠 |
--- a/resources/test/fixtures/flake8_simplify/SIM117.py
+++ b/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()
--- a/src/checkers/ast.rs
+++ b/src/checkers/ast.rs
@ -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) {
--- a/src/flake8_simplify/mod.rs
+++ b/src/flake8_simplify/mod.rs
@ -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(
--- a/src/flake8_simplify/plugins/ast_with.rs
+++ b/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),
+        ));
+    }
+}
--- a/src/flake8_simplify/plugins/mod.rs
+++ b/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;
--- a/src/flake8_simplify/snapshots/ruff__flake8_simplifytestsSIM117_SIM117.py.snap
+++ b/src/flake8_simplify/snapshots/ruff__flake8_simplifytestsSIM117_SIM117.py.snap
@ -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: ~
+
--- a/src/registry.rs
+++ b/src/registry.rs
@ -217,6 +217,7 @@ pub enum CheckCode {
    YTT302,
    YTT303,
    // flake8-simplify
+    SIM117,
    SIM102,
    SIM105,
    SIM118,
@ -953,6 +954,7 @@ pub enum CheckKind {
    SysVersionCmpStr10,
    SysVersionSlice1Referenced,
    // flake8-simplify
+    MultipleWithStatements,
    NestedIfStatements,
    AAndNotA(String),
    AOrNotA(String),
@ -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()),
@ -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,
@ -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,
@ -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")
            }
--- a/src/registry_gen.rs
+++ b/src/registry_gen.rs
@ -521,6 +521,7 @@ pub enum CheckCodePrefix {
    SIM102,
    SIM105,
    SIM11,
+    SIM117,
    SIM118,
    SIM2,
    SIM22,
@ -807,6 +808,7 @@ impl CheckCodePrefix {
                CheckCode::YTT301,
                CheckCode::YTT302,
                CheckCode::YTT303,
+                CheckCode::SIM117,
                CheckCode::SIM102,
                CheckCode::SIM105,
                CheckCode::SIM118,
@ -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,
@ -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,
@ -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,