Skip to content

Commit

Permalink
Avoid invalid combination of force-sort-within-types and `lines-bet…
Browse files Browse the repository at this point in the history
…ween-types` (#9041)

Closes #8792.
  • Loading branch information
charliermarsh authored Dec 7, 2023
1 parent 981a070 commit ebc7ac3
Show file tree
Hide file tree
Showing 4 changed files with 39 additions and 2 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
from a import x
import b
from c import y
import d
22 changes: 22 additions & 0 deletions crates/ruff_linter/src/rules/isort/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -200,6 +200,7 @@ fn format_import_block(
// Add a blank lines between direct and from imports.
if settings.from_first
&& lines_between_types > 0
&& !settings.force_sort_within_sections
&& line_insertion == Some(LineInsertion::Necessary)
{
for _ in 0..lines_between_types {
Expand All @@ -225,6 +226,7 @@ fn format_import_block(
// Add a blank lines between direct and from imports.
if !settings.from_first
&& lines_between_types > 0
&& !settings.force_sort_within_sections
&& line_insertion == Some(LineInsertion::Necessary)
{
for _ in 0..lines_between_types {
Expand Down Expand Up @@ -722,6 +724,26 @@ mod tests {
Ok(())
}

#[test_case(Path::new("force_sort_within_sections_lines_between.py"))]
fn force_sort_within_sections_lines_between(path: &Path) -> Result<()> {
let snapshot = format!("force_sort_within_sections_{}", path.to_string_lossy());
let mut diagnostics = test_path(
Path::new("isort").join(path).as_path(),
&LinterSettings {
isort: super::settings::Settings {
force_sort_within_sections: true,
lines_between_types: 2,
..super::settings::Settings::default()
},
src: vec![test_resource_path("fixtures/isort")],
..LinterSettings::for_rule(Rule::UnsortedImports)
},
)?;
diagnostics.sort_by_key(Ranged::start);
assert_messages!(snapshot, diagnostics);
Ok(())
}

#[test_case(Path::new("comment.py"))]
#[test_case(Path::new("comments_and_newlines.py"))]
#[test_case(Path::new("docstring.py"))]
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
---

11 changes: 9 additions & 2 deletions crates/ruff_workspace/src/options.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2109,6 +2109,13 @@ impl IsortOptions {
warn_user_once!("`sections` is ignored when `no-sections` is set to `true`");
}

// Verify that if `force_sort_within_sections` is `True`, then `lines_between_types` is set to `0`.
let force_sort_within_sections = self.force_sort_within_sections.unwrap_or_default();
let lines_between_types = self.lines_between_types.unwrap_or_default();
if force_sort_within_sections && lines_between_types != 0 {
warn_user_once!("`lines-between-types` is ignored when `force-sort-within-sections` is set to `true`");
}

// Extract any configuration options that deal with user-defined sections.
let mut section_order: Vec<_> = self
.section_order
Expand Down Expand Up @@ -2240,7 +2247,7 @@ impl IsortOptions {
required_imports: BTreeSet::from_iter(self.required_imports.unwrap_or_default()),
combine_as_imports: self.combine_as_imports.unwrap_or(false),
force_single_line: self.force_single_line.unwrap_or(false),
force_sort_within_sections: self.force_sort_within_sections.unwrap_or(false),
force_sort_within_sections,
case_sensitive: self.case_sensitive.unwrap_or(false),
force_wrap_aliases: self.force_wrap_aliases.unwrap_or(false),
detect_same_package: self.detect_same_package.unwrap_or(true),
Expand All @@ -2263,7 +2270,7 @@ impl IsortOptions {
variables: BTreeSet::from_iter(self.variables.unwrap_or_default()),
no_lines_before: BTreeSet::from_iter(no_lines_before),
lines_after_imports: self.lines_after_imports.unwrap_or(-1),
lines_between_types: self.lines_between_types.unwrap_or_default(),
lines_between_types,
forced_separate: Vec::from_iter(self.forced_separate.unwrap_or_default()),
section_order,
no_sections,
Expand Down

0 comments on commit ebc7ac3

Please sign in to comment.