mirror of
https://github.com/astral-sh/ruff.git
synced 2025-09-28 21:05:08 +00:00
Implement isort's default-section
setting (#10149)
## Summary
This fixes https://github.com/astral-sh/ruff/issues/7868.
Support isort's `default-section` feature which allows any imports that
match sections that are not in `section-order` to be mapped to a
specifically named section.
https://pycqa.github.io/isort/docs/configuration/options.html#default-section
This has a few implications:
- It is no longer required that all known sections are defined in
`section-order`.
- This is technically a bw-incompat change because currently if folks
define custom groups, and do not define a `section-order`, the code used
to add all known sections to `section-order` while emitting warnings.
**However, when this happened, users would be seeing warnings so I do
not think it should count as a bw-incompat change.**
## Test Plan
- Added a new test.
- Did not break any existing tests.
Finally, I ran the following config against Pyramid's complex codebase
that was previously using isort and this change worked there.
### pyramid's previous isort config
5f7e286b06/pyproject.toml (L22-L37)
```toml
[tool.isort]
profile = "black"
multi_line_output = 3
src_paths = ["src", "tests"]
skip_glob = ["docs/*"]
include_trailing_comma = true
force_grid_wrap = false
combine_as_imports = true
line_length = 79
force_sort_within_sections = true
no_lines_before = "THIRDPARTY"
sections = "FUTURE,THIRDPARTY,FIRSTPARTY,LOCALFOLDER"
default_section = "THIRDPARTY"
known_first_party = "pyramid"
```
### tested with ruff isort config
```toml
[tool.ruff.lint.isort]
case-sensitive = true
combine-as-imports = true
force-sort-within-sections = true
section-order = [
"future",
"third-party",
"first-party",
"local-folder",
]
default-section = "third-party"
known-first-party = [
"pyramid",
]
```
This commit is contained in:
parent
8e0a70cfa3
commit
c9931a548f
11 changed files with 174 additions and 23 deletions
|
@ -311,6 +311,7 @@ linter.isort.section_order = [
|
||||||
known { type = first_party },
|
known { type = first_party },
|
||||||
known { type = local_folder },
|
known { type = local_folder },
|
||||||
]
|
]
|
||||||
|
linter.isort.default_section = known { type = third_party }
|
||||||
linter.isort.no_sections = false
|
linter.isort.no_sections = false
|
||||||
linter.isort.from_first = false
|
linter.isort.from_first = false
|
||||||
linter.isort.length_sort = false
|
linter.isort.length_sort = false
|
||||||
|
|
9
crates/ruff_linter/resources/test/fixtures/isort/default_section_user_defined.py
vendored
Normal file
9
crates/ruff_linter/resources/test/fixtures/isort/default_section_user_defined.py
vendored
Normal file
|
@ -0,0 +1,9 @@
|
||||||
|
from __future__ import annotations
|
||||||
|
|
||||||
|
import django.settings
|
||||||
|
from library import foo
|
||||||
|
import os
|
||||||
|
import pytz
|
||||||
|
import sys
|
||||||
|
|
||||||
|
from . import local
|
9
crates/ruff_linter/resources/test/fixtures/isort/no_standard_library.py
vendored
Normal file
9
crates/ruff_linter/resources/test/fixtures/isort/no_standard_library.py
vendored
Normal file
|
@ -0,0 +1,9 @@
|
||||||
|
from __future__ import annotations
|
||||||
|
|
||||||
|
import os
|
||||||
|
import django.settings
|
||||||
|
from library import foo
|
||||||
|
import pytz
|
||||||
|
|
||||||
|
from . import local
|
||||||
|
import sys
|
|
@ -304,6 +304,8 @@ pub(crate) fn typing_only_runtime_import(
|
||||||
&checker.settings.isort.known_modules,
|
&checker.settings.isort.known_modules,
|
||||||
checker.settings.target_version,
|
checker.settings.target_version,
|
||||||
checker.settings.isort.no_sections,
|
checker.settings.isort.no_sections,
|
||||||
|
&checker.settings.isort.section_order,
|
||||||
|
&checker.settings.isort.default_section,
|
||||||
) {
|
) {
|
||||||
ImportSection::Known(ImportType::LocalFolder | ImportType::FirstParty) => {
|
ImportSection::Known(ImportType::LocalFolder | ImportType::FirstParty) => {
|
||||||
ImportType::FirstParty
|
ImportType::FirstParty
|
||||||
|
|
|
@ -84,6 +84,7 @@ enum Reason<'a> {
|
||||||
NoMatch,
|
NoMatch,
|
||||||
UserDefinedSection,
|
UserDefinedSection,
|
||||||
NoSections,
|
NoSections,
|
||||||
|
DisabledSection(&'a ImportSection),
|
||||||
}
|
}
|
||||||
|
|
||||||
#[allow(clippy::too_many_arguments)]
|
#[allow(clippy::too_many_arguments)]
|
||||||
|
@ -96,9 +97,11 @@ pub(crate) fn categorize<'a>(
|
||||||
known_modules: &'a KnownModules,
|
known_modules: &'a KnownModules,
|
||||||
target_version: PythonVersion,
|
target_version: PythonVersion,
|
||||||
no_sections: bool,
|
no_sections: bool,
|
||||||
|
section_order: &'a [ImportSection],
|
||||||
|
default_section: &'a ImportSection,
|
||||||
) -> &'a ImportSection {
|
) -> &'a ImportSection {
|
||||||
let module_base = module_name.split('.').next().unwrap();
|
let module_base = module_name.split('.').next().unwrap();
|
||||||
let (import_type, reason) = {
|
let (mut import_type, mut reason) = {
|
||||||
if matches!(level, None | Some(0)) && module_base == "__future__" {
|
if matches!(level, None | Some(0)) && module_base == "__future__" {
|
||||||
(&ImportSection::Known(ImportType::Future), Reason::Future)
|
(&ImportSection::Known(ImportType::Future), Reason::Future)
|
||||||
} else if no_sections {
|
} else if no_sections {
|
||||||
|
@ -134,12 +137,14 @@ pub(crate) fn categorize<'a>(
|
||||||
Reason::KnownFirstParty,
|
Reason::KnownFirstParty,
|
||||||
)
|
)
|
||||||
} else {
|
} else {
|
||||||
(
|
(default_section, Reason::NoMatch)
|
||||||
&ImportSection::Known(ImportType::ThirdParty),
|
|
||||||
Reason::NoMatch,
|
|
||||||
)
|
|
||||||
}
|
}
|
||||||
};
|
};
|
||||||
|
// If a value is not in `section_order` then map it to `default_section`.
|
||||||
|
if !section_order.contains(import_type) {
|
||||||
|
reason = Reason::DisabledSection(import_type);
|
||||||
|
import_type = default_section;
|
||||||
|
}
|
||||||
debug!(
|
debug!(
|
||||||
"Categorized '{}' as {:?} ({:?})",
|
"Categorized '{}' as {:?} ({:?})",
|
||||||
module_name, import_type, reason
|
module_name, import_type, reason
|
||||||
|
@ -176,6 +181,8 @@ pub(crate) fn categorize_imports<'a>(
|
||||||
known_modules: &'a KnownModules,
|
known_modules: &'a KnownModules,
|
||||||
target_version: PythonVersion,
|
target_version: PythonVersion,
|
||||||
no_sections: bool,
|
no_sections: bool,
|
||||||
|
section_order: &'a [ImportSection],
|
||||||
|
default_section: &'a ImportSection,
|
||||||
) -> BTreeMap<&'a ImportSection, ImportBlock<'a>> {
|
) -> BTreeMap<&'a ImportSection, ImportBlock<'a>> {
|
||||||
let mut block_by_type: BTreeMap<&ImportSection, ImportBlock> = BTreeMap::default();
|
let mut block_by_type: BTreeMap<&ImportSection, ImportBlock> = BTreeMap::default();
|
||||||
// Categorize `Stmt::Import`.
|
// Categorize `Stmt::Import`.
|
||||||
|
@ -189,6 +196,8 @@ pub(crate) fn categorize_imports<'a>(
|
||||||
known_modules,
|
known_modules,
|
||||||
target_version,
|
target_version,
|
||||||
no_sections,
|
no_sections,
|
||||||
|
section_order,
|
||||||
|
default_section,
|
||||||
);
|
);
|
||||||
block_by_type
|
block_by_type
|
||||||
.entry(import_type)
|
.entry(import_type)
|
||||||
|
@ -207,6 +216,8 @@ pub(crate) fn categorize_imports<'a>(
|
||||||
known_modules,
|
known_modules,
|
||||||
target_version,
|
target_version,
|
||||||
no_sections,
|
no_sections,
|
||||||
|
section_order,
|
||||||
|
default_section,
|
||||||
);
|
);
|
||||||
block_by_type
|
block_by_type
|
||||||
.entry(classification)
|
.entry(classification)
|
||||||
|
@ -225,6 +236,8 @@ pub(crate) fn categorize_imports<'a>(
|
||||||
known_modules,
|
known_modules,
|
||||||
target_version,
|
target_version,
|
||||||
no_sections,
|
no_sections,
|
||||||
|
section_order,
|
||||||
|
default_section,
|
||||||
);
|
);
|
||||||
block_by_type
|
block_by_type
|
||||||
.entry(classification)
|
.entry(classification)
|
||||||
|
@ -243,6 +256,8 @@ pub(crate) fn categorize_imports<'a>(
|
||||||
known_modules,
|
known_modules,
|
||||||
target_version,
|
target_version,
|
||||||
no_sections,
|
no_sections,
|
||||||
|
section_order,
|
||||||
|
default_section,
|
||||||
);
|
);
|
||||||
block_by_type
|
block_by_type
|
||||||
.entry(classification)
|
.entry(classification)
|
||||||
|
|
|
@ -173,6 +173,8 @@ fn format_import_block(
|
||||||
&settings.known_modules,
|
&settings.known_modules,
|
||||||
target_version,
|
target_version,
|
||||||
settings.no_sections,
|
settings.no_sections,
|
||||||
|
&settings.section_order,
|
||||||
|
&settings.default_section,
|
||||||
);
|
);
|
||||||
|
|
||||||
let mut output = String::new();
|
let mut output = String::new();
|
||||||
|
@ -934,6 +936,65 @@ mod tests {
|
||||||
Ok(())
|
Ok(())
|
||||||
}
|
}
|
||||||
|
|
||||||
|
#[test_case(Path::new("default_section_user_defined.py"))]
|
||||||
|
fn default_section_can_map_to_user_defined_section(path: &Path) -> Result<()> {
|
||||||
|
let snapshot = format!(
|
||||||
|
"default_section_can_map_to_user_defined_section_{}",
|
||||||
|
path.to_string_lossy()
|
||||||
|
);
|
||||||
|
let diagnostics = test_path(
|
||||||
|
Path::new("isort").join(path).as_path(),
|
||||||
|
&LinterSettings {
|
||||||
|
isort: super::settings::Settings {
|
||||||
|
known_modules: KnownModules::new(
|
||||||
|
vec![],
|
||||||
|
vec![],
|
||||||
|
vec![],
|
||||||
|
vec![],
|
||||||
|
FxHashMap::from_iter([("django".to_string(), vec![pattern("django")])]),
|
||||||
|
),
|
||||||
|
section_order: vec![
|
||||||
|
ImportSection::Known(ImportType::Future),
|
||||||
|
ImportSection::UserDefined("django".to_string()),
|
||||||
|
ImportSection::Known(ImportType::FirstParty),
|
||||||
|
ImportSection::Known(ImportType::LocalFolder),
|
||||||
|
],
|
||||||
|
force_sort_within_sections: true,
|
||||||
|
default_section: ImportSection::UserDefined("django".to_string()),
|
||||||
|
..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_standard_library.py"))]
|
||||||
|
fn no_standard_library(path: &Path) -> Result<()> {
|
||||||
|
let snapshot = format!("no_standard_library_{}", path.to_string_lossy());
|
||||||
|
let diagnostics = test_path(
|
||||||
|
Path::new("isort").join(path).as_path(),
|
||||||
|
&LinterSettings {
|
||||||
|
isort: super::settings::Settings {
|
||||||
|
section_order: vec![
|
||||||
|
ImportSection::Known(ImportType::Future),
|
||||||
|
ImportSection::Known(ImportType::ThirdParty),
|
||||||
|
ImportSection::Known(ImportType::FirstParty),
|
||||||
|
ImportSection::Known(ImportType::LocalFolder),
|
||||||
|
],
|
||||||
|
force_sort_within_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"))]
|
#[test_case(Path::new("no_lines_before.py"))]
|
||||||
fn no_lines_before(path: &Path) -> Result<()> {
|
fn no_lines_before(path: &Path) -> Result<()> {
|
||||||
let snapshot = format!("no_lines_before.py_{}", path.to_string_lossy());
|
let snapshot = format!("no_lines_before.py_{}", path.to_string_lossy());
|
||||||
|
|
|
@ -67,6 +67,7 @@ pub struct Settings {
|
||||||
pub lines_between_types: usize,
|
pub lines_between_types: usize,
|
||||||
pub forced_separate: Vec<String>,
|
pub forced_separate: Vec<String>,
|
||||||
pub section_order: Vec<ImportSection>,
|
pub section_order: Vec<ImportSection>,
|
||||||
|
pub default_section: ImportSection,
|
||||||
pub no_sections: bool,
|
pub no_sections: bool,
|
||||||
pub from_first: bool,
|
pub from_first: bool,
|
||||||
pub length_sort: bool,
|
pub length_sort: bool,
|
||||||
|
@ -97,6 +98,7 @@ impl Default for Settings {
|
||||||
lines_between_types: 0,
|
lines_between_types: 0,
|
||||||
forced_separate: Vec::new(),
|
forced_separate: Vec::new(),
|
||||||
section_order: ImportType::iter().map(ImportSection::Known).collect(),
|
section_order: ImportType::iter().map(ImportSection::Known).collect(),
|
||||||
|
default_section: ImportSection::Known(ImportType::ThirdParty),
|
||||||
no_sections: false,
|
no_sections: false,
|
||||||
from_first: false,
|
from_first: false,
|
||||||
length_sort: false,
|
length_sort: false,
|
||||||
|
@ -132,6 +134,7 @@ impl Display for Settings {
|
||||||
self.lines_between_types,
|
self.lines_between_types,
|
||||||
self.forced_separate | array,
|
self.forced_separate | array,
|
||||||
self.section_order | array,
|
self.section_order | array,
|
||||||
|
self.default_section,
|
||||||
self.no_sections,
|
self.no_sections,
|
||||||
self.from_first,
|
self.from_first,
|
||||||
self.length_sort,
|
self.length_sort,
|
||||||
|
|
|
@ -0,0 +1,4 @@
|
||||||
|
---
|
||||||
|
source: crates/ruff_linter/src/rules/isort/mod.rs
|
||||||
|
---
|
||||||
|
|
|
@ -0,0 +1,29 @@
|
||||||
|
---
|
||||||
|
source: crates/ruff_linter/src/rules/isort/mod.rs
|
||||||
|
---
|
||||||
|
no_standard_library.py:1:1: I001 [*] Import block is un-sorted or un-formatted
|
||||||
|
|
|
||||||
|
1 | / from __future__ import annotations
|
||||||
|
2 | |
|
||||||
|
3 | | import os
|
||||||
|
4 | | import django.settings
|
||||||
|
5 | | from library import foo
|
||||||
|
6 | | import pytz
|
||||||
|
7 | |
|
||||||
|
8 | | from . import local
|
||||||
|
9 | | import sys
|
||||||
|
|
|
||||||
|
= help: Organize imports
|
||||||
|
|
||||||
|
ℹ Safe fix
|
||||||
|
1 1 | from __future__ import annotations
|
||||||
|
2 2 |
|
||||||
|
3 |-import os
|
||||||
|
4 3 | import django.settings
|
||||||
|
5 4 | from library import foo
|
||||||
|
5 |+import os
|
||||||
|
6 6 | import pytz
|
||||||
|
7 |+import sys
|
||||||
|
7 8 |
|
||||||
|
8 9 | from . import local
|
||||||
|
9 |-import sys
|
|
@ -2099,6 +2099,16 @@ pub struct IsortOptions {
|
||||||
)]
|
)]
|
||||||
pub section_order: Option<Vec<ImportSection>>,
|
pub section_order: Option<Vec<ImportSection>>,
|
||||||
|
|
||||||
|
/// Define a default section for any imports that don't fit into the specified `section-order`.
|
||||||
|
#[option(
|
||||||
|
default = r#"third-party"#,
|
||||||
|
value_type = "str",
|
||||||
|
example = r#"
|
||||||
|
default-section = "third-party"
|
||||||
|
"#
|
||||||
|
)]
|
||||||
|
pub default_section: Option<ImportSection>,
|
||||||
|
|
||||||
/// Put all imports into the same section bucket.
|
/// Put all imports into the same section bucket.
|
||||||
///
|
///
|
||||||
/// For example, rather than separating standard library and third-party imports, as in:
|
/// For example, rather than separating standard library and third-party imports, as in:
|
||||||
|
@ -2226,6 +2236,9 @@ impl IsortOptions {
|
||||||
if no_sections && self.section_order.is_some() {
|
if no_sections && self.section_order.is_some() {
|
||||||
warn_user_once!("`section-order` is ignored when `no-sections` is set to `true`");
|
warn_user_once!("`section-order` is ignored when `no-sections` is set to `true`");
|
||||||
}
|
}
|
||||||
|
if no_sections && self.default_section.is_some() {
|
||||||
|
warn_user_once!("`default-section` is ignored when `no-sections` is set to `true`");
|
||||||
|
}
|
||||||
if no_sections && self.sections.is_some() {
|
if no_sections && self.sections.is_some() {
|
||||||
warn_user_once!("`sections` is ignored when `no-sections` is set to `true`");
|
warn_user_once!("`sections` is ignored when `no-sections` is set to `true`");
|
||||||
}
|
}
|
||||||
|
@ -2241,6 +2254,10 @@ impl IsortOptions {
|
||||||
let mut section_order: Vec<_> = self
|
let mut section_order: Vec<_> = self
|
||||||
.section_order
|
.section_order
|
||||||
.unwrap_or_else(|| ImportType::iter().map(ImportSection::Known).collect());
|
.unwrap_or_else(|| ImportType::iter().map(ImportSection::Known).collect());
|
||||||
|
let default_section = self
|
||||||
|
.default_section
|
||||||
|
.unwrap_or(ImportSection::Known(ImportType::ThirdParty));
|
||||||
|
|
||||||
let known_first_party = self
|
let known_first_party = self
|
||||||
.known_first_party
|
.known_first_party
|
||||||
.map(|names| {
|
.map(|names| {
|
||||||
|
@ -2344,24 +2361,13 @@ impl IsortOptions {
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
// Add all built-in sections to `section_order`, if not already present.
|
// Verify that `default_section` is in `section_order`.
|
||||||
for section in ImportType::iter().map(ImportSection::Known) {
|
if !section_order.contains(&default_section) {
|
||||||
if !section_order.contains(§ion) {
|
|
||||||
warn_user_once!(
|
warn_user_once!(
|
||||||
"`section-order` is missing built-in section: `{:?}`",
|
"`section-order` must contain `default-section`: {:?}",
|
||||||
section
|
default_section,
|
||||||
);
|
);
|
||||||
section_order.push(section);
|
section_order.push(default_section.clone());
|
||||||
}
|
|
||||||
}
|
|
||||||
|
|
||||||
// Add all user-defined sections to `section-order`, if not already present.
|
|
||||||
for section_name in sections.keys() {
|
|
||||||
let section = ImportSection::UserDefined(section_name.clone());
|
|
||||||
if !section_order.contains(§ion) {
|
|
||||||
warn_user_once!("`section-order` is missing section: `{:?}`", section);
|
|
||||||
section_order.push(section);
|
|
||||||
}
|
|
||||||
}
|
}
|
||||||
|
|
||||||
Ok(isort::settings::Settings {
|
Ok(isort::settings::Settings {
|
||||||
|
@ -2394,6 +2400,7 @@ impl IsortOptions {
|
||||||
lines_between_types,
|
lines_between_types,
|
||||||
forced_separate: Vec::from_iter(self.forced_separate.unwrap_or_default()),
|
forced_separate: Vec::from_iter(self.forced_separate.unwrap_or_default()),
|
||||||
section_order,
|
section_order,
|
||||||
|
default_section,
|
||||||
no_sections,
|
no_sections,
|
||||||
from_first,
|
from_first,
|
||||||
length_sort: self.length_sort.unwrap_or(false),
|
length_sort: self.length_sort.unwrap_or(false),
|
||||||
|
|
11
ruff.schema.json
generated
11
ruff.schema.json
generated
|
@ -1474,6 +1474,17 @@
|
||||||
"type": "string"
|
"type": "string"
|
||||||
}
|
}
|
||||||
},
|
},
|
||||||
|
"default-section": {
|
||||||
|
"description": "Define a default section for any imports that don't fit into the specified `section-order`.",
|
||||||
|
"anyOf": [
|
||||||
|
{
|
||||||
|
"$ref": "#/definitions/ImportSection"
|
||||||
|
},
|
||||||
|
{
|
||||||
|
"type": "null"
|
||||||
|
}
|
||||||
|
]
|
||||||
|
},
|
||||||
"detect-same-package": {
|
"detect-same-package": {
|
||||||
"description": "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.\n\nThis heuristic is often unnecessary when `src` is configured to detect all first-party sources; however, if `src` is _not_ configured, this heuristic can be useful to detect first-party imports from _within_ (but not _across_) first-party packages.",
|
"description": "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.\n\nThis heuristic is often unnecessary when `src` is configured to detect all first-party sources; however, if `src` is _not_ configured, this heuristic can be useful to detect first-party imports from _within_ (but not _across_) first-party packages.",
|
||||||
"type": [
|
"type": [
|
||||||
|
|
Loading…
Add table
Add a link
Reference in a new issue