Avoid removing trailing comments on pass statements (#2235)

This isn't super consistent with some other rules, but... if you have a lone body, with a `pass`, followed by a comment, it's probably surprising if it gets removed. Let's retain the comment.

Closes #2231.
This commit is contained in:
Charlie Marsh 2023-01-26 17:29:13 -05:00 committed by GitHub
parent 9d3a5530af
commit a6ec2eb044
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
4 changed files with 81 additions and 29 deletions

View file

@ -1,12 +1,12 @@
class Foo: class Foo:
"""buzz""" """buzz"""
pass # PIE790 pass
if foo: if foo:
"""foo""" """foo"""
pass # PIE790 pass
def multi_statement() -> None: def multi_statement() -> None:
@ -18,28 +18,28 @@ if foo:
pass pass
else: else:
"""bar""" """bar"""
pass # PIE790 pass
while True: while True:
pass pass
else: else:
"""bar""" """bar"""
pass # PIE790 pass
for _ in range(10): for _ in range(10):
pass pass
else: else:
"""bar""" """bar"""
pass # PIE790 pass
async for _ in range(10): async for _ in range(10):
pass pass
else: else:
"""bar""" """bar"""
pass # PIE790 pass
def foo() -> None: def foo() -> None:
@ -47,7 +47,7 @@ def foo() -> None:
buzz buzz
""" """
pass # PIE790 pass
async def foo(): async def foo():
@ -55,14 +55,14 @@ async def foo():
buzz buzz
""" """
pass # PIE790 pass
try: try:
""" """
buzz buzz
""" """
pass # PIE790 pass
except ValueError: except ValueError:
pass pass
@ -71,29 +71,34 @@ try:
bar() bar()
except ValueError: except ValueError:
"""bar""" """bar"""
pass # PIE790 pass
for _ in range(10): for _ in range(10):
"""buzz""" """buzz"""
pass # PIE790 pass
async for _ in range(10): async for _ in range(10):
"""buzz""" """buzz"""
pass # PIE790 pass
while cond: while cond:
"""buzz""" """buzz"""
pass # PIE790 pass
with bar: with bar:
"""buzz""" """buzz"""
pass # PIE790 pass
async with bar: async with bar:
"""buzz""" """buzz"""
pass # PIE790 pass
def foo() -> None:
"""buzz"""
pass # bar
class Foo: class Foo:

View file

@ -676,6 +676,24 @@ pub fn match_trailing_content(stmt: &Stmt, locator: &Locator) -> bool {
false false
} }
/// If a `Stmt` has a trailing comment, return the index of the hash.
pub fn match_trailing_comment(stmt: &Stmt, locator: &Locator) -> Option<usize> {
let range = Range::new(
stmt.end_location.unwrap(),
Location::new(stmt.end_location.unwrap().row() + 1, 0),
);
let suffix = locator.slice_source_code_range(&range);
for (i, char) in suffix.chars().enumerate() {
if char == '#' {
return Some(i);
}
if !char.is_whitespace() {
return None;
}
}
None
}
/// Return the number of trailing empty lines following a statement. /// Return the number of trailing empty lines following a statement.
pub fn count_trailing_lines(stmt: &Stmt, locator: &Locator) -> usize { pub fn count_trailing_lines(stmt: &Stmt, locator: &Locator) -> usize {
let suffix = let suffix =

View file

@ -4,12 +4,13 @@ use rustc_hash::FxHashSet;
use rustpython_ast::{Constant, Expr, ExprKind, Keyword, Stmt, StmtKind}; use rustpython_ast::{Constant, Expr, ExprKind, Keyword, Stmt, StmtKind};
use crate::ast::comparable::ComparableExpr; use crate::ast::comparable::ComparableExpr;
use crate::ast::helpers::unparse_expr; use crate::ast::helpers::{match_trailing_comment, unparse_expr};
use crate::ast::types::{Range, RefEquality}; use crate::ast::types::{Range, RefEquality};
use crate::autofix::helpers::delete_stmt; use crate::autofix::helpers::delete_stmt;
use crate::checkers::ast::Checker; use crate::checkers::ast::Checker;
use crate::define_violation; use crate::define_violation;
use crate::fix::Fix; use crate::fix::Fix;
use crate::message::Location;
use crate::python::identifiers::is_identifier; use crate::python::identifiers::is_identifier;
use crate::registry::{Diagnostic, Rule}; use crate::registry::{Diagnostic, Rule};
use crate::violation::{AlwaysAutofixableViolation, Violation}; use crate::violation::{AlwaysAutofixableViolation, Violation};
@ -112,19 +113,29 @@ pub fn no_unnecessary_pass(checker: &mut Checker, body: &[Stmt]) {
let mut diagnostic = let mut diagnostic =
Diagnostic::new(NoUnnecessaryPass, Range::from_located(pass_stmt)); Diagnostic::new(NoUnnecessaryPass, Range::from_located(pass_stmt));
if checker.patch(&Rule::NoUnnecessaryPass) { if checker.patch(&Rule::NoUnnecessaryPass) {
match delete_stmt( if let Some(index) = match_trailing_comment(pass_stmt, checker.locator) {
pass_stmt, diagnostic.amend(Fix::deletion(
None, pass_stmt.location,
&[], Location::new(
checker.locator, pass_stmt.end_location.unwrap().row(),
checker.indexer, pass_stmt.end_location.unwrap().column() + index,
checker.stylist, ),
) { ));
Ok(fix) => { } else {
diagnostic.amend(fix); match delete_stmt(
} pass_stmt,
Err(e) => { None,
error!("Failed to delete `pass` statement: {}", e); &[],
checker.locator,
checker.indexer,
checker.stylist,
) {
Ok(fix) => {
diagnostic.amend(fix);
}
Err(e) => {
error!("Failed to delete `pass` statement: {}", e);
}
} }
} }
} }

View file

@ -290,4 +290,22 @@ expression: diagnostics
row: 97 row: 97
column: 0 column: 0
parent: ~ parent: ~
- kind:
NoUnnecessaryPass: ~
location:
row: 101
column: 4
end_location:
row: 101
column: 8
fix:
content:
- ""
location:
row: 101
column: 4
end_location:
row: 101
column: 10
parent: ~