From 8165925e016a7e5f8fc038db834400288ca33c42 Mon Sep 17 00:00:00 2001 From: Dhruv Manilawala Date: Tue, 26 Sep 2023 19:43:59 +0530 Subject: [PATCH] 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`
Diagnostics output:

```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. ```

Diff output:

```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. ```

### `dhruv/consistent-cell-indices`
Diagnostic output:

```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. ```

Diff output:

```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. ```

fixes: #6673 --- crates/ruff_cli/src/diagnostics.rs | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/crates/ruff_cli/src/diagnostics.rs b/crates/ruff_cli/src/diagnostics.rs index 13de795a86b8b..ed4455786132f 100644 --- a/crates/ruff_cli/src/diagnostics.rs +++ b/crates/ruff_cli/src/diagnostics.rs @@ -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)) =