Skip to content

Commit

Permalink
isort: Support disabling sections with no-sections = true (#8657)
Browse files Browse the repository at this point in the history
## Summary

This adds a ``no-sections`` option for isort in the linter, similar to
the ``no_sections`` option that exists in upstream isort
(https://pycqa.github.io/isort/docs/configuration/options.html#no-sections)

This option puts all imports except for ``__future__`` into the same
section, and is mostly used by monorepos.

I've taken a bit of a leap in assuming that ruff wants to support the
exact same option; more than happy to refactor if you'd prefer a
different way of setting this up.

Fixes #8653

## Test Plan

I've added a test and have run it on a large Python codebase that uses
isort with --no-sections. The option is disabled by default.
  • Loading branch information
jelmer authored Nov 14, 2023
1 parent 5612779 commit 9d76e4e
Show file tree
Hide file tree
Showing 8 changed files with 93 additions and 3 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
from __future__ import annotations

import django.settings
import os
import pytz
import sys
from . import local
from library import foo
Original file line number Diff line number Diff line change
Expand Up @@ -285,6 +285,7 @@ pub(crate) fn typing_only_runtime_import(
checker.settings.isort.detect_same_package,
&checker.settings.isort.known_modules,
checker.settings.target_version,
checker.settings.isort.no_sections,
) {
ImportSection::Known(ImportType::LocalFolder | ImportType::FirstParty) => {
ImportType::FirstParty
Expand Down
18 changes: 15 additions & 3 deletions crates/ruff_linter/src/rules/isort/categorize.rs
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,7 @@ enum Reason<'a> {
SourceMatch(&'a Path),
NoMatch,
UserDefinedSection,
NoSections,
}

#[allow(clippy::too_many_arguments)]
Expand All @@ -72,16 +73,22 @@ pub(crate) fn categorize<'a>(
detect_same_package: bool,
known_modules: &'a KnownModules,
target_version: PythonVersion,
no_sections: bool,
) -> &'a ImportSection {
let module_base = module_name.split('.').next().unwrap();
let (import_type, reason) = {
if level.is_some_and(|level| level > 0) {
if matches!(level, None | Some(0)) && module_base == "__future__" {
(&ImportSection::Known(ImportType::Future), Reason::Future)
} else if no_sections {
(
&ImportSection::Known(ImportType::FirstParty),
Reason::NoSections,
)
} else if level.is_some_and(|level| level > 0) {
(
&ImportSection::Known(ImportType::LocalFolder),
Reason::NonZeroLevel,
)
} else if module_base == "__future__" {
(&ImportSection::Known(ImportType::Future), Reason::Future)
} else if let Some((import_type, reason)) = known_modules.categorize(module_name) {
(import_type, reason)
} else if is_known_standard_library(target_version.minor(), module_base) {
Expand Down Expand Up @@ -141,6 +148,7 @@ pub(crate) fn categorize_imports<'a>(
detect_same_package: bool,
known_modules: &'a KnownModules,
target_version: PythonVersion,
no_sections: bool,
) -> BTreeMap<&'a ImportSection, ImportBlock<'a>> {
let mut block_by_type: BTreeMap<&ImportSection, ImportBlock> = BTreeMap::default();
// Categorize `Stmt::Import`.
Expand All @@ -153,6 +161,7 @@ pub(crate) fn categorize_imports<'a>(
detect_same_package,
known_modules,
target_version,
no_sections,
);
block_by_type
.entry(import_type)
Expand All @@ -170,6 +179,7 @@ pub(crate) fn categorize_imports<'a>(
detect_same_package,
known_modules,
target_version,
no_sections,
);
block_by_type
.entry(classification)
Expand All @@ -187,6 +197,7 @@ pub(crate) fn categorize_imports<'a>(
detect_same_package,
known_modules,
target_version,
no_sections,
);
block_by_type
.entry(classification)
Expand All @@ -204,6 +215,7 @@ pub(crate) fn categorize_imports<'a>(
detect_same_package,
known_modules,
target_version,
no_sections,
);
block_by_type
.entry(classification)
Expand Down
19 changes: 19 additions & 0 deletions crates/ruff_linter/src/rules/isort/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -153,6 +153,7 @@ fn format_import_block(
settings.detect_same_package,
&settings.known_modules,
target_version,
settings.no_sections,
);

let mut output = String::new();
Expand Down Expand Up @@ -836,6 +837,24 @@ mod tests {
Ok(())
}

#[test_case(Path::new("no_sections.py"))]
fn no_sections(path: &Path) -> Result<()> {
let snapshot = format!("no_sections_{}", path.to_string_lossy());
let diagnostics = test_path(
Path::new("isort").join(path).as_path(),
&LinterSettings {
isort: super::settings::Settings {
no_sections: true,
..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("no_lines_before.py"))]
fn no_lines_before(path: &Path) -> Result<()> {
let snapshot = format!("no_lines_before.py_{}", path.to_string_lossy());
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 @@ -56,6 +56,7 @@ pub struct Settings {
pub lines_between_types: usize,
pub forced_separate: Vec<String>,
pub section_order: Vec<ImportSection>,
pub no_sections: bool,
}

impl Default for Settings {
Expand All @@ -82,6 +83,7 @@ impl Default for Settings {
lines_between_types: 0,
forced_separate: Vec::new(),
section_order: ImportType::iter().map(ImportSection::Known).collect(),
no_sections: 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
---

37 changes: 37 additions & 0 deletions crates/ruff_workspace/src/options.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1978,6 +1978,33 @@ pub struct IsortOptions {
)]
pub section_order: Option<Vec<ImportSection>>,

/// Put all imports into the same section bucket.
///
/// For example, rather than separating standard library and third-party imports, as in:
/// ```python
/// import os
/// import sys
///
/// import numpy
/// import pandas
/// ```
///
/// Setting `no-sections = true` will instead group all imports into a single section:
/// ```python
/// import os
/// import numpy
/// import pandas
/// import sys
/// ```
#[option(
default = r#"false"#,
value_type = "bool",
example = r#"
no-sections = true
"#
)]
pub no_sections: Option<bool>,

/// Whether to automatically mark imports from within the same package as first-party.
/// For example, when `detect-same-package = true`, then when analyzing files within the
/// `foo` package, any imports from within the `foo` package will be considered first-party.
Expand Down Expand Up @@ -2013,6 +2040,15 @@ impl IsortOptions {
pub fn try_into_settings(
self,
) -> Result<isort::settings::Settings, isort::settings::SettingsError> {
// Verify that if `no_sections` is set, then `section_order` is empty.
let no_sections = self.no_sections.unwrap_or_default();
if no_sections && self.section_order.is_some() {
warn_user_once!("`section-order` is ignored when `no-sections` is set to `true`");
}
if no_sections && self.sections.is_some() {
warn_user_once!("`sections` is ignored when `no-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 @@ -2169,6 +2205,7 @@ impl IsortOptions {
lines_between_types: self.lines_between_types.unwrap_or_default(),
forced_separate: Vec::from_iter(self.forced_separate.unwrap_or_default()),
section_order,
no_sections,
})
}
}
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 9d76e4e

Please sign in to comment.