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] class attributes wrongly trigger reflexive assignments #129

Closed
sondrelg opened this issue Mar 29, 2022 · 6 comments · Fixed by #132
Closed

[SIM909] class attributes wrongly trigger reflexive assignments #129

sondrelg opened this issue Mar 29, 2022 · 6 comments · Fixed by #132
Assignees
Labels
enhancement New feature or request

Comments

@sondrelg
Copy link

Desired change

  • Rule(s): SIM909
  • Adjustment: Consider allowing declarations on classes

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?

@sondrelg sondrelg added the enhancement New feature or request label Mar 29, 2022
@MartinThoma MartinThoma changed the title [Adjust Rule] SIM909 [SIM909] class attributes wrongly trigger reflexive assignments Mar 29, 2022
@MartinThoma
Copy link
Owner

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
Copy link
Owner

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=[],
)

MartinThoma added a commit that referenced this issue Mar 29, 2022
Class attribute assignments are not reflexive assignments

Closes #129
MartinThoma added a commit that referenced this issue Mar 29, 2022
Class attribute assignments are not reflexive assignments

Closes #129
@sondrelg
Copy link
Author

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

@MartinThoma
Copy link
Owner

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

@sondrelg
Copy link
Author

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

@MartinThoma
Copy link
Owner

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants