diff --git a/crates/ruff_linter/resources/test/fixtures/isort/from_first.py b/crates/ruff_linter/resources/test/fixtures/isort/from_first.py new file mode 100644 index 00000000000000..a15a573d282157 --- /dev/null +++ b/crates/ruff_linter/resources/test/fixtures/isort/from_first.py @@ -0,0 +1,5 @@ +from __future__ import blah + +from os import path + +import os diff --git a/crates/ruff_linter/src/rules/isort/mod.rs b/crates/ruff_linter/src/rules/isort/mod.rs index 10a3f7b1d974a7..5fce5a86acfc8a 100644 --- a/crates/ruff_linter/src/rules/isort/mod.rs +++ b/crates/ruff_linter/src/rules/isort/mod.rs @@ -145,6 +145,15 @@ fn format_import_block( target_version: PythonVersion, settings: &Settings, ) -> String { + #[derive(Debug, Copy, Clone, PartialEq, Eq)] + enum LineInsertion { + /// A blank line should be inserted as soon as the next import is + /// of a different type (i.e., direct vs. `from`). + Necessary, + /// A blank line has already been inserted. + Inserted, + } + // Categorize by type (e.g., first-party vs. third-party). let mut block_by_type = categorize_imports( block, @@ -182,13 +191,24 @@ fn format_import_block( pending_lines_before = false; } - let mut lines_inserted = false; - let mut has_direct_import = false; + let mut line_insertion = None; let mut is_first_statement = true; let lines_between_types = settings.lines_between_types; for import in imports { match import { Import((alias, comments)) => { + // Add a blank lines between direct and from imports. + if settings.from_first + && lines_between_types > 0 + && line_insertion == Some(LineInsertion::Necessary) + { + for _ in 0..lines_between_types { + output.push_str(&stylist.line_ending()); + } + + line_insertion = Some(LineInsertion::Inserted); + } + output.push_str(&format::format_import( &alias, &comments, @@ -196,17 +216,22 @@ fn format_import_block( stylist, )); - has_direct_import = true; + if !settings.from_first { + line_insertion = Some(LineInsertion::Necessary); + } } ImportFrom((import_from, comments, trailing_comma, aliases)) => { - // Add a blank lines between direct and from imports - if lines_between_types > 0 && has_direct_import && !lines_inserted { + // Add a blank lines between direct and from imports. + if !settings.from_first + && lines_between_types > 0 + && line_insertion == Some(LineInsertion::Necessary) + { for _ in 0..lines_between_types { output.push_str(&stylist.line_ending()); } - lines_inserted = true; + line_insertion = Some(LineInsertion::Inserted); } output.push_str(&format::format_import_from( @@ -221,6 +246,10 @@ fn format_import_block( settings.split_on_trailing_comma && matches!(trailing_comma, TrailingComma::Present), )); + + if settings.from_first { + line_insertion = Some(LineInsertion::Necessary); + } } } is_first_statement = false; @@ -819,6 +848,25 @@ mod tests { Ok(()) } + #[test_case(Path::new("from_first.py"))] + fn from_first(path: &Path) -> Result<()> { + let snapshot = format!("from_first_{}", path.to_string_lossy()); + let diagnostics = test_path( + Path::new("isort").join(path).as_path(), + &LinterSettings { + isort: super::settings::Settings { + from_first: true, + lines_between_types: 1, + ..super::settings::Settings::default() + }, + src: vec![test_resource_path("fixtures/isort")], + ..LinterSettings::for_rule(Rule::UnsortedImports) + }, + )?; + assert_messages!(snapshot, diagnostics); + Ok(()) + } + #[test_case(Path::new("relative_imports_order.py"))] fn closest_to_furthest(path: &Path) -> Result<()> { let snapshot = format!("closest_to_furthest_{}", path.to_string_lossy()); diff --git a/crates/ruff_linter/src/rules/isort/order.rs b/crates/ruff_linter/src/rules/isort/order.rs index 06002fd3ebb9b8..1d7102b3cae353 100644 --- a/crates/ruff_linter/src/rules/isort/order.rs +++ b/crates/ruff_linter/src/rules/isort/order.rs @@ -82,11 +82,19 @@ pub(crate) fn order_imports<'a>( settings, ) }); - ordered_straight_imports - .into_iter() - .map(Import) - .chain(ordered_from_imports.into_iter().map(ImportFrom)) - .collect() + if settings.from_first { + ordered_from_imports + .into_iter() + .map(ImportFrom) + .chain(ordered_straight_imports.into_iter().map(Import)) + .collect() + } else { + ordered_straight_imports + .into_iter() + .map(Import) + .chain(ordered_from_imports.into_iter().map(ImportFrom)) + .collect() + } }; ordered_imports diff --git a/crates/ruff_linter/src/rules/isort/settings.rs b/crates/ruff_linter/src/rules/isort/settings.rs index 6e27a2debf4ee9..d4937520fa7aae 100644 --- a/crates/ruff_linter/src/rules/isort/settings.rs +++ b/crates/ruff_linter/src/rules/isort/settings.rs @@ -57,6 +57,7 @@ pub struct Settings { pub forced_separate: Vec, pub section_order: Vec, pub no_sections: bool, + pub from_first: bool, } impl Default for Settings { @@ -84,6 +85,7 @@ impl Default for Settings { forced_separate: Vec::new(), section_order: ImportType::iter().map(ImportSection::Known).collect(), no_sections: false, + from_first: false, } } } diff --git a/crates/ruff_linter/src/rules/isort/snapshots/ruff_linter__rules__isort__tests__from_first_from_first.py.snap b/crates/ruff_linter/src/rules/isort/snapshots/ruff_linter__rules__isort__tests__from_first_from_first.py.snap new file mode 100644 index 00000000000000..ed369f0fd61f02 --- /dev/null +++ b/crates/ruff_linter/src/rules/isort/snapshots/ruff_linter__rules__isort__tests__from_first_from_first.py.snap @@ -0,0 +1,4 @@ +--- +source: crates/ruff_linter/src/rules/isort/mod.rs +--- + diff --git a/crates/ruff_workspace/src/options.rs b/crates/ruff_workspace/src/options.rs index 5967186b444a9e..fff62ddd6ff336 100644 --- a/crates/ruff_workspace/src/options.rs +++ b/crates/ruff_workspace/src/options.rs @@ -2021,6 +2021,32 @@ pub struct IsortOptions { )] pub detect_same_package: Option, + /// Whether to place `import from` imports before straight imports when sorting. + /// + /// For example, by default, imports will be sorted such that straight imports appear + /// before `import from` imports, as in: + /// ```python + /// import os + /// import sys + /// from typing import List + /// ``` + /// + /// Setting `from-first = true` will instead sort such that `import from` imports appear + /// before straight imports, as in: + /// ```python + /// from typing import List + /// import os + /// import sys + /// ``` + #[option( + default = r#"false"#, + value_type = "bool", + example = r#" + from-first = true + "# + )] + pub from_first: Option, + // Tables are required to go last. /// A list of mappings from section names to modules. /// By default custom sections are output last, but this can be overridden with `section-order`. @@ -2098,6 +2124,7 @@ impl IsortOptions { .map_err(isort::settings::SettingsError::InvalidExtraStandardLibrary)? .unwrap_or_default(); let no_lines_before = self.no_lines_before.unwrap_or_default(); + let from_first = self.from_first.unwrap_or_default(); let sections = self.sections.unwrap_or_default(); // Verify that `sections` doesn't contain any built-in sections. @@ -2206,6 +2233,7 @@ impl IsortOptions { forced_separate: Vec::from_iter(self.forced_separate.unwrap_or_default()), section_order, no_sections, + from_first, }) } } diff --git a/ruff.schema.json b/ruff.schema.json index 8ac638a4c11d19..57613064df667d 100644 --- a/ruff.schema.json +++ b/ruff.schema.json @@ -1440,6 +1440,13 @@ "type": "string" } }, + "from-first": { + "description": "Whether to place `import from` imports before straight imports when sorting.\n\nFor example, by default, imports will be sorted such that straight imports appear before `import from` imports, as in: ```python import os import sys from typing import List ```\n\nSetting `from-first = true` will instead sort such that `import from` imports appear before straight imports, as in: ```python from typing import List import os import sys ```", + "type": [ + "boolean", + "null" + ] + }, "known-first-party": { "description": "A list of modules to consider first-party, regardless of whether they can be identified as such via introspection of the local filesystem.\n\nSupports glob patterns. For more information on the glob syntax, refer to the [`globset` documentation](https://docs.rs/globset/latest/globset/#syntax).", "type": [