diff --git a/README.md b/README.md index 02890d5516..3c1be25744 100644 --- a/README.md +++ b/README.md @@ -1059,7 +1059,8 @@ For more, see [flake8-pie](https://pypi.org/project/flake8-pie/0.16.0/) on PyPI. | Code | Name | Message | Fix | | ---- | ---- | ------- | --- | -| PIE807 | PreferListBuiltin | | 🛠 | +| PIE790 | NoUnnecessaryPass | Unnecessary `pass` statement | 🛠 | +| PIE807 | PreferListBuiltin | Prefer `list()` over useless lambda | 🛠 | ### Ruff-specific rules (RUF) diff --git a/resources/test/fixtures/flake8_pie/PIE790.py b/resources/test/fixtures/flake8_pie/PIE790.py new file mode 100644 index 0000000000..d9888d5b20 --- /dev/null +++ b/resources/test/fixtures/flake8_pie/PIE790.py @@ -0,0 +1,115 @@ +class Foo: + """buzz""" + + pass # PIE790 + + +if foo: + """foo""" + pass # PIE790 + + +if foo: + pass +else: + """bar""" + pass # PIE790 + + +while True: + pass +else: + """bar""" + pass # PIE790 + + +for _ in range(10): + pass +else: + """bar""" + pass # PIE790 + + +async for _ in range(10): + pass +else: + """bar""" + pass # PIE790 + + +def foo() -> None: + """ + buzz + """ + + pass # PIE790 + + +async def foo(): + """ + buzz + """ + + pass # PIE790 + + +try: + """ + buzz + """ + pass # PIE790 +except ValueError: + pass + + +try: + bar() +except ValueError: + """bar""" + pass # PIE790 + + +for _ in range(10): + """buzz""" + pass # PIE790 + +async for _ in range(10): + """buzz""" + pass # PIE790 + +while cond: + """buzz""" + pass # PIE790 + + +with bar: + """buzz""" + pass # PIE790 + +async with bar: + """buzz""" + pass # PIE790 + + +class Foo: + # bar + pass + + +if foo: + # foo + pass + + +class Error(Exception): + pass + + +try: + foo() +except NetworkError: + pass + + +def foo() -> None: + pass diff --git a/ruff.schema.json b/ruff.schema.json index 3a987c14de..512353bc8e 100644 --- a/ruff.schema.json +++ b/ruff.schema.json @@ -763,6 +763,9 @@ "PGH003", "PGH004", "PIE", + "PIE7", + "PIE79", + "PIE790", "PIE8", "PIE80", "PIE807", diff --git a/src/ast/visitor.rs b/src/ast/visitor.rs index c6e91018e3..6db3b3f3b6 100644 --- a/src/ast/visitor.rs +++ b/src/ast/visitor.rs @@ -62,6 +62,15 @@ pub trait Visitor<'a> { fn visit_pattern(&mut self, pattern: &'a Pattern) { walk_pattern(self, pattern); } + fn visit_body(&mut self, body: &'a [Stmt]) { + walk_body(self, body); + } +} + +pub fn walk_body<'a, V: Visitor<'a> + ?Sized>(visitor: &mut V, body: &'a [Stmt]) { + for stmt in body { + visitor.visit_stmt(stmt); + } } pub fn walk_stmt<'a, V: Visitor<'a> + ?Sized>(visitor: &mut V, stmt: &'a Stmt) { @@ -80,9 +89,7 @@ pub fn walk_stmt<'a, V: Visitor<'a> + ?Sized>(visitor: &mut V, stmt: &'a Stmt) { for expr in returns { visitor.visit_annotation(expr); } - for stmt in body { - visitor.visit_stmt(stmt); - } + visitor.visit_body(body); } StmtKind::AsyncFunctionDef { args, @@ -98,9 +105,7 @@ pub fn walk_stmt<'a, V: Visitor<'a> + ?Sized>(visitor: &mut V, stmt: &'a Stmt) { for expr in returns { visitor.visit_annotation(expr); } - for stmt in body { - visitor.visit_stmt(stmt); - } + visitor.visit_body(body); } StmtKind::ClassDef { bases, @@ -118,9 +123,7 @@ pub fn walk_stmt<'a, V: Visitor<'a> + ?Sized>(visitor: &mut V, stmt: &'a Stmt) { for expr in decorator_list { visitor.visit_expr(expr); } - for stmt in body { - visitor.visit_stmt(stmt); - } + visitor.visit_body(body); } StmtKind::Return { value } => { if let Some(expr) = value { @@ -164,12 +167,8 @@ pub fn walk_stmt<'a, V: Visitor<'a> + ?Sized>(visitor: &mut V, stmt: &'a Stmt) { } => { visitor.visit_expr(iter); visitor.visit_expr(target); - for stmt in body { - visitor.visit_stmt(stmt); - } - for stmt in orelse { - visitor.visit_stmt(stmt); - } + visitor.visit_body(body); + visitor.visit_body(orelse); } StmtKind::AsyncFor { target, @@ -180,46 +179,30 @@ pub fn walk_stmt<'a, V: Visitor<'a> + ?Sized>(visitor: &mut V, stmt: &'a Stmt) { } => { visitor.visit_expr(iter); visitor.visit_expr(target); - for stmt in body { - visitor.visit_stmt(stmt); - } - for stmt in orelse { - visitor.visit_stmt(stmt); - } + visitor.visit_body(body); + visitor.visit_body(orelse); } StmtKind::While { test, body, orelse } => { visitor.visit_expr(test); - for stmt in body { - visitor.visit_stmt(stmt); - } - for stmt in orelse { - visitor.visit_stmt(stmt); - } + visitor.visit_body(body); + visitor.visit_body(orelse); } StmtKind::If { test, body, orelse } => { visitor.visit_expr(test); - for stmt in body { - visitor.visit_stmt(stmt); - } - for stmt in orelse { - visitor.visit_stmt(stmt); - } + visitor.visit_body(body); + visitor.visit_body(orelse); } StmtKind::With { items, body, .. } => { for withitem in items { visitor.visit_withitem(withitem); } - for stmt in body { - visitor.visit_stmt(stmt); - } + visitor.visit_body(body); } StmtKind::AsyncWith { items, body, .. } => { for withitem in items { visitor.visit_withitem(withitem); } - for stmt in body { - visitor.visit_stmt(stmt); - } + visitor.visit_body(body); } StmtKind::Match { subject, cases } => { // TODO(charlie): Handle `cases`. @@ -242,18 +225,12 @@ pub fn walk_stmt<'a, V: Visitor<'a> + ?Sized>(visitor: &mut V, stmt: &'a Stmt) { orelse, finalbody, } => { - for stmt in body { - visitor.visit_stmt(stmt); - } + visitor.visit_body(body); for excepthandler in handlers { visitor.visit_excepthandler(excepthandler); } - for stmt in orelse { - visitor.visit_stmt(stmt); - } - for stmt in finalbody { - visitor.visit_stmt(stmt); - } + visitor.visit_body(orelse); + visitor.visit_body(finalbody); } StmtKind::Assert { test, msg } => { visitor.visit_expr(test); @@ -469,9 +446,7 @@ pub fn walk_excepthandler<'a, V: Visitor<'a> + ?Sized>( if let Some(expr) = type_ { visitor.visit_expr(expr); } - for stmt in body { - visitor.visit_stmt(stmt); - } + visitor.visit_body(body); } } } @@ -522,9 +497,7 @@ pub fn walk_match_case<'a, V: Visitor<'a> + ?Sized>(visitor: &mut V, match_case: if let Some(expr) = &match_case.guard { visitor.visit_expr(expr); } - for stmt in &match_case.body { - visitor.visit_stmt(stmt); - } + visitor.visit_body(&match_case.body); } pub fn walk_pattern<'a, V: Visitor<'a> + ?Sized>(visitor: &mut V, pattern: &'a Pattern) { diff --git a/src/checkers/ast.rs b/src/checkers/ast.rs index fbf0886fbf..f1c44ae1cf 100644 --- a/src/checkers/ast.rs +++ b/src/checkers/ast.rs @@ -1445,9 +1445,7 @@ where globals, }))); - for stmt in body { - self.visit_stmt(stmt); - } + self.visit_body(body); } StmtKind::Try { body, @@ -1459,19 +1457,13 @@ where if self.settings.enabled.contains(&CheckCode::B012) { flake8_bugbear::plugins::jump_statement_in_finally(self, finalbody); } - for stmt in body { - self.visit_stmt(stmt); - } + self.visit_body(body); self.except_handlers.pop(); for excepthandler in handlers { self.visit_excepthandler(excepthandler); } - for stmt in orelse { - self.visit_stmt(stmt); - } - for stmt in finalbody { - self.visit_stmt(stmt); - } + self.visit_body(orelse); + self.visit_body(finalbody); } StmtKind::AnnAssign { target, @@ -2995,6 +2987,13 @@ where self.check_builtin_arg_shadowing(&arg.node.arg, arg); } + + fn visit_body(&mut self, body: &'b [Stmt]) { + if self.settings.enabled.contains(&CheckCode::PIE790) { + flake8_pie::plugins::no_unnecessary_pass(self, body); + } + visitor::walk_body(self, body); + } } impl<'a> Checker<'a> { @@ -3572,9 +3571,7 @@ impl<'a> Checker<'a> { StmtKind::FunctionDef { body, args, .. } | StmtKind::AsyncFunctionDef { body, args, .. } => { self.visit_arguments(args); - for stmt in body { - self.visit_stmt(stmt); - } + self.visit_body(body); } _ => unreachable!("Expected StmtKind::FunctionDef | StmtKind::AsyncFunctionDef"), } diff --git a/src/flake8_pie/mod.rs b/src/flake8_pie/mod.rs index 39ff994703..aff26bd123 100644 --- a/src/flake8_pie/mod.rs +++ b/src/flake8_pie/mod.rs @@ -13,6 +13,7 @@ mod tests { use crate::settings; #[test_case(CheckCode::PIE807, Path::new("PIE807.py"); "PIE807")] + #[test_case(CheckCode::PIE790, Path::new("PIE790.py"); "PIE790")] fn checks(check_code: CheckCode, path: &Path) -> Result<()> { let snapshot = format!("{}_{}", check_code.as_ref(), path.to_string_lossy()); let checks = test_path( diff --git a/src/flake8_pie/plugins.rs b/src/flake8_pie/plugins.rs index 58a3ce91e0..a96e8b9a0d 100644 --- a/src/flake8_pie/plugins.rs +++ b/src/flake8_pie/plugins.rs @@ -1,10 +1,42 @@ -use rustpython_ast::{Expr, ExprKind}; +use rustpython_ast::{Constant, Expr, ExprKind, Stmt, StmtKind}; use crate::ast::types::Range; use crate::autofix::Fix; use crate::checkers::ast::Checker; use crate::registry::{Check, CheckCode, CheckKind}; +/// PIE790 +pub fn no_unnecessary_pass(checker: &mut Checker, body: &[Stmt]) { + if body.len() > 1 { + // This only catches the case in which a docstring makes a `pass` statement redundant. + // Consider removing all `pass` statements instead. + let docstring_stmt = &body[0]; + let pass_stmt = &body[1]; + let StmtKind::Expr { value } = &docstring_stmt.node else { + return; + }; + if matches!( + value.node, + ExprKind::Constant { + value: Constant::Str(..), + .. + } + ) { + if matches!(pass_stmt.node, StmtKind::Pass) { + let mut check = + Check::new(CheckKind::NoUnnecessaryPass, Range::from_located(pass_stmt)); + if checker.patch(&CheckCode::PIE790) { + check.amend(Fix::deletion( + pass_stmt.location, + pass_stmt.end_location.unwrap(), + )); + } + checker.add_check(check); + } + } + } +} + /// PIE807 pub fn prefer_list_builtin(checker: &mut Checker, expr: &Expr) { let ExprKind::Lambda { args, body } = &expr.node else { diff --git a/src/flake8_pie/snapshots/ruff__flake8_pie__tests__PIE790_PIE790.py.snap b/src/flake8_pie/snapshots/ruff__flake8_pie__tests__PIE790_PIE790.py.snap new file mode 100644 index 0000000000..8ede770470 --- /dev/null +++ b/src/flake8_pie/snapshots/ruff__flake8_pie__tests__PIE790_PIE790.py.snap @@ -0,0 +1,245 @@ +--- +source: src/flake8_pie/mod.rs +expression: checks +--- +- kind: NoUnnecessaryPass + location: + row: 4 + column: 4 + end_location: + row: 4 + column: 8 + fix: + content: "" + location: + row: 4 + column: 4 + end_location: + row: 4 + column: 8 + parent: ~ +- kind: NoUnnecessaryPass + location: + row: 9 + column: 4 + end_location: + row: 9 + column: 8 + fix: + content: "" + location: + row: 9 + column: 4 + end_location: + row: 9 + column: 8 + parent: ~ +- kind: NoUnnecessaryPass + location: + row: 16 + column: 4 + end_location: + row: 16 + column: 8 + fix: + content: "" + location: + row: 16 + column: 4 + end_location: + row: 16 + column: 8 + parent: ~ +- kind: NoUnnecessaryPass + location: + row: 23 + column: 4 + end_location: + row: 23 + column: 8 + fix: + content: "" + location: + row: 23 + column: 4 + end_location: + row: 23 + column: 8 + parent: ~ +- kind: NoUnnecessaryPass + location: + row: 30 + column: 4 + end_location: + row: 30 + column: 8 + fix: + content: "" + location: + row: 30 + column: 4 + end_location: + row: 30 + column: 8 + parent: ~ +- kind: NoUnnecessaryPass + location: + row: 37 + column: 4 + end_location: + row: 37 + column: 8 + fix: + content: "" + location: + row: 37 + column: 4 + end_location: + row: 37 + column: 8 + parent: ~ +- kind: NoUnnecessaryPass + location: + row: 45 + column: 4 + end_location: + row: 45 + column: 8 + fix: + content: "" + location: + row: 45 + column: 4 + end_location: + row: 45 + column: 8 + parent: ~ +- kind: NoUnnecessaryPass + location: + row: 53 + column: 4 + end_location: + row: 53 + column: 8 + fix: + content: "" + location: + row: 53 + column: 4 + end_location: + row: 53 + column: 8 + parent: ~ +- kind: NoUnnecessaryPass + location: + row: 60 + column: 4 + end_location: + row: 60 + column: 8 + fix: + content: "" + location: + row: 60 + column: 4 + end_location: + row: 60 + column: 8 + parent: ~ +- kind: NoUnnecessaryPass + location: + row: 69 + column: 4 + end_location: + row: 69 + column: 8 + fix: + content: "" + location: + row: 69 + column: 4 + end_location: + row: 69 + column: 8 + parent: ~ +- kind: NoUnnecessaryPass + location: + row: 74 + column: 4 + end_location: + row: 74 + column: 8 + fix: + content: "" + location: + row: 74 + column: 4 + end_location: + row: 74 + column: 8 + parent: ~ +- kind: NoUnnecessaryPass + location: + row: 78 + column: 4 + end_location: + row: 78 + column: 8 + fix: + content: "" + location: + row: 78 + column: 4 + end_location: + row: 78 + column: 8 + parent: ~ +- kind: NoUnnecessaryPass + location: + row: 82 + column: 4 + end_location: + row: 82 + column: 8 + fix: + content: "" + location: + row: 82 + column: 4 + end_location: + row: 82 + column: 8 + parent: ~ +- kind: NoUnnecessaryPass + location: + row: 87 + column: 4 + end_location: + row: 87 + column: 8 + fix: + content: "" + location: + row: 87 + column: 4 + end_location: + row: 87 + column: 8 + parent: ~ +- kind: NoUnnecessaryPass + location: + row: 91 + column: 4 + end_location: + row: 91 + column: 8 + fix: + content: "" + location: + row: 91 + column: 4 + end_location: + row: 91 + column: 8 + parent: ~ + diff --git a/src/registry.rs b/src/registry.rs index 42afce29a8..066bad9968 100644 --- a/src/registry.rs +++ b/src/registry.rs @@ -390,6 +390,7 @@ pub enum CheckCode { PT025, PT026, // flake8-pie + PIE790, PIE807, // Ruff RUF001, @@ -1115,6 +1116,7 @@ pub enum CheckKind { ErroneousUseFixturesOnFixture, UseFixturesWithoutParameters, // flake8-pie + NoUnnecessaryPass, PreferListBuiltin, // Ruff AmbiguousUnicodeCharacterString(char, char), @@ -1568,6 +1570,7 @@ impl CheckCode { CheckCode::PT025 => CheckKind::ErroneousUseFixturesOnFixture, CheckCode::PT026 => CheckKind::UseFixturesWithoutParameters, // flake8-pie + CheckCode::PIE790 => CheckKind::NoUnnecessaryPass, CheckCode::PIE807 => CheckKind::PreferListBuiltin, // Ruff CheckCode::RUF001 => CheckKind::AmbiguousUnicodeCharacterString('𝐁', 'B'), @@ -1934,6 +1937,7 @@ impl CheckCode { CheckCode::YTT302 => CheckCategory::Flake82020, CheckCode::YTT303 => CheckCategory::Flake82020, // flake8-pie + CheckCode::PIE790 => CheckCategory::Flake8Pie, CheckCode::PIE807 => CheckCategory::Flake8Pie, // Ruff CheckCode::RUF001 => CheckCategory::Ruff, @@ -2301,6 +2305,7 @@ impl CheckKind { CheckKind::ErroneousUseFixturesOnFixture => &CheckCode::PT025, CheckKind::UseFixturesWithoutParameters => &CheckCode::PT026, // flake8-pie + CheckKind::NoUnnecessaryPass => &CheckCode::PIE790, CheckKind::PreferListBuiltin => &CheckCode::PIE807, // Ruff CheckKind::AmbiguousUnicodeCharacterString(..) => &CheckCode::RUF001, @@ -3353,7 +3358,8 @@ impl CheckKind { "Useless `pytest.mark.usefixtures` without parameters".to_string() } // flake8-pie - CheckKind::PreferListBuiltin => String::new(), + CheckKind::NoUnnecessaryPass => "Unnecessary `pass` statement".to_string(), + CheckKind::PreferListBuiltin => "Prefer `list()` over useless lambda".to_string(), // Ruff CheckKind::AmbiguousUnicodeCharacterString(confusable, representant) => { format!( @@ -3501,6 +3507,7 @@ impl CheckKind { | CheckKind::NoOverIndentation | CheckKind::NoSurroundingWhitespace | CheckKind::NoUnderIndentation + | CheckKind::NoUnnecessaryPass | CheckKind::NoneComparison(..) | CheckKind::NotInTest | CheckKind::NotIsTest @@ -3662,6 +3669,7 @@ impl CheckKind { CheckKind::NoBlankLineBeforeClass(..) => { Some("Remove blank line(s) before class docstring".to_string()) } + CheckKind::NoUnnecessaryPass => Some("Remove unnecessary `pass`".to_string()), CheckKind::OneBlankLineBeforeClass(..) => { Some("Insert 1 blank line before class docstring".to_string()) } diff --git a/src/registry_gen.rs b/src/registry_gen.rs index 448dce7e2d..6f6de91490 100644 --- a/src/registry_gen.rs +++ b/src/registry_gen.rs @@ -383,6 +383,9 @@ pub enum CheckCodePrefix { PGH003, PGH004, PIE, + PIE7, + PIE79, + PIE790, PIE8, PIE80, PIE807, @@ -948,6 +951,7 @@ impl CheckCodePrefix { CheckCode::PT024, CheckCode::PT025, CheckCode::PT026, + CheckCode::PIE790, CheckCode::PIE807, CheckCode::RUF001, CheckCode::RUF002, @@ -2179,7 +2183,10 @@ impl CheckCodePrefix { CheckCodePrefix::PGH002 => vec![CheckCode::PGH002], CheckCodePrefix::PGH003 => vec![CheckCode::PGH003], CheckCodePrefix::PGH004 => vec![CheckCode::PGH004], - CheckCodePrefix::PIE => vec![CheckCode::PIE807], + CheckCodePrefix::PIE => vec![CheckCode::PIE790, CheckCode::PIE807], + CheckCodePrefix::PIE7 => vec![CheckCode::PIE790], + CheckCodePrefix::PIE79 => vec![CheckCode::PIE790], + CheckCodePrefix::PIE790 => vec![CheckCode::PIE790], CheckCodePrefix::PIE8 => vec![CheckCode::PIE807], CheckCodePrefix::PIE80 => vec![CheckCode::PIE807], CheckCodePrefix::PIE807 => vec![CheckCode::PIE807], @@ -3400,6 +3407,9 @@ impl CheckCodePrefix { CheckCodePrefix::PGH003 => SuffixLength::Three, CheckCodePrefix::PGH004 => SuffixLength::Three, CheckCodePrefix::PIE => SuffixLength::Zero, + CheckCodePrefix::PIE7 => SuffixLength::One, + CheckCodePrefix::PIE79 => SuffixLength::Two, + CheckCodePrefix::PIE790 => SuffixLength::Three, CheckCodePrefix::PIE8 => SuffixLength::One, CheckCodePrefix::PIE80 => SuffixLength::Two, CheckCodePrefix::PIE807 => SuffixLength::Three,