Skip to content

Commit 128e186

Browse files
committed
perf(formatter/sort_imports): Precompute import metadata (#15580)
- Before: Compute metadata for sort inside of `sort_by()` - After: Collect these data before `sort_by()`
1 parent cd31cc1 commit 128e186

File tree

4 files changed

+227
-221
lines changed

4 files changed

+227
-221
lines changed

crates/oxc_formatter/src/ir_transform/sort_imports/import_unit.rs

Lines changed: 99 additions & 170 deletions
Original file line numberDiff line numberDiff line change
@@ -1,186 +1,98 @@
1-
use std::path::Path;
1+
use std::{borrow::Cow, path::Path};
22

33
use cow_utils::CowUtils;
44
use phf::phf_set;
55

66
use crate::{formatter::format_element::FormatElement, options};
77

8-
use super::source_line::{ImportLine, SourceLine};
9-
10-
#[derive(Debug)]
11-
pub struct ImportUnits(pub Vec<SortableImport>);
12-
13-
impl IntoIterator for ImportUnits {
14-
type Item = SortableImport;
15-
type IntoIter = std::vec::IntoIter<SortableImport>;
16-
17-
fn into_iter(self) -> Self::IntoIter {
18-
self.0.into_iter()
19-
}
20-
}
21-
22-
impl ImportUnits {
23-
pub fn sort_imports(&mut self, elements: &[FormatElement], options: &options::SortImports) {
24-
let imports_len = self.0.len();
25-
26-
// Perform sorting only if needed
27-
if imports_len < 2 {
28-
return;
29-
}
30-
31-
// Separate imports into:
32-
// - sortable: indices of imports that should be sorted
33-
// - fixed: indices of imports that should be ignored
34-
// - e.g. side-effect imports when `sort_side_effects: false`, with ignore comments, etc...
35-
let mut sortable_indices = vec![];
36-
let mut fixed_indices = vec![];
37-
for (idx, si) in self.0.iter().enumerate() {
38-
if si.is_ignored(options) {
39-
fixed_indices.push(idx);
40-
} else {
41-
sortable_indices.push(idx);
42-
}
43-
}
44-
45-
// Sort indices by comparing their corresponding import groups, then sources
46-
sortable_indices.sort_by(|&a, &b| {
47-
let metadata_a = self.0[a].get_metadata(elements);
48-
let metadata_b = self.0[b].get_metadata(elements);
49-
50-
// First, compare by group index
51-
let group_idx_a = metadata_a.match_group(&options.groups);
52-
let group_idx_b = metadata_b.match_group(&options.groups);
53-
54-
let group_ord = group_idx_a.cmp(&group_idx_b);
55-
if group_ord != std::cmp::Ordering::Equal {
56-
return if options.order.is_desc() { group_ord.reverse() } else { group_ord };
57-
}
58-
59-
// Within the same group, compare by source
60-
let source_ord = if options.ignore_case {
61-
metadata_a.source.cow_to_lowercase().cmp(&metadata_b.source.cow_to_lowercase())
62-
} else {
63-
metadata_a.source.cmp(metadata_b.source)
64-
};
65-
66-
if options.order.is_desc() { source_ord.reverse() } else { source_ord }
67-
});
68-
69-
// Create a permutation map
70-
let mut permutation = vec![0; imports_len];
71-
let mut sortable_iter = sortable_indices.into_iter();
72-
for (idx, perm) in permutation.iter_mut().enumerate() {
73-
// NOTE: This is O(n), but side-effect imports are usually few
74-
if fixed_indices.contains(&idx) {
75-
*perm = idx;
76-
} else if let Some(sorted_idx) = sortable_iter.next() {
77-
*perm = sorted_idx;
78-
}
79-
}
80-
debug_assert!(
81-
permutation.iter().copied().collect::<rustc_hash::FxHashSet<_>>().len() == imports_len,
82-
"`permutation` must be a valid permutation, all indices must be unique."
83-
);
84-
85-
// Apply permutation in-place using cycle decomposition
86-
let mut visited = vec![false; imports_len];
87-
for idx in 0..imports_len {
88-
// Already visited or already in the correct position
89-
if visited[idx] || permutation[idx] == idx {
90-
continue;
91-
}
92-
// Follow the cycle
93-
let mut current = idx;
94-
loop {
95-
let next = permutation[current];
96-
visited[current] = true;
97-
if next == idx {
98-
break;
99-
}
100-
self.0.swap(current, next);
101-
current = next;
102-
}
103-
}
104-
debug_assert!(self.0.len() == imports_len, "Length must remain the same after sorting.");
105-
}
106-
}
107-
108-
// ---
8+
use super::source_line::{ImportLineMetadata, SourceLine};
1099

11010
#[derive(Debug, Clone)]
111-
pub struct SortableImport {
11+
pub struct SortableImport<'a> {
11212
pub leading_lines: Vec<SourceLine>,
11313
pub import_line: SourceLine,
14+
pub group_idx: usize,
15+
pub normalized_source: Cow<'a, str>,
16+
pub is_ignored: bool,
11417
}
11518

116-
impl SortableImport {
19+
impl<'a> SortableImport<'a> {
11720
pub fn new(leading_lines: Vec<SourceLine>, import_line: SourceLine) -> Self {
118-
Self { leading_lines, import_line }
21+
Self {
22+
leading_lines,
23+
import_line,
24+
// These will be computed by `collect_sort_keys()`
25+
group_idx: 0,
26+
normalized_source: Cow::Borrowed(""),
27+
is_ignored: false,
28+
}
11929
}
12030

121-
/// Get all import metadata in one place.
122-
pub fn get_metadata<'a>(&self, elements: &'a [FormatElement]) -> ImportMetadata<'a> {
123-
let SourceLine::Import(ImportLine {
124-
source_idx,
125-
is_side_effect,
126-
is_type_import,
127-
has_default_specifier,
128-
has_namespace_specifier,
129-
has_named_specifier,
130-
..
131-
}) = &self.import_line
31+
/// Pre-compute keys needed for sorting.
32+
#[must_use]
33+
pub fn collect_sort_keys(
34+
mut self,
35+
elements: &'a [FormatElement],
36+
options: &options::SortImports,
37+
) -> Self {
38+
let SourceLine::Import(
39+
_,
40+
ImportLineMetadata {
41+
source_idx,
42+
is_side_effect,
43+
is_type_import,
44+
has_default_specifier,
45+
has_namespace_specifier,
46+
has_named_specifier,
47+
},
48+
) = &self.import_line
13249
else {
13350
unreachable!("`import_line` must be of type `SourceLine::Import`.");
13451
};
13552

136-
// Strip quotes and params
137-
let source = match &elements[*source_idx] {
138-
FormatElement::Text { text, .. } => *text,
139-
_ => unreachable!(
140-
"`source_idx` must point to either `LocatedTokenText` or `Text` in the `elements`."
141-
),
142-
};
143-
let source = source.trim_matches('"').trim_matches('\'');
144-
let source = source.split('?').next().unwrap_or(source);
53+
let source = extract_source_text(elements, *source_idx);
14554

146-
ImportMetadata {
147-
source,
55+
// Pre-compute normalized source for case-insensitive comparison
56+
self.normalized_source =
57+
if options.ignore_case { source.cow_to_lowercase() } else { Cow::Borrowed(source) };
58+
59+
// Create group matcher from import characteristics
60+
let matcher = ImportGroupMatcher {
14861
is_side_effect: *is_side_effect,
14962
is_type_import: *is_type_import,
15063
is_style_import: is_style(source),
15164
has_default_specifier: *has_default_specifier,
15265
has_namespace_specifier: *has_namespace_specifier,
15366
has_named_specifier: *has_named_specifier,
154-
path_kind: ImportPathKind::new(source),
155-
}
156-
}
67+
path_kind: to_path_kind(source),
68+
};
69+
self.group_idx = matcher.match_group(&options.groups);
15770

158-
/// Check if this import should be ignored (not sorted).
159-
pub fn is_ignored(&self, options: &options::SortImports) -> bool {
160-
match self.import_line {
161-
SourceLine::Import(ImportLine { is_side_effect, .. }) => {
162-
// TODO: Check ignore comments?
163-
!options.sort_side_effects && is_side_effect
164-
}
165-
_ => unreachable!("`import_line` must be of type `SourceLine::Import`."),
166-
}
71+
// TODO: Check ignore comments?
72+
self.is_ignored = !options.sort_side_effects && *is_side_effect;
73+
74+
self
16775
}
16876
}
16977

170-
/// Metadata about an import for sorting purposes.
78+
// ---
79+
80+
/// Helper for matching imports to configured groups.
81+
///
82+
/// Contains all characteristics of an import needed to determine which group it belongs to,
83+
/// such as whether it's a type import, side-effect import, style import, and what kind of path it uses.
17184
#[derive(Debug, Clone)]
172-
pub struct ImportMetadata<'a> {
173-
pub source: &'a str,
174-
pub is_side_effect: bool,
175-
pub is_type_import: bool,
176-
pub is_style_import: bool,
177-
pub has_default_specifier: bool,
178-
pub has_namespace_specifier: bool,
179-
pub has_named_specifier: bool,
180-
pub path_kind: ImportPathKind,
85+
struct ImportGroupMatcher {
86+
is_side_effect: bool,
87+
is_type_import: bool,
88+
is_style_import: bool,
89+
has_default_specifier: bool,
90+
has_namespace_specifier: bool,
91+
has_named_specifier: bool,
92+
path_kind: ImportPathKind,
18193
}
18294

183-
impl ImportMetadata<'_> {
95+
impl ImportGroupMatcher {
18496
/// Match this import against the configured groups and return the group index.
18597
/// Returns the index of the first matching group, or the index of "unknown" group if present,
18698
/// or the last index + 1 if no match found.
@@ -427,6 +339,22 @@ impl ImportModifier {
427339
}
428340
}
429341

342+
// ---
343+
344+
/// Extract the import source text from format elements.
345+
///
346+
/// This removes quotes and query parameters from the source string.
347+
/// For example, `"./foo?bar"` becomes `./foo`.
348+
fn extract_source_text<'a>(elements: &'a [FormatElement], source_idx: usize) -> &'a str {
349+
let source = match &elements[source_idx] {
350+
FormatElement::Text { text, .. } => *text,
351+
_ => unreachable!("`source_idx` must point to the `Text` in the `elements`."),
352+
};
353+
354+
let source = source.trim_matches('"').trim_matches('\'');
355+
source.split('?').next().unwrap_or(source)
356+
}
357+
430358
// spellchecker:off
431359
static STYLE_EXTENSIONS: phf::Set<&'static str> = phf_set! {
432360
"css",
@@ -439,6 +367,7 @@ static STYLE_EXTENSIONS: phf::Set<&'static str> = phf_set! {
439367
};
440368
// spellchecker:on
441369

370+
/// Check if an import source is a style file based on its extension.
442371
fn is_style(source: &str) -> bool {
443372
Path::new(source)
444373
.extension()
@@ -456,12 +385,12 @@ static NODE_BUILTINS: phf::Set<&'static str> = phf_set! {
456385
"zlib",
457386
};
458387

388+
/// Check if an import source is a Node.js or Bun builtin module.
459389
fn is_builtin(source: &str) -> bool {
460390
source.starts_with("node:") || source.starts_with("bun:") || NODE_BUILTINS.contains(source)
461391
}
462392

463-
/// Classification of import path types for grouping.
464-
#[derive(Debug, Clone, Copy, PartialEq, Eq)]
393+
#[derive(Debug, Clone, Copy, PartialEq, Eq, Default)]
465394
pub enum ImportPathKind {
466395
/// Node.js builtin module (e.g., `node:fs`, `fs`)
467396
Builtin,
@@ -476,30 +405,30 @@ pub enum ImportPathKind {
476405
/// Index file import (e.g., `./`, `../`)
477406
Index,
478407
/// Unknown or unclassified
408+
#[default]
479409
Unknown,
480410
}
481411

482-
impl ImportPathKind {
483-
fn new(source: &str) -> Self {
484-
if is_builtin(source) {
485-
return Self::Builtin;
486-
}
412+
/// Determine the path kind for an import source.
413+
fn to_path_kind(source: &str) -> ImportPathKind {
414+
if is_builtin(source) {
415+
return ImportPathKind::Builtin;
416+
}
487417

488-
if source.starts_with('.') {
489-
if source == "." || source == ".." || source.ends_with('/') {
490-
return Self::Index;
491-
}
492-
if source.starts_with("../") {
493-
return Self::Parent;
494-
}
495-
return Self::Sibling;
418+
if source.starts_with('.') {
419+
if source == "." || source == ".." || source.ends_with('/') {
420+
return ImportPathKind::Index;
496421
}
497-
498-
// TODO: This can be changed via `options.internalPattern`
499-
if source.starts_with('~') || source.starts_with('@') {
500-
return Self::Internal;
422+
if source.starts_with("../") {
423+
return ImportPathKind::Parent;
501424
}
425+
return ImportPathKind::Sibling;
426+
}
502427

503-
Self::External
428+
// TODO: This can be changed via `options.internalPattern`
429+
if source.starts_with('~') || source.starts_with('@') {
430+
return ImportPathKind::Internal;
504431
}
432+
433+
ImportPathKind::External
505434
}

0 commit comments

Comments
 (0)