Skip to content

Commit

Permalink
SIM909: Avoid reflexive assignments
Browse files Browse the repository at this point in the history
Closes #114
  • Loading branch information
MartinThoma committed Mar 28, 2022
1 parent 7ac7d32 commit 4686234
Show file tree
Hide file tree
Showing 5 changed files with 113 additions and 2 deletions.
22 changes: 22 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,7 @@ General Code Style:
* [`SIM112`](https://github.com/MartinThoma/flake8-simplify/issues/19): Use CAPITAL environment variables ([example](#SIM112))
* `SIM122`: Reserved for SIM902 once it's stable
* `SIM123`: Reserved for SIM903 once it's stable
* `SIM124`: Reserved for SIM909 once it's stable

**Experimental rules:**

Expand Down Expand Up @@ -631,3 +632,24 @@ if "some_key" in some_dict:
# Good
name = some_dict.get("some_key", "some_default")
```

### SIM909

Thank you Ryan Delaney for the idea!

```python
# Bad
foo = foo

# Good: Nothing. Reflexive assignments have no purpose.
```

or

```python
# Bad
foo = foo = 42

# Good
foo = 42
```
3 changes: 2 additions & 1 deletion flake8_simplify/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
from typing import Any, Generator, List, Tuple, Type

# First party
from flake8_simplify.rules.ast_assign import get_sim904
from flake8_simplify.rules.ast_assign import get_sim904, get_sim909
from flake8_simplify.rules.ast_bool_op import (
get_sim101,
get_sim109,
Expand Down Expand Up @@ -68,6 +68,7 @@ def __init__(self) -> None:

def visit_Assign(self, node: ast.Assign) -> Any:
self.errors += get_sim904(node)
self.errors += get_sim909(node)
self.generic_visit(node)

def visit_Call(self, node: ast.Call) -> Any:
Expand Down
44 changes: 44 additions & 0 deletions flake8_simplify/rules/ast_assign.py
Original file line number Diff line number Diff line change
Expand Up @@ -67,3 +67,47 @@ def get_sim904(node: ast.Assign) -> List[Tuple[int, int, str]]:
)
)
return errors


def get_sim909(node: ast.Assign) -> List[Tuple[int, int, str]]:
"""
Avoid reflexive assignments
Example
-------
Code:
# Bad
foo = foo
# Good: Just remove them
Bad AST:
[
Assign(
targets=[Name(id='foo', ctx=Store())],
value=Name(id='foo', ctx=Load()),
type_comment=None,
),
]
"""
RULE = "SIM909 Remove reflexive assignment '{code}'"
errors: List[Tuple[int, int, str]] = []

names = []
if isinstance(node.value, (ast.Name, ast.Subscript, ast.Tuple)):
names.append(to_source(node.value))
for target in node.targets:
names.append(to_source(target))

if len(names) == len(set(names)):
return errors

code = to_source(node)

errors.append(
(
node.lineno,
node.col_offset,
RULE.format(code=code),
)
)
return errors
2 changes: 1 addition & 1 deletion flake8_simplify/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ def __init__(self, orig: ast.For) -> None:


def to_source(
node: Union[None, ast.expr, ast.Expr, ast.withitem, ast.slice]
node: Union[None, ast.expr, ast.Expr, ast.withitem, ast.slice, ast.Assign]
) -> str:
if node is None:
return "None"
Expand Down
44 changes: 44 additions & 0 deletions tests/test_900_rules.py
Original file line number Diff line number Diff line change
Expand Up @@ -193,3 +193,47 @@ def test_sim908():
"2:0 SIM908 Use 'some_dict.get(\"some_key\")' instead of "
'\'if "some_key" in some_dict: some_dict["some_key"]\''
}


@pytest.mark.parametrize(
("s", "msg"),
(
# Credits to Ryan Delaney for the following two example
# https://github.com/MartinThoma/flake8-simplify/issues/114
(
"foo = foo",
"1:0 SIM909 Remove reflexive assignment 'foo = foo'",
),
(
"foo = foo = 42",
"1:0 SIM909 Remove reflexive assignment 'foo = foo = 42'",
),
(
"foo = bar = foo = 42",
"1:0 SIM909 Remove reflexive assignment 'foo = bar = foo = 42'",
),
(
"n, m = n, m",
"1:0 SIM909 Remove reflexive assignment 'n, m = n, m'",
),
(
"a['foo'] = a['foo']",
"1:0 SIM909 Remove reflexive assignment 'a['foo'] = a['foo']'",
),
),
ids=["simple", "double", "multiple", "tuple", "dict"],
)
def test_sim909(s, msg):
results = _results(s)
assert results == {msg}


@pytest.mark.parametrize(
("s"),
("n, m = m, n", "foo = 'foo'"),
ids=["tuple-switch", "variable"],
)
def test_sim909_false_positives(s):
results = _results(s)
for result in results:
assert "SIM909" not in result

0 comments on commit 4686234

Please sign in to comment.