From 830cec576408dc2eb2630af4adcbfb2bfcc34556 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jelmer=20Vernoo=C4=B3?= Date: Mon, 13 Nov 2023 19:35:53 +0000 Subject: [PATCH] isort: Add support for the ``from-first`` setting This setting behaves similarly to the ``from_first`` setting in isort upstream, and sorts "from X import Y" type imports before straight imports. Fixes #8662 --- .../test/fixtures/isort/from_first.py | 5 ++ crates/ruff_linter/src/rules/isort/mod.rs | 48 +++++++++++++++++-- crates/ruff_linter/src/rules/isort/order.rs | 18 +++++-- .../ruff_linter/src/rules/isort/settings.rs | 2 + ...sort__tests__from_first_from_first.py.snap | 4 ++ crates/ruff_workspace/src/options.rs | 12 +++++ ruff.schema.json | 7 +++ 7 files changed, 88 insertions(+), 8 deletions(-) create mode 100644 crates/ruff_linter/resources/test/fixtures/isort/from_first.py create mode 100644 crates/ruff_linter/src/rules/isort/snapshots/ruff_linter__rules__isort__tests__from_first_from_first.py.snap 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 0000000000000..a15a573d28215 --- /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 73380ca4eefd0..d4b6d46763644 100644 --- a/crates/ruff_linter/src/rules/isort/mod.rs +++ b/crates/ruff_linter/src/rules/isort/mod.rs @@ -183,12 +183,25 @@ fn format_import_block( } let mut lines_inserted = false; - let mut has_direct_import = false; + let mut needs_lines_inserted = false; 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 + && needs_lines_inserted + && !lines_inserted + { + for _ in 0..lines_between_types { + output.push_str(&stylist.line_ending()); + } + + lines_inserted = true; + } + output.push_str(&format::format_import( &alias, &comments, @@ -196,12 +209,18 @@ fn format_import_block( stylist, )); - has_direct_import = true; + if !settings.from_first { + needs_lines_inserted = true; + } } 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 { + if !settings.from_first + && lines_between_types > 0 + && needs_lines_inserted + && !lines_inserted + { for _ in 0..lines_between_types { output.push_str(&stylist.line_ending()); } @@ -221,6 +240,10 @@ fn format_import_block( settings.split_on_trailing_comma && matches!(trailing_comma, TrailingComma::Present), )); + + if settings.from_first { + needs_lines_inserted = true; + } } } is_first_statement = false; @@ -819,6 +842,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 06002fd3ebb9b..1d7102b3cae35 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 6e27a2debf4ee..d4937520fa7aa 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 0000000000000..ed369f0fd61f0 --- /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 5967186b444a9..5598bbc21e98a 100644 --- a/crates/ruff_workspace/src/options.rs +++ b/crates/ruff_workspace/src/options.rs @@ -2021,6 +2021,16 @@ pub struct IsortOptions { )] pub detect_same_package: Option, + /// Sort "from .. import .." imports before straight imports + #[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 +2108,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 +2217,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 22756f2dc68d1..89cac52f833a1 100644 --- a/ruff.schema.json +++ b/ruff.schema.json @@ -1440,6 +1440,13 @@ "type": "string" } }, + "from-first": { + "description": "Sort \"from .. import ..\" imports before straight imports", + "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": [