Skip to content

Conversation

@VascoSch92
Copy link
Contributor

@VascoSch92 VascoSch92 commented May 1, 2025

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

It seems that the fix was always marked as unsafe #14401

Unsafety example

This first example is a little extreme. In fact, the class Foo overrides the __getitem__ method but in a very special, way. The difference lies in the fact that zip(letters, letters[1:]) call the slice letters[1:] which is behaving weird in this case, while itertools.pairwise(letters) call just __getitem__(0), __getitem__(1), ... and so on.

Note that the diagnostic is emitted: playground

I don't know if we want to mention this problem, as there is a subtile bug in the python implementation of Foo which make the rule unsafe.

from dataclasses import dataclass
import itertools

@dataclass
class Foo:
    letters: str
    
    def __getitem__(self, index):
        return self.letters[index] + "_foo"


letters = Foo("ABCD")
zip_ = zip(letters, letters[1:])
for a, b in zip_:
    print(a, b) # A_foo B, B_foo C, C_foo D, D_foo _
    
pair = itertools.pairwise(letters)
for a, b in pair:
    print(a, b) # A_foo B_foo, B_foo C_foo, C_foo D_foo

This other example is much probable.
here, itertools.pairwise was shadowed by a costume function (playground)

from dataclasses import dataclass
from itertools import pairwise

def pairwise(a):
    return []
    
letters = "ABCD"
zip_ = zip(letters, letters[1:])
print([(a, b) for a, b in zip_]) # [('A', 'B'), ('B', 'C'), ('C', 'D')]

pair = pairwise(letters)
print(pair) # []

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

github-actions bot commented May 1, 2025

ruff-ecosystem results

Linter (stable)

✅ ecosystem check detected no linter changes.

Linter (preview)

✅ ecosystem check detected no linter changes.

@MichaReiser MichaReiser added the documentation Improvements or additions to documentation label May 1, 2025
@VascoSch92
Copy link
Contributor Author

@dylwil3 Hey ;-) I think it is ready to review

@VascoSch92 VascoSch92 requested review from dylwil3 and ntBre May 13, 2025 20:31
@VascoSch92
Copy link
Contributor Author

@ntBre , @dylwil3 let me know what do you think :-)

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! And nice catch on the slicing behavior. That is very tricky.

@ntBre ntBre merged commit 1e4377c into astral-sh:main May 14, 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 `RUF007` (astral-sh#15584 )

It seems that the fix was always marked as unsafe astral-sh#14401

## Unsafety example

This first example is a little extreme. In fact, the class `Foo`
overrides the `__getitem__` method but in a very special, way. The
difference lies in the fact that `zip(letters, letters[1:])` call the
slice `letters[1:]` which is behaving weird in this case, while
`itertools.pairwise(letters)` call just `__getitem__(0), __getitem__(1),
...` and so on.

Note that the diagnostic is emitted: [playground](https://play.ruff.rs)

I don't know if we want to mention this problem, as there is a subtile
bug in the python implementation of `Foo` which make the rule unsafe.

```python
from dataclasses import dataclass
import itertools

@DataClass
class Foo:
    letters: str
    
    def __getitem__(self, index):
        return self.letters[index] + "_foo"


letters = Foo("ABCD")
zip_ = zip(letters, letters[1:])
for a, b in zip_:
    print(a, b) # A_foo B, B_foo C, C_foo D, D_foo _
    
pair = itertools.pairwise(letters)
for a, b in pair:
    print(a, b) # A_foo B_foo, B_foo C_foo, C_foo D_foo
```

This other example is much probable.
here, `itertools.pairwise` was shadowed by a costume function
[(playground)](https://play.ruff.rs)

```python
from dataclasses import dataclass
from itertools import pairwise

def pairwise(a):
    return []
    
letters = "ABCD"
zip_ = zip(letters, letters[1:])
print([(a, b) for a, b in zip_]) # [('A', 'B'), ('B', 'C'), ('C', 'D')]

pair = pairwise(letters)
print(pair) # []
```
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.

4 participants