Skip to content

Commit ab35571

Browse files
committed
fix(formatter/sort-imports): Check comment after newline
1 parent 8babdf9 commit ab35571

File tree

5 files changed

+294
-41
lines changed

5 files changed

+294
-41
lines changed

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

Lines changed: 41 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -153,41 +153,52 @@ impl SortImportsTransform {
153153
// After sorting, flatten everything back to `FormatElement`s.
154154
let mut next_elements = ArenaVec::with_capacity_in(prev_elements.len(), allocator);
155155

156-
let mut chunks_iter = chunks.into_iter().enumerate().peekable();
157-
while let Some((_idx, chunk)) = chunks_iter.next() {
156+
let mut chunks_iter = chunks.into_iter().peekable();
157+
while let Some(chunk) = chunks_iter.next() {
158158
match chunk {
159159
// Boundary chunks: Just output as-is
160160
PartitionedChunk::Boundary(line) => {
161161
line.write(prev_elements, &mut next_elements, true);
162162
}
163163
// Import chunks: Sort and output
164164
PartitionedChunk::Imports(_) => {
165-
// For ease of implementation, we will convert `ImportChunk` into multiple `SortableImport`s.
165+
// Convert `ImportChunk` into `SortableImport`s.
166+
//
167+
// `SortableImport` is a logical unit of 1 import statement with its leading lines.
168+
// `OrphanContent` tracks comments separated by empty lines with their slot positions.
169+
//
170+
// Comments attach based on empty line separation:
171+
// - Comments directly before an `import` (no empty line) → `SortableImport.leading_lines`
172+
// - Comments followed by an empty line → `orphan_contents` (stay at slot position)
166173
//
167-
// `SortableImport` is a logical unit of 1 import statement + its N leading lines.
168-
// And there may be trailing lines after all import statements in the chunk.
169174
// e.g.
170175
// ```
171-
// const THIS_IS_BOUNDARY = true;
172-
// // comment for A
173-
// import A from "a"; // sortable1
174-
// import B from "b"; // sortable2
175-
//
176-
// // comment for C and empty line above + below
176+
// // orphan (after_slot: None)
177177
//
178-
// // another comment for C
179-
// import C from "c"; // sortable3
180-
// // trailing comment and empty line below for this chunk
178+
// // leading for A
179+
// import A from "a";
180+
// // orphan (after_slot: Some(0))
181181
//
182-
// const YET_ANOTHER_BOUNDARY = true;
182+
// // leading for B
183+
// import B from "b";
184+
// // chunk trailing
183185
// ```
184-
let (sorted_imports, trailing_lines) =
186+
let (sorted_imports, orphan_contents, trailing_lines) =
185187
chunk.into_sorted_import_units(&self.groups, &self.options);
186188

187-
// Output sorted import units
189+
// Output leading orphan content (after_slot: None)
190+
for orphan in &orphan_contents {
191+
if orphan.after_slot.is_none() {
192+
for line in &orphan.lines {
193+
line.write(prev_elements, &mut next_elements, true);
194+
}
195+
}
196+
}
197+
198+
// Output sorted import units with orphan content at their slot positions
188199
let mut prev_group_idx = None;
189200
let mut prev_was_ignored = false;
190-
for sorted_import in sorted_imports {
201+
for (slot_idx, sorted_import) in sorted_imports.iter().enumerate() {
191202
// Insert newline when:
192203
// 1. Group changes
193204
// 2. Previous import was not ignored (don't insert after ignored)
@@ -204,16 +215,25 @@ impl SortImportsTransform {
204215
}
205216

206217
// Output leading lines and import line
207-
for line in sorted_import.leading_lines {
218+
for line in &sorted_import.leading_lines {
208219
line.write(
209220
prev_elements,
210221
&mut next_elements,
211222
self.options.partition_by_newline,
212223
);
213224
}
214225
sorted_import.import_line.write(prev_elements, &mut next_elements, false);
226+
227+
// Output orphan content that belongs after this slot
228+
for orphan in &orphan_contents {
229+
if orphan.after_slot == Some(slot_idx) {
230+
for line in &orphan.lines {
231+
line.write(prev_elements, &mut next_elements, false);
232+
}
233+
}
234+
}
215235
}
216-
// And output trailing lines
236+
// And output chunk's trailing lines
217237
//
218238
// Special care is needed for the last empty line.
219239
// We should preserve it only if the next chunk is a boundary.
@@ -235,7 +255,7 @@ impl SortImportsTransform {
235255
// ```
236256
let next_chunk_is_boundary = chunks_iter
237257
.peek()
238-
.is_some_and(|(_, c)| matches!(c, PartitionedChunk::Boundary(_)));
258+
.is_some_and(|c| matches!(c, PartitionedChunk::Boundary(_)));
239259
for (idx, line) in trailing_lines.iter().enumerate() {
240260
let is_last_empty_line =
241261
idx == trailing_lines.len() - 1 && matches!(line, SourceLine::Empty);

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

Lines changed: 84 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,25 @@
1-
use crate::ir_transform::sort_imports::{
2-
compute_metadata::compute_import_metadata,
3-
group_config::GroupName,
4-
options::SortImportsOptions,
5-
sortable_imports::{SortSortableImports, SortableImport},
6-
source_line::SourceLine,
1+
use crate::{
2+
formatter::format_element::LineMode,
3+
ir_transform::sort_imports::{
4+
compute_metadata::compute_import_metadata,
5+
group_config::GroupName,
6+
options::SortImportsOptions,
7+
sortable_imports::{SortSortableImports, SortableImport},
8+
source_line::SourceLine,
9+
},
710
};
811

12+
/// Orphan content (comments/empty lines separated by empty line from the next import).
13+
/// These stay at their original slot position after sorting.
14+
#[derive(Debug)]
15+
pub struct OrphanContent<'a> {
16+
pub lines: Vec<SourceLine<'a>>,
17+
/// The slot position:
18+
/// - `None`: before the first import (leading orphan)
19+
/// - `Some(n)`: after the Nth import (0-indexed)
20+
pub after_slot: Option<usize>,
21+
}
22+
923
#[derive(Debug)]
1024
pub enum PartitionedChunk<'a> {
1125
/// A chunk containing import statements,
@@ -42,39 +56,90 @@ impl<'a> PartitionedChunk<'a> {
4256
matches!(self, Self::Imports(lines) if lines.is_empty())
4357
}
4458

45-
/// Convert this import chunk into a list of sortable import units and trailing lines.
46-
/// Returns a tuple of `(sortable_imports, trailing_lines)`.
59+
/// Convert this import chunk into `SortableImport` units with `OrphanContent`.
60+
/// Returns a tuple of `(sortable_imports, orphan_contents, trailing_lines)`.
61+
///
62+
/// - `sortable_imports`: Import statements with their attached leading lines.
63+
/// - `leading_lines`: Comments directly before this import (no empty line between).
64+
/// - `orphan_contents`: Orphan comments (separated by empty line from next import) with their slot positions.
65+
/// - `after_slot: None` = before first import, `Some(n)` = after slot n
66+
/// - `trailing_lines`: Lines at the end of the chunk after all imports.
4767
#[must_use]
4868
pub fn into_sorted_import_units(
4969
self,
5070
groups: &[Vec<GroupName>],
5171
options: &SortImportsOptions,
52-
) -> (Vec<SortableImport<'a>>, Vec<SourceLine<'a>>) {
72+
) -> (Vec<SortableImport<'a>>, Vec<OrphanContent<'a>>, Vec<SourceLine<'a>>) {
5373
let Self::Imports(lines) = self else {
5474
unreachable!(
5575
"`into_import_units()` must be called on `PartitionedChunk::Imports` only."
5676
);
5777
};
5878

59-
let mut sortable_imports = vec![];
60-
let mut current_leading_lines = vec![];
79+
let mut sortable_imports: Vec<SortableImport<'a>> = vec![];
80+
let mut orphan_contents: Vec<OrphanContent<'a>> = vec![];
81+
82+
// Comments separated from the next import by empty line.
83+
// These stay at their slot position, not attached to any import.
84+
let mut orphan_pending: Vec<SourceLine<'a>> = vec![];
85+
// Comments directly before the next import (no empty line between).
86+
// These attach to the next import as leading lines.
87+
let mut current_pending: Vec<SourceLine<'a>> = vec![];
88+
6189
for line in lines {
6290
match line {
6391
SourceLine::Import(_, ref metadata) => {
92+
// Handle orphan content (separated by empty line from this import)
93+
// These stay at their original slot position, not attached to any import.
94+
if !orphan_pending.is_empty() {
95+
// Slot position: None = before first import, Some(n) = after slot n
96+
let after_slot = if sortable_imports.is_empty() {
97+
None
98+
} else {
99+
Some(sortable_imports.len() - 1)
100+
};
101+
// For leading orphans (None), keep all lines including empty lines
102+
// For other orphans, keep only comments
103+
let lines: Vec<_> = if after_slot.is_none() {
104+
std::mem::take(&mut orphan_pending)
105+
} else {
106+
std::mem::take(&mut orphan_pending)
107+
.into_iter()
108+
.filter_map(|orphan| {
109+
if let SourceLine::CommentOnly(range, _) = orphan {
110+
Some(SourceLine::CommentOnly(range, LineMode::Hard))
111+
} else {
112+
None
113+
}
114+
})
115+
.collect()
116+
};
117+
if !lines.is_empty() {
118+
orphan_contents.push(OrphanContent { lines, after_slot });
119+
}
120+
}
121+
64122
let is_side_effect = metadata.is_side_effect;
65123
let (group_idx, normalized_source, is_ignored) =
66124
compute_import_metadata(metadata, groups, options);
125+
67126
sortable_imports.push(SortableImport {
68-
leading_lines: std::mem::take(&mut current_leading_lines),
127+
leading_lines: std::mem::take(&mut current_pending),
69128
import_line: line,
70129
is_side_effect,
71130
group_idx,
72131
normalized_source,
73132
is_ignored,
74133
});
75134
}
76-
SourceLine::CommentOnly(..) | SourceLine::Empty => {
77-
current_leading_lines.push(line);
135+
SourceLine::Empty => {
136+
// Empty line separates comments from the next import.
137+
// Move `current_pending` to `orphan_pending`, then add empty line.
138+
orphan_pending.append(&mut current_pending);
139+
orphan_pending.push(line);
140+
}
141+
SourceLine::CommentOnly(..) => {
142+
current_pending.push(line);
78143
}
79144
SourceLine::Others(..) => {
80145
unreachable!(
@@ -84,12 +149,14 @@ impl<'a> PartitionedChunk<'a> {
84149
}
85150
}
86151

87-
// Any remaining comments/lines are trailing
88-
let trailing_lines = current_leading_lines;
152+
// Any remaining lines are trailing
153+
// Combine orphan and current pending as trailing lines
154+
orphan_pending.append(&mut current_pending);
155+
let trailing_lines = orphan_pending;
89156

90157
// Let's sort this chunk!
91158
sortable_imports.sort(options);
92159

93-
(sortable_imports, trailing_lines)
160+
(sortable_imports, orphan_contents, trailing_lines)
94161
}
95162
}

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ use crate::ir_transform::sort_imports::{options::SortImportsOptions, source_line
66

77
#[derive(Debug)]
88
pub struct SortableImport<'a> {
9+
/// Comments directly before this import (no empty line between).
910
pub leading_lines: Vec<SourceLine<'a>>,
1011
pub import_line: SourceLine<'a>,
1112
// These are used for sorting and computed by `compute_import_metadata()`

crates/oxc_formatter/tests/ir_transform/mod.rs

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,8 @@ pub fn assert_format(code: &str, config_json: &str, expected: &str) {
1515
actual, expected,
1616
r"
1717
💥 First format does not match expected!
18+
============== input ==============
19+
{code}
1820
============== actual =============
1921
{actual}
2022
============= expected ============
@@ -25,13 +27,15 @@ pub fn assert_format(code: &str, config_json: &str, expected: &str) {
2527
);
2628

2729
// Check idempotency
28-
let actual = format_code(&actual, &options);
30+
let actual2 = format_code(&actual, &options);
2931
assert_eq!(
30-
actual, expected,
32+
actual2, expected,
3133
r"
3234
💥 Formatting is not idempotent!
33-
============== actual =============
35+
============== input ==============
3436
{actual}
37+
============== actual =============
38+
{actual2}
3539
============= expected ============
3640
{expected}
3741
============== config =============

0 commit comments

Comments
 (0)