[pycodestyle] Add an autofix for E402

crates/ruff_linter/resources/test/fixtures/pycodestyle/E402_comments.py

@@ -0,0 +1,25 @@
+import os
+
+print("something")
+
+import last
+
+import late  # comment-late
+
+from late_paren1 import (  # comment-late_paren1
+    value
+)
+
+from late_paren2 import (
+    value  # comment-late_paren2
+)
+
+from late_paren3 import (
+    value
+)  # comment-late_paren3
+
+from late_paren4 import (
+    value1,
+    value2,  # comment-late_paren4
+    value3,
+)

crates/ruff_linter/resources/test/fixtures/pycodestyle/E402_docstring.py

@@ -0,0 +1,5 @@
+"""module docstring"""
+
+print("something")
+
+import os

crates/ruff_linter/resources/test/fixtures/pycodestyle/E402_future.py

@@ -0,0 +1,5 @@
+from __future__ import annotations
+
+print("something")
+
+import os

crates/ruff_linter/resources/test/fixtures/pycodestyle/E402_shebang.py

@@ -0,0 +1,5 @@
+#!/usr/bin/python3
+
+print("something")
+
+import os

crates/ruff_linter/resources/test/fixtures/pycodestyle/E402_shebang_docstring_and_future.py

@@ -0,0 +1,9 @@
+#!/usr/bin/python3
+
+"""docstring"""
+
+from __future__ import annotations
+
+print("something")
+
+import os

crates/ruff_linter/src/importer/mod.rs

@@ -17,8 +17,9 @@ use ruff_python_parser::Parsed;
 use ruff_python_semantic::{
     ImportedName, MemberNameImport, ModuleNameImport, NameImport, SemanticModel,
 };
-use ruff_python_trivia::textwrap::indent;
-use ruff_text_size::{Ranged, TextSize};
+use ruff_python_trivia::{PythonWhitespace, textwrap::indent};
+use ruff_source_file::LineRanges;
+use ruff_text_size::{Ranged, TextRange, TextSize};
 
 use crate::cst::matchers::{match_aliases, match_import_from, match_statement};
 use crate::fix;
@@ -77,16 +78,23 @@ impl<'a> Importer<'a> {
             // Insert after the last top-level import.
             Insertion::end_of_statement(stmt, self.source, self.stylist).into_edit(&required_import)
         } else {
-            // Check if there are any future imports that we need to respect
-            if let Some(last_future_import) = self.find_last_future_import() {
-                // Insert after the last future import
-                Insertion::end_of_statement(last_future_import, self.source, self.stylist)
-                    .into_edit(&required_import)
-            } else {
-                // Insert at the start of the file.
-                Insertion::start_of_file(self.python_ast, self.source, self.stylist, None)
-                    .into_edit(&required_import)
-            }
+            self.add_at_start(&required_import)
+        }
+    }
+
+    /// Add an existing import statement to the start of the file.
+    pub(crate) fn add_import_at_start(&self, import: &Stmt) -> Edit {
+        let range = TextRange::new(import.start(), self.source.line_end(import.end()));
+        self.add_at_start(self.source[range].trim_whitespace())
+    }
+
+    fn add_at_start(&self, text: &str) -> Edit {
+        if let Some(last_future_import) = self.find_last_future_import() {
+            Insertion::end_of_statement(last_future_import, self.source, self.stylist)
+                .into_edit(text)
+        } else {
+            Insertion::start_of_file(self.python_ast, self.source, self.stylist, None)
+                .into_edit(text)
         }
     }
 

crates/ruff_linter/src/preview.rs

@@ -335,6 +335,11 @@ pub(crate) const fn is_incorrect_dict_iterator_comprehension_enabled(
     settings.preview.is_enabled()
 }
 
+// https://github.com/astral-sh/ruff/pull/22212
+pub(crate) const fn is_e402_fix_enabled(settings: &LinterSettings) -> bool {
+    settings.preview.is_enabled()
+}
+
 // https://github.com/astral-sh/ruff/pull/23260
 pub(crate) const fn is_up006_future_annotations_fix_enabled(settings: &LinterSettings) -> bool {
     settings.preview.is_enabled()

crates/ruff_linter/src/rules/pycodestyle/mod.rs

@@ -76,6 +76,22 @@ mod tests {
     }
 
     #[test_case(Rule::LineTooLong, Path::new("E501_5.py"))]
+    #[test_case(Rule::ModuleImportNotAtTopOfFile, Path::new("E40.py"))]
+    #[test_case(Rule::ModuleImportNotAtTopOfFile, Path::new("E402_0.py"))]
+    #[test_case(Rule::ModuleImportNotAtTopOfFile, Path::new("E402_1.py"))]
+    #[test_case(Rule::ModuleImportNotAtTopOfFile, Path::new("E402_2.py"))]
+    #[test_case(Rule::ModuleImportNotAtTopOfFile, Path::new("E402_3.py"))]
+    #[test_case(Rule::ModuleImportNotAtTopOfFile, Path::new("E402_4.py"))]
+    #[test_case(Rule::ModuleImportNotAtTopOfFile, Path::new("E402_5.py"))]
+    #[test_case(Rule::ModuleImportNotAtTopOfFile, Path::new("E402_docstring.py"))]
+    #[test_case(Rule::ModuleImportNotAtTopOfFile, Path::new("E402_comments.py"))]
+    #[test_case(Rule::ModuleImportNotAtTopOfFile, Path::new("E402_future.py"))]
+    #[test_case(Rule::ModuleImportNotAtTopOfFile, Path::new("E402_shebang.py"))]
+    #[test_case(
+        Rule::ModuleImportNotAtTopOfFile,
+        Path::new("E402_shebang_docstring_and_future.py")
+    )]
+    #[test_case(Rule::ModuleImportNotAtTopOfFile, Path::new("E402.ipynb"))]
     #[test_case(Rule::RedundantBackslash, Path::new("E502.py"))]
     #[test_case(Rule::TooManyNewlinesAtEndOfFile, Path::new("W391_0.py"))]
     #[test_case(Rule::TooManyNewlinesAtEndOfFile, Path::new("W391_1.py"))]

crates/ruff_linter/src/rules/pycodestyle/rules/module_import_not_at_top_of_file.rs

@@ -1,9 +1,11 @@
 use ruff_macros::{ViolationMetadata, derive_message_formats};
 use ruff_python_ast::{PySourceType, Stmt};
-use ruff_text_size::Ranged;
+use ruff_source_file::LineRanges;
+use ruff_text_size::{Ranged, TextRange};
 
-use crate::Violation;
 use crate::checkers::ast::Checker;
+use crate::preview::is_e402_fix_enabled;
+use crate::{Edit, Fix, FixAvailability, Violation};
 
 /// ## What it does
 /// Checks for imports that are not at the top of the file.
@@ -38,6 +40,12 @@ use crate::checkers::ast::Checker;
 /// ## Notebook behavior
 /// For Jupyter notebooks, this rule checks for imports that are not at the top of a *cell*.
 ///
+/// ## Fix safety
+/// This rule's fix is marked as unsafe as imports moved to the top of the file
+/// are placed above existing imports, in reverse order than they were in the
+/// file. Re-ordering imports is unsafe as it can change the execution order of
+/// the imported code.
+///
 /// [PEP 8]: https://peps.python.org/pep-0008/#imports
 #[derive(ViolationMetadata)]
 #[violation_metadata(stable_since = "v0.0.28")]
@@ -46,6 +54,8 @@ pub(crate) struct ModuleImportNotAtTopOfFile {
 }
 
 impl Violation for ModuleImportNotAtTopOfFile {
+    const FIX_AVAILABILITY: FixAvailability = FixAvailability::Sometimes;
+
     #[derive_message_formats]
     fn message(&self) -> String {
         if self.source_type.is_ipynb() {
@@ -54,16 +64,54 @@ impl Violation for ModuleImportNotAtTopOfFile {
             "Module level import not at top of file".to_string()
         }
     }
+
+    fn fix_title(&self) -> Option<String> {
+        if self.source_type.is_ipynb() {
+            Some("Move module level imports to top of cell".to_string())
+        } else {
+            Some("Move module level imports to top of file".to_string())
+        }
+    }
 }
 
 /// E402
 pub(crate) fn module_import_not_at_top_of_file(checker: &Checker, stmt: &Stmt) {
     if checker.semantic().seen_import_boundary() && checker.semantic().at_top_level() {
-        checker.report_diagnostic(
+        let mut diagnostic = checker.report_diagnostic(
             ModuleImportNotAtTopOfFile {
                 source_type: checker.source_type,
             },
             stmt.range(),
         );
+
+        if !is_e402_fix_enabled(checker.settings()) {
+            return;
+        }
+
+        // Support for fixing notebooks is not yet implemented.
+        if checker.cell_offsets().is_some() {
+            return;
+        }
+
+        let indexer = checker.indexer();
+        let locator = checker.locator();
+
+        // 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 indexer.preceded_by_multi_statement_line(stmt, locator.contents())
+            || indexer.followed_by_multi_statement_line(stmt, locator.contents())
+        {
+            return;
+        }
+
+        let edit = checker.importer().add_import_at_start(stmt);
+
+        // Include trailing comments and the newline in the removal.
+        let removal_range = TextRange::new(stmt.start(), locator.full_line_end(stmt.end()));
+
+        diagnostic.set_fix(Fix::unsafe_edits(
+            Edit::range_deletion(removal_range),
+            [edit],
+        ));
     }
 }

crates/ruff_linter/src/rules/pycodestyle/snapshots/ruff_linter__rules__pycodestyle__tests__E402_E40.py.snap

@@ -11,6 +11,7 @@ E402 Module level import not at top of file
 57 | #: E402
 58 | import foo
    |
+help: Move module level imports to top of file
 
 E402 Module level import not at top of file
   --> E40.py:58:1
@@ -22,6 +23,7 @@ E402 Module level import not at top of file
 59 |
 60 | a = 1
    |
+help: Move module level imports to top of file
 
 E402 Module level import not at top of file
   --> E40.py:62:1
@@ -33,6 +35,7 @@ E402 Module level import not at top of file
 63 |
 64 | #: E401
    |
+help: Move module level imports to top of file
 
 E402 Module level import not at top of file
   --> E40.py:65:1
@@ -42,6 +45,7 @@ E402 Module level import not at top of file
    | ^^^^^^^^^^^^^^^^^^^^^^^^^^
 66 | import re as regex, string; x = 1
    |
+help: Move module level imports to top of file
 
 E402 Module level import not at top of file
   --> E40.py:66:1
@@ -53,6 +57,7 @@ E402 Module level import not at top of file
 67 |
 68 | x = 1; import re as regex, string
    |
+help: Move module level imports to top of file
 
 E402 Module level import not at top of file
   --> E40.py:68:8
@@ -62,3 +67,4 @@ E402 Module level import not at top of file
 68 | x = 1; import re as regex, string
    |        ^^^^^^^^^^^^^^^^^^^^^^^^^^
    |
+help: Move module level imports to top of file

crates/ruff_linter/src/rules/pycodestyle/snapshots/ruff_linter__rules__pycodestyle__tests__E402_E402.ipynb.snap

@@ -11,6 +11,7 @@ E402 Module level import not at top of cell
 10 |
 11 | import a
    |
+help: Move module level imports to top of cell
 
 E402 Module level import not at top of cell
   --> E402.ipynb:22:1
@@ -21,6 +22,7 @@ E402 Module level import not at top of cell
    | ^^^^^^^^
 23 | import ok
    |
+help: Move module level imports to top of cell
 
 E402 Module level import not at top of cell
   --> E402.ipynb:30:1
@@ -30,3 +32,4 @@ E402 Module level import not at top of cell
 31 |
 32 | %%time
    |
+help: Move module level imports to top of cell

crates/ruff_linter/src/rules/pycodestyle/snapshots/ruff_linter__rules__pycodestyle__tests__E402_E402_0.py.snap

@@ -9,6 +9,7 @@ E402 Module level import not at top of file
 35 | import h
    | ^^^^^^^^
    |
+help: Move module level imports to top of file
 
 E402 Module level import not at top of file
   --> E402_0.py:45:1
@@ -18,6 +19,7 @@ E402 Module level import not at top of file
 45 | import k; import l
    | ^^^^^^^^
    |
+help: Move module level imports to top of file
 
 E402 Module level import not at top of file
   --> E402_0.py:45:11
@@ -27,3 +29,4 @@ E402 Module level import not at top of file
 45 | import k; import l
    |           ^^^^^^^^
    |
+help: Move module level imports to top of file

crates/ruff_linter/src/rules/pycodestyle/snapshots/ruff_linter__rules__pycodestyle__tests__E402_E402_1.py.snap

@@ -11,6 +11,7 @@ E402 Module level import not at top of file
 6 |
 7 | """Some other docstring."""
   |
+help: Move module level imports to top of file
 
 E402 Module level import not at top of file
  --> E402_1.py:9:1
@@ -20,3 +21,4 @@ E402 Module level import not at top of file
 9 | import c
   | ^^^^^^^^
   |
+help: Move module level imports to top of file