diff --git a/Cargo.lock b/Cargo.lock index 67fdea2c65e43..7822bd527a484 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1234,6 +1234,12 @@ dependencies = [ "windows-sys", ] +[[package]] +name = "natord" +version = "1.0.9" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "308d96db8debc727c3fd9744aac51751243420e46edf401010908da7f8d5e57c" + [[package]] name = "new_debug_unreachable" version = "1.0.4" @@ -1880,6 +1886,7 @@ dependencies = [ "itertools", "libcst", "log", + "natord", "nohash-hasher", "notify", "num-bigint", diff --git a/Cargo.toml b/Cargo.toml index b593a8a5ac667..a8f3fc15e12b3 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -34,6 +34,7 @@ ignore = { version = "0.4.18" } itertools = { version = "0.10.5" } libcst = { git = "https://github.com/charliermarsh/LibCST", rev = "f2f0b7a487a8725d161fe8b3ed73a6758b21e177" } log = { version = "0.4.17" } +natord = { version = "1.0.9" } nohash-hasher = { version = "0.2.0" } notify = { version = "5.0.0" } num-bigint = { version = "0.4.3" } diff --git a/resources/test/fixtures/isort/natural_order.py b/resources/test/fixtures/isort/natural_order.py new file mode 100644 index 0000000000000..ad36cb428c62e --- /dev/null +++ b/resources/test/fixtures/isort/natural_order.py @@ -0,0 +1,16 @@ +import numpy1 +import numpy10 +import numpy2 +from numpy import ( + cos, + int8, + sin, + int32, + int64, + tan, + uint8, + uint16, + int16, + uint32, + uint64, +) diff --git a/src/isort/mod.rs b/src/isort/mod.rs index f2cd8f27b7701..b1c7d96f11ed9 100644 --- a/src/isort/mod.rs +++ b/src/isort/mod.rs @@ -1,4 +1,4 @@ -use std::cmp::Reverse; +use std::cmp::Ordering; use std::collections::{BTreeMap, BTreeSet}; use std::path::{Path, PathBuf}; @@ -9,7 +9,7 @@ use rustpython_ast::{Stmt, StmtKind}; use crate::isort::categorize::{categorize, ImportType}; use crate::isort::comments::Comment; -use crate::isort::sorting::{member_key, module_key}; +use crate::isort::sorting::{cmp_import_froms, cmp_members, cmp_modules}; use crate::isort::track::{Block, Trailer}; use crate::isort::types::{ AliasData, CommentSet, ImportBlock, ImportFromData, Importable, OrderedImportBlock, @@ -379,7 +379,7 @@ fn sort_imports(block: ImportBlock) -> OrderedImportBlock { block .import .into_iter() - .sorted_by_cached_key(|(alias, _)| module_key(alias.name, alias.asname)), + .sorted_by(|(alias1, _), (alias2, _)| cmp_modules(alias1, alias2)), ); // Sort `StmtKind::ImportFrom`. @@ -446,23 +446,19 @@ fn sort_imports(block: ImportBlock) -> OrderedImportBlock { comments, aliases .into_iter() - .sorted_by_cached_key(|(alias, _)| member_key(alias.name, alias.asname)) + .sorted_by(|(alias1, _), (alias2, _)| cmp_members(alias1, alias2)) .collect::>(), ) }) - .sorted_by_cached_key(|(import_from, _, aliases)| { - // Sort each `StmtKind::ImportFrom` by module key, breaking ties based on - // members. - ( - Reverse(import_from.level), - import_from - .module - .as_ref() - .map(|module| module_key(module, None)), - aliases - .first() - .map(|(alias, _)| member_key(alias.name, alias.asname)), - ) + .sorted_by(|(import_from1, _, aliases1), (import_from2, _, aliases2)| { + cmp_import_froms(import_from1, import_from2).then_with(|| { + match (aliases1.first(), aliases2.first()) { + (None, None) => Ordering::Equal, + (None, Some(_)) => Ordering::Less, + (Some(_), None) => Ordering::Greater, + (Some((alias1, _)), Some((alias2, _))) => cmp_members(alias1, alias2), + } + }) }), ); @@ -569,6 +565,7 @@ mod tests { #[test_case(Path::new("insert_empty_lines.py"))] #[test_case(Path::new("insert_empty_lines.pyi"))] #[test_case(Path::new("leading_prefix.py"))] + #[test_case(Path::new("natural_order.py"))] #[test_case(Path::new("no_reorder_within_section.py"))] #[test_case(Path::new("no_wrap_star.py"))] #[test_case(Path::new("order_by_type.py"))] diff --git a/src/isort/snapshots/ruff__isort__tests__natural_order.py.snap b/src/isort/snapshots/ruff__isort__tests__natural_order.py.snap new file mode 100644 index 0000000000000..bea63bbe274dc --- /dev/null +++ b/src/isort/snapshots/ruff__isort__tests__natural_order.py.snap @@ -0,0 +1,20 @@ +--- +source: src/isort/mod.rs +expression: checks +--- +- kind: UnsortedImports + location: + row: 1 + column: 0 + end_location: + row: 17 + column: 0 + fix: + content: "import numpy1\nimport numpy2\nimport numpy10\nfrom numpy import (\n cos,\n int8,\n int16,\n int32,\n int64,\n sin,\n tan,\n uint8,\n uint16,\n uint32,\n uint64,\n)\n" + location: + row: 1 + column: 0 + end_location: + row: 17 + column: 0 + diff --git a/src/isort/sorting.rs b/src/isort/sorting.rs index 2fe433410506d..22cd98006fc8e 100644 --- a/src/isort/sorting.rs +++ b/src/isort/sorting.rs @@ -1,4 +1,7 @@ /// See: +use std::cmp::Ordering; + +use crate::isort::types::{AliasData, ImportFromData}; use crate::python::string; #[derive(PartialOrd, Ord, PartialEq, Eq)] @@ -8,29 +11,57 @@ pub enum Prefix { Variables, } -pub fn module_key<'a>( - name: &'a str, - asname: Option<&'a String>, -) -> (String, &'a str, Option<&'a String>) { - (name.to_lowercase(), name, asname) +fn prefix(name: &str) -> Prefix { + if name.len() > 1 && string::is_upper(name) { + // Ex) `CONSTANT` + Prefix::Constants + } else if name.chars().next().map_or(false, char::is_uppercase) { + // Ex) `Class` + Prefix::Classes + } else { + // Ex) `variable` + Prefix::Variables + } +} + +/// Compare two top-level modules. +pub fn cmp_modules(alias1: &AliasData, alias2: &AliasData) -> Ordering { + natord::compare_ignore_case(alias1.name, alias2.name) + .then_with(|| natord::compare(alias1.name, alias2.name)) + .then_with(|| match (alias1.asname, alias2.asname) { + (None, None) => Ordering::Equal, + (None, Some(_)) => Ordering::Less, + (Some(_), None) => Ordering::Greater, + (Some(asname1), Some(asname2)) => natord::compare(asname1, asname2), + }) +} + +/// Compare two member imports within `StmtKind::ImportFrom` blocks. +pub fn cmp_members(alias1: &AliasData, alias2: &AliasData) -> Ordering { + prefix(alias1.name) + .cmp(&prefix(alias2.name)) + .then_with(|| cmp_modules(alias1, alias2)) +} + +/// Compare two relative import levels. +pub fn cmp_levels(level1: Option<&usize>, level2: Option<&usize>) -> Ordering { + match (level1, level2) { + (None, None) => Ordering::Equal, + (None, Some(_)) => Ordering::Less, + (Some(_), None) => Ordering::Greater, + (Some(level1), Some(level2)) => level2.cmp(level1), + } } -pub fn member_key<'a>( - name: &'a str, - asname: Option<&'a String>, -) -> (Prefix, String, Option<&'a String>) { - ( - if name.len() > 1 && string::is_upper(name) { - // Ex) `CONSTANT` - Prefix::Constants - } else if name.chars().next().map_or(false, char::is_uppercase) { - // Ex) `Class` - Prefix::Classes - } else { - // Ex) `variable` - Prefix::Variables - }, - name.to_lowercase(), - asname, - ) +/// Compare two `StmtKind::ImportFrom` blocks. +pub fn cmp_import_froms(import_from1: &ImportFromData, import_from2: &ImportFromData) -> Ordering { + cmp_levels(import_from1.level, import_from2.level).then_with(|| { + match (&import_from1.module, import_from2.module) { + (None, None) => Ordering::Equal, + (None, Some(_)) => Ordering::Less, + (Some(_), None) => Ordering::Greater, + (Some(module1), Some(module2)) => natord::compare_ignore_case(module1, module2) + .then_with(|| natord::compare(module1, module2)), + } + }) }