Implement SIM118 (key in dict) of flake8-simplify (#1195)

LICENSE

@@ -438,6 +438,31 @@ are:
     SOFTWARE.
   """
 
+- flake8-simplify, licensed as follows:
+  """
+    MIT License
+
+    Copyright (c) 2020 Martin Thoma
+
+    Permission is hereby granted, free of charge, to any person obtaining a copy
+    of this software and associated documentation files (the "Software"), to deal
+    in the Software without restriction, including without limitation the rights
+    to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+    copies of the Software, and to permit persons to whom the Software is
+    furnished to do so, subject to the following conditions:
+
+    The above copyright notice and this permission notice shall be included in all
+    copies or substantial portions of the Software.
+
+    THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+    IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+    FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
+    AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+    LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+    OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
+    SOFTWARE.
+  """
+
 - isort, licensed as follows:
   """
     The MIT License (MIT)

README.md

@@ -90,6 +90,7 @@ of [Conda](https://docs.conda.io/en/latest/):
    1. [flake8-print (T20)](#flake8-print-t20)
    1. [flake8-quotes (Q)](#flake8-quotes-q)
    1. [flake8-return (RET)](#flake8-return-ret)
+   1. [flake8-simplify (SIM)](#flake8-simplify-sim)
    1. [flake8-tidy-imports (TID)](#flake8-tidy-imports-tid)
    1. [flake8-unused-arguments (ARG)](#flake8-unused-arguments-arg)
    1. [eradicate (ERA)](#eradicate-era)
@@ -771,6 +772,14 @@ For more, see [flake8-return](https://pypi.org/project/flake8-return/1.2.0/) on
 | RET507 | SuperfluousElseContinue | Unnecessary `else` after `continue` statement |  |
 | RET508 | SuperfluousElseBreak | Unnecessary `else` after `break` statement |  |
 
+### flake8-simplify (SIM)
+
+For more, see [flake8-simplify](https://pypi.org/project/flake8-simplify/0.19.3/) on PyPI.
+
+| Code | Name | Message | Fix |
+| ---- | ---- | ------- | --- |
+| SIM118 | KeyInDict | Use 'key in dict' instead of 'key in dict.keys() | 🛠 |
+
 ### flake8-tidy-imports (TID)
 
 For more, see [flake8-tidy-imports](https://pypi.org/project/flake8-tidy-imports/4.8.0/) on PyPI.

resources/test/fixtures/flake8_simplify/SIM118.py

@@ -0,0 +1,12 @@
+key in dict.keys() # SIM118
+
+foo["bar"] in dict.keys() # SIM118
+
+foo() in dict.keys() # SIM118
+
+for key in dict.keys(): # SIM118
+    pass
+
+for key in list(dict.keys()):
+    if some_property(key):
+        del dict[key]

src/check_ast.rs

@@ -37,7 +37,7 @@ use crate::visibility::{module_visibility, transition_scope, Modifier, Visibilit
 use crate::{
     docstrings, flake8_2020, flake8_annotations, flake8_bandit, flake8_blind_except,
     flake8_boolean_trap, flake8_bugbear, flake8_builtins, flake8_comprehensions, flake8_debugger,
-    flake8_import_conventions, flake8_print, flake8_return, flake8_tidy_imports,
+    flake8_import_conventions, flake8_print, flake8_return, flake8_simplify, flake8_tidy_imports,
     flake8_unused_arguments, mccabe, pep8_naming, pycodestyle, pydocstyle, pyflakes, pygrep_hooks,
     pylint, pyupgrade, visibility,
 };
@@ -1059,6 +1059,9 @@ where
                 if self.settings.enabled.contains(&CheckCode::PLW0120) {
                     pylint::plugins::useless_else_on_loop(self, stmt, body, orelse);
                 }
+                if self.settings.enabled.contains(&CheckCode::SIM118) {
+                    flake8_simplify::plugins::key_in_dict_for(self, target, iter);
+                }
             }
             StmtKind::Try { handlers, .. } => {
                 if self.settings.enabled.contains(&CheckCode::F707) {
@@ -2090,6 +2093,16 @@ where
                         comparators,
                     );
                 }
+
+                if self.settings.enabled.contains(&CheckCode::SIM118) {
+                    flake8_simplify::plugins::key_in_dict_compare(
+                        self,
+                        expr,
+                        left,
+                        ops,
+                        comparators,
+                    );
+                }
             }
             ExprKind::Constant {
                 value: Constant::Str(value),

src/checks.rs

@@ -206,6 +206,8 @@ pub enum CheckCode {
     YTT301,
     YTT302,
     YTT303,
+    // flake8-simplify
+    SIM118,
     // pyupgrade
     UP001,
     UP003,
@@ -337,6 +339,7 @@ pub enum CheckCategory {
     Flake8Print,
     Flake8Quotes,
     Flake8Return,
+    Flake8Simplify,
     Flake8TidyImports,
     Flake8UnusedArguments,
     Eradicate,
@@ -377,6 +380,7 @@ impl CheckCategory {
             CheckCategory::Flake8Quotes => "flake8-quotes",
             CheckCategory::Flake8Return => "flake8-return",
             CheckCategory::Flake8TidyImports => "flake8-tidy-imports",
+            CheckCategory::Flake8Simplify => "flake8-simplify",
             CheckCategory::Flake8UnusedArguments => "flake8-unused-arguments",
             CheckCategory::Isort => "isort",
             CheckCategory::McCabe => "mccabe",
@@ -406,6 +410,7 @@ impl CheckCategory {
             CheckCategory::Flake8Print => vec![CheckCodePrefix::T20],
             CheckCategory::Flake8Quotes => vec![CheckCodePrefix::Q],
             CheckCategory::Flake8Return => vec![CheckCodePrefix::RET],
+            CheckCategory::Flake8Simplify => vec![CheckCodePrefix::SIM],
             CheckCategory::Flake8TidyImports => vec![CheckCodePrefix::TID],
             CheckCategory::Flake8UnusedArguments => vec![CheckCodePrefix::ARG],
             CheckCategory::Isort => vec![CheckCodePrefix::I],
@@ -481,6 +486,10 @@ impl CheckCategory {
                 "https://pypi.org/project/flake8-return/1.2.0/",
                 &Platform::PyPI,
             )),
+            CheckCategory::Flake8Simplify => Some((
+                "https://pypi.org/project/flake8-simplify/0.19.3/",
+                &Platform::PyPI,
+            )),
             CheckCategory::Flake8TidyImports => Some((
                 "https://pypi.org/project/flake8-tidy-imports/4.8.0/",
                 &Platform::PyPI,
@@ -746,6 +755,8 @@ pub enum CheckKind {
     SysVersion0Referenced,
     SysVersionCmpStr10,
     SysVersionSlice1Referenced,
+    // flake8-simplify
+    KeyInDict(String, String),
     // pyupgrade
     TypeOfPrimitive(Primitive),
     UselessMetaclassType,
@@ -1082,6 +1093,8 @@ impl CheckCode {
             CheckCode::YTT303 => CheckKind::SysVersionSlice1Referenced,
             // flake8-blind-except
             CheckCode::BLE001 => CheckKind::BlindExcept("Exception".to_string()),
+            // flake8-simplify
+            CheckCode::SIM118 => CheckKind::KeyInDict("key".to_string(), "dict".to_string()),
             // pyupgrade
             CheckCode::UP001 => CheckKind::UselessMetaclassType,
             CheckCode::UP003 => CheckKind::TypeOfPrimitive(Primitive::Str),
@@ -1442,6 +1455,7 @@ impl CheckCode {
             CheckCode::S105 => CheckCategory::Flake8Bandit,
             CheckCode::S106 => CheckCategory::Flake8Bandit,
             CheckCode::S107 => CheckCategory::Flake8Bandit,
+            CheckCode::SIM118 => CheckCategory::Flake8Simplify,
             CheckCode::T100 => CheckCategory::Flake8Debugger,
             CheckCode::T201 => CheckCategory::Flake8Print,
             CheckCode::T203 => CheckCategory::Flake8Print,
@@ -1650,6 +1664,8 @@ impl CheckKind {
             CheckKind::SysVersion0Referenced => &CheckCode::YTT301,
             CheckKind::SysVersionCmpStr10 => &CheckCode::YTT302,
             CheckKind::SysVersionSlice1Referenced => &CheckCode::YTT303,
+            // flake8-simplify
+            CheckKind::KeyInDict(..) => &CheckCode::SIM118,
             // pyupgrade
             CheckKind::TypeOfPrimitive(_) => &CheckCode::UP003,
             CheckKind::UselessMetaclassType => &CheckCode::UP001,
@@ -2316,6 +2332,10 @@ impl CheckKind {
             CheckKind::SysVersionSlice1Referenced => {
                 "`sys.version[:1]` referenced (python10), use `sys.version_info`".to_string()
             }
+            // flake8-simplify
+            CheckKind::KeyInDict(key, dict) => {
+                format!("Use '{key} in {dict}' instead of '{key} in {dict}.keys()")
+            }
             // pyupgrade
             CheckKind::TypeOfPrimitive(primitive) => {
                 format!("Use `{}` instead of `type(...)`", primitive.builtin())
@@ -2660,6 +2680,7 @@ impl CheckKind {
                 | CheckKind::ImplicitReturn
                 | CheckKind::ImplicitReturnValue
                 | CheckKind::IsLiteral
+                | CheckKind::KeyInDict(..)
                 | CheckKind::MisplacedComparisonConstant(..)
                 | CheckKind::NewLineAfterLastParagraph
                 | CheckKind::NewLineAfterSectionName(..)

src/checks_gen.rs

@@ -382,6 +382,10 @@ pub enum CheckCodePrefix {
     S105,
     S106,
     S107,
+    SIM,
+    SIM1,
+    SIM11,
+    SIM118,
     T,
     T1,
     T10,
@@ -1494,6 +1498,10 @@ impl CheckCodePrefix {
             CheckCodePrefix::S105 => vec![CheckCode::S105],
             CheckCodePrefix::S106 => vec![CheckCode::S106],
             CheckCodePrefix::S107 => vec![CheckCode::S107],
+            CheckCodePrefix::SIM => vec![CheckCode::SIM118],
+            CheckCodePrefix::SIM1 => vec![CheckCode::SIM118],
+            CheckCodePrefix::SIM11 => vec![CheckCode::SIM118],
+            CheckCodePrefix::SIM118 => vec![CheckCode::SIM118],
             CheckCodePrefix::T => vec![CheckCode::T100, CheckCode::T201, CheckCode::T203],
             CheckCodePrefix::T1 => vec![CheckCode::T100],
             CheckCodePrefix::T10 => vec![CheckCode::T100],
@@ -2203,6 +2211,10 @@ impl CheckCodePrefix {
             CheckCodePrefix::S105 => SuffixLength::Three,
             CheckCodePrefix::S106 => SuffixLength::Three,
             CheckCodePrefix::S107 => SuffixLength::Three,
+            CheckCodePrefix::SIM => SuffixLength::Zero,
+            CheckCodePrefix::SIM1 => SuffixLength::One,
+            CheckCodePrefix::SIM11 => SuffixLength::Two,
+            CheckCodePrefix::SIM118 => SuffixLength::Three,
             CheckCodePrefix::T => SuffixLength::Zero,
             CheckCodePrefix::T1 => SuffixLength::One,
             CheckCodePrefix::T10 => SuffixLength::Two,
@@ -2303,6 +2315,7 @@ pub const CATEGORIES: &[CheckCodePrefix] = &[
     CheckCodePrefix::RET,
     CheckCodePrefix::RUF,
     CheckCodePrefix::S,
+    CheckCodePrefix::SIM,
     CheckCodePrefix::T,
     CheckCodePrefix::TID,
     CheckCodePrefix::UP,

src/flake8_simplify/mod.rs

@@ -0,0 +1,29 @@
+pub mod plugins;
+
+#[cfg(test)]
+mod tests {
+    use std::convert::AsRef;
+    use std::path::Path;
+
+    use anyhow::Result;
+    use test_case::test_case;
+
+    use crate::checks::CheckCode;
+    use crate::linter::test_path;
+    use crate::settings;
+
+    #[test_case(CheckCode::SIM118, Path::new("SIM118.py"); "SIM118")]
+    fn checks(check_code: CheckCode, path: &Path) -> Result<()> {
+        let snapshot = format!("{}_{}", check_code.as_ref(), path.to_string_lossy());
+        let mut checks = test_path(
+            Path::new("./resources/test/fixtures/flake8_simplify")
+                .join(path)
+                .as_path(),
+            &settings::Settings::for_rule(check_code),
+            true,
+        )?;
+        checks.sort_by_key(|check| check.location);
+        insta::assert_yaml_snapshot!(snapshot, checks);
+        Ok(())
+    }
+}

src/flake8_simplify/plugins/key_in_dict.rs

@@ -0,0 +1,74 @@
+use rustpython_ast::{Cmpop, Expr, ExprKind};
+
+use crate::ast::types::Range;
+use crate::autofix::Fix;
+use crate::check_ast::Checker;
+use crate::checks::{Check, CheckCode, CheckKind};
+
+/// SIM118
+fn key_in_dict(checker: &mut Checker, left: &Expr, right: &Expr, range: Range) {
+    let ExprKind::Call {
+        func,
+        args,
+        keywords,
+    } = &right.node else {
+        return;
+    };
+    if !(args.is_empty() && keywords.is_empty()) {
+        return;
+    }
+
+    let ExprKind::Attribute { attr, value, .. } = &func.node else {
+        return;
+    };
+    if attr != "keys" {
+        return;
+    }
+
+    let mut check = Check::new(
+        CheckKind::KeyInDict(left.to_string(), value.to_string()),
+        range,
+    );
+    if checker.patch(&CheckCode::SIM118) {
+        let content = right.to_string().replace(".keys()", "");
+        check.amend(Fix::replacement(
+            content,
+            right.location,
+            right.end_location.unwrap(),
+        ));
+    }
+    checker.add_check(check);
+}
+
+/// SIM118 in a for loop
+pub fn key_in_dict_for(checker: &mut Checker, target: &Expr, iter: &Expr) {
+    key_in_dict(
+        checker,
+        target,
+        iter,
+        Range {
+            location: target.location,
+            end_location: iter.end_location.unwrap(),
+        },
+    );
+}
+
+/// SIM118 in a comparison
+pub fn key_in_dict_compare(
+    checker: &mut Checker,
+    expr: &Expr,
+    left: &Expr,
+    ops: &[Cmpop],
+    comparators: &[Expr],
+) {
+    if !matches!(ops[..], [Cmpop::In]) {
+        return;
+    }
+
+    if comparators.len() != 1 {
+        return;
+    }
+    let right = comparators.first().unwrap();
+
+    key_in_dict(checker, left, right, Range::from_located(expr));
+}

src/flake8_simplify/plugins/mod.rs

@@ -0,0 +1,3 @@
+pub use key_in_dict::{key_in_dict_compare, key_in_dict_for};
+
+mod key_in_dict;

src/flake8_simplify/snapshots/ruff__flake8_simplify__tests__SIM118_SIM118.py.snap

@@ -0,0 +1,77 @@
+---
+source: src/flake8_simplify/mod.rs
+expression: checks
+---
+- kind:
+    KeyInDict:
+      - key
+      - dict
+  location:
+    row: 1
+    column: 0
+  end_location:
+    row: 1
+    column: 18
+  fix:
+    content: dict
+    location:
+      row: 1
+      column: 7
+    end_location:
+      row: 1
+      column: 18
+- kind:
+    KeyInDict:
+      - "foo['bar']"
+      - dict
+  location:
+    row: 3
+    column: 0
+  end_location:
+    row: 3
+    column: 25
+  fix:
+    content: dict
+    location:
+      row: 3
+      column: 14
+    end_location:
+      row: 3
+      column: 25
+- kind:
+    KeyInDict:
+      - foo()
+      - dict
+  location:
+    row: 5
+    column: 0
+  end_location:
+    row: 5
+    column: 20
+  fix:
+    content: dict
+    location:
+      row: 5
+      column: 9
+    end_location:
+      row: 5
+      column: 20
+- kind:
+    KeyInDict:
+      - key
+      - dict
+  location:
+    row: 7
+    column: 4
+  end_location:
+    row: 7
+    column: 22
+  fix:
+    content: dict
+    location:
+      row: 7
+      column: 11
+    end_location:
+      row: 7
+      column: 22
+

src/lib.rs

@@ -53,6 +53,7 @@ mod flake8_import_conventions;
 mod flake8_print;
 pub mod flake8_quotes;
 mod flake8_return;
+mod flake8_simplify;
 pub mod flake8_tidy_imports;
 mod flake8_unused_arguments;
 pub mod fs;