From 5862a17e3bf0e26723ac206742e66ced9f27fd4f Mon Sep 17 00:00:00 2001 From: liuxingbaoyu <30521560+liuxingbaoyu@users.noreply.github.com> Date: Thu, 10 Nov 2022 04:24:20 +0800 Subject: [PATCH 1/2] improve --- packages/babel-generator/src/index.ts | 10 ++++ packages/babel-generator/src/printer.ts | 52 +++++++++---------- .../test/fixtures/regression/15161/output.js | 4 +- 3 files changed, 37 insertions(+), 29 deletions(-) diff --git a/packages/babel-generator/src/index.ts b/packages/babel-generator/src/index.ts index 7f8defd37e75..2b3982cfe986 100644 --- a/packages/babel-generator/src/index.ts +++ b/packages/babel-generator/src/index.ts @@ -109,6 +109,16 @@ function normalizeOptions( format.indent.adjustMultilineComment = false; } + const { auxiliaryCommentBefore, auxiliaryCommentAfter, shouldPrintComment } = + format; + + if (auxiliaryCommentBefore && !shouldPrintComment(auxiliaryCommentBefore)) { + format.auxiliaryCommentBefore = undefined; + } + if (auxiliaryCommentAfter && !shouldPrintComment(auxiliaryCommentAfter)) { + format.auxiliaryCommentAfter = undefined; + } + return format; } diff --git a/packages/babel-generator/src/printer.ts b/packages/babel-generator/src/printer.ts index 401e19294d4f..ad42fa651de8 100644 --- a/packages/babel-generator/src/printer.ts +++ b/packages/babel-generator/src/printer.ts @@ -903,35 +903,35 @@ class Printer { } } - // Returns `true` if the comment cannot be printed in this position due to + // Returns `-1` if the comment cannot be printed in this position due to // line terminators, signaling that the print comments loop can stop and - // resume printing comments at the next posisble position. This happens when + // resume printing comments at the next possible position. This happens when // printing inner comments, since if we have an inner comment with a multiline // there is at least one inner position where line terminators are allowed. - _printComment( - comment: t.Comment, - skipNewLines: COMMENT_SKIP_NEWLINE, - ): boolean { + _shouldPrintComment(comment: t.Comment): -1 | 0 | 1 { // Some plugins (such as flow-strip-types) use this to mark comments as removed using the AST-root 'comments' property, // where they can't manually mutate the AST node comment lists. - if (comment.ignore) return false; + if (comment.ignore) return 0; - if (this._printedComments.has(comment)) return false; - - const noLineTerminator = this._noLineTerminator; + if (this._printedComments.has(comment)) return 0; if ( - noLineTerminator && + this._noLineTerminator && (HAS_NEWLINE.test(comment.value) || HAS_BlOCK_COMMENT_END.test(comment.value)) ) { - return true; + return -1; } - if (!this.format.shouldPrintComment(comment.value)) return false; - this._printedComments.add(comment); + if (!this.format.shouldPrintComment(comment.value)) return 0; + + return 1; + } + + _printComment(comment: t.Comment, skipNewLines: COMMENT_SKIP_NEWLINE) { + const noLineTerminator = this._noLineTerminator; const isBlockComment = comment.type === "CommentBlock"; // Add a newline before and after a block comment, unless explicitly @@ -999,8 +999,6 @@ class Printer { if (printNewLines && skipNewLines !== COMMENT_SKIP_NEWLINE.SKIP_TRAILING) { this.newline(1); } - - return false; } _printComments( @@ -1025,8 +1023,12 @@ class Printer { for (let i = 0; i < len; i++) { const comment = comments[i]; - const printed = this._printedComments.has(comment); - if (hasLoc && comment.loc && !printed) { + const shouldPrint = this._shouldPrintComment(comment); + if (shouldPrint === -1) { + hasLoc = false; + break; + } + if (hasLoc && comment.loc && shouldPrint === 1) { const commentStartLine = comment.loc.start.line; const commentEndLine = comment.loc.end.line; if (type === COMMENT_TYPE.LEADING) { @@ -1061,7 +1063,7 @@ class Printer { lastLine = commentEndLine; maybeNewline(offset); - if (this._printComment(comment, COMMENT_SKIP_NEWLINE.SKIP_ALL)) break; + this._printComment(comment, COMMENT_SKIP_NEWLINE.SKIP_ALL); if (i + 1 === len) { maybeNewline(Math.min(1, nodeEndLine - lastLine)); // TODO: Improve here when inner comments processing is stronger @@ -1077,8 +1079,9 @@ class Printer { } } else { hasLoc = false; - - if (printed) continue; + if (shouldPrint !== 1) { + continue; + } if (len === 1) { const singleLine = comment.loc @@ -1100,9 +1103,7 @@ class Printer { : COMMENT_SKIP_NEWLINE.DEFAULT, ); } else if (shouldSkipNewline && type === COMMENT_TYPE.TRAILING) { - if (this._printComment(comment, COMMENT_SKIP_NEWLINE.SKIP_ALL)) { - break; - } + this._printComment(comment, COMMENT_SKIP_NEWLINE.SKIP_ALL); } else { this._printComment(comment, COMMENT_SKIP_NEWLINE.DEFAULT); } @@ -1117,7 +1118,7 @@ class Printer { // /*:: b: ?string*/ // } - const skippedDueToNewline = this._printComment( + this._printComment( comment, i === 0 ? COMMENT_SKIP_NEWLINE.SKIP_LEADING @@ -1125,7 +1126,6 @@ class Printer { ? COMMENT_SKIP_NEWLINE.SKIP_TRAILING : COMMENT_SKIP_NEWLINE.DEFAULT, ); - if (skippedDueToNewline) break; } else { this._printComment(comment, COMMENT_SKIP_NEWLINE.DEFAULT); } diff --git a/packages/babel-generator/test/fixtures/regression/15161/output.js b/packages/babel-generator/test/fixtures/regression/15161/output.js index ef5f61e62396..9cd08e01a530 100644 --- a/packages/babel-generator/test/fixtures/regression/15161/output.js +++ b/packages/babel-generator/test/fixtures/regression/15161/output.js @@ -1,3 +1 @@ -var test = ( -) => { -}; \ No newline at end of file +var test = () => {}; \ No newline at end of file From ca65490972433c526eac3b669ab8f2f39bb6e510 Mon Sep 17 00:00:00 2001 From: liuxingbaoyu <30521560+liuxingbaoyu@users.noreply.github.com> Date: Tue, 15 Nov 2022 00:44:05 +0800 Subject: [PATCH 2/2] review --- packages/babel-generator/src/printer.ts | 54 ++++++++++++++----------- 1 file changed, 31 insertions(+), 23 deletions(-) diff --git a/packages/babel-generator/src/printer.ts b/packages/babel-generator/src/printer.ts index ad42fa651de8..9f0f915bb577 100644 --- a/packages/babel-generator/src/printer.ts +++ b/packages/babel-generator/src/printer.ts @@ -35,9 +35,15 @@ const enum COMMENT_TYPE { const enum COMMENT_SKIP_NEWLINE { DEFAULT, - SKIP_ALL, - SKIP_LEADING, - SKIP_TRAILING, + ALL, + LEADING, + TRAILING, +} + +const enum PRINT_COMMENT_HINT { + SKIP, + ALLOW, + DEFER, } export type Format = { @@ -903,31 +909,33 @@ class Printer { } } - // Returns `-1` if the comment cannot be printed in this position due to + // Returns `PRINT_COMMENT_HINT.DEFER` if the comment cannot be printed in this position due to // line terminators, signaling that the print comments loop can stop and // resume printing comments at the next possible position. This happens when // printing inner comments, since if we have an inner comment with a multiline // there is at least one inner position where line terminators are allowed. - _shouldPrintComment(comment: t.Comment): -1 | 0 | 1 { + _shouldPrintComment(comment: t.Comment): PRINT_COMMENT_HINT { // Some plugins (such as flow-strip-types) use this to mark comments as removed using the AST-root 'comments' property, // where they can't manually mutate the AST node comment lists. - if (comment.ignore) return 0; + if (comment.ignore) return PRINT_COMMENT_HINT.SKIP; - if (this._printedComments.has(comment)) return 0; + if (this._printedComments.has(comment)) return PRINT_COMMENT_HINT.SKIP; if ( this._noLineTerminator && (HAS_NEWLINE.test(comment.value) || HAS_BlOCK_COMMENT_END.test(comment.value)) ) { - return -1; + return PRINT_COMMENT_HINT.DEFER; } this._printedComments.add(comment); - if (!this.format.shouldPrintComment(comment.value)) return 0; + if (!this.format.shouldPrintComment(comment.value)) { + return PRINT_COMMENT_HINT.SKIP; + } - return 1; + return PRINT_COMMENT_HINT.ALLOW; } _printComment(comment: t.Comment, skipNewLines: COMMENT_SKIP_NEWLINE) { @@ -938,13 +946,13 @@ class Printer { // disallowed const printNewLines = isBlockComment && - skipNewLines !== COMMENT_SKIP_NEWLINE.SKIP_ALL && + skipNewLines !== COMMENT_SKIP_NEWLINE.ALL && !this._noLineTerminator; if ( printNewLines && this._buf.hasContent() && - skipNewLines !== COMMENT_SKIP_NEWLINE.SKIP_LEADING + skipNewLines !== COMMENT_SKIP_NEWLINE.LEADING ) { this.newline(1); } @@ -996,7 +1004,7 @@ class Printer { this.newline(1, true); } - if (printNewLines && skipNewLines !== COMMENT_SKIP_NEWLINE.SKIP_TRAILING) { + if (printNewLines && skipNewLines !== COMMENT_SKIP_NEWLINE.TRAILING) { this.newline(1); } } @@ -1024,11 +1032,11 @@ class Printer { const comment = comments[i]; const shouldPrint = this._shouldPrintComment(comment); - if (shouldPrint === -1) { + if (shouldPrint === PRINT_COMMENT_HINT.DEFER) { hasLoc = false; break; } - if (hasLoc && comment.loc && shouldPrint === 1) { + if (hasLoc && comment.loc && shouldPrint === PRINT_COMMENT_HINT.ALLOW) { const commentStartLine = comment.loc.start.line; const commentEndLine = comment.loc.end.line; if (type === COMMENT_TYPE.LEADING) { @@ -1049,7 +1057,7 @@ class Printer { lastLine = commentEndLine; maybeNewline(offset); - this._printComment(comment, COMMENT_SKIP_NEWLINE.SKIP_ALL); + this._printComment(comment, COMMENT_SKIP_NEWLINE.ALL); if (i + 1 === len) { maybeNewline( @@ -1063,7 +1071,7 @@ class Printer { lastLine = commentEndLine; maybeNewline(offset); - this._printComment(comment, COMMENT_SKIP_NEWLINE.SKIP_ALL); + this._printComment(comment, COMMENT_SKIP_NEWLINE.ALL); if (i + 1 === len) { maybeNewline(Math.min(1, nodeEndLine - lastLine)); // TODO: Improve here when inner comments processing is stronger @@ -1075,11 +1083,11 @@ class Printer { lastLine = commentEndLine; maybeNewline(offset); - this._printComment(comment, COMMENT_SKIP_NEWLINE.SKIP_ALL); + this._printComment(comment, COMMENT_SKIP_NEWLINE.ALL); } } else { hasLoc = false; - if (shouldPrint !== 1) { + if (shouldPrint !== PRINT_COMMENT_HINT.ALLOW) { continue; } @@ -1099,11 +1107,11 @@ class Printer { comment, (shouldSkipNewline && node.type !== "ObjectExpression") || (singleLine && isFunction(parent, { body: node })) - ? COMMENT_SKIP_NEWLINE.SKIP_ALL + ? COMMENT_SKIP_NEWLINE.ALL : COMMENT_SKIP_NEWLINE.DEFAULT, ); } else if (shouldSkipNewline && type === COMMENT_TYPE.TRAILING) { - this._printComment(comment, COMMENT_SKIP_NEWLINE.SKIP_ALL); + this._printComment(comment, COMMENT_SKIP_NEWLINE.ALL); } else { this._printComment(comment, COMMENT_SKIP_NEWLINE.DEFAULT); } @@ -1121,9 +1129,9 @@ class Printer { this._printComment( comment, i === 0 - ? COMMENT_SKIP_NEWLINE.SKIP_LEADING + ? COMMENT_SKIP_NEWLINE.LEADING : i === len - 1 - ? COMMENT_SKIP_NEWLINE.SKIP_TRAILING + ? COMMENT_SKIP_NEWLINE.TRAILING : COMMENT_SKIP_NEWLINE.DEFAULT, ); } else {