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

PIE794 false positive when mutating class variable. Also leads to a bad fix. #8497

Closed
Skylion007 opened this issue Nov 5, 2023 · 1 comment · Fixed by #8634
Closed

PIE794 false positive when mutating class variable. Also leads to a bad fix. #8497

Skylion007 opened this issue Nov 5, 2023 · 1 comment · Fixed by #8634
Assignees
Labels
bug Something isn't working

Comments

@Skylion007
Copy link
Contributor

Skylion007 commented Nov 5, 2023

class Person:
    name = "Foo"
    name = name + " Bar"

with unsafe fixes applied becomes:

class Person:
    name = "Foo"

It may not be the cleanest way of writing that, but it probably shouldn't trigger.

class Person:
    names = ["Foo"]
    names = names + ["Bar"]

is a use case I've actually seen in real code that probably should not be flagged.

ruff version is 0.1.4.

@Skylion007
Copy link
Contributor Author

making it do += seems to be a decent mitigation for the objects which define it, but that may not always be an option depending on the Python object.

pytorchmergebot pushed a commit to pytorch/pytorch that referenced this issue Nov 5, 2023
Enables some tests that were incorrectly not being run and enables PIE794 globally. This rule checks if a classvar is defined twice as flags it as it is likely a bug. In fact, we found several cases where it was a bug. It does have a couple of false positives which I flagged upstream and replaced with noqas: astral-sh/ruff#8497

Pull Request resolved: #112989
Approved by: https://github.com/malfet
@dhruvmanila dhruvmanila added the bug Something isn't working label Nov 6, 2023
xuhancn pushed a commit to xuhancn/pytorch that referenced this issue Nov 7, 2023
…#112989)

Enables some tests that were incorrectly not being run and enables PIE794 globally. This rule checks if a classvar is defined twice as flags it as it is likely a bug. In fact, we found several cases where it was a bug. It does have a couple of false positives which I flagged upstream and replaced with noqas: astral-sh/ruff#8497

Pull Request resolved: pytorch#112989
Approved by: https://github.com/malfet
@charliermarsh charliermarsh self-assigned this Nov 12, 2023
Skylion007 added a commit to Skylion007/pytorch that referenced this issue Nov 14, 2023
…#112989)

Enables some tests that were incorrectly not being run and enables PIE794 globally. This rule checks if a classvar is defined twice as flags it as it is likely a bug. In fact, we found several cases where it was a bug. It does have a couple of false positives which I flagged upstream and replaced with noqas: astral-sh/ruff#8497

Pull Request resolved: pytorch#112989
Approved by: https://github.com/malfet
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants