Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

SIM 121: Added #52

Merged
merged 8 commits into from Mar 28, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
24 changes: 23 additions & 1 deletion README.md
Expand Up @@ -48,6 +48,7 @@ Python-specific rules:
* [`SIM117`](https://github.com/MartinThoma/flake8-simplify/issues/35): Merge with-statements that use the same scope ([example](#SIM117))
* [`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`: Reserved for [SIM908](#sim908) once it's stable
* `SIM124`: Reserved for [SIM904](#sim904) once it's stable
* `SIM125`: Reserved for [SIM905](#sim905) once it's stable
* `SIM126`: Reserved for [SIM906](#sim906) once it's stable
Expand Down Expand Up @@ -357,18 +358,20 @@ 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.


### SIM201

```python
Expand Down Expand Up @@ -599,6 +602,8 @@ os.path.join(a, b, c)
This rule will be renamed to `SIM127` after its 6-month trial period is over.
Please report any issues you encounter with this rule!

The trial period starts on 28th of March and will end on 28th of September 2022.

```python
# Bad
def foo(a: Union[int, None]) -> Union[int, None]:
Expand All @@ -609,3 +614,20 @@ def foo(a: Union[int, None]) -> Union[int, None]:
def foo(a: Optional[int]) -> Optional[int]:
return a
```

### SIM908

This rule will be renamed to `SIM121` after its 6-month trial period is over.
Please report any issues you encounter with this rule!

The trial period starts on 28th of March and will end on 28th of September 2022.

```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")
```
2 changes: 2 additions & 0 deletions flake8_simplify/__init__.py
Expand Up @@ -37,6 +37,7 @@
get_sim114,
get_sim116,
get_sim401,
get_sim908,
)
from flake8_simplify.rules.ast_ifexp import get_sim210, get_sim211, get_sim212
from flake8_simplify.rules.ast_subscript import get_sim907
Expand Down Expand Up @@ -101,6 +102,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_sim908(node)
self.errors += get_sim401(node)
self.generic_visit(node)

Expand Down
39 changes: 39 additions & 0 deletions flake8_simplify/rules/ast_if.py
Expand Up @@ -291,6 +291,45 @@ def get_sim116(node: ast.If) -> List[Tuple[int, int, str]]:
return errors


def get_sim908(node: ast.If) -> List[Tuple[int, int, str]]:
"""
Get all if-blocks which only check if a key is in a dictionary.
"""
RULE = (
"SIM908 Use '{dictname}.get({key})' instead of "
"'if {key} in {dictname}: {dictname}[{key}]'"
)
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,
RULE.format(key=key, dictname=dictname),
)
)
return errors


def get_sim401(node: ast.If) -> List[Tuple[int, int, str]]:
"""
Get all calls that should use default values for dictionary access.
Expand Down
12 changes: 12 additions & 0 deletions tests/test_900_rules.py
Expand Up @@ -181,3 +181,15 @@ def test_sim907():
assert results == {
"1:11 SIM907 Use 'Optional[int]' instead of 'Union[int, None]'"
}


def test_sim908():
results = _results(
"""name = "some_default"
if "some_key" in some_dict:
name = some_dict["some_key"]"""
)
assert results == {
"2:0 SIM908 Use 'some_dict.get(\"some_key\")' instead of "
'\'if "some_key" in some_dict: some_dict["some_key"]\''
}