Skip to content

Commit

Permalink
Remove continuations when deleting statements
Browse files Browse the repository at this point in the history
  • Loading branch information
charliermarsh committed Jun 20, 2023
1 parent 36e01ad commit c2f596b
Show file tree
Hide file tree
Showing 17 changed files with 402 additions and 325 deletions.
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@

if True:
import foo1; x = 1
import foo2; x = 1
Expand All @@ -11,7 +10,6 @@
import foo4 \
; x = 1


if True:
x = 1; import foo5

Expand All @@ -20,12 +18,10 @@
x = 1; \
import foo6


if True:
x = 1 \
; import foo7


if True:
x = 1; import foo8; x = 1
x = 1; import foo9; x = 1
Expand All @@ -40,12 +36,27 @@
;import foo11 \
;x = 1

if True:
x = 1; \
\
import foo12

if True:
x = 1; \
\
import foo13


if True:
x = 1; \
# \
import foo14

# Continuation, but not as the last content in the file.
x = 1; \
import foo12
import foo15

# Continuation, followed by end-of-file. (Removing `import foo` would cause a syntax
# error.)
x = 1; \
import foo13
import foo16
48 changes: 24 additions & 24 deletions crates/ruff/src/autofix/edits.rs
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,6 @@ pub(crate) fn delete_stmt(
parent: Option<&Stmt>,
locator: &Locator,
indexer: &Indexer,
stylist: &Stylist,
) -> Edit {
if parent
.map(|parent| is_lone_child(stmt, parent))
Expand All @@ -39,18 +38,15 @@ pub(crate) fn delete_stmt(
// it with a `pass`.
Edit::range_replacement("pass".to_string(), stmt.range())
} else {
if let Some(semicolon) = trailing_semicolon(stmt, locator) {
if let Some(semicolon) = trailing_semicolon(stmt.end(), locator) {
let next = next_stmt_break(semicolon, locator);
Edit::deletion(stmt.start(), next)
} else if helpers::has_leading_content(stmt, locator) {
} else if helpers::has_leading_content(stmt.start(), locator) {
Edit::range_deletion(stmt.range())
} else if helpers::preceded_by_continuation(stmt, indexer, locator) {
if is_end_of_file(stmt, locator) && locator.is_at_start_of_line(stmt.start()) {
// Special-case: a file can't end in a continuation.
Edit::range_replacement(stylist.line_ending().to_string(), stmt.range())
} else {
Edit::range_deletion(stmt.range())
}
} else if let Some(start) =
helpers::preceded_by_continuations(stmt.start(), locator, indexer)
{
Edit::range_deletion(TextRange::new(start, stmt.end()))
} else {
let range = locator.full_lines_range(stmt.range());
Edit::range_deletion(range)
Expand All @@ -68,7 +64,7 @@ pub(crate) fn remove_unused_imports<'a>(
stylist: &Stylist,
) -> Result<Edit> {
match codemods::remove_imports(unused_imports, stmt, locator, stylist)? {
None => Ok(delete_stmt(stmt, parent, locator, indexer, stylist)),
None => Ok(delete_stmt(stmt, parent, locator, indexer)),
Some(content) => Ok(Edit::range_replacement(content, stmt.range())),
}
}
Expand Down Expand Up @@ -238,15 +234,15 @@ fn is_lone_child(child: &Stmt, parent: &Stmt) -> bool {

/// Return the location of a trailing semicolon following a `Stmt`, if it's part
/// of a multi-statement line.
fn trailing_semicolon(stmt: &Stmt, locator: &Locator) -> Option<TextSize> {
let contents = locator.after(stmt.end());
fn trailing_semicolon(offset: TextSize, locator: &Locator) -> Option<TextSize> {
let contents = locator.after(offset);

for line in NewlineWithTrailingNewline::from(contents) {
let trimmed = line.trim_whitespace_start();

if trimmed.starts_with(';') {
let colon_offset = line.text_len() - trimmed.text_len();
return Some(stmt.end() + line.start() + colon_offset);
return Some(offset + line.start() + colon_offset);
}

if !trimmed.starts_with('\\') {
Expand Down Expand Up @@ -284,16 +280,11 @@ fn next_stmt_break(semicolon: TextSize, locator: &Locator) -> TextSize {
locator.line_end(start_location)
}

/// Return `true` if a `Stmt` occurs at the end of a file.
fn is_end_of_file(stmt: &Stmt, locator: &Locator) -> bool {
stmt.end() == locator.contents().text_len()
}

#[cfg(test)]
mod tests {
use anyhow::Result;
use ruff_text_size::TextSize;
use rustpython_parser::ast::Suite;
use rustpython_parser::ast::{Ranged, Suite};
use rustpython_parser::Parse;

use ruff_python_ast::source_code::Locator;
Expand All @@ -306,19 +297,25 @@ mod tests {
let program = Suite::parse(contents, "<filename>")?;
let stmt = program.first().unwrap();
let locator = Locator::new(contents);
assert_eq!(trailing_semicolon(stmt, &locator), None);
assert_eq!(trailing_semicolon(stmt.end(), &locator), None);

let contents = "x = 1; y = 1";
let program = Suite::parse(contents, "<filename>")?;
let stmt = program.first().unwrap();
let locator = Locator::new(contents);
assert_eq!(trailing_semicolon(stmt, &locator), Some(TextSize::from(5)));
assert_eq!(
trailing_semicolon(stmt.end(), &locator),
Some(TextSize::from(5))
);

let contents = "x = 1 ; y = 1";
let program = Suite::parse(contents, "<filename>")?;
let stmt = program.first().unwrap();
let locator = Locator::new(contents);
assert_eq!(trailing_semicolon(stmt, &locator), Some(TextSize::from(6)));
assert_eq!(
trailing_semicolon(stmt.end(), &locator),
Some(TextSize::from(6))
);

let contents = r#"
x = 1 \
Expand All @@ -328,7 +325,10 @@ x = 1 \
let program = Suite::parse(contents, "<filename>")?;
let stmt = program.first().unwrap();
let locator = Locator::new(contents);
assert_eq!(trailing_semicolon(stmt, &locator), Some(TextSize::from(10)));
assert_eq!(
trailing_semicolon(stmt.end(), &locator),
Some(TextSize::from(10))
);

Ok(())
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,6 @@ pub(crate) fn duplicate_class_field_definition<'a, 'b>(
Some(parent),
checker.locator,
checker.indexer,
checker.stylist,
);
diagnostic.set_fix(Fix::suggested(edit).isolate(checker.isolation(Some(parent))));
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -67,13 +67,7 @@ pub(crate) fn no_unnecessary_pass(checker: &mut Checker, body: &[Stmt]) {
let edit = if let Some(index) = trailing_comment_start_offset(stmt, checker.locator) {
Edit::range_deletion(stmt.range().add_end(index))
} else {
autofix::edits::delete_stmt(
stmt,
None,
checker.locator,
checker.indexer,
checker.stylist,
)
autofix::edits::delete_stmt(stmt, None, checker.locator, checker.indexer)
};
diagnostic.set_fix(Fix::automatic(edit));
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -69,13 +69,8 @@ pub(crate) fn ellipsis_in_non_empty_class_body<'a>(

let mut diagnostic = Diagnostic::new(EllipsisInNonEmptyClassBody, stmt.range());
if checker.patch(diagnostic.kind.rule()) {
let edit = autofix::edits::delete_stmt(
stmt,
Some(parent),
checker.locator,
checker.indexer,
checker.stylist,
);
let edit =
autofix::edits::delete_stmt(stmt, Some(parent), checker.locator, checker.indexer);
diagnostic.set_fix(Fix::automatic(edit).isolate(checker.isolation(Some(parent))));
}
checker.diagnostics.push(diagnostic);
Expand Down
9 changes: 2 additions & 7 deletions crates/ruff/src/rules/flake8_pyi/rules/pass_in_class_body.rs
Original file line number Diff line number Diff line change
Expand Up @@ -39,13 +39,8 @@ pub(crate) fn pass_in_class_body<'a>(

let mut diagnostic = Diagnostic::new(PassInClassBody, stmt.range());
if checker.patch(diagnostic.kind.rule()) {
let edit = autofix::edits::delete_stmt(
stmt,
Some(parent),
checker.locator,
checker.indexer,
checker.stylist,
);
let edit =
autofix::edits::delete_stmt(stmt, Some(parent), checker.locator, checker.indexer);
diagnostic.set_fix(Fix::automatic(edit).isolate(checker.isolation(Some(parent))));
}
checker.diagnostics.push(diagnostic);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -95,13 +95,7 @@ pub(crate) fn str_or_repr_defined_in_stub(checker: &mut Checker, stmt: &Stmt) {
if checker.patch(diagnostic.kind.rule()) {
let stmt = checker.semantic().stmt();
let parent = checker.semantic().stmt_parent();
let edit = delete_stmt(
stmt,
parent,
checker.locator,
checker.indexer,
checker.stylist,
);
let edit = delete_stmt(stmt, parent, checker.locator, checker.indexer);
diagnostic.set_fix(
Fix::automatic(edit).isolate(checker.isolation(checker.semantic().stmt_parent())),
);
Expand Down
9 changes: 2 additions & 7 deletions crates/ruff/src/rules/flake8_return/rules/function.rs
Original file line number Diff line number Diff line change
Expand Up @@ -517,13 +517,8 @@ fn unnecessary_assign(checker: &mut Checker, stack: &Stack) {
// Delete the `return` statement. There's no need to treat this as an isolated
// edit, since we're editing the preceding statement, so no conflicting edit would
// be allowed to remove that preceding statement.
let delete_return = edits::delete_stmt(
stmt,
None,
checker.locator,
checker.indexer,
checker.stylist,
);
let delete_return =
edits::delete_stmt(stmt, None, checker.locator, checker.indexer);

// Replace the `x = 1` statement with `return 1`.
let content = checker.locator.slice(assign.range());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -62,13 +62,7 @@ pub(crate) fn empty_type_checking_block(checker: &mut Checker, stmt: &ast::StmtI
// Delete the entire type-checking block.
let stmt = checker.semantic().stmt();
let parent = checker.semantic().stmt_parent();
let edit = autofix::edits::delete_stmt(
stmt,
parent,
checker.locator,
checker.indexer,
checker.stylist,
);
let edit = autofix::edits::delete_stmt(stmt, parent, checker.locator, checker.indexer);
diagnostic.set_fix(Fix::automatic(edit).isolate(checker.isolation(parent)));
}
checker.diagnostics.push(diagnostic);
Expand Down
4 changes: 2 additions & 2 deletions crates/ruff/src/rules/pycodestyle/rules/lambda_assignment.rs
Original file line number Diff line number Diff line change
Expand Up @@ -78,8 +78,8 @@ pub(crate) fn lambda_assignment(
// See https://github.com/astral-sh/ruff/issues/3046
if checker.patch(diagnostic.kind.rule())
&& !checker.semantic().scope().kind.is_class()
&& !has_leading_content(stmt, checker.locator)
&& !has_trailing_content(stmt, checker.locator)
&& !has_leading_content(stmt.start(), checker.locator)
&& !has_trailing_content(stmt.end(), checker.locator)
{
let first_line = checker.locator.line(stmt.start());
let indentation = leading_indentation(first_line);
Expand Down
16 changes: 2 additions & 14 deletions crates/ruff/src/rules/pyflakes/rules/unused_variable.rs
Original file line number Diff line number Diff line change
Expand Up @@ -210,13 +210,7 @@ fn remove_unused_variable(
Some(Fix::suggested(edit))
} else {
// If (e.g.) assigning to a constant (`x = 1`), delete the entire statement.
let edit = delete_stmt(
stmt,
parent,
checker.locator,
checker.indexer,
checker.stylist,
);
let edit = delete_stmt(stmt, parent, checker.locator, checker.indexer);
Some(Fix::suggested(edit).isolate(checker.isolation(parent)))
};
}
Expand All @@ -241,13 +235,7 @@ fn remove_unused_variable(
Some(Fix::suggested(edit))
} else {
// If (e.g.) assigning to a constant (`x = 1`), delete the entire statement.
let edit = delete_stmt(
stmt,
parent,
checker.locator,
checker.indexer,
checker.stylist,
);
let edit = delete_stmt(stmt, parent, checker.locator, checker.indexer);
Some(Fix::suggested(edit).isolate(checker.isolation(parent)))
};
}
Expand Down
Loading

0 comments on commit c2f596b

Please sign in to comment.