Use 1-based cell indices consistently for Notebooks (#7662)
## Summary
This PR fixes the bug where the cell indices displayed in the `--diff` output
and the ones in the normal output were different. This was due to the fact that
the `--diff` output was using the `enumerate` function to iterate over
the cells which starts at 0.
## Test Plan
Ran the following command with and without the `--diff` flag:
```console
cargo run --bin ruff -- check --no-cache --isolated ~/playground/ruff/notebooks/test.ipynb
```
### `main`
<details><summary>Diagnostics output:</summary>
<p>
```console
$ cargo run --bin ruff -- check --no-cache --isolated ~/playground/ruff/notebooks/test.ipynb
/Users/dhruv/playground/ruff/notebooks/test.ipynb:cell 3:2:8: F401 [*] `math` imported but unused
/Users/dhruv/playground/ruff/notebooks/test.ipynb:cell 5:1:8: F811 Redefinition of unused `random` from line 1
/Users/dhruv/playground/ruff/notebooks/test.ipynb:cell 5:2:8: F401 [*] `pprint` imported but unused
/Users/dhruv/playground/ruff/notebooks/test.ipynb:cell 6:2:4: F632 [*] Use `==` to compare constant literals
/Users/dhruv/playground/ruff/notebooks/test.ipynb:cell 6:3:38: F632 [*] Use `==` to compare constant literals
Found 5 errors.
[*] 4 potentially fixable with the --fix option.
```
</p>
</details>
<details><summary>Diff output:</summary>
<p>
```console
$ cargo run --bin ruff -- check --no-cache --isolated ~/playground/ruff/notebooks/test.ipynb --diff
--- /Users/dhruv/playground/ruff/notebooks/test.ipynb:cell 2
+++ /Users/dhruv/playground/ruff/notebooks/test.ipynb:cell 2
@@ -1,2 +1 @@
-import random
-import math
+import random
--- /Users/dhruv/playground/ruff/notebooks/test.ipynb:cell 4
+++ /Users/dhruv/playground/ruff/notebooks/test.ipynb:cell 4
@@ -1,4 +1,3 @@
import random
-import pprint
random.randint(10, 20)
--- /Users/dhruv/playground/ruff/notebooks/test.ipynb:cell 5
+++ /Users/dhruv/playground/ruff/notebooks/test.ipynb:cell 5
@@ -1,3 +1,3 @@
foo = 1
-if foo is 2:
- raise ValueError(f"Invalid foo: {foo is 1}")
+if foo == 2:
+ raise ValueError(f"Invalid foo: {foo == 1}")
Would fix 4 errors.
```
</p>
</details>
### `dhruv/consistent-cell-indices`
<details><summary>Diagnostic output:</summary>
<p>
```console
$ cargo run --bin ruff -- check --no-cache --isolated ~/playground/ruff/notebooks/test.ipynb
/Users/dhruv/playground/ruff/notebooks/test.ipynb:cell 3:2:8: F401 [*] `math` imported but unused
/Users/dhruv/playground/ruff/notebooks/test.ipynb:cell 5:1:8: F811 Redefinition of unused `random` from line 1
/Users/dhruv/playground/ruff/notebooks/test.ipynb:cell 5:2:8: F401 [*] `pprint` imported but unused
/Users/dhruv/playground/ruff/notebooks/test.ipynb:cell 6:2:4: F632 [*] Use `==` to compare constant literals
/Users/dhruv/playground/ruff/notebooks/test.ipynb:cell 6:3:38: F632 [*] Use `==` to compare constant literals
Found 5 errors.
[*] 4 potentially fixable with the --fix option.
```
</p>
</details>
<details><summary>Diff output:</summary>
<p>
```console
$ cargo run --bin ruff -- check --no-cache --isolated ~/playground/ruff/notebooks/test.ipynb --diff
--- /Users/dhruv/playground/ruff/notebooks/test.ipynb:cell 3
+++ /Users/dhruv/playground/ruff/notebooks/test.ipynb:cell 3
@@ -1,2 +1 @@
-import random
-import math
+import random
--- /Users/dhruv/playground/ruff/notebooks/test.ipynb:cell 5
+++ /Users/dhruv/playground/ruff/notebooks/test.ipynb:cell 5
@@ -1,4 +1,3 @@
import random
-import pprint
random.randint(10, 20)
--- /Users/dhruv/playground/ruff/notebooks/test.ipynb:cell 6
+++ /Users/dhruv/playground/ruff/notebooks/test.ipynb:cell 6
@@ -1,3 +1,3 @@
foo = 1
-if foo is 2:
- raise ValueError(f"Invalid foo: {foo is 1}")
+if foo == 2:
+ raise ValueError(f"Invalid foo: {foo == 1}")
Would fix 4 errors.
```
</p>
</details>
fixes: #6673
This commit is contained in:
@@ -254,10 +254,9 @@ pub(crate) fn lint_path(
|
||||
// mutated it.
|
||||
let src_notebook = source_kind.as_ipy_notebook().unwrap();
|
||||
let mut stdout = io::stdout().lock();
|
||||
for ((idx, src_cell), dest_cell) in src_notebook
|
||||
.cells()
|
||||
.iter()
|
||||
.enumerate()
|
||||
// Cell indices are 1-based.
|
||||
for ((idx, src_cell), dest_cell) in (1u32..)
|
||||
.zip(src_notebook.cells().iter())
|
||||
.zip(dest_notebook.cells().iter())
|
||||
{
|
||||
let (Cell::Code(src_code_cell), Cell::Code(dest_code_cell)) =
|
||||
|
||||
Reference in New Issue
Block a user