Skip to content

Commit 5731e39

Browse files
lucablucab
authored
refactor(ast)!: store span details inside comment struct (#4132)
This tweaks `Comment` definition in order to internally store the start and end position of its span. Closes: #4069
1 parent 714bf1d commit 5731e39

File tree

26 files changed

+198
-184
lines changed

26 files changed

+198
-184
lines changed

‎crates/oxc_ast/src/trivia.rs

+30-26
Original file line numberDiff line numberDiff line change
@@ -12,13 +12,15 @@ use oxc_span::Span;
1212
#[derive(Debug, Clone, Copy)]
1313
pub struct Comment {
1414
pub kind: CommentKind,
15-
pub end: u32,
15+
/// The span of the comment text (without leading/trailing delimiters).
16+
pub span: Span,
1617
}
1718

1819
impl Comment {
1920
#[inline]
20-
pub fn new(end: u32, kind: CommentKind) -> Self {
21-
Self { kind, end }
21+
pub fn new(start: u32, end: u32, kind: CommentKind) -> Self {
22+
let span = Span::new(start, end);
23+
Self { kind, span }
2224
}
2325
}
2426

@@ -41,7 +43,7 @@ impl CommentKind {
4143
}
4244

4345
/// Sorted set of unique trivia comments, in ascending order by starting position.
44-
pub type SortedComments = Box<[(u32, Comment)]>;
46+
pub type SortedComments = Box<[Comment]>;
4547

4648
#[derive(Debug, Clone, Default)]
4749
pub struct Trivias(Arc<TriviasImpl>);
@@ -51,7 +53,7 @@ pub struct TriviasImpl {
5153
/// Unique comments, ordered by increasing span-start.
5254
comments: SortedComments,
5355

54-
irregular_whitespaces: Vec<Span>,
56+
irregular_whitespaces: Box<[Span]>,
5557
}
5658

5759
impl Deref for Trivias {
@@ -65,11 +67,14 @@ impl Deref for Trivias {
6567

6668
impl Trivias {
6769
pub fn new(comments: SortedComments, irregular_whitespaces: Vec<Span>) -> Trivias {
68-
Self(Arc::new(TriviasImpl { comments, irregular_whitespaces }))
70+
Self(Arc::new(TriviasImpl {
71+
comments,
72+
irregular_whitespaces: irregular_whitespaces.into_boxed_slice(),
73+
}))
6974
}
7075

71-
pub fn comments(&self) -> impl Iterator<Item = (CommentKind, Span)> + '_ {
72-
self.comments.iter().map(|(start, comment)| (comment.kind, Span::new(*start, comment.end)))
76+
pub fn comments(&self) -> impl Iterator<Item = &Comment> {
77+
self.comments.iter()
7378
}
7479

7580
pub fn comments_range<R>(&self, range: R) -> CommentsRange<'_>
@@ -83,21 +88,21 @@ impl Trivias {
8388
self.comments_range(span.start..span.end).count() > 0
8489
}
8590

86-
pub fn irregular_whitespaces(&self) -> &Vec<Span> {
91+
pub fn irregular_whitespaces(&self) -> &[Span] {
8792
&self.irregular_whitespaces
8893
}
8994
}
9095

9196
/// Double-ended iterator over a range of comments, by starting position.
9297
pub struct CommentsRange<'a> {
93-
comments: &'a [(u32, Comment)],
98+
comments: &'a [Comment],
9499
range: (Bound<u32>, Bound<u32>),
95100
current_start: usize,
96101
current_end: usize,
97102
}
98103

99104
impl<'a> CommentsRange<'a> {
100-
fn new(comments: &'a [(u32, Comment)], start: Bound<u32>, end: Bound<u32>) -> Self {
105+
fn new(comments: &'a [Comment], start: Bound<u32>, end: Bound<u32>) -> Self {
101106
// Directly skip all comments that are already known to start
102107
// outside the requested range.
103108
let partition_start = {
@@ -106,15 +111,15 @@ impl<'a> CommentsRange<'a> {
106111
Bound::Included(x) => x,
107112
Bound::Excluded(x) => x.saturating_add(1),
108113
};
109-
comments.partition_point(|(start, _)| *start < range_start)
114+
comments.partition_point(|comment| comment.span.start < range_start)
110115
};
111116
let partition_end = {
112117
let range_end = match end {
113118
Bound::Unbounded => u32::MAX,
114119
Bound::Included(x) => x,
115120
Bound::Excluded(x) => x.saturating_sub(1),
116121
};
117-
comments.partition_point(|(start, _)| *start <= range_end)
122+
comments.partition_point(|comment| comment.span.start <= range_end)
118123
};
119124
Self {
120125
comments,
@@ -126,14 +131,14 @@ impl<'a> CommentsRange<'a> {
126131
}
127132

128133
impl<'c> Iterator for CommentsRange<'c> {
129-
type Item = (&'c u32, &'c Comment);
134+
type Item = &'c Comment;
130135

131136
fn next(&mut self) -> Option<Self::Item> {
132137
if self.current_start < self.current_end {
133-
for (start, comment) in &self.comments[self.current_start..self.current_end] {
138+
for comment in &self.comments[self.current_start..self.current_end] {
134139
self.current_start = self.current_start.saturating_add(1);
135-
if self.range.contains(start) {
136-
return Some((start, comment));
140+
if self.range.contains(&comment.span.start) {
141+
return Some(comment);
137142
}
138143
}
139144
}
@@ -149,11 +154,10 @@ impl<'c> Iterator for CommentsRange<'c> {
149154
impl<'c> DoubleEndedIterator for CommentsRange<'c> {
150155
fn next_back(&mut self) -> Option<Self::Item> {
151156
if self.current_start < self.current_end {
152-
for (start, comment) in self.comments[self.current_start..self.current_end].iter().rev()
153-
{
157+
for comment in self.comments[self.current_start..self.current_end].iter().rev() {
154158
self.current_end = self.current_end.saturating_sub(1);
155-
if self.range.contains(start) {
156-
return Some((start, comment));
159+
if self.range.contains(&comment.span.start) {
160+
return Some(comment);
157161
}
158162
}
159163
}
@@ -170,11 +174,11 @@ mod test {
170174
#[test]
171175
fn test_comments_range() {
172176
let comments: SortedComments = vec![
173-
(0, Comment { end: 4, kind: CommentKind::SingleLine }),
174-
(5, Comment { end: 9, kind: CommentKind::SingleLine }),
175-
(10, Comment { end: 13, kind: CommentKind::SingleLine }),
176-
(14, Comment { end: 17, kind: CommentKind::SingleLine }),
177-
(18, Comment { end: 23, kind: CommentKind::SingleLine }),
177+
Comment { span: Span::new(0, 4), kind: CommentKind::SingleLine },
178+
Comment { span: Span::new(5, 9), kind: CommentKind::SingleLine },
179+
Comment { span: Span::new(10, 13), kind: CommentKind::SingleLine },
180+
Comment { span: Span::new(14, 17), kind: CommentKind::SingleLine },
181+
Comment { span: Span::new(18, 23), kind: CommentKind::SingleLine },
178182
]
179183
.into_boxed_slice();
180184
let full_len = comments.len();

‎crates/oxc_codegen/src/annotation_comment.rs

+13-17
Original file line numberDiff line numberDiff line change
@@ -12,42 +12,38 @@ static MATCHER: Lazy<DoubleArrayAhoCorasick<usize>> = Lazy::new(|| {
1212
pub fn get_leading_annotate_comment<const MINIFY: bool>(
1313
node_start: u32,
1414
codegen: &mut Codegen<{ MINIFY }>,
15-
) -> Option<(u32, Comment)> {
15+
) -> Option<Comment> {
1616
let maybe_leading_comment = codegen.try_get_leading_comment(node_start);
17-
let (comment_start, comment) = maybe_leading_comment?;
17+
let comment = maybe_leading_comment?;
1818
let real_end = match comment.kind {
19-
CommentKind::SingleLine => comment.end,
20-
CommentKind::MultiLine => comment.end + 2,
19+
CommentKind::SingleLine => comment.span.end,
20+
CommentKind::MultiLine => comment.span.end + 2,
2121
};
2222
let source_code = codegen.source_text;
2323
let content_between = &source_code[real_end as usize..node_start as usize];
2424
// Used for VariableDeclaration (Rollup only respects "const" and only for the first one)
2525
if content_between.chars().all(|ch| ch.is_ascii_whitespace()) {
26-
let comment_content = &source_code[*comment_start as usize..comment.end as usize];
26+
let comment_content = &source_code[comment.span.start as usize..comment.span.end as usize];
2727
if MATCHER.find_iter(&comment_content).next().is_some() {
28-
return Some((*comment_start, *comment));
28+
return Some(*comment);
2929
}
3030
None
3131
} else {
3232
None
3333
}
3434
}
3535

36-
pub fn print_comment<const MINIFY: bool>(
37-
comment_start: u32,
38-
comment: Comment,
39-
p: &mut Codegen<{ MINIFY }>,
40-
) {
36+
pub fn print_comment<const MINIFY: bool>(comment: Comment, p: &mut Codegen<{ MINIFY }>) {
4137
match comment.kind {
4238
CommentKind::SingleLine => {
4339
p.print_str("//");
44-
p.print_range_of_source_code(comment_start as usize..comment.end as usize);
40+
p.print_range_of_source_code(comment.span.start as usize..comment.span.end as usize);
4541
p.print_soft_newline();
4642
p.print_indent();
4743
}
4844
CommentKind::MultiLine => {
4945
p.print_str("/*");
50-
p.print_range_of_source_code(comment_start as usize..comment.end as usize);
46+
p.print_range_of_source_code(comment.span.start as usize..comment.span.end as usize);
5147
p.print_str("*/");
5248
p.print_soft_space();
5349
}
@@ -58,11 +54,11 @@ pub fn gen_comment<const MINIFY: bool>(node_start: u32, codegen: &mut Codegen<{
5854
if !codegen.comment_options.preserve_annotate_comments {
5955
return;
6056
}
61-
if let Some((comment_start, comment)) = codegen.try_take_moved_comment(node_start) {
62-
print_comment::<MINIFY>(comment_start, comment, codegen);
57+
if let Some(comment) = codegen.try_take_moved_comment(node_start) {
58+
print_comment::<MINIFY>(comment, codegen);
6359
}
6460
let maybe_leading_annotate_comment = get_leading_annotate_comment(node_start, codegen);
65-
if let Some((comment_start, comment)) = maybe_leading_annotate_comment {
66-
print_comment::<MINIFY>(comment_start, comment, codegen);
61+
if let Some(comment) = maybe_leading_annotate_comment {
62+
print_comment::<MINIFY>(comment, codegen);
6763
}
6864
}

‎crates/oxc_codegen/src/lib.rs

+4-4
Original file line numberDiff line numberDiff line change
@@ -517,7 +517,7 @@ impl<'a, const MINIFY: bool> Codegen<'a, MINIFY> {
517517
}
518518
}
519519

520-
pub(crate) type MoveCommentMap = FxHashMap<u32, (u32, Comment)>;
520+
pub(crate) type MoveCommentMap = FxHashMap<u32, Comment>;
521521

522522
// Comment related
523523
impl<'a, const MINIFY: bool> Codegen<'a, MINIFY> {
@@ -541,15 +541,15 @@ impl<'a, const MINIFY: bool> Codegen<'a, MINIFY> {
541541
///
542542
/// }, b = 10000;
543543
/// ```
544-
fn move_comment(&mut self, position: u32, full_comment_info: (u32, Comment)) {
544+
fn move_comment(&mut self, position: u32, full_comment_info: Comment) {
545545
self.move_comment_map.insert(position, full_comment_info);
546546
}
547547

548-
fn try_get_leading_comment(&self, start: u32) -> Option<(&u32, &Comment)> {
548+
fn try_get_leading_comment(&self, start: u32) -> Option<&Comment> {
549549
self.trivias.comments_range(0..start).next_back()
550550
}
551551

552-
fn try_take_moved_comment(&mut self, node_start: u32) -> Option<(u32, Comment)> {
552+
fn try_take_moved_comment(&mut self, node_start: u32) -> Option<Comment> {
553553
self.move_comment_map.remove(&node_start)
554554
}
555555
}

‎crates/oxc_linter/src/disable_directives.rs

+29-19
Original file line numberDiff line numberDiff line change
@@ -93,8 +93,8 @@ impl<'a> DisableDirectivesBuilder<'a> {
9393
// This algorithm iterates through the comments and builds all intervals
9494
// for matching disable and enable pairs.
9595
// Wrongly ordered matching pairs are not taken into consideration.
96-
for (_, span) in self.trivias.clone().comments() {
97-
let text = span.source_text(self.source_text);
96+
for comment in self.trivias.clone().comments() {
97+
let text = comment.span.source_text(self.source_text);
9898
let text = text.trim_start();
9999

100100
if let Some(text) =
@@ -103,66 +103,72 @@ impl<'a> DisableDirectivesBuilder<'a> {
103103
// `eslint-disable`
104104
if text.trim().is_empty() {
105105
if self.disable_all_start.is_none() {
106-
self.disable_all_start = Some(span.end);
106+
self.disable_all_start = Some(comment.span.end);
107107
}
108-
self.disable_all_comments.push(span);
108+
self.disable_all_comments.push(comment.span);
109109
continue;
110110
}
111111

112112
// `eslint-disable-next-line`
113113
if let Some(text) = text.strip_prefix("-next-line") {
114114
// Get the span up to the next new line
115-
let stop = self.source_text[span.end as usize..]
115+
let stop = self.source_text[comment.span.end as usize..]
116116
.lines()
117117
.take(2)
118-
.fold(span.end, |acc, line| acc + line.len() as u32);
118+
.fold(comment.span.end, |acc, line| acc + line.len() as u32);
119119
if text.trim().is_empty() {
120-
self.add_interval(span.end, stop, DisabledRule::All);
121-
self.disable_all_comments.push(span);
120+
self.add_interval(comment.span.end, stop, DisabledRule::All);
121+
self.disable_all_comments.push(comment.span);
122122
} else {
123123
// `eslint-disable-next-line rule_name1, rule_name2`
124124
let mut rules = vec![];
125125
Self::get_rule_names(text, |rule_name| {
126-
self.add_interval(span.end, stop, DisabledRule::Single(rule_name));
126+
self.add_interval(
127+
comment.span.end,
128+
stop,
129+
DisabledRule::Single(rule_name),
130+
);
127131
rules.push(rule_name);
128132
});
129-
self.disable_rule_comments.push(DisableRuleComment { span, rules });
133+
self.disable_rule_comments
134+
.push(DisableRuleComment { span: comment.span, rules });
130135
}
131136
continue;
132137
}
133138

134139
// `eslint-disable-line`
135140
if let Some(text) = text.strip_prefix("-line") {
136141
// Get the span between the preceding newline to this comment
137-
let start = self.source_text[..=span.start as usize]
142+
let start = self.source_text[..=comment.span.start as usize]
138143
.lines()
139144
.next_back()
140-
.map_or(0, |line| span.start - (line.len() as u32 - 1));
141-
let stop = span.start;
145+
.map_or(0, |line| comment.span.start - (line.len() as u32 - 1));
146+
let stop = comment.span.start;
142147

143148
// `eslint-disable-line`
144149
if text.trim().is_empty() {
145150
self.add_interval(start, stop, DisabledRule::All);
146-
self.disable_all_comments.push(span);
151+
self.disable_all_comments.push(comment.span);
147152
} else {
148153
// `eslint-disable-line rule-name1, rule-name2`
149154
let mut rules = vec![];
150155
Self::get_rule_names(text, |rule_name| {
151156
self.add_interval(start, stop, DisabledRule::Single(rule_name));
152157
rules.push(rule_name);
153158
});
154-
self.disable_rule_comments.push(DisableRuleComment { span, rules });
159+
self.disable_rule_comments
160+
.push(DisableRuleComment { span: comment.span, rules });
155161
}
156162
continue;
157163
}
158164

159165
// `eslint-disable rule-name1, rule-name2`
160166
let mut rules = vec![];
161167
Self::get_rule_names(text, |rule_name| {
162-
self.disable_start_map.entry(rule_name).or_insert(span.end);
168+
self.disable_start_map.entry(rule_name).or_insert(comment.span.end);
163169
rules.push(rule_name);
164170
});
165-
self.disable_rule_comments.push(DisableRuleComment { span, rules });
171+
self.disable_rule_comments.push(DisableRuleComment { span: comment.span, rules });
166172

167173
continue;
168174
}
@@ -173,13 +179,17 @@ impl<'a> DisableDirectivesBuilder<'a> {
173179
// `eslint-enable`
174180
if text.trim().is_empty() {
175181
if let Some(start) = self.disable_all_start.take() {
176-
self.add_interval(start, span.start, DisabledRule::All);
182+
self.add_interval(start, comment.span.start, DisabledRule::All);
177183
}
178184
} else {
179185
// `eslint-enable rule-name1, rule-name2`
180186
Self::get_rule_names(text, |rule_name| {
181187
if let Some(start) = self.disable_start_map.remove(rule_name) {
182-
self.add_interval(start, span.start, DisabledRule::Single(rule_name));
188+
self.add_interval(
189+
start,
190+
comment.span.start,
191+
DisabledRule::Single(rule_name),
192+
);
183193
}
184194
});
185195
}

‎crates/oxc_linter/src/rules/eslint/default_case.rs

+2-4
Original file line numberDiff line numberDiff line change
@@ -72,10 +72,8 @@ impl Rule for DefaultCase {
7272
.trivias()
7373
.comments_range(last_case.span.start..switch.span.end)
7474
.last()
75-
.is_some_and(|(start, comment)| {
76-
let raw = Span::new(*start, comment.end)
77-
.source_text(ctx.semantic().source_text())
78-
.trim();
75+
.is_some_and(|comment| {
76+
let raw = comment.span.source_text(ctx.semantic().source_text()).trim();
7977
match &self.comment_pattern {
8078
Some(comment_pattern) => comment_pattern.is_match(raw),
8179
None => raw.eq_ignore_ascii_case("no default"),

0 commit comments

Comments
 (0)