Implement unnecessary-pass-statement (#1580)

This commit is contained in:
Charlie Marsh 2023-01-02 22:15:24 -05:00 committed by GitHub
parent ca7fe686d5
commit 93259acb31
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
10 changed files with 458 additions and 73 deletions

View File

@ -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)

View File

@ -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

View File

@ -763,6 +763,9 @@
"PGH003",
"PGH004",
"PIE",
"PIE7",
"PIE79",
"PIE790",
"PIE8",
"PIE80",
"PIE807",

View File

@ -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) {

View File

@ -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"),
}

View File

@ -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(

View File

@ -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 {

View File

@ -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: ~

View File

@ -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())
}

View File

@ -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,