[flake8-simplify] Add Rule for SIM115 (Use context handler for op…

…ening files) (#1782)

ref: #998

Co-authored-by: Charlie Marsh <[email protected]>

README.md

@@ -975,6 +975,7 @@ For more, see [flake8-simplify](https://pypi.org/project/flake8-simplify/0.19.3/
 
 | Code | Name | Message | Fix |
 | ---- | ---- | ------- | --- |
+| SIM115 | OpenFileWithContextHandler | Use context handler for opening files |  |
 | SIM101 | DuplicateIsinstanceCall | Multiple `isinstance` calls for `...`, merge into a single call | 🛠 |
 | SIM102 | NestedIfStatements | Use a single `if` statement instead of nested `if` statements |  |
 | SIM103 | ReturnBoolConditionDirectly | Return the condition `...` directly | 🛠 |

resources/test/fixtures/flake8_simplify/SIM115.py

@@ -0,0 +1,6 @@
+f = open('foo.txt')  # SIM115
+data = f.read()
+f.close()
+
+with open('foo.txt') as f:  # OK
+    data = f.read()

ruff.schema.json

@@ -1520,6 +1520,7 @@
         "SIM110",
         "SIM111",
         "SIM112",
+        "SIM115",
         "SIM117",
         "SIM118",
         "SIM2",

src/checkers/ast.rs

@@ -2321,6 +2321,11 @@ where
                             args, keywords,
                         ));
                 }
+
+                // flake8-simplify
+                if self.settings.enabled.contains(&RuleCode::SIM115) {
+                    flake8_simplify::rules::open_file_with_context_handler(self, func);
+                }
             }
             ExprKind::Dict { keys, values } => {
                 if self.settings.enabled.contains(&RuleCode::F601)

src/flake8_simplify/mod.rs

@@ -22,14 +22,15 @@ mod tests {
     #[test_case(RuleCode::SIM110, Path::new("SIM110.py"); "SIM110")]
     #[test_case(RuleCode::SIM111, Path::new("SIM111.py"); "SIM111")]
     #[test_case(RuleCode::SIM112, Path::new("SIM112.py"); "SIM112")]
+    #[test_case(RuleCode::SIM115, Path::new("SIM115.py"); "SIM115")]
     #[test_case(RuleCode::SIM117, Path::new("SIM117.py"); "SIM117")]
+    #[test_case(RuleCode::SIM118, Path::new("SIM118.py"); "SIM118")]
     #[test_case(RuleCode::SIM201, Path::new("SIM201.py"); "SIM201")]
     #[test_case(RuleCode::SIM202, Path::new("SIM202.py"); "SIM202")]
     #[test_case(RuleCode::SIM208, Path::new("SIM208.py"); "SIM208")]
     #[test_case(RuleCode::SIM210, Path::new("SIM210.py"); "SIM210")]
     #[test_case(RuleCode::SIM211, Path::new("SIM211.py"); "SIM211")]
     #[test_case(RuleCode::SIM212, Path::new("SIM212.py"); "SIM212")]
-    #[test_case(RuleCode::SIM118, Path::new("SIM118.py"); "SIM118")]
     #[test_case(RuleCode::SIM220, Path::new("SIM220.py"); "SIM220")]
     #[test_case(RuleCode::SIM221, Path::new("SIM221.py"); "SIM221")]
     #[test_case(RuleCode::SIM222, Path::new("SIM222.py"); "SIM222")]

src/flake8_simplify/rules/mod.rs

@@ -10,6 +10,7 @@ pub use ast_ifexp::{
 pub use ast_unary_op::{double_negation, negation_with_equal_op, negation_with_not_equal_op};
 pub use ast_with::multiple_with_statements;
 pub use key_in_dict::{key_in_dict_compare, key_in_dict_for};
+pub use open_file_with_context_handler::open_file_with_context_handler;
 pub use return_in_try_except_finally::return_in_try_except_finally;
 pub use use_contextlib_suppress::use_contextlib_suppress;
 pub use yoda_conditions::yoda_conditions;
@@ -22,6 +23,7 @@ mod ast_ifexp;
 mod ast_unary_op;
 mod ast_with;
 mod key_in_dict;
+mod open_file_with_context_handler;
 mod return_in_try_except_finally;
 mod use_contextlib_suppress;
 mod yoda_conditions;

src/flake8_simplify/rules/open_file_with_context_handler.rs

@@ -0,0 +1,30 @@
+use rustpython_ast::Expr;
+use rustpython_parser::ast::StmtKind;
+
+use crate::ast::helpers::{collect_call_paths, dealias_call_path, match_call_path};
+use crate::ast::types::Range;
+use crate::checkers::ast::Checker;
+use crate::registry::Diagnostic;
+use crate::violations;
+
+/// SIM115
+pub fn open_file_with_context_handler(checker: &mut Checker, func: &Expr) {
+    if match_call_path(
+        &dealias_call_path(collect_call_paths(func), &checker.import_aliases),
+        "",
+        "open",
+        &checker.from_imports,
+    ) {
+        if checker.is_builtin("open") {
+            match checker.current_stmt().node {
+                StmtKind::With { .. } => (),
+                _ => {
+                    checker.diagnostics.push(Diagnostic::new(
+                        violations::OpenFileWithContextHandler,
+                        Range::from_located(func),
+                    ));
+                }
+            }
+        }
+    }
+}

src/flake8_simplify/snapshots/ruff__flake8_simplify__tests__SIM115_SIM115.py.snap

@@ -0,0 +1,15 @@
+---
+source: src/flake8_simplify/mod.rs
+expression: diagnostics
+---
+- kind:
+    OpenFileWithContextHandler: ~
+  location:
+    row: 1
+    column: 4
+  end_location:
+    row: 1
+    column: 8
+  fix: ~
+  parent: ~
+

src/registry.rs

@@ -294,6 +294,7 @@ define_rule_mapping!(
     YTT302 => violations::SysVersionCmpStr10,
     YTT303 => violations::SysVersionSlice1Referenced,
     // flake8-simplify
+    SIM115 => violations::OpenFileWithContextHandler,
     SIM101 => violations::DuplicateIsinstanceCall,
     SIM102 => violations::NestedIfStatements,
     SIM103 => violations::ReturnBoolConditionDirectly,

src/violations.rs

@@ -2677,6 +2677,20 @@ impl Violation for SysVersionSlice1Referenced {
 }
 
 // flake8-simplify
+
+define_violation!(
+    pub struct OpenFileWithContextHandler;
+);
+impl Violation for OpenFileWithContextHandler {
+    fn message(&self) -> String {
+        "Use context handler for opening files".to_string()
+    }
+
+    fn placeholder() -> Self {
+        OpenFileWithContextHandler
+    }
+}
+
 define_violation!(
     pub struct UseCapitalEnvironmentVariables(pub String, pub String);
 );