Skip to content

refactor(ast)!: store span details inside comment struct #4132

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Boshen merged 6 commits into from
Jul 9, 2024
Merged

refactor(ast)!: store span details inside comment struct #4132

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
0f5e3e0
refactor(ast)!: store span details inside comment struct
lucab Jul 9, 2024
9b2d773
ast: inline span getter
lucab Jul 9, 2024
e8f7dbe
ast: change comment iterator item type
lucab Jul 9, 2024
0c7e4fc
comment: make span public
lucab Jul 9, 2024
28c79b9
wasm: fix iterator type
lucab Jul 9, 2024
019489a
prettier: fix clippy lint
lucab Jul 9, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
56 changes: 30 additions & 26 deletions crates/oxc_ast/src/trivia.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,13 +12,15 @@ use oxc_span::Span;
#[derive(Debug, Clone, Copy)]
pub struct Comment {
pub kind: CommentKind,
pub end: u32,
/// The span of the comment text (without leading/trailing delimiters).
pub span: Span,
}

impl Comment {
#[inline]
pub fn new(end: u32, kind: CommentKind) -> Self {
Self { kind, end }
pub fn new(start: u32, end: u32, kind: CommentKind) -> Self {
let span = Span::new(start, end);
Self { kind, span }
}
}

Expand All @@ -41,7 +43,7 @@ impl CommentKind {
}

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

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

irregular_whitespaces: Vec<Span>,
irregular_whitespaces: Box<[Span]>,
}

impl Deref for Trivias {
Expand All @@ -65,11 +67,14 @@ impl Deref for Trivias {

impl Trivias {
pub fn new(comments: SortedComments, irregular_whitespaces: Vec<Span>) -> Trivias {
Self(Arc::new(TriviasImpl { comments, irregular_whitespaces }))
Self(Arc::new(TriviasImpl {
comments,
irregular_whitespaces: irregular_whitespaces.into_boxed_slice(),
}))
}

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

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

pub fn irregular_whitespaces(&self) -> &Vec<Span> {
pub fn irregular_whitespaces(&self) -> &[Span] {
&self.irregular_whitespaces
}
}

/// Double-ended iterator over a range of comments, by starting position.
pub struct CommentsRange<'a> {
comments: &'a [(u32, Comment)],
comments: &'a [Comment],
range: (Bound<u32>, Bound<u32>),
current_start: usize,
current_end: usize,
}

impl<'a> CommentsRange<'a> {
fn new(comments: &'a [(u32, Comment)], start: Bound<u32>, end: Bound<u32>) -> Self {
fn new(comments: &'a [Comment], start: Bound<u32>, end: Bound<u32>) -> Self {
// Directly skip all comments that are already known to start
// outside the requested range.
let partition_start = {
Expand All @@ -106,15 +111,15 @@ impl<'a> CommentsRange<'a> {
Bound::Included(x) => x,
Bound::Excluded(x) => x.saturating_add(1),
};
comments.partition_point(|(start, _)| *start < range_start)
comments.partition_point(|comment| comment.span.start < range_start)
};
let partition_end = {
let range_end = match end {
Bound::Unbounded => u32::MAX,
Bound::Included(x) => x,
Bound::Excluded(x) => x.saturating_sub(1),
};
comments.partition_point(|(start, _)| *start <= range_end)
comments.partition_point(|comment| comment.span.start <= range_end)
};
Self {
comments,
Expand All @@ -126,14 +131,14 @@ impl<'a> CommentsRange<'a> {
}

impl<'c> Iterator for CommentsRange<'c> {
type Item = (&'c u32, &'c Comment);
type Item = &'c Comment;

fn next(&mut self) -> Option<Self::Item> {
if self.current_start < self.current_end {
for (start, comment) in &self.comments[self.current_start..self.current_end] {
for comment in &self.comments[self.current_start..self.current_end] {
self.current_start = self.current_start.saturating_add(1);
if self.range.contains(start) {
return Some((start, comment));
if self.range.contains(&comment.span.start) {
return Some(comment);
}
}
}
Expand All @@ -149,11 +154,10 @@ impl<'c> Iterator for CommentsRange<'c> {
impl<'c> DoubleEndedIterator for CommentsRange<'c> {
fn next_back(&mut self) -> Option<Self::Item> {
if self.current_start < self.current_end {
for (start, comment) in self.comments[self.current_start..self.current_end].iter().rev()
{
for comment in self.comments[self.current_start..self.current_end].iter().rev() {
self.current_end = self.current_end.saturating_sub(1);
if self.range.contains(start) {
return Some((start, comment));
if self.range.contains(&comment.span.start) {
return Some(comment);
}
}
}
Expand All @@ -170,11 +174,11 @@ mod test {
#[test]
fn test_comments_range() {
let comments: SortedComments = vec![
(0, Comment { end: 4, kind: CommentKind::SingleLine }),
(5, Comment { end: 9, kind: CommentKind::SingleLine }),
(10, Comment { end: 13, kind: CommentKind::SingleLine }),
(14, Comment { end: 17, kind: CommentKind::SingleLine }),
(18, Comment { end: 23, kind: CommentKind::SingleLine }),
Comment { span: Span::new(0, 4), kind: CommentKind::SingleLine },
Comment { span: Span::new(5, 9), kind: CommentKind::SingleLine },
Comment { span: Span::new(10, 13), kind: CommentKind::SingleLine },
Comment { span: Span::new(14, 17), kind: CommentKind::SingleLine },
Comment { span: Span::new(18, 23), kind: CommentKind::SingleLine },
]
.into_boxed_slice();
let full_len = comments.len();
Expand Down
30 changes: 13 additions & 17 deletions crates/oxc_codegen/src/annotation_comment.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,42 +12,38 @@ static MATCHER: Lazy<DoubleArrayAhoCorasick<usize>> = Lazy::new(|| {
pub fn get_leading_annotate_comment<const MINIFY: bool>(
node_start: u32,
codegen: &mut Codegen<{ MINIFY }>,
) -> Option<(u32, Comment)> {
) -> Option<Comment> {
let maybe_leading_comment = codegen.try_get_leading_comment(node_start);
let (comment_start, comment) = maybe_leading_comment?;
let comment = maybe_leading_comment?;
let real_end = match comment.kind {
CommentKind::SingleLine => comment.end,
CommentKind::MultiLine => comment.end + 2,
CommentKind::SingleLine => comment.span.end,
CommentKind::MultiLine => comment.span.end + 2,
};
let source_code = codegen.source_text;
let content_between = &source_code[real_end as usize..node_start as usize];
// Used for VariableDeclaration (Rollup only respects "const" and only for the first one)
if content_between.chars().all(|ch| ch.is_ascii_whitespace()) {
let comment_content = &source_code[*comment_start as usize..comment.end as usize];
let comment_content = &source_code[comment.span.start as usize..comment.span.end as usize];
if MATCHER.find_iter(&comment_content).next().is_some() {
return Some((*comment_start, *comment));
return Some(*comment);
}
None
} else {
None
}
}

pub fn print_comment<const MINIFY: bool>(
comment_start: u32,
comment: Comment,
p: &mut Codegen<{ MINIFY }>,
) {
pub fn print_comment<const MINIFY: bool>(comment: Comment, p: &mut Codegen<{ MINIFY }>) {
match comment.kind {
CommentKind::SingleLine => {
p.print_str("//");
p.print_range_of_source_code(comment_start as usize..comment.end as usize);
p.print_range_of_source_code(comment.span.start as usize..comment.span.end as usize);
p.print_soft_newline();
p.print_indent();
}
CommentKind::MultiLine => {
p.print_str("/*");
p.print_range_of_source_code(comment_start as usize..comment.end as usize);
p.print_range_of_source_code(comment.span.start as usize..comment.span.end as usize);
p.print_str("*/");
p.print_soft_space();
}
Expand All @@ -58,11 +54,11 @@ pub fn gen_comment<const MINIFY: bool>(node_start: u32, codegen: &mut Codegen<{
if !codegen.comment_options.preserve_annotate_comments {
return;
}
if let Some((comment_start, comment)) = codegen.try_take_moved_comment(node_start) {
print_comment::<MINIFY>(comment_start, comment, codegen);
if let Some(comment) = codegen.try_take_moved_comment(node_start) {
print_comment::<MINIFY>(comment, codegen);
}
let maybe_leading_annotate_comment = get_leading_annotate_comment(node_start, codegen);
if let Some((comment_start, comment)) = maybe_leading_annotate_comment {
print_comment::<MINIFY>(comment_start, comment, codegen);
if let Some(comment) = maybe_leading_annotate_comment {
print_comment::<MINIFY>(comment, codegen);
}
}
8 changes: 4 additions & 4 deletions crates/oxc_codegen/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -517,7 +517,7 @@ impl<'a, const MINIFY: bool> Codegen<'a, MINIFY> {
}
}

pub(crate) type MoveCommentMap = FxHashMap<u32, (u32, Comment)>;
pub(crate) type MoveCommentMap = FxHashMap<u32, Comment>;

// Comment related
impl<'a, const MINIFY: bool> Codegen<'a, MINIFY> {
Expand All @@ -541,15 +541,15 @@ impl<'a, const MINIFY: bool> Codegen<'a, MINIFY> {
///
/// }, b = 10000;
/// ```
fn move_comment(&mut self, position: u32, full_comment_info: (u32, Comment)) {
fn move_comment(&mut self, position: u32, full_comment_info: Comment) {
self.move_comment_map.insert(position, full_comment_info);
}

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

fn try_take_moved_comment(&mut self, node_start: u32) -> Option<(u32, Comment)> {
fn try_take_moved_comment(&mut self, node_start: u32) -> Option<Comment> {
self.move_comment_map.remove(&node_start)
}
}
48 changes: 29 additions & 19 deletions crates/oxc_linter/src/disable_directives.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -93,8 +93,8 @@ impl<'a> DisableDirectivesBuilder<'a> {
// This algorithm iterates through the comments and builds all intervals
// for matching disable and enable pairs.
// Wrongly ordered matching pairs are not taken into consideration.
for (_, span) in self.trivias.clone().comments() {
let text = span.source_text(self.source_text);
for comment in self.trivias.clone().comments() {
let text = comment.span.source_text(self.source_text);
let text = text.trim_start();

if let Some(text) =
Expand All @@ -103,66 +103,72 @@ impl<'a> DisableDirectivesBuilder<'a> {
// `eslint-disable`
if text.trim().is_empty() {
if self.disable_all_start.is_none() {
self.disable_all_start = Some(span.end);
self.disable_all_start = Some(comment.span.end);
}
self.disable_all_comments.push(span);
self.disable_all_comments.push(comment.span);
continue;
}

// `eslint-disable-next-line`
if let Some(text) = text.strip_prefix("-next-line") {
// Get the span up to the next new line
let stop = self.source_text[span.end as usize..]
let stop = self.source_text[comment.span.end as usize..]
.lines()
.take(2)
.fold(span.end, |acc, line| acc + line.len() as u32);
.fold(comment.span.end, |acc, line| acc + line.len() as u32);
if text.trim().is_empty() {
self.add_interval(span.end, stop, DisabledRule::All);
self.disable_all_comments.push(span);
self.add_interval(comment.span.end, stop, DisabledRule::All);
self.disable_all_comments.push(comment.span);
} else {
// `eslint-disable-next-line rule_name1, rule_name2`
let mut rules = vec![];
Self::get_rule_names(text, |rule_name| {
self.add_interval(span.end, stop, DisabledRule::Single(rule_name));
self.add_interval(
comment.span.end,
stop,
DisabledRule::Single(rule_name),
);
rules.push(rule_name);
});
self.disable_rule_comments.push(DisableRuleComment { span, rules });
self.disable_rule_comments
.push(DisableRuleComment { span: comment.span, rules });
}
continue;
}

// `eslint-disable-line`
if let Some(text) = text.strip_prefix("-line") {
// Get the span between the preceding newline to this comment
let start = self.source_text[..=span.start as usize]
let start = self.source_text[..=comment.span.start as usize]
.lines()
.next_back()
.map_or(0, |line| span.start - (line.len() as u32 - 1));
let stop = span.start;
.map_or(0, |line| comment.span.start - (line.len() as u32 - 1));
let stop = comment.span.start;

// `eslint-disable-line`
if text.trim().is_empty() {
self.add_interval(start, stop, DisabledRule::All);
self.disable_all_comments.push(span);
self.disable_all_comments.push(comment.span);
} else {
// `eslint-disable-line rule-name1, rule-name2`
let mut rules = vec![];
Self::get_rule_names(text, |rule_name| {
self.add_interval(start, stop, DisabledRule::Single(rule_name));
rules.push(rule_name);
});
self.disable_rule_comments.push(DisableRuleComment { span, rules });
self.disable_rule_comments
.push(DisableRuleComment { span: comment.span, rules });
}
continue;
}

// `eslint-disable rule-name1, rule-name2`
let mut rules = vec![];
Self::get_rule_names(text, |rule_name| {
self.disable_start_map.entry(rule_name).or_insert(span.end);
self.disable_start_map.entry(rule_name).or_insert(comment.span.end);
rules.push(rule_name);
});
self.disable_rule_comments.push(DisableRuleComment { span, rules });
self.disable_rule_comments.push(DisableRuleComment { span: comment.span, rules });

continue;
}
Expand All @@ -173,13 +179,17 @@ impl<'a> DisableDirectivesBuilder<'a> {
// `eslint-enable`
if text.trim().is_empty() {
if let Some(start) = self.disable_all_start.take() {
self.add_interval(start, span.start, DisabledRule::All);
self.add_interval(start, comment.span.start, DisabledRule::All);
}
} else {
// `eslint-enable rule-name1, rule-name2`
Self::get_rule_names(text, |rule_name| {
if let Some(start) = self.disable_start_map.remove(rule_name) {
self.add_interval(start, span.start, DisabledRule::Single(rule_name));
self.add_interval(
start,
comment.span.start,
DisabledRule::Single(rule_name),
);
}
});
}
Expand Down
6 changes: 2 additions & 4 deletions crates/oxc_linter/src/rules/eslint/default_case.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -72,10 +72,8 @@ impl Rule for DefaultCase {
.trivias()
.comments_range(last_case.span.start..switch.span.end)
.last()
.is_some_and(|(start, comment)| {
let raw = Span::new(*start, comment.end)
.source_text(ctx.semantic().source_text())
.trim();
.is_some_and(|comment| {
let raw = comment.span.source_text(ctx.semantic().source_text()).trim();
match &self.comment_pattern {
Some(comment_pattern) => comment_pattern.is_match(raw),
None => raw.eq_ignore_ascii_case("no default"),
Expand Down
Loading