8000 [`flake8-simplify`] add fix safety section (`SIM210`) by VascoSch92 · Pull Request #18100 · astral-sh/ruff · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

[flake8-simplify] add fix safety section (SIM210) #18100

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

Merged
merged 2 commits into from
May 15, 2025

Conversation

VascoSch92
Copy link
Contributor

The PR add the fix safety section for rule SIM210 (#15584 )

It is a little cheating, as the Fix safety section is copy/pasted by #18086 as the problem is the same.

Unsafe Fix Example

class Foo():
    def __eq__(self, other):
        return 0

def foo():
    return True if Foo() == 0 else False

def foo_fix():
    return Foo() == 0

print(foo()) # False
print(foo_fix()) # 0

@VascoSch92 VascoSch92 mentioned this pull request May 14, 2025
71 tasks
Copy link
Contributor
github-actions bot commented May 14, 2025

ruff-ecosystem results

Linter (stable)

✅ ecosystem check detected no linter changes.

Linter (preview)

✅ ecosystem check detected no linter changes.

@ntBre ntBre added the documentation Improvements or additions to documentation label May 14, 2025
Copy link
Contributor
@ntBre ntBre left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! I think this time we do need a note about comments.

/// return a proper Boolean. While the fix will try to wrap non-boolean values in a call to bool,
/// custom implementations of comparison functions like `__eq__` can avoid the bool call and still
/// lead to altered behavior.
///
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This one actually does drop comments 😆 : https://play.ruff.rs/a415e7a8-c951-4ed8-af98-a0ebcbc41a40

but only in the case of multi-line if-expressions, which were easiest for me to cause with implicitly-concatenated strings, so it might be pretty rare.

Copy link
Contributor Author
@VascoSch92 VascoSch92 May 14, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I will update ;)

I admit that I was lazy and i didn't check

@VascoSch92 VascoSch92 requested a review from ntBre May 15, 2025 06:15
@VascoSch92
Copy link
Contributor Author

Thanks! I think this time we do need a note about comments.

Added ;-)

Copy link
Contributor
@ntBre ntBre left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you!

@ntBre ntBre merged commit e5435eb into astral-sh:main May 15, 2025
34 checks passed
Glyphack pushed a commit to Glyphack/ruff that referenced this pull request May 21, 2025
The PR add the `fix safety` section for rule `SIM210` (astral-sh#15584 )

It is a little cheating, as the Fix safety section is copy/pasted by
astral-sh#18086 as the problem is the same.

### Unsafe Fix Example

```python
class Foo():
    def __eq__(self, other):
        return 0

def foo():
    return True if Foo() == 0 else False

def foo_fix():
    return Foo() == 0

print(foo()) # False
print(foo_fix()) # 0
```
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants
0