From c97609e65c98544b2f7eef852eddb705f929db88 Mon Sep 17 00:00:00 2001 From: Martin Thoma Date: Tue, 29 Mar 2022 11:48:22 +0200 Subject: [PATCH] SIM909: Fix false-positive (#132) Class attribute assignments are not reflexive assignments Closes #129 --- README.md | 6 ++++-- flake8_simplify/__init__.py | 4 ++-- flake8_simplify/rules/ast_assign.py | 9 ++++++--- flake8_simplify/utils.py | 16 ++++++++++++++++ tests/test_900_rules.py | 15 +++++++++++++-- 5 files changed, 41 insertions(+), 9 deletions(-) diff --git a/README.md b/README.md index c592b89..f73d258 100644 --- a/README.md +++ b/README.md @@ -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 @@ -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 @@ -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 diff --git a/flake8_simplify/__init__.py b/flake8_simplify/__init__.py index f7f37ce..2207de6 100644 --- a/flake8_simplify/__init__.py +++ b/flake8_simplify/__init__.py @@ -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__) @@ -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: diff --git a/flake8_simplify/rules/ast_assign.py b/flake8_simplify/rules/ast_assign.py index 6dadb8f..a6a42fe 100644 --- a/flake8_simplify/rules/ast_assign.py +++ b/flake8_simplify/rules/ast_assign.py @@ -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]]: @@ -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 @@ -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( ( diff --git a/flake8_simplify/utils.py b/flake8_simplify/utils.py index 89a38fe..71a29df 100644 --- a/flake8_simplify/utils.py +++ b/flake8_simplify/utils.py @@ -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: diff --git a/tests/test_900_rules.py b/tests/test_900_rules.py index 9f49abf..029080b 100644 --- a/tests/test_900_rules.py +++ b/tests/test_900_rules.py @@ -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)