feat(parser,codegen): parse and print leading comments #5779

Boshen · 2024-09-15T06:31:50Z

Got a prototype working ... I am going to break this apart into stacked PRs, with proper testing.

NOTE: only special leading comments such as jsdoc, /* @... */ are printed.

graphite-app · 2024-09-15T06:31:56Z

Your org has enabled the Graphite merge queue for merging into main

Add the label “0-merge” to the PR and Graphite will automatically add it to the merge queue when it’s ready to merge. Or use the label “hotfix” to add to the merge queue as a hot fix.

You must have a Graphite account and log in to Graphite in order to use the merge queue. Sign up using this link.

Boshen · 2024-09-15T06:32:13Z

tasks/coverage/codegen_sourcemap.snap

This snapshot is kind of useless.

Boshen · 2024-09-15T06:32:35Z

tasks/transform_conformance/babel.snap.md

-Unresolved reference IDs mismatch for "LongNameModule":
-after transform: [ReferenceId(1), ReferenceId(5)]
-rebuilt        : [ReferenceId(1)]
+x Output mismatch


We need to move the spans ... cc @Dunqing

Boshen · 2024-09-15T06:34:32Z

crates/oxc_codegen/src/lib.rs

+            return;
+        }
+        let Some(source_text) = self.source_text else { return };
+        let Some(comments) = self.leading_comments.remove(&start) else { return };


Performance killer :-/ we need to somehow carry over whether a token has leading comments to the ast node, for a fast path.

Boshen · 2024-09-15T06:34:51Z

crates/oxc_codegen/src/lib.rs

+            .filter(|comment| {
+                let s = comment.span.source_text(source_text);
+                // only print jsdoc and `@__PURE__`, `@license`, `@preserve` etc
+                (comment.is_multi_line() && s.starts_with('*')) || s.trim_start().starts_with('@')


Need to find all the special comments :-/

Boshen · 2024-09-15T06:36:39Z

crates/oxc_ast/src/trivia.rs

+    pub preceded_by_newline: Option<bool>,
+    pub followed_by_newline: Option<bool>,
+
+    pub attached_to: u32,
 }


Need to study what information is required by prettier, cc @leaysgur

I don't remember where this came from

oxc/crates/oxc_prettier/src/comments/mod.rs

Lines 54 to 65 in 91bb06e

bitflags! {

#[derive(Debug, Clone, Copy)]

pub struct CommentFlags: u8 {

const Leading = 1 << 0; // Check comment is a leading comment

const Trailing = 1 << 1; // Check comment is a trailing comment

const Dangling = 1 << 2; // Check comment is a dangling comment

const Block = 1 << 3; // Check comment is a block comment

const Line = 1 << 4; // Check comment is a line comment

const PrettierIgnore = 1 << 5; // Check comment is a `prettier-ignore` comment

const First = 1 << 6; // Check comment is the first attached comment

const Last = 1 << 7; // Check comment is the last attached comment

}

I'm still on the way to read through these area, but as far as I can tell are:

Prettier performs 3 phases to collect comment information

1: Find preceding, enclosing and|or following node for each comment

Relatively intuitive, but there is exception

2: Classify comments into ownLine, endOfLine and remaining group

3: Attach comments to each node with 1 + 2

This one using so many utils...

Final AST nodes contain comments field which are ESTree.Comment[] and Comment is extended with leading and trailing flag.

I found a few more extra properties but not understand yet

😅

I don't remember where this came from

Maybe this?

https://github.com/prettier/prettier/blob/aa3853b7765645b3f3d8a76e41cf6d70b93c01fd/src/language-js/utils/index.js#L1005

Uh, only the leading and trailing flags are extended in the preparation phase, but other of CommentFlags are dynamically retrieved at runtime...

codspeed-hq · 2024-09-15T06:39:07Z

CodSpeed Performance Report

Merging #5779 will degrade performances by 29.62%

_{Comparing 09-14-feat_codegen_print_comments (f431e04) with main (3230ae5)}

Summary

❌ 14 regressions
✅ 15 untouched benchmarks

⚠️ Please fix the performance issues or acknowledge them on CodSpeed.

Benchmarks breakdown

	Benchmark	`main`	`09-14-feat_codegen_print_comments`	Change
❌	`codegen[checker.ts]`	20.8 ms	27.6 ms	-24.57%
❌	`codegen_sourcemap[checker.ts]`	72.5 ms	79.3 ms	-8.64%
❌	`isolated-declarations[vue-id.ts]`	391.9 ms	412.3 ms	-4.95%
❌	`lexer[RadixUIAdoptionSection.jsx]`	22.4 µs	26.1 µs	-13.99%
❌	`lexer[antd.js]`	19.9 ms	27.4 ms	-27.49%
❌	`lexer[cal.com.tsx]`	4.9 ms	6.8 ms	-27.79%
❌	`lexer[checker.ts]`	12 ms	15.8 ms	-24.4%
❌	`lexer[pdf.mjs]`	3.2 ms	4.5 ms	-29.62%
❌	`parser[RadixUIAdoptionSection.jsx]`	77.3 µs	81.1 µs	-4.73%
❌	`parser[antd.js]`	106.7 ms	112.9 ms	-5.56%
❌	`parser[cal.com.tsx]`	24.5 ms	26.1 ms	-6.2%
❌	`parser[checker.ts]`	53.3 ms	56.2 ms	-5.19%
❌	`parser[pdf.mjs]`	17.4 ms	18.5 ms	-5.76%
❌	`sourcemap[cal.com.tsx]`	61.5 ms	64.6 ms	-4.81%

Boshen · 2024-09-15T06:41:22Z

crates/oxc_ast/src/trivia.rs

@@ -8,19 +8,39 @@ use std::{

 use oxc_span::Span;

+#[derive(Debug, Clone, Copy, Eq, PartialEq)]
+pub enum CommentPosition {
+    Leading,


Leading comments are all comments before a token.

Boshen · 2024-09-15T06:41:54Z

crates/oxc_ast/src/trivia.rs

+#[derive(Debug, Clone, Copy, Eq, PartialEq)]
+pub enum CommentPosition {
+    Leading,
+    Trailing,


Trailing comments are all comments tailing a token and before the newline.

feat(parser,codegen): parse and print leading comments

f431e04

github-actions bot added A-parser Area - Parser A-ast Area - AST A-transformer Area - Transformer / Transpiler A-codegen Area - Code Generation labels Sep 15, 2024

Boshen commented Sep 15, 2024

View reviewed changes

tasks/coverage/codegen_sourcemap.snap

Copy link

Member Author

Boshen Sep 15, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This snapshot is kind of useless.

Boshen commented Sep 15, 2024

View reviewed changes

Boshen closed this Sep 17, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(parser,codegen): parse and print leading comments #5779

feat(parser,codegen): parse and print leading comments #5779

Boshen commented Sep 15, 2024

graphite-app bot commented Sep 15, 2024

Boshen Sep 15, 2024

Boshen Sep 15, 2024

Boshen Sep 15, 2024

Boshen Sep 15, 2024

Boshen Sep 15, 2024

leaysgur Sep 15, 2024 •

edited

Loading

leaysgur Sep 17, 2024 •

edited

Loading

codspeed-hq bot commented Sep 15, 2024

Boshen Sep 15, 2024

Boshen Sep 15, 2024

	bitflags! {
	#[derive(Debug, Clone, Copy)]
	pub struct CommentFlags: u8 {
	const Leading = 1 << 0; // Check comment is a leading comment
	const Trailing = 1 << 1; // Check comment is a trailing comment
	const Dangling = 1 << 2; // Check comment is a dangling comment
	const Block = 1 << 3; // Check comment is a block comment
	const Line = 1 << 4; // Check comment is a line comment
	const PrettierIgnore = 1 << 5; // Check comment is a `prettier-ignore` comment
	const First = 1 << 6; // Check comment is the first attached comment
	const Last = 1 << 7; // Check comment is the last attached comment
	}

feat(parser,codegen): parse and print leading comments #5779

feat(parser,codegen): parse and print leading comments #5779

Conversation

Boshen commented Sep 15, 2024

graphite-app bot commented Sep 15, 2024

Your org has enabled the Graphite merge queue for merging into main

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

leaysgur Sep 15, 2024 • edited Loading

Choose a reason for hiding this comment

leaysgur Sep 17, 2024 • edited Loading

Choose a reason for hiding this comment

codspeed-hq bot commented Sep 15, 2024

Merging #5779 will degrade performances by 29.62%

Summary

Benchmarks breakdown

Choose a reason for hiding this comment

Choose a reason for hiding this comment

leaysgur Sep 15, 2024 •

edited

Loading

leaysgur Sep 17, 2024 •

edited

Loading