From 334752416402686383e0248eb118d4d845de4a3f Mon Sep 17 00:00:00 2001 From: Tom Kuson Date: Fri, 29 Sep 2023 02:39:11 +0100 Subject: [PATCH] Extend `unnecessary-pass` (`PIE790`) to trigger on all unnecessary `pass` statements (#7697) ## Summary Extend `unnecessary-pass` (`PIE790`) to trigger on all unnecessary `pass` statements by checking for `pass` statements in any class or function body with more than one statement. Closes #7600. ## Test Plan `cargo test` --- .../test/fixtures/flake8_pie/PIE790.py | 11 +++ .../src/rules/flake8_pie/rules/mod.rs | 4 +- .../flake8_pie/rules/no_unnecessary_pass.rs | 78 ------------------- .../flake8_pie/rules/unnecessary_pass.rs | 77 ++++++++++++++++++ ...__flake8_pie__tests__PIE790_PIE790.py.snap | 33 ++++++++ 5 files changed, 123 insertions(+), 80 deletions(-) delete mode 100644 crates/ruff_linter/src/rules/flake8_pie/rules/no_unnecessary_pass.rs create mode 100644 crates/ruff_linter/src/rules/flake8_pie/rules/unnecessary_pass.rs diff --git a/crates/ruff_linter/resources/test/fixtures/flake8_pie/PIE790.py b/crates/ruff_linter/resources/test/fixtures/flake8_pie/PIE790.py index 5f35fd443a108..ae89164ff100a 100644 --- a/crates/ruff_linter/resources/test/fixtures/flake8_pie/PIE790.py +++ b/crates/ruff_linter/resources/test/fixtures/flake8_pie/PIE790.py @@ -123,3 +123,14 @@ class Error(Exception): def foo() -> None: pass + + +def foo(): + print("foo") + pass + + +def foo(): + """A docstring.""" + print("foo") + pass diff --git a/crates/ruff_linter/src/rules/flake8_pie/rules/mod.rs b/crates/ruff_linter/src/rules/flake8_pie/rules/mod.rs index 79012144acf6d..aa030ca27519a 100644 --- a/crates/ruff_linter/src/rules/flake8_pie/rules/mod.rs +++ b/crates/ruff_linter/src/rules/flake8_pie/rules/mod.rs @@ -1,17 +1,17 @@ pub(crate) use duplicate_class_field_definition::*; pub(crate) use multiple_starts_ends_with::*; -pub(crate) use no_unnecessary_pass::*; pub(crate) use non_unique_enums::*; pub(crate) use reimplemented_list_builtin::*; pub(crate) use unnecessary_dict_kwargs::*; +pub(crate) use unnecessary_pass::*; pub(crate) use unnecessary_range_start::*; pub(crate) use unnecessary_spread::*; mod duplicate_class_field_definition; mod multiple_starts_ends_with; -mod no_unnecessary_pass; mod non_unique_enums; mod reimplemented_list_builtin; mod unnecessary_dict_kwargs; +mod unnecessary_pass; mod unnecessary_range_start; mod unnecessary_spread; diff --git a/crates/ruff_linter/src/rules/flake8_pie/rules/no_unnecessary_pass.rs b/crates/ruff_linter/src/rules/flake8_pie/rules/no_unnecessary_pass.rs deleted file mode 100644 index bcc54da6e152f..0000000000000 --- a/crates/ruff_linter/src/rules/flake8_pie/rules/no_unnecessary_pass.rs +++ /dev/null @@ -1,78 +0,0 @@ -use ruff_python_ast::Stmt; - -use ruff_diagnostics::AlwaysFixableViolation; -use ruff_diagnostics::{Diagnostic, Edit, Fix}; -use ruff_macros::{derive_message_formats, violation}; -use ruff_python_ast::helpers::is_docstring_stmt; -use ruff_python_ast::whitespace::trailing_comment_start_offset; -use ruff_text_size::Ranged; - -use crate::checkers::ast::Checker; -use crate::fix; -use crate::registry::AsRule; - -/// ## What it does -/// Checks for unnecessary `pass` statements in class and function bodies. -/// where it is not needed syntactically (e.g., when an indented docstring is -/// present). -/// -/// ## Why is this bad? -/// When a function or class definition contains a docstring, an additional -/// `pass` statement is redundant. -/// -/// ## Example -/// ```python -/// def foo(): -/// """Placeholder docstring.""" -/// pass -/// ``` -/// -/// Use instead: -/// ```python -/// def foo(): -/// """Placeholder docstring.""" -/// ``` -/// -/// ## References -/// - [Python documentation: The `pass` statement](https://docs.python.org/3/reference/simple_stmts.html#the-pass-statement) -#[violation] -pub struct UnnecessaryPass; - -impl AlwaysFixableViolation for UnnecessaryPass { - #[derive_message_formats] - fn message(&self) -> String { - format!("Unnecessary `pass` statement") - } - - fn fix_title(&self) -> String { - "Remove unnecessary `pass`".to_string() - } -} - -/// PIE790 -pub(crate) fn no_unnecessary_pass(checker: &mut Checker, body: &[Stmt]) { - let [first, second, ..] = body else { - return; - }; - // This only catches the case in which a docstring makes a `pass` statement - // redundant. Consider removing all `pass` statements instead. - if !is_docstring_stmt(first) { - return; - } - - // The second statement must be a `pass` statement. - if !second.is_pass_stmt() { - return; - } - - let mut diagnostic = Diagnostic::new(UnnecessaryPass, second.range()); - if checker.patch(diagnostic.kind.rule()) { - let edit = if let Some(index) = trailing_comment_start_offset(second, checker.locator()) { - Edit::range_deletion(second.range().add_end(index)) - } else { - fix::edits::delete_stmt(second, None, checker.locator(), checker.indexer()) - }; - diagnostic.set_fix(Fix::automatic(edit)); - } - checker.diagnostics.push(diagnostic); -} diff --git a/crates/ruff_linter/src/rules/flake8_pie/rules/unnecessary_pass.rs b/crates/ruff_linter/src/rules/flake8_pie/rules/unnecessary_pass.rs new file mode 100644 index 0000000000000..433aeaa0b78e9 --- /dev/null +++ b/crates/ruff_linter/src/rules/flake8_pie/rules/unnecessary_pass.rs @@ -0,0 +1,77 @@ +use ruff_python_ast::Stmt; + +use ruff_diagnostics::AlwaysFixableViolation; +use ruff_diagnostics::{Diagnostic, Edit, Fix}; +use ruff_macros::{derive_message_formats, violation}; +use ruff_python_ast::whitespace::trailing_comment_start_offset; +use ruff_text_size::Ranged; + +use crate::checkers::ast::Checker; +use crate::fix; +use crate::registry::AsRule; + +/// ## What it does +/// Checks for unnecessary `pass` statements in functions, classes, and other +/// blocks. +/// +/// ## Why is this bad? +/// In Python, the `pass` statement serves as a placeholder, allowing for +/// syntactically correct empty code blocks. The primary purpose of the `pass` +/// statement is to avoid syntax errors in situations where a statement is +/// syntactically required, but no code needs to be executed. +/// +/// If a `pass` statement is present in a code block that includes at least +/// one other statement (even, e.g., a docstring), it is unnecessary and should +/// be removed. +/// +/// ## Example +/// ```python +/// def func(): +/// """Placeholder docstring.""" +/// pass +/// ``` +/// +/// Use instead: +/// ```python +/// def func(): +/// """Placeholder docstring.""" +/// ``` +/// +/// ## References +/// - [Python documentation: The `pass` statement](https://docs.python.org/3/reference/simple_stmts.html#the-pass-statement) +#[violation] +pub struct UnnecessaryPass; + +impl AlwaysFixableViolation for UnnecessaryPass { + #[derive_message_formats] + fn message(&self) -> String { + format!("Unnecessary `pass` statement") + } + + fn fix_title(&self) -> String { + "Remove unnecessary `pass`".to_string() + } +} + +/// PIE790 +pub(crate) fn no_unnecessary_pass(checker: &mut Checker, body: &[Stmt]) { + if body.len() < 2 { + return; + } + + body.iter() + .filter(|stmt| stmt.is_pass_stmt()) + .for_each(|stmt| { + let mut diagnostic = Diagnostic::new(UnnecessaryPass, stmt.range()); + if checker.patch(diagnostic.kind.rule()) { + let edit = + if let Some(index) = trailing_comment_start_offset(stmt, checker.locator()) { + Edit::range_deletion(stmt.range().add_end(index)) + } else { + fix::edits::delete_stmt(stmt, None, checker.locator(), checker.indexer()) + }; + diagnostic.set_fix(Fix::automatic(edit)); + } + checker.diagnostics.push(diagnostic); + }); +} diff --git a/crates/ruff_linter/src/rules/flake8_pie/snapshots/ruff_linter__rules__flake8_pie__tests__PIE790_PIE790.py.snap b/crates/ruff_linter/src/rules/flake8_pie/snapshots/ruff_linter__rules__flake8_pie__tests__PIE790_PIE790.py.snap index 8a10e5db448fa..c7ad48b9af144 100644 --- a/crates/ruff_linter/src/rules/flake8_pie/snapshots/ruff_linter__rules__flake8_pie__tests__PIE790_PIE790.py.snap +++ b/crates/ruff_linter/src/rules/flake8_pie/snapshots/ruff_linter__rules__flake8_pie__tests__PIE790_PIE790.py.snap @@ -317,4 +317,37 @@ PIE790.py:101:5: PIE790 [*] Unnecessary `pass` statement 103 103 | 104 104 | class Foo: +PIE790.py:130:5: PIE790 [*] Unnecessary `pass` statement + | +128 | def foo(): +129 | print("foo") +130 | pass + | ^^^^ PIE790 + | + = help: Remove unnecessary `pass` + +ℹ Fix +127 127 | +128 128 | def foo(): +129 129 | print("foo") +130 |- pass +131 130 | +132 131 | +133 132 | def foo(): + +PIE790.py:136:5: PIE790 [*] Unnecessary `pass` statement + | +134 | """A docstring.""" +135 | print("foo") +136 | pass + | ^^^^ PIE790 + | + = help: Remove unnecessary `pass` + +ℹ Fix +133 133 | def foo(): +134 134 | """A docstring.""" +135 135 | print("foo") +136 |- pass +