Skip to content

Commit

Permalink
feat(codegen): print jsdoc comments that are attached to statements a…
Browse files Browse the repository at this point in the history
…nd class elements (#5845)

I am unable to print all comments correctly. Comments have way too much semantic meaning in JavaScript.

This PR reduces the scope to only print jsdoc comments that are attached to statements and class elements, in order to get isolated declarations shipped.
  • Loading branch information
Boshen committed Sep 18, 2024
1 parent 3120c6c commit bcdbba3
Show file tree
Hide file tree
Showing 8 changed files with 363 additions and 22 deletions.
4 changes: 4 additions & 0 deletions crates/oxc_ast/src/trivia.rs
Original file line number Diff line number Diff line change
Expand Up @@ -104,6 +104,10 @@ impl Comment {
pub fn real_span_start(&self) -> u32 {
self.span.start - 2
}

pub fn is_jsdoc(&self, source_text: &str) -> bool {
self.is_leading() && self.is_block() && self.span.source_text(source_text).starts_with('*')
}
}

/// Sorted set of unique trivia comments, in ascending order by starting position.
Expand Down
74 changes: 74 additions & 0 deletions crates/oxc_codegen/src/comment.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,74 @@
use oxc_syntax::identifier::is_line_terminator;
use rustc_hash::FxHashMap;

use oxc_ast::{Comment, CommentKind, Trivias};

use crate::Codegen;

pub type CommentsMap = FxHashMap</* attached_to */ u32, Vec<Comment>>;

impl<'a> Codegen<'a> {
pub(crate) fn build_leading_comments(&mut self, source_text: &str, trivias: &Trivias) {
let mut leading_comments: CommentsMap = FxHashMap::default();
for comment in trivias
.comments()
.copied()
.filter(|comment| Self::should_keep_comment(comment, source_text))
{
leading_comments.entry(comment.attached_to).or_default().push(comment);
}
self.leading_comments = leading_comments;
}

fn should_keep_comment(comment: &Comment, source_text: &str) -> bool {
comment.is_jsdoc(source_text)
&& comment.preceded_by_newline
// webpack comment `/*****/`
&& !comment.span.source_text(source_text).chars().all(|c| c == '*')
}

pub(crate) fn print_leading_comments(&mut self, start: u32) {
if self.options.minify {
return;
}
let Some(source_text) = self.source_text else { return };
let Some(comments) = self.leading_comments.remove(&start) else { return };

let first = comments.first().unwrap();
if first.preceded_by_newline {
// Skip printing newline if this comment is already on a newline.
if self.peek_nth(0).is_some_and(|c| c != '\n' && c != '\t') {
self.print_char(b'\n');
self.print_indent();
}
}

for comment in &comments {
let s = comment.real_span().source_text(source_text);
match comment.kind {
CommentKind::Line => {
self.print_str(s);
}
CommentKind::Block => {
// Print block comments with our own indentation.
let lines = s.split(is_line_terminator);
for line in lines {
if !line.starts_with("/*") {
self.print_indent();
}
self.print_str(line.trim_start());
if !line.ends_with("*/") {
self.print_hard_newline();
}
}
}
}
}

let last = comments.last().unwrap();
if last.is_line() || last.followed_by_newline {
self.print_hard_newline();
self.print_indent();
}
}
}
7 changes: 7 additions & 0 deletions crates/oxc_codegen/src/gen.rs
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,7 @@ impl<'a> Gen for Directive<'a> {

impl<'a> Gen for Statement<'a> {
fn gen(&self, p: &mut Codegen, ctx: Context) {
p.print_leading_comments(self.span().start);
match self {
Self::BlockStatement(stmt) => stmt.print(p, ctx),
Self::BreakStatement(stmt) => stmt.print(p, ctx),
Expand Down Expand Up @@ -468,6 +469,7 @@ impl<'a> Gen for ReturnStatement<'a> {
fn gen(&self, p: &mut Codegen, _ctx: Context) {
p.add_source_mapping(self.span.start);
p.print_indent();
p.print_space_before_identifier();
p.print_str("return");
if let Some(arg) = &self.argument {
p.print_hard_space();
Expand Down Expand Up @@ -2204,22 +2206,27 @@ impl<'a> Gen for ClassElement<'a> {
fn gen(&self, p: &mut Codegen, ctx: Context) {
match self {
Self::StaticBlock(elem) => {
p.print_leading_comments(elem.span.start);
elem.print(p, ctx);
p.print_soft_newline();
}
Self::MethodDefinition(elem) => {
p.print_leading_comments(elem.span.start);
elem.print(p, ctx);
p.print_soft_newline();
}
Self::PropertyDefinition(elem) => {
p.print_leading_comments(elem.span.start);
elem.print(p, ctx);
p.print_semicolon_after_statement();
}
Self::AccessorProperty(elem) => {
p.print_leading_comments(elem.span.start);
elem.print(p, ctx);
p.print_semicolon_after_statement();
}
Self::TSIndexSignature(elem) => {
p.print_leading_comments(elem.span.start);
elem.print(p, ctx);
p.print_semicolon_after_statement();
}
Expand Down
9 changes: 6 additions & 3 deletions crates/oxc_codegen/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
mod annotation_comment;
mod binary_expr_visitor;
mod comment;
mod context;
mod gen;
mod operator;
Expand All @@ -25,10 +26,9 @@ use oxc_syntax::{
};
use rustc_hash::FxHashMap;

use self::annotation_comment::AnnotationComment;
use crate::{
binary_expr_visitor::BinaryExpressionVisitor, operator::Operator,
sourcemap_builder::SourcemapBuilder,
annotation_comment::AnnotationComment, binary_expr_visitor::BinaryExpressionVisitor,
comment::CommentsMap, operator::Operator, sourcemap_builder::SourcemapBuilder,
};
pub use crate::{
context::Context,
Expand Down Expand Up @@ -75,6 +75,7 @@ pub struct Codegen<'a> {
source_text: Option<&'a str>,

trivias: Trivias,
leading_comments: CommentsMap,

mangler: Option<Mangler>,

Expand Down Expand Up @@ -142,6 +143,7 @@ impl<'a> Codegen<'a> {
comment_options: CommentOptions::default(),
source_text: None,
trivias: Trivias::default(),
leading_comments: CommentsMap::default(),
mangler: None,
code: vec![],
needs_semicolon: false,
Expand Down Expand Up @@ -200,6 +202,7 @@ impl<'a> Codegen<'a> {
trivias: Trivias,
options: CommentOptions,
) -> Self {
self.build_leading_comments(source_text, &trivias);
self.trivias = trivias;
self.comment_options = options;
self.with_source_text(source_text)
Expand Down
78 changes: 78 additions & 0 deletions crates/oxc_codegen/tests/integration/jsdoc.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,78 @@
use crate::snapshot;

#[test]
fn comment() {
let cases = vec![
r"
/** This is a description of the foo function. */
function foo() {
}
/**
* Represents a book.
* @constructor
* @param {string} title - The title of the book.
* @param {string} author - The author of the book.
*/
function Book(title, author) {
}
/** Class representing a point. */
class Point {
/**
* Create a point.
* @param {number} x - The x value.
* @param {number} y - The y value.
*/
constructor(x, y) {
}
/**
* Get the x value.
* @return {number} The x value.
*/
getX() {
}
/**
* Get the y value.
* @return {number} The y value.
*/
getY() {
}
/**
* Convert a string containing two comma-separated numbers into a point.
* @param {string} str - The string containing two comma-separated numbers.
* @return {Point} A Point object.
*/
static fromString(str) {
}
}
/** Class representing a point. */
const Point = class {
}
/**
* Shirt module.
* @module my/shirt
*/
/** Button the shirt. */
exports.button = function() {
};
/** Unbutton the shirt. */
exports.unbutton = function() {
};
this.Book = function(title) {
/** The title of the book. */
this.title = title;
}
",
];

snapshot("jsodc", &cases);
}
1 change: 1 addition & 0 deletions crates/oxc_codegen/tests/integration/main.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
#![allow(clippy::missing_panics_doc)]
pub mod esbuild;
pub mod jsdoc;
pub mod pure_comments;
pub mod tester;
pub mod ts;
Expand Down
121 changes: 121 additions & 0 deletions crates/oxc_codegen/tests/integration/snapshots/jsodc.snap
Original file line number Diff line number Diff line change
@@ -0,0 +1,121 @@
---
source: crates/oxc_codegen/tests/integration/main.rs
---
########## 0

/** This is a description of the foo function. */
function foo() {
}

/**
* Represents a book.
* @constructor
* @param {string} title - The title of the book.
* @param {string} author - The author of the book.
*/
function Book(title, author) {
}

/** Class representing a point. */
class Point {
/**
* Create a point.
* @param {number} x - The x value.
* @param {number} y - The y value.
*/
constructor(x, y) {
}

/**
* Get the x value.
* @return {number} The x value.
*/
getX() {
}

/**
* Get the y value.
* @return {number} The y value.
*/
getY() {
}

/**
* Convert a string containing two comma-separated numbers into a point.
* @param {string} str - The string containing two comma-separated numbers.
* @return {Point} A Point object.
*/
static fromString(str) {
}
}

/** Class representing a point. */
const Point = class {
}

/**
* Shirt module.
* @module my/shirt
*/

/** Button the shirt. */
exports.button = function() {
};

/** Unbutton the shirt. */
exports.unbutton = function() {
};

this.Book = function(title) {
/** The title of the book. */
this.title = title;
}

----------
/** This is a description of the foo function. */
function foo() {}
/**
* Represents a book.
* @constructor
* @param {string} title - The title of the book.
* @param {string} author - The author of the book.
*/
function Book(title, author) {}
/** Class representing a point. */
class Point {
/**
* Create a point.
* @param {number} x - The x value.
* @param {number} y - The y value.
*/
constructor(x, y) {}
/**
* Get the x value.
* @return {number} The x value.
*/
getX() {}
/**
* Get the y value.
* @return {number} The y value.
*/
getY() {}
/**
* Convert a string containing two comma-separated numbers into a point.
* @param {string} str - The string containing two comma-separated numbers.
* @return {Point} A Point object.
*/
static fromString(str) {}
}
/** Class representing a point. */
const Point = class {};
/**
* Shirt module.
* @module my/shirt
*//** Button the shirt. */
exports.button = function() {};
/** Unbutton the shirt. */
exports.unbutton = function() {};
this.Book = function(title) {
/** The title of the book. */
this.title = title;
};
Loading

0 comments on commit bcdbba3

Please sign in to comment.