mirror of
https://github.com/astral-sh/ruff.git
synced 2025-07-29 07:53:52 +00:00
Detect automagic-like assignments in notebooks (#9653)
## Summary Given a statement like `colors = 6`, we currently treat the cell as an automagic (since `colors` is an automagic) -- i.e., we assume it's equivalent to `%colors = 6`. This PR adds some additional detection whereby if the statement is an _assignment_, we avoid treating it as such. I audited the list of automagics, and I believe this is safe for all of them. Closes https://github.com/astral-sh/ruff/issues/8526. Closes https://github.com/astral-sh/ruff/issues/9648. ## Test Plan `cargo test`
This commit is contained in:
parent
c8074b0e18
commit
bea8f2ee3a
6 changed files with 113 additions and 7 deletions
|
@ -795,6 +795,23 @@ mod tests {
|
|||
Ok(())
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_undefined_name() -> Result<(), NotebookError> {
|
||||
let actual = notebook_path("undefined_name.ipynb");
|
||||
let expected = notebook_path("undefined_name.ipynb");
|
||||
let TestedNotebook {
|
||||
messages,
|
||||
source_notebook,
|
||||
..
|
||||
} = assert_notebook_path(
|
||||
&actual,
|
||||
expected,
|
||||
&settings::LinterSettings::for_rule(Rule::UndefinedName),
|
||||
)?;
|
||||
assert_messages!(messages, actual, source_notebook);
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_json_consistency() -> Result<()> {
|
||||
let actual_path = notebook_path("before_fix.ipynb");
|
||||
|
|
|
@ -0,0 +1,10 @@
|
|||
---
|
||||
source: crates/ruff_linter/src/linter.rs
|
||||
---
|
||||
undefined_name.ipynb:cell 3:1:7: F821 Undefined name `undefined`
|
||||
|
|
||||
1 | print(undefined)
|
||||
| ^^^^^^^^^ F821
|
||||
|
|
||||
|
||||
|
8
crates/ruff_notebook/resources/test/fixtures/jupyter/cell/automagic_assignment.json
vendored
Normal file
8
crates/ruff_notebook/resources/test/fixtures/jupyter/cell/automagic_assignment.json
vendored
Normal file
|
@ -0,0 +1,8 @@
|
|||
{
|
||||
"execution_count": null,
|
||||
"cell_type": "code",
|
||||
"id": "1",
|
||||
"metadata": {},
|
||||
"outputs": [],
|
||||
"source": ["colors = 6\n", "print(colors)"]
|
||||
}
|
59
crates/ruff_notebook/resources/test/fixtures/jupyter/undefined_name.ipynb
vendored
Normal file
59
crates/ruff_notebook/resources/test/fixtures/jupyter/undefined_name.ipynb
vendored
Normal file
|
@ -0,0 +1,59 @@
|
|||
{
|
||||
"cells": [
|
||||
{
|
||||
"cell_type": "code",
|
||||
"execution_count": null,
|
||||
"id": "a0efffbc-85f1-4513-bf49-5387ec3a2a4e",
|
||||
"metadata": {},
|
||||
"outputs": [],
|
||||
"source": [
|
||||
"colors = 6"
|
||||
]
|
||||
},
|
||||
{
|
||||
"cell_type": "code",
|
||||
"execution_count": null,
|
||||
"id": "6e0b2b50-43f2-4f59-951d-9404dd560ae4",
|
||||
"metadata": {
|
||||
"is_executing": true
|
||||
},
|
||||
"outputs": [],
|
||||
"source": [
|
||||
"print(colors)"
|
||||
]
|
||||
},
|
||||
{
|
||||
"cell_type": "code",
|
||||
"execution_count": null,
|
||||
"outputs": [],
|
||||
"source": [
|
||||
"print(undefined)"
|
||||
],
|
||||
"metadata": {
|
||||
"collapsed": false
|
||||
},
|
||||
"id": "884e0e4e686fd56"
|
||||
}
|
||||
],
|
||||
"metadata": {
|
||||
"kernelspec": {
|
||||
"display_name": "Python (ruff)",
|
||||
"language": "python",
|
||||
"name": "ruff"
|
||||
},
|
||||
"language_info": {
|
||||
"codemirror_mode": {
|
||||
"name": "ipython",
|
||||
"version": 3
|
||||
},
|
||||
"file_extension": ".py",
|
||||
"mimetype": "text/x-python",
|
||||
"name": "python",
|
||||
"nbconvert_exporter": "python",
|
||||
"pygments_lexer": "ipython3",
|
||||
"version": "3.11.3"
|
||||
}
|
||||
},
|
||||
"nbformat": 4,
|
||||
"nbformat_minor": 5
|
||||
}
|
|
@ -80,10 +80,11 @@ impl Cell {
|
|||
// ```
|
||||
//
|
||||
// See: https://ipython.readthedocs.io/en/stable/interactive/magics.html
|
||||
if lines
|
||||
.peek()
|
||||
.and_then(|line| line.split_whitespace().next())
|
||||
.is_some_and(|token| {
|
||||
if let Some(line) = lines.peek() {
|
||||
let mut tokens = line.split_whitespace();
|
||||
|
||||
// The first token must be an automagic, like `load_exit`.
|
||||
if tokens.next().is_some_and(|token| {
|
||||
matches!(
|
||||
token,
|
||||
"alias"
|
||||
|
@ -164,10 +165,20 @@ impl Cell {
|
|||
| "xdel"
|
||||
| "xmode"
|
||||
)
|
||||
})
|
||||
{
|
||||
}) {
|
||||
// The second token must _not_ be an operator, like `=` (to avoid false positives).
|
||||
// The assignment operators can never follow an automagic. Some binary operators
|
||||
// _can_, though (e.g., `cd -` is valid), so we omit them.
|
||||
if !tokens.next().is_some_and(|token| {
|
||||
matches!(
|
||||
token,
|
||||
"=" | "+=" | "-=" | "*=" | "/=" | "//=" | "%=" | "**=" | "&=" | "|=" | "^="
|
||||
)
|
||||
}) {
|
||||
return true;
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// Detect cell magics (which operate on multiple lines).
|
||||
lines.any(|line| {
|
||||
|
|
|
@ -454,6 +454,7 @@ mod tests {
|
|||
#[test_case("cell_magic", false)]
|
||||
#[test_case("valid_cell_magic", true)]
|
||||
#[test_case("automagic", false)]
|
||||
#[test_case("automagic_assignment", true)]
|
||||
#[test_case("automagics", false)]
|
||||
#[test_case("automagic_before_code", false)]
|
||||
#[test_case("automagic_after_code", true)]
|
||||
|
|
Loading…
Add table
Add a link
Reference in a new issue