Skip to content

Commit

Permalink
SIM909: Fix false-positive (#132)
Browse files Browse the repository at this point in the history
Class attribute assignments are not reflexive assignments

Closes #129
  • Loading branch information
MartinThoma committed Mar 29, 2022
1 parent 61d9d53 commit c97609e
Show file tree
Hide file tree
Showing 5 changed files with 41 additions and 9 deletions.
6 changes: 4 additions & 2 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,6 @@ Python-specific rules:
* `SIM119`: ![](https://img.shields.io/badge/-removed-lightgrey) Moved to [flake8-scream](https://github.com/MartinThoma/flake8-scream) due to [issue 63](https://github.com/MartinThoma/flake8-simplify/issues/63)
* `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
* `SIM127`: Reserved for [SIM907](#sim907) once it's stable
Expand Down Expand Up @@ -100,12 +99,12 @@ the code will change to another number.
Current experimental rules:

* `SIM901`: Use comparisons directly instead of wrapping them in a `bool(...)` call ([example](#SIM901))

* `SIM904`: Assign values to dictionary directly at initialization ([example](#SIM904))
* [`SIM905`](https://github.com/MartinThoma/flake8-simplify/issues/86): Split string directly if only constants are used ([example](#SIM905))
* [`SIM906`](https://github.com/MartinThoma/flake8-simplify/issues/101): Merge nested os.path.join calls ([example](#SIM906))
* [`SIM907`](https://github.com/MartinThoma/flake8-simplify/issues/64): Use Optional[Type] instead of Union[Type, None] ([example](#SIM907))
* [`SIM908`](https://github.com/MartinThoma/flake8-simplify/issues/50): Use dict.get(key) ([example](#SIM908))
* [`SIM909`](https://github.com/MartinThoma/flake8-simplify/issues/114): Avoid reflexive assignments ([example](#SIM909))

## Disabling Rules

Expand Down Expand Up @@ -594,6 +593,9 @@ name = some_dict.get("some_key", "some_default")

Thank you Ryan Delaney for the idea!

This rule will be renamed to `SIM124` 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
Expand Down
4 changes: 2 additions & 2 deletions flake8_simplify/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@
get_sim208,
)
from flake8_simplify.rules.ast_with import get_sim117
from flake8_simplify.utils import Call, For, If, UnaryOp
from flake8_simplify.utils import Assign, Call, For, If, UnaryOp

logger = logging.getLogger(__name__)

Expand All @@ -66,7 +66,7 @@ def __init__(self) -> None:

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

def visit_Call(self, node: ast.Call) -> Any:
Expand Down
9 changes: 6 additions & 3 deletions flake8_simplify/rules/ast_assign.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
from typing import List, Tuple

# First party
from flake8_simplify.utils import expression_uses_variable, to_source
from flake8_simplify.utils import Assign, expression_uses_variable, to_source


def get_sim904(node: ast.Assign) -> List[Tuple[int, int, str]]:
Expand Down Expand Up @@ -69,7 +69,7 @@ def get_sim904(node: ast.Assign) -> List[Tuple[int, int, str]]:
return errors


def get_sim909(node: ast.Assign) -> List[Tuple[int, int, str]]:
def get_sim909(node: Assign) -> List[Tuple[int, int, str]]:
"""
Avoid reflexive assignments
Expand Down Expand Up @@ -101,7 +101,10 @@ def get_sim909(node: ast.Assign) -> List[Tuple[int, int, str]]:
if len(names) == len(set(names)):
return errors

code = to_source(node)
if isinstance(node.parent, ast.ClassDef):
return errors

code = to_source(node.orig)

errors.append(
(
Expand Down
16 changes: 16 additions & 0 deletions flake8_simplify/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,22 @@ def __init__(self, orig: ast.For) -> None:
self.previous_sibling = orig.previous_sibling # type: ignore


class Assign(ast.Assign):
"""For mypy so that it knows that added attributes exist."""

def __init__(self, orig: ast.Assign) -> None:
self.targets = orig.targets
self.value = orig.value
# For all ast.*:
self.orig = orig
self.lineno = orig.lineno
self.col_offset = orig.col_offset

# Added attributes
self.parent: ast.AST = orig.parent # type: ignore
self.previous_sibling = orig.previous_sibling # type: ignore


def to_source(
node: Union[None, ast.expr, ast.Expr, ast.withitem, ast.slice, ast.Assign]
) -> str:
Expand Down
15 changes: 13 additions & 2 deletions tests/test_900_rules.py
Original file line number Diff line number Diff line change
Expand Up @@ -163,8 +163,19 @@ def test_sim909(s, msg):

@pytest.mark.parametrize(
("s"),
("n, m = m, n", "foo = 'foo'"),
ids=["tuple-switch", "variable"],
(
"n, m = m, n",
"foo = 'foo'",
# Credits to Sondre Lillebø Gundersen for the following example
# https://github.com/MartinThoma/flake8-simplify/issues/129
"""database = Database(url=url)
metadata = sqlalchemy.MetaData()
class BaseMeta:
metadata = metadata
database = database""",
),
ids=["tuple-switch", "variable", "class-attributes"],
)
def test_sim909_false_positives(s):
results = _results(s)
Expand Down

0 comments on commit c97609e

Please sign in to comment.