Skip to content

Commit

Permalink
fix(codegen): dedupe pure annotation comments (#4862)
Browse files Browse the repository at this point in the history
Close #4843
  • Loading branch information
IWANABETHATGUY committed Aug 13, 2024
1 parent 1bdde2c commit 1808529
Show file tree
Hide file tree
Showing 4 changed files with 88 additions and 18 deletions.
78 changes: 66 additions & 12 deletions crates/oxc_codegen/src/annotation_comment.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
use daachorse::DoubleArrayAhoCorasick;
use once_cell::sync::Lazy;
use oxc_ast::{Comment, CommentKind};
use oxc_span::Span;

use crate::Codegen;
static MATCHER: Lazy<DoubleArrayAhoCorasick<usize>> = Lazy::new(|| {
Expand All @@ -9,8 +10,46 @@ static MATCHER: Lazy<DoubleArrayAhoCorasick<usize>> = Lazy::new(|| {
DoubleArrayAhoCorasick::new(patterns).unwrap()
});

bitflags::bitflags! {
/// In theory this should be a enum,but using bitflags is easy to merge many flags into one
/// bitset, which is used to unique annotation comment in codegen
#[derive(Debug, Default, Clone, Copy)]
pub(crate) struct AnnotationKind: u8 {
const NO_SIDE_EFFECTS = 1 << 0;
const PURE = 1 << 1;
}
}

#[derive(Debug, Clone, Copy)]
pub struct AnnotationComment {
pub(crate) annotation_kind: AnnotationKind,
pub(crate) comment: Comment,
}

impl AnnotationComment {
pub fn annotation_kind(&self) -> AnnotationKind {
self.annotation_kind
}

pub fn span(&self) -> Span {
self.comment.span
}
pub fn kind(&self) -> CommentKind {
self.comment.kind
}
}

impl From<(Comment, AnnotationKind)> for AnnotationComment {
fn from(value: (Comment, AnnotationKind)) -> Self {
Self { annotation_kind: value.1, comment: value.0 }
}
}

impl<'a, const MINIFY: bool> Codegen<'a, MINIFY> {
pub(crate) fn get_leading_annotate_comment(&mut self, node_start: u32) -> Option<Comment> {
pub(crate) fn get_leading_annotate_comment(
&mut self,
node_start: u32,
) -> Option<AnnotationComment> {
let maybe_leading_comment = self.try_get_leading_comment(node_start);
let comment = maybe_leading_comment?;
if self.latest_consumed_comment_end >= comment.span.end {
Expand All @@ -26,16 +65,21 @@ impl<'a, const MINIFY: bool> Codegen<'a, MINIFY> {
if content_between.chars().all(|ch| ch.is_ascii_whitespace()) {
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);
if let Some(m) = MATCHER.find_iter(&comment_content).next() {
let annotation_kind = match m.value() {
0 | 1 => AnnotationKind::NO_SIDE_EFFECTS,
2 | 3 => AnnotationKind::PURE,
_ => unreachable!(),
};
return Some((*comment, annotation_kind).into());
}
None
} else {
None
}
}

pub(crate) fn print_comment(&mut self, comment: Comment) {
pub(crate) fn print_comment(&mut self, comment: AnnotationComment) {
// ```js
// /*#__PURE__*/
// Object.getOwnPropertyNames(Symbol)
Expand All @@ -48,23 +92,24 @@ impl<'a, const MINIFY: bool> Codegen<'a, MINIFY> {
// ```
// in this example, `Object.getOwnPropertyNames(Symbol)` and `Object.getOwnPropertyNames(Symbol).filter()`, `Object.getOwnPropertyNames(Symbol).filter().map()`
// share the same leading comment. since they both are call expr and has same span start, we need to avoid print the same comment multiple times.
if self.latest_consumed_comment_end >= comment.span.end {
let comment_span = comment.span();
if self.latest_consumed_comment_end >= comment_span.end {
return;
}
self.latest_consumed_comment_end = comment.span.end;
match comment.kind {
self.latest_consumed_comment_end = comment_span.end;
match comment.kind() {
CommentKind::SingleLine => {
self.print_str("//");
self.print_range_of_source_code(
comment.span.start as usize..comment.span.end as usize,
comment_span.start as usize..comment_span.end as usize,
);
self.print_soft_newline();
self.print_indent();
}
CommentKind::MultiLine => {
self.print_str("/*");
self.print_range_of_source_code(
comment.span.start as usize..comment.span.end as usize,
comment_span.start as usize..comment_span.end as usize,
);
self.print_str("*/");
self.print_soft_space();
Expand All @@ -74,16 +119,25 @@ impl<'a, const MINIFY: bool> Codegen<'a, MINIFY> {
self.start_of_default_export = self.code_len();
}

pub(crate) fn gen_comment(&mut self, node_start: u32) {
pub(crate) fn gen_comments(&mut self, node_start: u32) {
if !self.comment_options.preserve_annotate_comments {
return;
}
let mut annotation_kind_set = AnnotationKind::empty();
if let Some(comment) = self.try_take_moved_comment(node_start) {
self.print_comment(comment);
let kind = comment.annotation_kind();
if !annotation_kind_set.intersects(kind) {
annotation_kind_set.insert(kind);
self.print_comment(comment);
}
}
let maybe_leading_annotate_comment = self.get_leading_annotate_comment(node_start);
if let Some(comment) = maybe_leading_annotate_comment {
self.print_comment(comment);
let kind = comment.annotation_kind();
if !annotation_kind_set.intersects(kind) {
annotation_kind_set.insert(kind);
self.print_comment(comment);
}
}
}
}
6 changes: 3 additions & 3 deletions crates/oxc_codegen/src/gen.rs
Original file line number Diff line number Diff line change
Expand Up @@ -632,7 +632,7 @@ impl<'a, const MINIFY: bool> Gen<MINIFY> for Function<'a> {
fn gen(&self, p: &mut Codegen<{ MINIFY }>, ctx: Context) {
let n = p.code_len();
let wrap = self.is_expression() && (p.start_of_stmt == n || p.start_of_default_export == n);
p.gen_comment(self.span.start);
p.gen_comments(self.span.start);
p.wrap(wrap, |p| {
p.print_space_before_identifier();
p.add_source_mapping(self.span.start);
Expand Down Expand Up @@ -868,7 +868,7 @@ impl<'a, const MINIFY: bool> Gen<MINIFY> for ExportNamedDeclaration<'a> {
if p.comment_options.preserve_annotate_comments {
match &self.declaration {
Some(Declaration::FunctionDeclaration(_)) => {
p.gen_comment(self.span.start);
p.gen_comments(self.span.start);
}
Some(Declaration::VariableDeclaration(var_decl))
if matches!(var_decl.kind, VariableDeclarationKind::Const) =>
Expand Down Expand Up @@ -1638,7 +1638,7 @@ impl<'a, const MINIFY: bool> Gen<MINIFY> for PropertyKey<'a> {
impl<'a, const MINIFY: bool> GenExpr<MINIFY> for ArrowFunctionExpression<'a> {
fn gen_expr(&self, p: &mut Codegen<{ MINIFY }>, precedence: Precedence, ctx: Context) {
p.wrap(precedence >= Precedence::Assign, |p| {
p.gen_comment(self.span.start);
p.gen_comments(self.span.start);
if self.r#async {
p.add_source_mapping(self.span.start);
p.print_str("async");
Expand Down
8 changes: 5 additions & 3 deletions crates/oxc_codegen/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,8 @@ pub use crate::{
gen::{Gen, GenExpr},
};

use self::annotation_comment::AnnotationComment;

/// Code generator without whitespace removal.
pub type CodeGenerator<'a> = Codegen<'a, false>;

Expand Down Expand Up @@ -519,7 +521,7 @@ impl<'a, const MINIFY: bool> Codegen<'a, MINIFY> {
}
}

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

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

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<Comment> {
fn try_take_moved_comment(&mut self, node_start: u32) -> Option<AnnotationComment> {
self.move_comment_map.remove(&node_start)
}
}
14 changes: 14 additions & 0 deletions crates/oxc_codegen/tests/integration/pure_comments.rs
Original file line number Diff line number Diff line change
Expand Up @@ -184,4 +184,18 @@ const builtInSymbols = new Set(
test("(/* @__PURE__ */ new Foo()).bar();\n", "(/* @__PURE__ */ new Foo()).bar();\n");

test("(/* @__PURE__ */ Foo()).bar();\n", "(/* @__PURE__ */ Foo()).bar();\n");

// https://github.com/oxc-project/oxc/issues/4843
test(
r"
/* #__NO_SIDE_EFFECTS__ */
const defineSSRCustomElement = /* @__NO_SIDE_EFFECTS__ */ (
options,
extraOptions,
) => {
return /* @__PURE__ */ defineCustomElement(options, extraOptions, hydrate);
};
",
"const defineSSRCustomElement = /* #__NO_SIDE_EFFECTS__ */ (options, extraOptions) => {\n\treturn /* @__PURE__ */ defineCustomElement(options, extraOptions, hydrate);\n};\n",
);
}

0 comments on commit 1808529

Please sign in to comment.