Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

isort: Add support for the from-first setting #8663

Merged
merged 3 commits into from
Nov 21, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
from __future__ import blah

from os import path

import os
60 changes: 54 additions & 6 deletions crates/ruff_linter/src/rules/isort/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -182,31 +191,47 @@ 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,
is_first_statement,
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(
Expand All @@ -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;
Expand Down Expand Up @@ -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());
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
---

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

/// 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<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 +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.
Expand Down Expand Up @@ -2206,6 +2233,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.

Loading