Repository: MartinThoma/flake8-simplify (python)

Issue:
[Adjust Rule] SIM909
## Desired change

* **Rule(s)**: SIM909
* **Adjustment**: Consider allowing declarations on classes

## Explanation

I have this code:

```python
database = Database(url=url)
metadata = sqlalchemy.MetaData()

class BaseMeta:
    metadata = metadata
    database = database
```

The `BaseMeta` class is used for database models as the `Meta` baseclass.

This is currently flagged by SIM909, but after reading through the rationale of the rule, I think this could be considered a false positive. Assignment of `metadata` to `metadata` on a class is not the same as outside a class right - it actually serves a purpose 🙂

What do you think? 

Pull request description:
SIM909: Fix false-positive
Class attribute assignments are not reflexive assignments

Closes #129

Patch:
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 @@ from flake8_simplify.rules.ast_unary_op import (
     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 @@ class Visitor(ast.NodeVisitor):
 
     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 @@ import ast
 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 @@ class For(ast.For):
         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:


Test patch:
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)