Skip to content

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
@github-actions
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