-
Notifications
You must be signed in to change notification settings - Fork 1.2k
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
Add
cell
field to JSON output format (#7664)
## Summary This PR adds a new `cell` field to the JSON output format which indicates the Notebook cell this diagnostic (and fix) belongs to. It also updates the location for the diagnostic and fixes as per the `NotebookIndex`. It will be used in the VSCode extension to display the diagnostic in the correct cell. The diagnostic and edit start and end source locations are translated for the notebook as per the `NotebookIndex`. The end source location for an edit needs some special handling. ### Edit end location To understand this, the following context is required: 1. Visible lines in Jupyter Notebook vs JSON array strings: The newline is part of the string in the JSON format. This means that if there are 3 visible lines in a cell where the last line is empty then the JSON would contain 2 strings in the source array, both ending with a newline: **JSON format:** ```json [ "# first line\n", "# second line\n", ] ``` **Notebook view:** ```python 1 # first line 2 # second line 3 ``` 2. If an edit needs to remove an entire line including the newline, then the end location would be the start of the next row. To remove a statement in the following code: ```python import os ``` The edit would be: ``` start: row 1, col 1 end: row 2, col 1 ``` Now, here's where the problem lies. The notebook index doesn't have any information for row 2 because it doesn't exists in the actual notebook. The newline was added by Ruff to concatenate the source code and it's removed before writing back. But, the edit is computed looking at that newline. This means that while translating the end location for an edit belong to a Notebook, we need to check if both the start and end location belongs to the same cell. If not, then the end location should be the first character of the next row and if so, translate that back to the last character of the previous row. Taking the above example, the translated location for Notebook would be: ``` start: row 1, col 1 end: row 1, col 10 ``` ## Test Plan Add test cases for notebook output in the JSON format and update existing snapshots.
- Loading branch information
1 parent
1e184e6
commit 66179af
Showing
11 changed files
with
256 additions
and
19 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -168,6 +168,7 @@ import os | |
----- stdout ----- | ||
[ | ||
{ | ||
"cell": null, | ||
"code": "F401", | ||
"end_location": { | ||
"column": 10, | ||
|
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -17,6 +17,7 @@ exit_code: 1 | |
----- stdout ----- | ||
[ | ||
{ | ||
"cell": null, | ||
"code": "F401", | ||
"end_location": { | ||
"column": 10, | ||
|
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
105 changes: 105 additions & 0 deletions
105
...ruff_linter/src/message/snapshots/ruff_linter__message__json__tests__notebook_output.snap
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,105 @@ | ||
--- | ||
source: crates/ruff_linter/src/message/json.rs | ||
expression: content | ||
--- | ||
[ | ||
{ | ||
"cell": 1, | ||
"code": "F401", | ||
"end_location": { | ||
"column": 10, | ||
"row": 2 | ||
}, | ||
"filename": "notebook.ipynb", | ||
"fix": { | ||
"applicability": "safe", | ||
"edits": [ | ||
{ | ||
"content": "", | ||
"end_location": { | ||
"column": 10, | ||
"row": 2 | ||
}, | ||
"location": { | ||
"column": 1, | ||
"row": 2 | ||
} | ||
} | ||
], | ||
"message": "Remove unused import: `os`" | ||
}, | ||
"location": { | ||
"column": 8, | ||
"row": 2 | ||
}, | ||
"message": "`os` imported but unused", | ||
"noqa_row": 2, | ||
"url": "https://docs.astral.sh/ruff/rules/unused-import" | ||
}, | ||
{ | ||
"cell": 2, | ||
"code": "F401", | ||
"end_location": { | ||
"column": 12, | ||
"row": 2 | ||
}, | ||
"filename": "notebook.ipynb", | ||
"fix": { | ||
"applicability": "safe", | ||
"edits": [ | ||
{ | ||
"content": "", | ||
"end_location": { | ||
"column": 1, | ||
"row": 3 | ||
}, | ||
"location": { | ||
"column": 1, | ||
"row": 2 | ||
} | ||
} | ||
], | ||
"message": "Remove unused import: `math`" | ||
}, | ||
"location": { | ||
"column": 8, | ||
"row": 2 | ||
}, | ||
"message": "`math` imported but unused", | ||
"noqa_row": 2, | ||
"url": "https://docs.astral.sh/ruff/rules/unused-import" | ||
}, | ||
{ | ||
"cell": 3, | ||
"code": "F841", | ||
"end_location": { | ||
"column": 6, | ||
"row": 4 | ||
}, | ||
"filename": "notebook.ipynb", | ||
"fix": { | ||
"applicability": "unsafe", | ||
"edits": [ | ||
{ | ||
"content": "", | ||
"end_location": { | ||
"column": 10, | ||
"row": 4 | ||
}, | ||
"location": { | ||
"column": 1, | ||
"row": 4 | ||
} | ||
} | ||
], | ||
"message": "Remove assignment to unused variable `x`" | ||
}, | ||
"location": { | ||
"column": 5, | ||
"row": 4 | ||
}, | ||
"message": "Local variable `x` is assigned to but never used", | ||
"noqa_row": 4, | ||
"url": "https://docs.astral.sh/ruff/rules/unused-variable" | ||
} | ||
] |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
8 changes: 8 additions & 0 deletions
8
...inter/src/message/snapshots/ruff_linter__message__json_lines__tests__notebook_output.snap
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,8 @@ | ||
--- | ||
source: crates/ruff_linter/src/message/json_lines.rs | ||
expression: content | ||
--- | ||
{"cell":1,"code":"F401","end_location":{"column":10,"row":2},"filename":"notebook.ipynb","fix":{"applicability":"safe","edits":[{"content":"","end_location":{"column":10,"row":2},"location":{"column":1,"row":2}}],"message":"Remove unused import: `os`"},"location":{"column":8,"row":2},"message":"`os` imported but unused","noqa_row":2,"url":"https://docs.astral.sh/ruff/rules/unused-import"} | ||
{"cell":2,"code":"F401","end_location":{"column":12,"row":2},"filename":"notebook.ipynb","fix":{"applicability":"safe","edits":[{"content":"","end_location":{"column":1,"row":3},"location":{"column":1,"row":2}}],"message":"Remove unused import: `math`"},"location":{"column":8,"row":2},"message":"`math` imported but unused","noqa_row":2,"url":"https://docs.astral.sh/ruff/rules/unused-import"} | ||
{"cell":3,"code":"F841","end_location":{"column":6,"row":4},"filename":"notebook.ipynb","fix":{"applicability":"unsafe","edits":[{"content":"","end_location":{"column":10,"row":4},"location":{"column":1,"row":4}}],"message":"Remove assignment to unused variable `x`"},"location":{"column":5,"row":4},"message":"Local variable `x` is assigned to but never used","noqa_row":4,"url":"https://docs.astral.sh/ruff/rules/unused-variable"} | ||
|
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.