Skip to content

Commit

Permalink
Detect automagic-like assignments in notebooks
Browse files Browse the repository at this point in the history
  • Loading branch information
charliermarsh committed Jan 29, 2024
1 parent b9139a3 commit 67c19dc
Show file tree
Hide file tree
Showing 6 changed files with 113 additions and 7 deletions.
17 changes: 17 additions & 0 deletions crates/ruff_linter/src/linter.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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");
Expand Down
Original file line number Diff line number Diff line change
@@ -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
|


Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
{
"execution_count": null,
"cell_type": "code",
"id": "1",
"metadata": {},
"outputs": [],
"source": ["colors = 6\n", "print(colors)"]
}
Original file line number Diff line number Diff line change
@@ -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
}
25 changes: 18 additions & 7 deletions crates/ruff_notebook/src/cell.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -164,9 +165,19 @@ impl Cell {
| "xdel"
| "xmode"
)
})
{
return true;
}) {
// 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).
Expand Down
1 change: 1 addition & 0 deletions crates/ruff_notebook/src/notebook.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)]
Expand Down

0 comments on commit 67c19dc

Please sign in to comment.