From 1ac2699b5e993716e33607ec8f7544334e5f42a5 Mon Sep 17 00:00:00 2001 From: Dhruv Manilawala Date: Sat, 5 Aug 2023 06:15:01 +0530 Subject: [PATCH] Update `F841` autofix to not remove line magic expr (#6141) ## Summary Update `F841` autofix to not remove line magic expr ## Test Plan Added test case for assignment statement with and without type annotation fixes: #6116 --- .../fixtures/jupyter/unused_variable.ipynb | 49 +++++++++++++ .../jupyter/unused_variable_expected.ipynb | 49 +++++++++++++ crates/ruff/src/jupyter/notebook.rs | 12 ++++ ...ter__notebook__tests__unused_variable.snap | 72 +++++++++++++++++++ crates/ruff_python_ast/src/helpers.rs | 1 + 5 files changed, 183 insertions(+) create mode 100644 crates/ruff/resources/test/fixtures/jupyter/unused_variable.ipynb create mode 100644 crates/ruff/resources/test/fixtures/jupyter/unused_variable_expected.ipynb create mode 100644 crates/ruff/src/jupyter/snapshots/ruff__jupyter__notebook__tests__unused_variable.snap diff --git a/crates/ruff/resources/test/fixtures/jupyter/unused_variable.ipynb b/crates/ruff/resources/test/fixtures/jupyter/unused_variable.ipynb new file mode 100644 index 0000000000000..d4b2f8f8af78d --- /dev/null +++ b/crates/ruff/resources/test/fixtures/jupyter/unused_variable.ipynb @@ -0,0 +1,49 @@ +{ + "cells": [ + { + "cell_type": "code", + "execution_count": null, + "id": "a0efffbc-85f1-4513-bf49-5387ec3a2a4e", + "metadata": {}, + "outputs": [], + "source": [ + "def f():\n", + " foo1 = %matplotlib --list\n", + " foo2: list[str] = %matplotlib --list" + ] + }, + { + "cell_type": "code", + "execution_count": null, + "id": "6e0b2b50-43f2-4f59-951d-9404dd560ae4", + "metadata": {}, + "outputs": [], + "source": [ + "def f():\n", + " bar1 = !pwd\n", + " bar2: str = !pwd" + ] + } + ], + "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 +} diff --git a/crates/ruff/resources/test/fixtures/jupyter/unused_variable_expected.ipynb b/crates/ruff/resources/test/fixtures/jupyter/unused_variable_expected.ipynb new file mode 100644 index 0000000000000..0c68e77de6179 --- /dev/null +++ b/crates/ruff/resources/test/fixtures/jupyter/unused_variable_expected.ipynb @@ -0,0 +1,49 @@ +{ + "cells": [ + { + "cell_type": "code", + "execution_count": null, + "id": "24426ef2-046c-453e-b809-05b56e7355e0", + "metadata": {}, + "outputs": [], + "source": [ + "def f():\n", + " %matplotlib --list\n", + " %matplotlib --list" + ] + }, + { + "cell_type": "code", + "execution_count": null, + "id": "3d98fdae-b86b-476e-b4db-9d3ce5562682", + "metadata": {}, + "outputs": [], + "source": [ + "def f():\n", + " !pwd\n", + " !pwd" + ] + } + ], + "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 +} diff --git a/crates/ruff/src/jupyter/notebook.rs b/crates/ruff/src/jupyter/notebook.rs index 652e54850f05f..4aaff0b1ff734 100644 --- a/crates/ruff/src/jupyter/notebook.rs +++ b/crates/ruff/src/jupyter/notebook.rs @@ -582,6 +582,18 @@ print("after empty cells") Ok(()) } + #[test] + fn test_unused_variable() -> Result<()> { + let path = "unused_variable.ipynb".to_string(); + let (diagnostics, source_kind, _) = test_notebook_path( + &path, + Path::new("unused_variable_expected.ipynb"), + &settings::Settings::for_rule(Rule::UnusedVariable), + )?; + assert_messages!(diagnostics, path, source_kind); + Ok(()) + } + #[test] fn test_json_consistency() -> Result<()> { let path = "before_fix.ipynb".to_string(); diff --git a/crates/ruff/src/jupyter/snapshots/ruff__jupyter__notebook__tests__unused_variable.snap b/crates/ruff/src/jupyter/snapshots/ruff__jupyter__notebook__tests__unused_variable.snap new file mode 100644 index 0000000000000..689e20e25ee18 --- /dev/null +++ b/crates/ruff/src/jupyter/snapshots/ruff__jupyter__notebook__tests__unused_variable.snap @@ -0,0 +1,72 @@ +--- +source: crates/ruff/src/jupyter/notebook.rs +--- +unused_variable.ipynb:cell 1:2:5: F841 [*] Local variable `foo1` is assigned to but never used + | +1 | def f(): +2 | foo1 = %matplotlib --list + | ^^^^ F841 +3 | foo2: list[str] = %matplotlib --list + | + = help: Remove assignment to unused variable `foo1` + +ℹ Suggested fix +1 1 | def f(): +2 |- foo1 = %matplotlib --list + 2 |+ %matplotlib --list +3 3 | foo2: list[str] = %matplotlib --list +4 4 | def f(): +5 5 | bar1 = !pwd + +unused_variable.ipynb:cell 1:3:5: F841 [*] Local variable `foo2` is assigned to but never used + | +1 | def f(): +2 | foo1 = %matplotlib --list +3 | foo2: list[str] = %matplotlib --list + | ^^^^ F841 + | + = help: Remove assignment to unused variable `foo2` + +ℹ Suggested fix +1 1 | def f(): +2 2 | foo1 = %matplotlib --list +3 |- foo2: list[str] = %matplotlib --list + 3 |+ %matplotlib --list +4 4 | def f(): +5 5 | bar1 = !pwd +6 6 | bar2: str = !pwd + +unused_variable.ipynb:cell 2:2:5: F841 [*] Local variable `bar1` is assigned to but never used + | +1 | def f(): +2 | bar1 = !pwd + | ^^^^ F841 +3 | bar2: str = !pwd + | + = help: Remove assignment to unused variable `bar1` + +ℹ Suggested fix +2 2 | foo1 = %matplotlib --list +3 3 | foo2: list[str] = %matplotlib --list +4 4 | def f(): +5 |- bar1 = !pwd + 5 |+ !pwd +6 6 | bar2: str = !pwd + +unused_variable.ipynb:cell 2:3:5: F841 [*] Local variable `bar2` is assigned to but never used + | +1 | def f(): +2 | bar1 = !pwd +3 | bar2: str = !pwd + | ^^^^ F841 + | + = help: Remove assignment to unused variable `bar2` + +ℹ Suggested fix +3 3 | foo2: list[str] = %matplotlib --list +4 4 | def f(): +5 5 | bar1 = !pwd +6 |- bar2: str = !pwd + 6 |+ !pwd + + diff --git a/crates/ruff_python_ast/src/helpers.rs b/crates/ruff_python_ast/src/helpers.rs index a445d624e01b6..2666f32de3452 100644 --- a/crates/ruff_python_ast/src/helpers.rs +++ b/crates/ruff_python_ast/src/helpers.rs @@ -111,6 +111,7 @@ where | Expr::Subscript(_) | Expr::Yield(_) | Expr::YieldFrom(_) + | Expr::LineMagic(_) ) }) }