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

SIM909: Fix false-positive #132

Merged
merged 1 commit into from
Mar 29, 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.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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