MartinThoma / flake8-simplify

❄ A flake8 plugin that helps you to simplify code
MIT License
185 stars 19 forks source link

[SIM909] class attributes wrongly trigger reflexive assignments #129

Closed sondrelg closed 2 years ago

sondrelg commented 2 years ago

Desired change

Explanation

I have this code:

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?

MartinThoma commented 2 years ago

You're absolutely right. SIM909 needs a fix. I hope in 1-2 hours I can release a new version with the fix.

Thank you for reporting it!

MartinThoma commented 2 years ago

Python

class BaseMeta:
    metadata = metadata

AST

$ python -m astpretty --no-show-offsets /dev/stdin <<< `cat example.py`
Module(
    body=[
        ClassDef(
            name='BaseMeta',
            bases=[],
            keywords=[],
            body=[
                Assign(
                    targets=[Name(id='metadata', ctx=Store())],
                    value=Name(id='metadata', ctx=Load()),
                    type_comment=None,
                ),
            ],
            decorator_list=[],
        ),
    ],
    type_ignores=[],
)
sondrelg commented 2 years ago

Let me know when the new release is out and I can test on my repo if you'd like 🙂

MartinThoma commented 2 years ago

flake8-simplify==0.19.2 was just released with the fix :-)

sondrelg commented 2 years ago

All the (now) false positives are gone, and it seems to run well. Thanks @MartinThoma!

MartinThoma commented 2 years ago

Nice! Thank you for reporting the issue and checking if the fix works!