diff --git a/resources/test/fixtures/isort/insert_empty_lines.pyi b/resources/test/fixtures/isort/insert_empty_lines.pyi new file mode 100644 index 0000000000..6103bc3405 --- /dev/null +++ b/resources/test/fixtures/isort/insert_empty_lines.pyi @@ -0,0 +1,41 @@ +import a +import b +x = 1 +import os +import sys +def f(): + pass +if True: + x = 1 + import collections + import typing +class X: pass +y = 1 +import os +import sys +"""Docstring""" + +if True: + import os + + +def f(): + pass + + +if True: + import os +def f(): + pass + +if True: + x = 1 + import collections + import typing + class X: pass + +if True: + x = 1 + import collections + import typing + def f(): pass diff --git a/src/check_imports.rs b/src/check_imports.rs index f6f531b9c0..ef34848797 100644 --- a/src/check_imports.rs +++ b/src/check_imports.rs @@ -1,5 +1,7 @@ //! Lint rules based on import analysis. +use std::path::Path; + use rustpython_parser::ast::Suite; use crate::ast::visitor::Visitor; @@ -33,8 +35,9 @@ pub fn check_imports( directives: &IsortDirectives, settings: &Settings, autofix: bool, + path: &Path, ) -> Vec { - let mut tracker = ImportTracker::new(directives); + let mut tracker = ImportTracker::new(directives, path); for stmt in python_ast { tracker.visit_stmt(stmt); } diff --git a/src/isort/mod.rs b/src/isort/mod.rs index 4bb556a013..e35ca1f26b 100644 --- a/src/isort/mod.rs +++ b/src/isort/mod.rs @@ -559,6 +559,7 @@ mod tests { #[test_case(Path::new("force_wrap_aliases.py"))] #[test_case(Path::new("import_from_after_import.py"))] #[test_case(Path::new("insert_empty_lines.py"))] + #[test_case(Path::new("insert_empty_lines.pyi"))] #[test_case(Path::new("leading_prefix.py"))] #[test_case(Path::new("no_reorder_within_section.py"))] #[test_case(Path::new("no_wrap_star.py"))] diff --git a/src/isort/snapshots/ruff__isort__tests__insert_empty_lines.pyi.snap b/src/isort/snapshots/ruff__isort__tests__insert_empty_lines.pyi.snap new file mode 100644 index 0000000000..c5d663da4a --- /dev/null +++ b/src/isort/snapshots/ruff__isort__tests__insert_empty_lines.pyi.snap @@ -0,0 +1,65 @@ +--- +source: src/isort/mod.rs +expression: checks +--- +- kind: UnsortedImports + location: + row: 1 + column: 0 + end_location: + row: 3 + column: 0 + fix: + content: "import a\nimport b\n\n" + location: + row: 1 + column: 0 + end_location: + row: 3 + column: 0 +- kind: UnsortedImports + location: + row: 4 + column: 0 + end_location: + row: 6 + column: 0 + fix: + content: "import os\nimport sys\n\n" + location: + row: 4 + column: 0 + end_location: + row: 6 + column: 0 +- kind: UnsortedImports + location: + row: 14 + column: 0 + end_location: + row: 16 + column: 0 + fix: + content: "import os\nimport sys\n\n" + location: + row: 14 + column: 0 + end_location: + row: 16 + column: 0 +- kind: UnsortedImports + location: + row: 33 + column: 0 + end_location: + row: 35 + column: 0 + fix: + content: " import collections\n import typing\n\n" + location: + row: 33 + column: 0 + end_location: + row: 35 + column: 0 + diff --git a/src/isort/track.rs b/src/isort/track.rs index 423c0d7f25..506314aa82 100644 --- a/src/isort/track.rs +++ b/src/isort/track.rs @@ -1,3 +1,5 @@ +use std::path::Path; + use rustpython_ast::{ Alias, Arg, Arguments, Boolop, Cmpop, Comprehension, Constant, Excepthandler, ExcepthandlerKind, Expr, ExprContext, Keyword, MatchCase, Operator, Pattern, Stmt, StmtKind, @@ -20,16 +22,18 @@ pub struct Block<'a> { } pub struct ImportTracker<'a> { - blocks: Vec>, directives: &'a IsortDirectives, + pyi: bool, + blocks: Vec>, split_index: usize, nested: bool, } impl<'a> ImportTracker<'a> { - pub fn new(directives: &'a IsortDirectives) -> Self { + pub fn new(directives: &'a IsortDirectives, path: &'a Path) -> Self { Self { directives, + pyi: path.extension().map_or(false, |ext| ext == "pyi"), blocks: vec![Block::default()], split_index: 0, nested: false, @@ -41,6 +45,34 @@ impl<'a> ImportTracker<'a> { self.blocks[index].imports.push(stmt); } + fn trailer_for(&self, stmt: &'a Stmt) -> Option { + if self.pyi { + // Black treats interface files differently, limiting to one newline + // (`Trailing::Sibling`), and avoiding inserting any newlines in nested function + // blocks. + if self.nested + && matches!( + stmt.node, + StmtKind::FunctionDef { .. } | StmtKind::AsyncFunctionDef { .. } + ) + { + None + } else { + Some(Trailer::Sibling) + } + } else if self.nested { + Some(Trailer::Sibling) + } else { + Some(match &stmt.node { + StmtKind::FunctionDef { .. } | StmtKind::AsyncFunctionDef { .. } => { + Trailer::FunctionDef + } + StmtKind::ClassDef { .. } => Trailer::ClassDef, + _ => Trailer::Sibling, + }) + } + } + fn finalize(&mut self, trailer: Option) { let index = self.blocks.len() - 1; if !self.blocks[index].imports.is_empty() { @@ -62,17 +94,7 @@ where // Track manual splits. while self.split_index < self.directives.splits.len() { if stmt.location.row() >= self.directives.splits[self.split_index] { - self.finalize(Some(if self.nested { - Trailer::Sibling - } else { - match &stmt.node { - StmtKind::FunctionDef { .. } | StmtKind::AsyncFunctionDef { .. } => { - Trailer::FunctionDef - } - StmtKind::ClassDef { .. } => Trailer::ClassDef, - _ => Trailer::Sibling, - } - })); + self.finalize(self.trailer_for(stmt)); self.split_index += 1; } else { break; @@ -87,17 +109,7 @@ where { self.track_import(stmt); } else { - self.finalize(Some(if self.nested { - Trailer::Sibling - } else { - match &stmt.node { - StmtKind::FunctionDef { .. } | StmtKind::AsyncFunctionDef { .. } => { - Trailer::FunctionDef - } - StmtKind::ClassDef { .. } => Trailer::ClassDef, - _ => Trailer::Sibling, - } - })); + self.finalize(self.trailer_for(stmt)); } // Track scope. diff --git a/src/linter.rs b/src/linter.rs index 2c4527e543..56587be811 100644 --- a/src/linter.rs +++ b/src/linter.rs @@ -92,6 +92,7 @@ pub(crate) fn check_path( &directives.isort, settings, autofix, + path, )); } }