Skip to content

Commit

Permalink
isort: Add support for the from-first setting
Browse files Browse the repository at this point in the history
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
  • Loading branch information
jelmer committed Nov 16, 2023
1 parent 2083352 commit 830cec5
Show file tree
Hide file tree
Showing 7 changed files with 88 additions and 8 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
from __future__ import blah

from os import path

import os
48 changes: 45 additions & 3 deletions crates/ruff_linter/src/rules/isort/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -183,25 +183,44 @@ 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,
is_first_statement,
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());
}
Expand All @@ -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;
Expand Down Expand Up @@ -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());
Expand Down
18 changes: 13 additions & 5 deletions crates/ruff_linter/src/rules/isort/order.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 2 additions & 0 deletions crates/ruff_linter/src/rules/isort/settings.rs
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,7 @@ pub struct Settings {
pub forced_separate: Vec<String>,
pub section_order: Vec<ImportSection>,
pub no_sections: bool,
pub from_first: bool,
}

impl Default for Settings {
Expand Down Expand Up @@ -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,
}
}
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
---
source: crates/ruff_linter/src/rules/isort/mod.rs
---

12 changes: 12 additions & 0 deletions crates/ruff_workspace/src/options.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2021,6 +2021,16 @@ pub struct IsortOptions {
)]
pub detect_same_package: Option<bool>,

/// Sort "from .. import .." imports before straight imports
#[option(
default = r#"false"#,
value_type = "bool",
example = r#"
from-first = true
"#
)]
pub from_first: Option<bool>,

// 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`.
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -2206,6 +2217,7 @@ impl IsortOptions {
forced_separate: Vec::from_iter(self.forced_separate.unwrap_or_default()),
section_order,
no_sections,
from_first,
})
}
}
Expand Down
7 changes: 7 additions & 0 deletions ruff.schema.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

0 comments on commit 830cec5

Please sign in to comment.