From 1e4377c9c63c1fc52f9fd1c7240c607e430c8784 Mon Sep 17 00:00:00 2001 From: Vasco Schiavo <115561717+VascoSch92@users.noreply.github.com> Date: Wed, 14 May 2025 17:07:11 +0200 Subject: [PATCH] [`ruff`] add fix safety section (`RUF007`) (#17755) 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](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) # [] ``` --- .../src/rules/ruff/rules/zip_instead_of_pairwise.rs | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/crates/ruff_linter/src/rules/ruff/rules/zip_instead_of_pairwise.rs b/crates/ruff_linter/src/rules/ruff/rules/zip_instead_of_pairwise.rs index 9a473eb32b..286560187a 100644 --- a/crates/ruff_linter/src/rules/ruff/rules/zip_instead_of_pairwise.rs +++ b/crates/ruff_linter/src/rules/ruff/rules/zip_instead_of_pairwise.rs @@ -29,6 +29,14 @@ use crate::{checkers::ast::Checker, importer::ImportRequest}; /// pairwise(letters) # ("A", "B"), ("B", "C"), ("C", "D") /// ``` /// +/// ## Fix safety +/// +/// The fix is always marked unsafe because it assumes that slicing an object +/// (e.g., `obj[1:]`) produces a value with the same type and iteration behavior +/// as the original object, which is not guaranteed for user-defined types that +/// override `__getitem__` without properly handling slices. Moreover, the fix +/// could delete comments. +/// /// ## References /// - [Python documentation: `itertools.pairwise`](https://docs.python.org/3/library/itertools.html#itertools.pairwise) #[derive(ViolationMetadata)]