From 8b0341069b8c5216e154eb82f25f683399012cc0 Mon Sep 17 00:00:00 2001 From: Michael Merickel Date: Tue, 27 Feb 2024 20:18:25 -0700 Subject: [PATCH 1/2] add tool.ruff.isort.default-section --- ...ow_settings__display_default_settings.snap | 1 + .../isort/default_section_user_defined.py | 9 ++++ .../rules/typing_only_runtime_import.rs | 2 + .../ruff_linter/src/rules/isort/categorize.rs | 25 ++++++++--- crates/ruff_linter/src/rules/isort/mod.rs | 34 +++++++++++++++ .../ruff_linter/src/rules/isort/settings.rs | 3 ++ ...fined_default_section_user_defined.py.snap | 5 +++ crates/ruff_workspace/src/options.rs | 43 +++++++++++-------- ruff.schema.json | 11 +++++ 9 files changed, 110 insertions(+), 23 deletions(-) create mode 100644 crates/ruff_linter/resources/test/fixtures/isort/default_section_user_defined.py create mode 100644 crates/ruff_linter/src/rules/isort/snapshots/ruff_linter__rules__isort__tests__default_section_user_defined_default_section_user_defined.py.snap diff --git a/crates/ruff/tests/snapshots/show_settings__display_default_settings.snap b/crates/ruff/tests/snapshots/show_settings__display_default_settings.snap index 02c735ec3dc94..e381800a21f87 100644 --- a/crates/ruff/tests/snapshots/show_settings__display_default_settings.snap +++ b/crates/ruff/tests/snapshots/show_settings__display_default_settings.snap @@ -312,6 +312,7 @@ linter.isort.section_order = [ known { type = first_party }, known { type = local_folder }, ] +linter.isort.default_section = known { type = third_party } linter.isort.no_sections = false linter.isort.from_first = false linter.isort.length_sort = false diff --git a/crates/ruff_linter/resources/test/fixtures/isort/default_section_user_defined.py b/crates/ruff_linter/resources/test/fixtures/isort/default_section_user_defined.py new file mode 100644 index 0000000000000..50d9218d9cfb1 --- /dev/null +++ b/crates/ruff_linter/resources/test/fixtures/isort/default_section_user_defined.py @@ -0,0 +1,9 @@ +from __future__ import annotations + +import django.settings +from library import foo +import os +import pytz +import sys + +from . import local diff --git a/crates/ruff_linter/src/rules/flake8_type_checking/rules/typing_only_runtime_import.rs b/crates/ruff_linter/src/rules/flake8_type_checking/rules/typing_only_runtime_import.rs index 17812c89989cc..8b12e0086a719 100644 --- a/crates/ruff_linter/src/rules/flake8_type_checking/rules/typing_only_runtime_import.rs +++ b/crates/ruff_linter/src/rules/flake8_type_checking/rules/typing_only_runtime_import.rs @@ -304,6 +304,8 @@ pub(crate) fn typing_only_runtime_import( &checker.settings.isort.known_modules, checker.settings.target_version, checker.settings.isort.no_sections, + &checker.settings.isort.section_order, + &checker.settings.isort.default_section, ) { ImportSection::Known(ImportType::LocalFolder | ImportType::FirstParty) => { ImportType::FirstParty diff --git a/crates/ruff_linter/src/rules/isort/categorize.rs b/crates/ruff_linter/src/rules/isort/categorize.rs index c00640ea12ac2..f7fe77dd71138 100644 --- a/crates/ruff_linter/src/rules/isort/categorize.rs +++ b/crates/ruff_linter/src/rules/isort/categorize.rs @@ -84,6 +84,7 @@ enum Reason<'a> { NoMatch, UserDefinedSection, NoSections, + DisabledSection(&'a ImportSection), } #[allow(clippy::too_many_arguments)] @@ -96,9 +97,11 @@ pub(crate) fn categorize<'a>( known_modules: &'a KnownModules, target_version: PythonVersion, no_sections: bool, + section_order: &'a [ImportSection], + default_section: &'a ImportSection, ) -> &'a ImportSection { 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__" { (&ImportSection::Known(ImportType::Future), Reason::Future) } else if no_sections { @@ -134,12 +137,14 @@ pub(crate) fn categorize<'a>( Reason::KnownFirstParty, ) } else { - ( - &ImportSection::Known(ImportType::ThirdParty), - Reason::NoMatch, - ) + (default_section, 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!( "Categorized '{}' as {:?} ({:?})", module_name, import_type, reason @@ -176,6 +181,8 @@ pub(crate) fn categorize_imports<'a>( known_modules: &'a KnownModules, target_version: PythonVersion, no_sections: bool, + section_order: &'a [ImportSection], + default_section: &'a ImportSection, ) -> BTreeMap<&'a ImportSection, ImportBlock<'a>> { let mut block_by_type: BTreeMap<&ImportSection, ImportBlock> = BTreeMap::default(); // Categorize `Stmt::Import`. @@ -189,6 +196,8 @@ pub(crate) fn categorize_imports<'a>( known_modules, target_version, no_sections, + section_order, + default_section, ); block_by_type .entry(import_type) @@ -207,6 +216,8 @@ pub(crate) fn categorize_imports<'a>( known_modules, target_version, no_sections, + section_order, + default_section, ); block_by_type .entry(classification) @@ -225,6 +236,8 @@ pub(crate) fn categorize_imports<'a>( known_modules, target_version, no_sections, + section_order, + default_section, ); block_by_type .entry(classification) @@ -243,6 +256,8 @@ pub(crate) fn categorize_imports<'a>( known_modules, target_version, no_sections, + section_order, + default_section, ); block_by_type .entry(classification) diff --git a/crates/ruff_linter/src/rules/isort/mod.rs b/crates/ruff_linter/src/rules/isort/mod.rs index a1df7209ffe62..cf5d6ba6785c4 100644 --- a/crates/ruff_linter/src/rules/isort/mod.rs +++ b/crates/ruff_linter/src/rules/isort/mod.rs @@ -163,6 +163,8 @@ fn format_import_block( &settings.known_modules, target_version, settings.no_sections, + &settings.section_order, + &settings.default_section, ); let mut output = String::new(); @@ -924,6 +926,38 @@ mod tests { 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_user_defined_{}", 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_lines_before.py"))] fn no_lines_before(path: &Path) -> Result<()> { let snapshot = format!("no_lines_before.py_{}", path.to_string_lossy()); diff --git a/crates/ruff_linter/src/rules/isort/settings.rs b/crates/ruff_linter/src/rules/isort/settings.rs index b5b13363b4c4a..f86d593a75968 100644 --- a/crates/ruff_linter/src/rules/isort/settings.rs +++ b/crates/ruff_linter/src/rules/isort/settings.rs @@ -67,6 +67,7 @@ pub struct Settings { pub lines_between_types: usize, pub forced_separate: Vec, pub section_order: Vec, + pub default_section: ImportSection, pub no_sections: bool, pub from_first: bool, pub length_sort: bool, @@ -97,6 +98,7 @@ impl Default for Settings { lines_between_types: 0, forced_separate: Vec::new(), section_order: ImportType::iter().map(ImportSection::Known).collect(), + default_section: ImportSection::Known(ImportType::ThirdParty), no_sections: false, from_first: false, length_sort: false, @@ -132,6 +134,7 @@ impl Display for Settings { self.lines_between_types, self.forced_separate | array, self.section_order | array, + self.default_section, self.no_sections, self.from_first, self.length_sort, diff --git a/crates/ruff_linter/src/rules/isort/snapshots/ruff_linter__rules__isort__tests__default_section_user_defined_default_section_user_defined.py.snap b/crates/ruff_linter/src/rules/isort/snapshots/ruff_linter__rules__isort__tests__default_section_user_defined_default_section_user_defined.py.snap new file mode 100644 index 0000000000000..d10c9da693e97 --- /dev/null +++ b/crates/ruff_linter/src/rules/isort/snapshots/ruff_linter__rules__isort__tests__default_section_user_defined_default_section_user_defined.py.snap @@ -0,0 +1,5 @@ +--- +source: crates/ruff_linter/src/rules/isort/mod.rs +assertion_line: 957 +--- + diff --git a/crates/ruff_workspace/src/options.rs b/crates/ruff_workspace/src/options.rs index 5696fce56bfdb..fde974e711b71 100644 --- a/crates/ruff_workspace/src/options.rs +++ b/crates/ruff_workspace/src/options.rs @@ -2096,6 +2096,16 @@ pub struct IsortOptions { )] pub section_order: Option>, + /// 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, + /// Put all imports into the same section bucket. /// /// For example, rather than separating standard library and third-party imports, as in: @@ -2223,6 +2233,9 @@ impl IsortOptions { 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.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() { warn_user_once!("`sections` is ignored when `no-sections` is set to `true`"); } @@ -2238,6 +2251,10 @@ impl IsortOptions { let mut section_order: Vec<_> = self .section_order .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 .known_first_party .map(|names| { @@ -2341,24 +2358,13 @@ impl IsortOptions { } } - // Add all built-in sections to `section_order`, if not already present. - for section in ImportType::iter().map(ImportSection::Known) { - if !section_order.contains(§ion) { - warn_user_once!( - "`section-order` is missing built-in section: `{:?}`", - section - ); - section_order.push(section); - } - } - - // 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); - } + // Verify that `default_section` is in `section_order`. + if !section_order.contains(&default_section) { + warn_user_once!( + "`section-order` must contain `default-section`: {:?}", + default_section, + ); + section_order.push(default_section.clone()); } Ok(isort::settings::Settings { @@ -2391,6 +2397,7 @@ impl IsortOptions { lines_between_types, forced_separate: Vec::from_iter(self.forced_separate.unwrap_or_default()), section_order, + default_section, no_sections, from_first, length_sort: self.length_sort.unwrap_or(false), diff --git a/ruff.schema.json b/ruff.schema.json index 674003ed05df2..d790b36654748 100644 --- a/ruff.schema.json +++ b/ruff.schema.json @@ -1474,6 +1474,17 @@ "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": { "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": [ From aeb1ee7ea195114b96e5cf41451ce9b682795ec6 Mon Sep 17 00:00:00 2001 From: Charlie Marsh Date: Thu, 29 Feb 2024 22:25:18 -0500 Subject: [PATCH 2/2] Add an additional test --- .../fixtures/isort/no_standard_library.py | 9 ++++++ crates/ruff_linter/src/rules/isort/mod.rs | 29 ++++++++++++++++++- ...tion_default_section_user_defined.py.snap} | 1 - ...andard_library_no_standard_library.py.snap | 29 +++++++++++++++++++ 4 files changed, 66 insertions(+), 2 deletions(-) create mode 100644 crates/ruff_linter/resources/test/fixtures/isort/no_standard_library.py rename crates/ruff_linter/src/rules/isort/snapshots/{ruff_linter__rules__isort__tests__default_section_user_defined_default_section_user_defined.py.snap => ruff_linter__rules__isort__tests__default_section_can_map_to_user_defined_section_default_section_user_defined.py.snap} (74%) create mode 100644 crates/ruff_linter/src/rules/isort/snapshots/ruff_linter__rules__isort__tests__no_standard_library_no_standard_library.py.snap diff --git a/crates/ruff_linter/resources/test/fixtures/isort/no_standard_library.py b/crates/ruff_linter/resources/test/fixtures/isort/no_standard_library.py new file mode 100644 index 0000000000000..73f88ce067a99 --- /dev/null +++ b/crates/ruff_linter/resources/test/fixtures/isort/no_standard_library.py @@ -0,0 +1,9 @@ +from __future__ import annotations + +import os +import django.settings +from library import foo +import pytz + +from . import local +import sys diff --git a/crates/ruff_linter/src/rules/isort/mod.rs b/crates/ruff_linter/src/rules/isort/mod.rs index cf5d6ba6785c4..a3a61631aef44 100644 --- a/crates/ruff_linter/src/rules/isort/mod.rs +++ b/crates/ruff_linter/src/rules/isort/mod.rs @@ -928,7 +928,10 @@ mod tests { #[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_user_defined_{}", path.to_string_lossy()); + 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 { @@ -958,6 +961,30 @@ mod tests { 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"))] fn no_lines_before(path: &Path) -> Result<()> { let snapshot = format!("no_lines_before.py_{}", path.to_string_lossy()); diff --git a/crates/ruff_linter/src/rules/isort/snapshots/ruff_linter__rules__isort__tests__default_section_user_defined_default_section_user_defined.py.snap b/crates/ruff_linter/src/rules/isort/snapshots/ruff_linter__rules__isort__tests__default_section_can_map_to_user_defined_section_default_section_user_defined.py.snap similarity index 74% rename from crates/ruff_linter/src/rules/isort/snapshots/ruff_linter__rules__isort__tests__default_section_user_defined_default_section_user_defined.py.snap rename to crates/ruff_linter/src/rules/isort/snapshots/ruff_linter__rules__isort__tests__default_section_can_map_to_user_defined_section_default_section_user_defined.py.snap index d10c9da693e97..ed369f0fd61f0 100644 --- a/crates/ruff_linter/src/rules/isort/snapshots/ruff_linter__rules__isort__tests__default_section_user_defined_default_section_user_defined.py.snap +++ b/crates/ruff_linter/src/rules/isort/snapshots/ruff_linter__rules__isort__tests__default_section_can_map_to_user_defined_section_default_section_user_defined.py.snap @@ -1,5 +1,4 @@ --- source: crates/ruff_linter/src/rules/isort/mod.rs -assertion_line: 957 --- diff --git a/crates/ruff_linter/src/rules/isort/snapshots/ruff_linter__rules__isort__tests__no_standard_library_no_standard_library.py.snap b/crates/ruff_linter/src/rules/isort/snapshots/ruff_linter__rules__isort__tests__no_standard_library_no_standard_library.py.snap new file mode 100644 index 0000000000000..8dfcfe5183d0b --- /dev/null +++ b/crates/ruff_linter/src/rules/isort/snapshots/ruff_linter__rules__isort__tests__no_standard_library_no_standard_library.py.snap @@ -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