SIM 121: Added

README.md

@@ -49,6 +49,7 @@ Python-specific rules:
 * [`SIM118`](https://github.com/MartinThoma/flake8-simplify/issues/40): Use 'key in dict' instead of 'key in dict.keys()' ([example](#SIM118))
 * [`SIM119`](https://github.com/MartinThoma/flake8-simplify/issues/37) ![](https://shields.io/badge/-legacyfix-inactive): Use dataclasses for data containers ([example](#SIM119))
 * `SIM120` ![](https://shields.io/badge/-legacyfix-inactive): Use 'class FooBar:' instead of 'class FooBar(object):' ([example](#SIM120))
+* [`SIM121`](https://github.com/MartinThoma/flake8-simplify/issues/50): Use `.get(key)` instead of `if key in dict`: dict[key]` ([example](#SIM121))
 
 Comparations:
 
@@ -306,18 +307,31 @@ A lot of projects use them:
 
 ### SIM120
 
-```
+```python
 # Bad
 class FooBar(object):
     ...
 
+
 # Good
 class FooBar:
     ...
 ```
 
 Both notations are equivalent in Python 3, but the second one is shorter.
 
+### SIM121
+
+```python
+# Bad
+name = "some_default"
+if "some_key" in some_dict:
+    name = some_dict["some_key"]
+
+# Good
+name = some_dict.get("some_key", "some_default")
+```
+
 ### SIM201
 
 ```python

flake8_simplify.py

@@ -80,6 +80,10 @@ def __init__(self, orig: ast.Call) -> None:
 SIM120 = (
     "SIM120 Use 'class {classname}:' instead of 'class {classname}(object):'"
 )
+SIM121 = (
+    "SIM121 Use '{dictname}.get({key})' istead of "
+    "'if {key} in {dictname}: {dictname}[{key}]'"
+)
 
 # Comparations
 SIM201 = "SIM201 Use '{left} != {right}' instead of 'not {left} == {right}'"
@@ -1110,6 +1114,41 @@ def _get_sim120(node: ast.ClassDef) -> List[Tuple[int, int, str]]:
     return errors
 
 
+def _get_sim121(node: ast.If) -> List[Tuple[int, int, str]]:
+    """
+    Get all if-blocks which only check if a key is in a dictionary.
+    """
+    errors: List[Tuple[int, int, str]] = []
+    if not (
+        isinstance(node.test, ast.Compare)
+        and len(node.test.ops) == 1
+        and isinstance(node.test.ops[0], ast.In)
+        and len(node.body) == 1
+        and len(node.orelse) == 0
+    ):
+        return errors
+    # We might still be left with a check if a value is in a list or in the body
+    # the developer might remove the element from the list
+    # We need to have a look at the body
+    if not (
+        isinstance(node.body[0], ast.Assign)
+        and isinstance(node.body[0].value, ast.Subscript)
+        and len(node.body[0].targets) == 1
+        and isinstance(node.body[0].targets[0], ast.Name)
+    ):
+        return errors
+    key = to_source(node.test.left)
+    dictname = to_source(node.test.comparators[0])
+    errors.append(
+        (
+            node.lineno,
+            node.col_offset,
+            SIM121.format(key=key, dictname=dictname),
+        )
+    )
+    return errors
+
+
 def get_if_body_pairs(node: ast.If) -> List[Tuple[ast.expr, List[ast.stmt]]]:
     pairs = [(node.test, node.body)]
     orelse = node.orelse
@@ -1611,6 +1650,7 @@ def visit_If(self, node: ast.If) -> None:
         self.errors += _get_sim108(node)
         self.errors += _get_sim114(node)
         self.errors += _get_sim116(node)
+        self.errors += _get_sim121(node)
         self.generic_visit(node)
 
     def visit_For(self, node: ast.For) -> None:

tests/test_simplify.py

@@ -491,6 +491,18 @@ def test_sim120():
     }
 
 
+def test_sim121():
+    results = _results(
+        """name = "some_default"
+if "some_key" in some_dict:
+    name = some_dict["some_key"]"""
+    )
+    assert results == {
+        "2:0 SIM121 Use 'some_dict.get(some_key)' istead of "
+        "'if some_key in some_dict: some_dict[some_key]'"
+    }
+
+
 def test_get_if_body_pairs():
     ret = ast.parse(
         """if a == b: