Avoid generating invalid statements when deleting from multi-statement lines (#1253)

foo.py

@@ -0,0 +1,3 @@
+X = 1; \
+
+# abc

resources/test/fixtures/isort/leading_prefix.py

@@ -4,3 +4,10 @@ import os
 if True:
     x = 1; import sys
     import os
+
+if True:
+    x = 1; \
+        import os
+
+x = 1; \
+import os

resources/test/fixtures/pyflakes/F811_20.py

@@ -7,7 +7,6 @@ import fu
 
 
 class bar:
-    # STOPSHIP: This errors.
     fu = 1
 
 

resources/test/fixtures/pyflakes/multi_statement_lines.py

@@ -0,0 +1,51 @@
+
+if True:
+    import foo; x = 1
+    import foo;     x = 1
+
+if True:
+    import foo; \
+x = 1
+
+if True:
+    import foo \
+        ; x = 1
+
+
+if True:
+    x = 1; import foo
+
+
+if True:
+    x = 1; \
+         import foo
+
+
+if True:
+    x = 1 \
+        ; import foo
+
+
+if True:
+    x = 1; import foo; x = 1
+    x = 1;     import foo;     x = 1
+
+if True:
+    x = 1; \
+        import foo; \
+    x = 1
+
+if True:
+    x = 1 \
+        ;import foo \
+        ;x = 1
+
+
+# Continuation, but not as the last content in the file.
+x = 1; \
+import foo
+
+# Continuation, followed by end-of-file. (Removing `import foo` would cause a syntax
+# error.)
+x = 1; \
+import foo

src/ast/helpers.rs

@@ -367,6 +367,39 @@ pub fn identifier_range(stmt: &Stmt, locator: &SourceCodeLocator) -> Range {
     Range::from_located(stmt)
 }
 
+/// Return `true` if a `Stmt` appears to be part of a multi-statement line, with
+/// other statements preceding it.
+pub fn preceded_by_continuation(stmt: &Stmt, locator: &SourceCodeLocator) -> bool {
+    // Does the previous line end in a continuation? This will have a specific
+    // false-positive, which is that if the previous line ends in a comment, it
+    // will be treated as a continuation. So we should only use this information to
+    // make conservative choices.
+    // TODO(charlie): Come up with a more robust strategy.
+    if stmt.location.row() > 1 {
+        let range = Range {
+            location: Location::new(stmt.location.row() - 1, 0),
+            end_location: Location::new(stmt.location.row(), 0),
+        };
+        let line = locator.slice_source_code_range(&range);
+        if line.trim().ends_with('\\') {
+            return true;
+        }
+    }
+    false
+}
+
+/// Return `true` if a `Stmt` appears to be part of a multi-statement line, with
+/// other statements preceding it.
+pub fn preceded_by_multi_statement_line(stmt: &Stmt, locator: &SourceCodeLocator) -> bool {
+    match_leading_content(stmt, locator) || preceded_by_continuation(stmt, locator)
+}
+
+/// Return `true` if a `Stmt` appears to be part of a multi-statement line, with
+/// other statements following it.
+pub fn followed_by_multi_statement_line(stmt: &Stmt, locator: &SourceCodeLocator) -> bool {
+    match_trailing_content(stmt, locator)
+}
+
 #[cfg(test)]
 mod tests {
     use anyhow::Result;
@@ -374,7 +407,7 @@ mod tests {
     use rustpython_ast::Location;
     use rustpython_parser::parser;
 
-    use crate::ast::helpers::{identifier_range, match_module_member};
+    use crate::ast::helpers::{identifier_range, match_module_member, match_trailing_content};
     use crate::ast::types::Range;
     use crate::source_code_locator::SourceCodeLocator;
 
@@ -521,6 +554,45 @@ mod tests {
         Ok(())
     }
 
+    #[test]
+    fn trailing_content() -> Result<()> {
+        let contents = "x = 1";
+        let program = parser::parse_program(contents, "<filename>")?;
+        let stmt = program.first().unwrap();
+        let locator = SourceCodeLocator::new(contents);
+        assert!(!match_trailing_content(stmt, &locator));
+
+        let contents = "x = 1; y = 2";
+        let program = parser::parse_program(contents, "<filename>")?;
+        let stmt = program.first().unwrap();
+        let locator = SourceCodeLocator::new(contents);
+        assert!(match_trailing_content(stmt, &locator));
+
+        let contents = "x = 1  ";
+        let program = parser::parse_program(contents, "<filename>")?;
+        let stmt = program.first().unwrap();
+        let locator = SourceCodeLocator::new(contents);
+        assert!(!match_trailing_content(stmt, &locator));
+
+        let contents = "x = 1  # Comment";
+        let program = parser::parse_program(contents, "<filename>")?;
+        let stmt = program.first().unwrap();
+        let locator = SourceCodeLocator::new(contents);
+        assert!(!match_trailing_content(stmt, &locator));
+
+        let contents = r#"
+x = 1
+y = 2
+"#
+        .trim();
+        let program = parser::parse_program(contents, "<filename>")?;
+        let stmt = program.first().unwrap();
+        let locator = SourceCodeLocator::new(contents);
+        assert!(!match_trailing_content(stmt, &locator));
+
+        Ok(())
+    }
+
     #[test]
     fn extract_identifier_range() -> Result<()> {
         let contents = "def f(): pass".trim();

src/autofix/helpers.rs

@@ -2,7 +2,11 @@ use anyhow::{bail, Result};
 use itertools::Itertools;
 use rustpython_parser::ast::{ExcepthandlerKind, Location, Stmt, StmtKind};
 
+use crate::ast::helpers;
+use crate::ast::helpers::to_absolute;
+use crate::ast::whitespace::LinesWithTrailingNewline;
 use crate::autofix::Fix;
+use crate::source_code_locator::SourceCodeLocator;
 
 /// Determine if a body contains only a single statement, taking into account
 /// deleted.
@@ -66,7 +70,87 @@ fn is_lone_child(child: &Stmt, parent: &Stmt, deleted: &[&Stmt]) -> Result<bool>
     }
 }
 
-pub fn remove_stmt(stmt: &Stmt, parent: Option<&Stmt>, deleted: &[&Stmt]) -> Result<Fix> {
+/// 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: &SourceCodeLocator) -> Option<Location> {
+    let contents = locator.slice_source_code_at(&stmt.end_location.unwrap());
+    for (row, line) in LinesWithTrailingNewline::from(&contents).enumerate() {
+        let trimmed = line.trim();
+        if trimmed.starts_with(';') {
+            let column = line
+                .char_indices()
+                .find_map(|(column, char)| if char == ';' { Some(column) } else { None })
+                .unwrap();
+            return Some(to_absolute(
+                Location::new(row + 1, column),
+                stmt.end_location.unwrap(),
+            ));
+        }
+        if !trimmed.starts_with('\\') {
+            break;
+        }
+    }
+    None
+}
+
+/// Find the next valid break for a `Stmt` after a semicolon.
+fn next_stmt_break(semicolon: Location, locator: &SourceCodeLocator) -> Location {
+    let start_location = Location::new(semicolon.row(), semicolon.column() + 1);
+    let contents = locator.slice_source_code_at(&start_location);
+    for (row, line) in LinesWithTrailingNewline::from(&contents).enumerate() {
+        let trimmed = line.trim();
+        // Skip past any continuations.
+        if trimmed.starts_with('\\') {
+            continue;
+        }
+        return if trimmed.is_empty() {
+            // If the line is empty, then despite the previous statement ending in a
+            // semicolon, we know that it's not a multi-statement line.
+            to_absolute(Location::new(row + 1, 0), start_location)
+        } else {
+            // Otherwise, find the start of the next statement. (Or, anything that isn't
+            // whitespace.)
+            let column = line
+                .char_indices()
+                .find_map(|(column, char)| {
+                    if char.is_whitespace() {
+                        None
+                    } else {
+                        Some(column)
+                    }
+                })
+                .unwrap();
+            to_absolute(Location::new(row + 1, column), start_location)
+        };
+    }
+    Location::new(start_location.row() + 1, 0)
+}
+
+/// Return `true` if a `Stmt` occurs at the end of a file.
+fn is_end_of_file(stmt: &Stmt, locator: &SourceCodeLocator) -> bool {
+    let contents = locator.slice_source_code_at(&stmt.end_location.unwrap());
+    contents.is_empty()
+}
+
+/// Return the `Fix` to use when deleting a `Stmt`.
+///
+/// In some cases, this is as simple as deleting the `Range` of the `Stmt`
+/// itself. However, there are a few exceptions:
+/// - If the `Stmt` is _not_ the terminal statement in a multi-statement line,
+///   we need to delete up to the start of the next statement (and avoid
+///   deleting any content that precedes the statement).
+/// - If the `Stmt` is the terminal statement in a multi-statement line, we need
+///   to avoid deleting any content that precedes the statement.
+/// - If the `Stmt` has no trailing and leading content, then it's convenient to
+///   remove the entire start and end lines.
+/// - If the `Stmt` is the last statement in its parent body, replace it with a
+///   `pass` instead.
+pub fn delete_stmt(
+    stmt: &Stmt,
+    parent: Option<&Stmt>,
+    deleted: &[&Stmt],
+    locator: &SourceCodeLocator,
+) -> Result<Fix> {
     if parent
         .map(|parent| is_lone_child(stmt, parent, deleted))
         .map_or(Ok(None), |v| v.map(Some))?
@@ -80,12 +164,103 @@ pub fn remove_stmt(stmt: &Stmt, parent: Option<&Stmt>, deleted: &[&Stmt]) -> Res
             stmt.end_location.unwrap(),
         ))
     } else {
-        // Otherwise, nuke the entire line.
-        // TODO(charlie): This logic assumes that there are no multi-statement physical
-        // lines.
-        Ok(Fix::deletion(
-            Location::new(stmt.location.row(), 0),
-            Location::new(stmt.end_location.unwrap().row() + 1, 0),
-        ))
+        Ok(if let Some(semicolon) = trailing_semicolon(stmt, locator) {
+            let next = next_stmt_break(semicolon, locator);
+            Fix::deletion(stmt.location, next)
+        } else if helpers::match_leading_content(stmt, locator) {
+            Fix::deletion(stmt.location, stmt.end_location.unwrap())
+        } else if helpers::preceded_by_continuation(stmt, locator) {
+            if is_end_of_file(stmt, locator) && stmt.location.column() == 0 {
+                // Special-case: a file can't end in a continuation.
+                Fix::replacement("\n".to_string(), stmt.location, stmt.end_location.unwrap())
+            } else {
+                Fix::deletion(stmt.location, stmt.end_location.unwrap())
+            }
+        } else {
+            Fix::deletion(
+                Location::new(stmt.location.row(), 0),
+                Location::new(stmt.end_location.unwrap().row() + 1, 0),
+            )
+        })
+    }
+}
+
+#[cfg(test)]
+mod tests {
+    use anyhow::Result;
+    use rustpython_ast::Location;
+    use rustpython_parser::parser;
+
+    use crate::autofix::helpers::{next_stmt_break, trailing_semicolon};
+    use crate::source_code_locator::SourceCodeLocator;
+
+    #[test]
+    fn find_semicolon() -> Result<()> {
+        let contents = "x = 1";
+        let program = parser::parse_program(contents, "<filename>")?;
+        let stmt = program.first().unwrap();
+        let locator = SourceCodeLocator::new(contents);
+        assert_eq!(trailing_semicolon(stmt, &locator), None);
+
+        let contents = "x = 1; y = 1";
+        let program = parser::parse_program(contents, "<filename>")?;
+        let stmt = program.first().unwrap();
+        let locator = SourceCodeLocator::new(contents);
+        assert_eq!(
+            trailing_semicolon(stmt, &locator),
+            Some(Location::new(1, 5))
+        );
+
+        let contents = "x = 1 ; y = 1";
+        let program = parser::parse_program(contents, "<filename>")?;
+        let stmt = program.first().unwrap();
+        let locator = SourceCodeLocator::new(contents);
+        assert_eq!(
+            trailing_semicolon(stmt, &locator),
+            Some(Location::new(1, 6))
+        );
+
+        let contents = r#"
+x = 1 \
+  ; y = 1
+"#
+        .trim();
+        let program = parser::parse_program(contents, "<filename>")?;
+        let stmt = program.first().unwrap();
+        let locator = SourceCodeLocator::new(contents);
+        assert_eq!(
+            trailing_semicolon(stmt, &locator),
+            Some(Location::new(2, 2))
+        );
+
+        Ok(())
+    }
+
+    #[test]
+    fn find_next_stmt_break() {
+        let contents = "x = 1; y = 1";
+        let locator = SourceCodeLocator::new(contents);
+        assert_eq!(
+            next_stmt_break(Location::new(1, 4), &locator),
+            Location::new(1, 5)
+        );
+
+        let contents = "x = 1 ; y = 1";
+        let locator = SourceCodeLocator::new(contents);
+        assert_eq!(
+            next_stmt_break(Location::new(1, 5), &locator),
+            Location::new(1, 6)
+        );
+
+        let contents = r#"
+x = 1 \
+  ; y = 1
+"#
+        .trim();
+        let locator = SourceCodeLocator::new(contents);
+        assert_eq!(
+            next_stmt_break(Location::new(2, 2), &locator),
+            Location::new(2, 4)
+        );
     }
 }

src/check_ast.rs

@@ -3425,11 +3425,11 @@ impl<'a> Checker<'a> {
                         let deleted: Vec<&Stmt> =
                             self.deletions.iter().map(|node| node.0).collect();
                         match pyflakes::fixes::remove_unused_imports(
-                            self.locator,
                             &unused_imports,
                             child,
                             parent,
                             &deleted,
+                            self.locator,
                         ) {
                             Ok(fix) => {
                                 if fix.content.is_empty() || fix.content == "pass" {

src/flake8_print/plugins/print_call.rs

@@ -23,7 +23,12 @@ pub fn print_call(checker: &mut Checker, expr: &Expr, func: &Expr) {
         let defined_in = checker.current_grandparent();
         if matches!(defined_by.0.node, StmtKind::Expr { .. }) {
             let deleted: Vec<&Stmt> = checker.deletions.iter().map(|node| node.0).collect();
-            match helpers::remove_stmt(defined_by.0, defined_in.map(|node| node.0), &deleted) {
+            match helpers::delete_stmt(
+                defined_by.0,
+                defined_in.map(|node| node.0),
+                &deleted,
+                checker.locator,
+            ) {
                 Ok(fix) => {
                     if fix.content.is_empty() || fix.content == "pass" {
                         checker.deletions.insert(defined_by.clone());

src/isort/plugins.rs

@@ -1,7 +1,9 @@
 use rustpython_ast::{Location, Stmt};
 use textwrap::{dedent, indent};
 
-use crate::ast::helpers::{count_trailing_lines, match_leading_content, match_trailing_content};
+use crate::ast::helpers::{
+    count_trailing_lines, followed_by_multi_statement_line, preceded_by_multi_statement_line,
+};
 use crate::ast::types::Range;
 use crate::ast::whitespace::leading_space;
 use crate::autofix::Fix;
@@ -39,6 +41,14 @@ pub fn check_imports(
 
     let range = extract_range(&block.imports);
 
+    // Special-cases: there's leading or trailing content in the import block. These
+    // are too hard to get right, and relatively rare, so flag but don't fix.
+    if preceded_by_multi_statement_line(block.imports.first().unwrap(), locator)
+        || followed_by_multi_statement_line(block.imports.last().unwrap(), locator)
+    {
+        return Some(Check::new(CheckKind::UnsortedImports, range));
+    }
+
     // Extract comments. Take care to grab any inline comments from the last line.
     let comments = comments::collect_comments(
         &Range {
@@ -48,9 +58,6 @@ pub fn check_imports(
         locator,
     );
 
-    // Special-cases: there's leading or trailing content in the import block.
-    let has_leading_content = match_leading_content(block.imports.first().unwrap(), locator);
-    let has_trailing_content = match_trailing_content(block.imports.last().unwrap(), locator);
     let num_trailing_lines = if block.trailer.is_none() {
         0
     } else {
@@ -70,46 +77,23 @@ pub fn check_imports(
         settings.isort.force_wrap_aliases,
     );
 
-    if has_leading_content || has_trailing_content {
+    // Expand the span the entire range, including leading and trailing space.
+    let range = Range {
+        location: Location::new(range.location.row(), 0),
+        end_location: Location::new(range.end_location.row() + 1 + num_trailing_lines, 0),
+    };
+    let actual = dedent(&locator.slice_source_code_range(&range));
+    if actual == expected {
+        None
+    } else {
         let mut check = Check::new(CheckKind::UnsortedImports, range);
         if autofix && settings.fixable.contains(check.kind.code()) {
-            let mut content = String::new();
-            if has_leading_content {
-                content.push('\n');
-            }
-            content.push_str(&indent(&expected, indentation));
             check.amend(Fix::replacement(
-                content,
-                // Preserve leading prefix (but put the imports on a new line).
-                if has_leading_content {
-                    range.location
-                } else {
-                    Location::new(range.location.row(), 0)
-                },
-                // TODO(charlie): Preserve trailing suffixes. Right now, we strip them.
-                Location::new(range.end_location.row() + 1 + num_trailing_lines, 0),
+                indent(&expected, indentation),
+                range.location,
+                range.end_location,
             ));
         }
         Some(check)
-    } else {
-        // Expand the span the entire range, including leading and trailing space.
-        let range = Range {
-            location: Location::new(range.location.row(), 0),
-            end_location: Location::new(range.end_location.row() + 1 + num_trailing_lines, 0),
-        };
-        let actual = dedent(&locator.slice_source_code_range(&range));
-        if actual == expected {
-            None
-        } else {
-            let mut check = Check::new(CheckKind::UnsortedImports, range);
-            if autofix && settings.fixable.contains(check.kind.code()) {
-                check.amend(Fix::replacement(
-                    indent(&expected, indentation),
-                    range.location,
-                    range.end_location,
-                ));
-            }
-            Some(check)
-        }
     }
 }

src/isort/snapshots/ruff__isort__tests__leading_prefix.py.snap

@@ -9,14 +9,7 @@ expression: checks
   end_location:
     row: 2
     column: 9
-  fix:
-    content: "\nimport os\nimport sys\n\n"
-    location:
-      row: 1
-      column: 7
-    end_location:
-      row: 4
-      column: 0
+  fix: ~
 - kind: UnsortedImports
   location:
     row: 5
@@ -24,12 +17,21 @@ expression: checks
   end_location:
     row: 6
     column: 13
-  fix:
-    content: "\n    import os\n    import sys\n"
-    location:
-      row: 5
-      column: 11
-    end_location:
-      row: 7
-      column: 0
+  fix: ~
+- kind: UnsortedImports
+  location:
+    row: 10
+    column: 8
+  end_location:
+    row: 10
+    column: 17
+  fix: ~
+- kind: UnsortedImports
+  location:
+    row: 13
+    column: 0
+  end_location:
+    row: 13
+    column: 9
+  fix: ~
 

src/isort/snapshots/ruff__isort__tests__trailing_suffix.py.snap

@@ -9,14 +9,7 @@ expression: checks
   end_location:
     row: 2
     column: 9
-  fix:
-    content: "import os\nimport sys\n\n"
-    location:
-      row: 1
-      column: 0
-    end_location:
-      row: 4
-      column: 0
+  fix: ~
 - kind: UnsortedImports
   location:
     row: 5
@@ -24,12 +17,5 @@ expression: checks
   end_location:
     row: 6
     column: 13
-  fix:
-    content: "    import os\n    import sys\n"
-    location:
-      row: 5
-      column: 0
-    end_location:
-      row: 7
-      column: 0
+  fix: ~
 

src/pyflakes/fixes.rs

@@ -14,11 +14,11 @@ use crate::source_code_locator::SourceCodeLocator;
 
 /// Generate a `Fix` to remove any unused imports from an `import` statement.
 pub fn remove_unused_imports(
-    locator: &SourceCodeLocator,
     unused_imports: &Vec<(&String, &Range)>,
     stmt: &Stmt,
     parent: Option<&Stmt>,
     deleted: &[&Stmt],
+    locator: &SourceCodeLocator,
 ) -> Result<Fix> {
     let module_text = locator.slice_source_code_range(&Range::from_located(stmt));
     let mut tree = match_module(&module_text)?;
@@ -65,7 +65,7 @@ pub fn remove_unused_imports(
     }
 
     if aliases.is_empty() {
-        helpers::remove_stmt(stmt, parent, deleted)
+        helpers::delete_stmt(stmt, parent, deleted, locator)
     } else {
         let mut state = CodegenState::default();
         tree.codegen(&mut state);
@@ -80,9 +80,9 @@ pub fn remove_unused_imports(
 
 /// Generate a `Fix` to remove unused keys from format dict.
 pub fn remove_unused_format_arguments_from_dict(
-    locator: &SourceCodeLocator,
     unused_arguments: &[&str],
     stmt: &Expr,
+    locator: &SourceCodeLocator,
 ) -> Result<Fix> {
     let module_text = locator.slice_source_code_range(&Range::from_located(stmt));
     let mut tree = match_module(&module_text)?;
@@ -126,9 +126,9 @@ pub fn remove_unused_format_arguments_from_dict(
 
 /// Generate a `Fix` to remove unused keyword arguments from format call.
 pub fn remove_unused_keyword_arguments_from_format_call(
-    locator: &SourceCodeLocator,
     unused_arguments: &[&str],
     location: Range,
+    locator: &SourceCodeLocator,
 ) -> Result<Fix> {
     let module_text = locator.slice_source_code_range(&location);
     let mut tree = match_module(&module_text)?;

src/pyflakes/mod.rs

@@ -154,6 +154,18 @@ mod tests {
         Ok(())
     }
 
+    #[test]
+    fn multi_statement_lines() -> Result<()> {
+        let mut checks = test_path(
+            Path::new("./resources/test/fixtures/pyflakes/multi_statement_lines.py"),
+            &settings::Settings::for_rule(CheckCode::F401),
+            true,
+        )?;
+        checks.sort_by_key(|check| check.location);
+        insta::assert_yaml_snapshot!(checks);
+        Ok(())
+    }
+
     /// A re-implementation of the Pyflakes test runner.
     /// Note that all tests marked with `#[ignore]` should be considered TODOs.
     fn flakes(contents: &str, expected: &[CheckCode]) -> Result<()> {

src/pyflakes/plugins/strings.rs

@@ -115,7 +115,7 @@ pub(crate) fn percent_format_extra_named_arguments(
         location,
     );
     if checker.patch(check.kind.code()) {
-        if let Ok(fix) = remove_unused_format_arguments_from_dict(checker.locator, &missing, right)
+        if let Ok(fix) = remove_unused_format_arguments_from_dict(&missing, right, checker.locator)
         {
             check.amend(fix);
         }
@@ -273,7 +273,7 @@ pub(crate) fn string_dot_format_extra_named_arguments(
     );
     if checker.patch(check.kind.code()) {
         if let Ok(fix) =
-            remove_unused_keyword_arguments_from_format_call(checker.locator, &missing, location)
+            remove_unused_keyword_arguments_from_format_call(&missing, location, checker.locator)
         {
             check.amend(fix);
         }

src/pyflakes/snapshots/ruff__pyflakes__tests__multi_statement_lines.snap

@@ -0,0 +1,239 @@
+---
+source: src/pyflakes/mod.rs
+expression: checks
+---
+- kind:
+    UnusedImport:
+      - foo
+      - false
+  location:
+    row: 3
+    column: 11
+  end_location:
+    row: 3
+    column: 14
+  fix:
+    content: ""
+    location:
+      row: 3
+      column: 4
+    end_location:
+      row: 3
+      column: 16
+- kind:
+    UnusedImport:
+      - foo
+      - false
+  location:
+    row: 4
+    column: 11
+  end_location:
+    row: 4
+    column: 14
+  fix:
+    content: ""
+    location:
+      row: 4
+      column: 4
+    end_location:
+      row: 4
+      column: 20
+- kind:
+    UnusedImport:
+      - foo
+      - false
+  location:
+    row: 7
+    column: 11
+  end_location:
+    row: 7
+    column: 14
+  fix:
+    content: ""
+    location:
+      row: 7
+      column: 4
+    end_location:
+      row: 8
+      column: 0
+- kind:
+    UnusedImport:
+      - foo
+      - false
+  location:
+    row: 11
+    column: 11
+  end_location:
+    row: 11
+    column: 14
+  fix:
+    content: ""
+    location:
+      row: 11
+      column: 4
+    end_location:
+      row: 12
+      column: 10
+- kind:
+    UnusedImport:
+      - foo
+      - false
+  location:
+    row: 16
+    column: 18
+  end_location:
+    row: 16
+    column: 21
+  fix:
+    content: ""
+    location:
+      row: 16
+      column: 11
+    end_location:
+      row: 16
+      column: 21
+- kind:
+    UnusedImport:
+      - foo
+      - false
+  location:
+    row: 21
+    column: 16
+  end_location:
+    row: 21
+    column: 19
+  fix:
+    content: ""
+    location:
+      row: 21
+      column: 9
+    end_location:
+      row: 21
+      column: 19
+- kind:
+    UnusedImport:
+      - foo
+      - false
+  location:
+    row: 26
+    column: 17
+  end_location:
+    row: 26
+    column: 20
+  fix:
+    content: ""
+    location:
+      row: 26
+      column: 10
+    end_location:
+      row: 26
+      column: 20
+- kind:
+    UnusedImport:
+      - foo
+      - false
+  location:
+    row: 30
+    column: 18
+  end_location:
+    row: 30
+    column: 21
+  fix:
+    content: ""
+    location:
+      row: 30
+      column: 11
+    end_location:
+      row: 30
+      column: 23
+- kind:
+    UnusedImport:
+      - foo
+      - false
+  location:
+    row: 31
+    column: 22
+  end_location:
+    row: 31
+    column: 25
+  fix:
+    content: ""
+    location:
+      row: 31
+      column: 15
+    end_location:
+      row: 31
+      column: 31
+- kind:
+    UnusedImport:
+      - foo
+      - false
+  location:
+    row: 35
+    column: 15
+  end_location:
+    row: 35
+    column: 18
+  fix:
+    content: ""
+    location:
+      row: 35
+      column: 8
+    end_location:
+      row: 36
+      column: 4
+- kind:
+    UnusedImport:
+      - foo
+      - false
+  location:
+    row: 40
+    column: 16
+  end_location:
+    row: 40
+    column: 19
+  fix:
+    content: ""
+    location:
+      row: 40
+      column: 9
+    end_location:
+      row: 41
+      column: 9
+- kind:
+    UnusedImport:
+      - foo
+      - false
+  location:
+    row: 46
+    column: 7
+  end_location:
+    row: 46
+    column: 10
+  fix:
+    content: ""
+    location:
+      row: 46
+      column: 0
+    end_location:
+      row: 46
+      column: 10
+- kind:
+    UnusedImport:
+      - foo
+      - false
+  location:
+    row: 51
+    column: 7
+  end_location:
+    row: 51
+    column: 10
+  fix:
+    content: "\n"
+    location:
+      row: 51
+      column: 0
+    end_location:
+      row: 51
+      column: 10
+

src/pyupgrade/fixes.rs

@@ -175,7 +175,7 @@ pub fn remove_unnecessary_future_import(
     }
 
     if aliases.is_empty() {
-        autofix::helpers::remove_stmt(stmt, parent, deleted)
+        autofix::helpers::delete_stmt(stmt, parent, deleted, locator)
     } else {
         let mut state = CodegenState::default();
         tree.codegen(&mut state);

src/pyupgrade/plugins/useless_metaclass_type.rs

@@ -16,7 +16,12 @@ pub fn useless_metaclass_type(checker: &mut Checker, stmt: &Stmt, value: &Expr,
         let deleted: Vec<&Stmt> = checker.deletions.iter().map(|node| node.0).collect();
         let defined_by = checker.current_parent();
         let defined_in = checker.current_grandparent();
-        match helpers::remove_stmt(defined_by.0, defined_in.map(|node| node.0), &deleted) {
+        match helpers::delete_stmt(
+            defined_by.0,
+            defined_in.map(|node| node.0),
+            &deleted,
+            checker.locator,
+        ) {
             Ok(fix) => {
                 if fix.content.is_empty() || fix.content == "pass" {
                     checker.deletions.insert(defined_by.clone());

src/resolver.rs

@@ -179,7 +179,7 @@ fn is_python_path(path: &Path) -> bool {
 }
 
 /// Return `true` if the `Entry` appears to be that of a Python file.
-fn is_python_entry(entry: &DirEntry) -> bool {
+pub fn is_python_entry(entry: &DirEntry) -> bool {
     is_python_path(entry.path())
         && !entry
             .file_type()

tests/black_compatibility_test.rs

@@ -15,7 +15,7 @@ use ruff::logging::{set_up_logging, LogLevel};
 use strum::IntoEnumIterator;
 use walkdir::WalkDir;
 
-/// Handles blackd process and allows submitting code to it for formatting.
+/// Handles `blackd` process and allows submitting code to it for formatting.
 struct Blackd {
     address: SocketAddr,
     server: process::Child,
@@ -155,19 +155,20 @@ fn test_ruff_black_compatibility() -> Result<()> {
 
     // Ignore some fixtures that currently trigger errors. `E999.py` especially, as
     // that is triggering a syntax error on purpose.
-    let excludes = ["E999.py", "B009_B010.py", "W605_1.py", "leading_prefix.py"];
+    let excludes = ["E999.py"];
 
     let paths: Vec<walkdir::DirEntry> = WalkDir::new(fixtures_dir)
         .into_iter()
         .filter(|entry| {
             entry.as_ref().map_or(true, |entry| {
-                let file_name = entry.path().file_name().unwrap().to_string_lossy();
-
-                (file_name.ends_with(".py") || file_name.ends_with(".pyi"))
-                    && !excludes.contains(&file_name.to_string().as_str())
+                entry
+                    .path()
+                    .extension()
+                    .map_or(false, |ext| ext == "py" || ext == "pyi")
+                    && !excludes.contains(&entry.path().file_name().unwrap().to_str().unwrap())
             })
         })
-        .filter_map(std::result::Result::ok)
+        .filter_map(Result::ok)
         .collect();
 
     let codes = CheckCategory::iter()