From d8856fe6850fe47216aec29c9b60abef1f5d78e3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Nicol=C3=B2=20Ribaudo?= Date: Wed, 2 Nov 2022 11:36:52 +0100 Subject: [PATCH] Improve printing of [no LineTerminator here] with comments --- .../src/generators/expressions.ts | 26 ++++------ .../babel-generator/src/generators/methods.ts | 36 +++++-------- .../babel-generator/src/generators/modules.ts | 20 +++----- .../src/generators/statements.ts | 19 +------ packages/babel-generator/src/printer.ts | 51 +++++++++++-------- .../inner-comment-async-arrows/input.js | 9 ++++ .../inner-comment-async-arrows/output.js | 9 ++++ 7 files changed, 76 insertions(+), 94 deletions(-) create mode 100644 packages/babel-generator/test/fixtures/regression/inner-comment-async-arrows/input.js create mode 100644 packages/babel-generator/test/fixtures/regression/inner-comment-async-arrows/output.js diff --git a/packages/babel-generator/src/generators/expressions.ts b/packages/babel-generator/src/generators/expressions.ts index 02d926d85226..01605a176257 100644 --- a/packages/babel-generator/src/generators/expressions.ts +++ b/packages/babel-generator/src/generators/expressions.ts @@ -26,14 +26,11 @@ export function UnaryExpression(this: Printer, node: t.UnaryExpression) { } export function DoExpression(this: Printer, node: t.DoExpression) { - // ensure no line terminator between `async` and `do` - this.ensureNoLineTerminator(() => { - if (node.async) { - this.word("async"); - this.space(); - } - this.word("do"); - }); + if (node.async) { + this.word("async", true); + this.space(); + } + this.word("do"); this.space(); this.print(node.body, node); } @@ -230,12 +227,10 @@ export function AwaitExpression(this: Printer, node: t.AwaitExpression) { } export function YieldExpression(this: Printer, node: t.YieldExpression) { - this.word("yield"); + this.word("yield", node.delegate); if (node.delegate) { - this.ensureNoLineTerminator(() => { - this.token("*"); - }); + this.token("*"); if (node.argument) { this.space(); // line terminators are allowed after yield* @@ -360,12 +355,9 @@ export function V8IntrinsicIdentifier( } export function ModuleExpression(this: Printer, node: t.ModuleExpression) { - this.word("module"); + this.word("module", true); this.space(); - // ensure no line terminator between `module` and `{` - this.ensureNoLineTerminator(() => { - this.token("{"); - }); + this.token("{"); this.indent(); const { body } = node; if (body.body.length || body.directives.length) { diff --git a/packages/babel-generator/src/generators/methods.ts b/packages/babel-generator/src/generators/methods.ts index c560540e64ce..dd16d761cabb 100644 --- a/packages/babel-generator/src/generators/methods.ts +++ b/packages/babel-generator/src/generators/methods.ts @@ -10,6 +10,7 @@ export function _params( this.token("("); this._parameters(node.params, node); this.token(")"); + this._noLineTerminator = true; this.print(node.returnType, node, node.type === "ArrowFunctionExpression"); } @@ -72,11 +73,8 @@ export function _methodHead(this: Printer, node: t.Method | t.TSDeclareMethod) { this.space(); } - const { _noLineTerminator } = this; if (node.async) { - // ensure no line terminator between async and class element name / * - this._noLineTerminator = true; - this.word("async"); + this.word("async", true); this.space(); } @@ -87,18 +85,15 @@ export function _methodHead(this: Printer, node: t.Method | t.TSDeclareMethod) { ) { if (node.generator) { this.token("*"); - this._noLineTerminator = _noLineTerminator; } } if (node.computed) { this.token("["); - this._noLineTerminator = _noLineTerminator; this.print(key, node); this.token("]"); } else { this.print(key, node); - this._noLineTerminator = _noLineTerminator; } if ( @@ -118,13 +113,14 @@ export function _predicate( | t.FunctionDeclaration | t.FunctionExpression | t.ArrowFunctionExpression, + noLineTerminatorAfter?: boolean, ) { if (node.predicate) { if (!node.returnType) { this.token(":"); } this.space(); - this.print(node.predicate, node); + this.print(node.predicate, node, noLineTerminatorAfter); } } @@ -172,10 +168,8 @@ export function ArrowFunctionExpression( this: Printer, node: t.ArrowFunctionExpression, ) { - const { _noLineTerminator } = this; if (node.async) { - this._noLineTerminator = true; - this.word("async"); + this.word("async", true); this.space(); } @@ -188,22 +182,18 @@ export function ArrowFunctionExpression( isIdentifier((firstParam = node.params[0])) && !hasTypesOrComments(node, firstParam) ) { - this.print(firstParam, node); - this._noLineTerminator = _noLineTerminator; + this.print(firstParam, node, true); } else { - this._noLineTerminator = _noLineTerminator; this._params(node); } - this._predicate(node); - this.ensureNoLineTerminator(() => { - this.space(); - // When printing (x)/*1*/=>{}, we remove the parentheses - // and thus there aren't two contiguous inner tokens. - // We forcefully print inner comments here. - this.printInnerComments(); - this.token("=>"); - }); + this._predicate(node, true); + this.space(); + // When printing (x)/*1*/=>{}, we remove the parentheses + // and thus there aren't two contiguous inner tokens. + // We forcefully print inner comments here. + this.printInnerComments(); + this.token("=>"); this.space(); diff --git a/packages/babel-generator/src/generators/modules.ts b/packages/babel-generator/src/generators/modules.ts index 3a711f4144a0..0b8d426f22f2 100644 --- a/packages/babel-generator/src/generators/modules.ts +++ b/packages/babel-generator/src/generators/modules.ts @@ -70,6 +70,7 @@ export function _printAssertions( this: Printer, node: Extract, ) { + this.word("assert"); this.space(); this.token("{"); this.space(); @@ -94,11 +95,8 @@ export function ExportAllDeclaration( this.space(); // @ts-expect-error Fixme: assertions is not defined in DeclareExportAllDeclaration if (node.assertions?.length) { - this.ensureNoLineTerminator(() => { - this.print(node.source, node); - this.space(); - this.word("assert"); - }); + this.print(node.source, node, true); + this.space(); // @ts-expect-error Fixme: assertions is not defined in DeclareExportAllDeclaration this._printAssertions(node); } else { @@ -169,11 +167,8 @@ export function ExportNamedDeclaration( this.word("from"); this.space(); if (node.assertions?.length) { - this.ensureNoLineTerminator(() => { - this.print(node.source, node); - this.space(); - this.word("assert"); - }); + this.print(node.source, node, true); + this.space(); this._printAssertions(node); } else { this.print(node.source, node); @@ -258,10 +253,7 @@ export function ImportDeclaration(this: Printer, node: t.ImportDeclaration) { if (node.assertions?.length) { this.print(node.source, node, true); - this.ensureNoLineTerminator(() => { - this.space(); - this.word("assert"); - }); + this.space(); this._printAssertions(node); } else { this.print(node.source, node); diff --git a/packages/babel-generator/src/generators/statements.ts b/packages/babel-generator/src/generators/statements.ts index 534230447e44..a333a2cc5db6 100644 --- a/packages/babel-generator/src/generators/statements.ts +++ b/packages/babel-generator/src/generators/statements.ts @@ -1,5 +1,4 @@ import type Printer from "../printer"; -import type { PrintJoinOptions } from "../printer"; import { isFor, isForStatement, @@ -262,12 +261,7 @@ export function VariableDeclaration( } const { kind } = node; - this.word(kind); - const { _noLineTerminator } = this; - if (kind === "using") { - // ensure no line break after `using` - this._noLineTerminator = true; - } + this.word(kind, kind === "using"); this.space(); let hasInits = false; @@ -293,16 +287,6 @@ export function VariableDeclaration( // bar = "foo"; // - let iterator: PrintJoinOptions["iterator"] | undefined; - if (kind === "using") { - // Ensure no line break between `using` and the first declarator - iterator = (_, i: number) => { - if (i === 0) { - this._noLineTerminator = _noLineTerminator; - } - }; - } - this.printList(node.declarations, node, { separator: hasInits ? function (this: Printer) { @@ -310,7 +294,6 @@ export function VariableDeclaration( this.newline(); } : undefined, - iterator, indent: node.declarations.length > 1 ? true : false, }); diff --git a/packages/babel-generator/src/printer.ts b/packages/babel-generator/src/printer.ts index 59b6e0e1c48b..994fb3b2f92e 100644 --- a/packages/babel-generator/src/printer.ts +++ b/packages/babel-generator/src/printer.ts @@ -22,6 +22,7 @@ const SCIENTIFIC_NOTATION = /e/i; const ZERO_DECIMAL_INTEGER = /\.0+$/; const NON_DECIMAL_LITERAL = /^0[box]/; const PURE_ANNOTATION_RE = /^\s*[@#]__PURE__\s*$/; +const NEWLINE = /\r\n|[\n\r\u2028\u2029]/; const { needsParens } = n; @@ -151,6 +152,7 @@ class Printer { } else { this._queue(charCodes.semicolon); } + this._noLineTerminator = false; } /** @@ -185,7 +187,7 @@ class Printer { * Writes a token that can't be safely parsed without taking whitespace into account. */ - word(str: string): void { + word(str: string, noLineTerminatorAfter: boolean = false): void { this._maybePrintInnerComments(); // prevent concatenating words and creating // comment out of division and regex @@ -200,6 +202,7 @@ class Printer { this._append(str, false); this._endsWithWord = true; + this._noLineTerminator = noLineTerminatorAfter; } /** @@ -243,6 +246,7 @@ class Printer { this._maybeAddAuxComment(); this._append(str, maybeNewline); + this._noLineTerminator = false; } tokenChar(char: number): void { @@ -263,6 +267,7 @@ class Printer { this._maybeAddAuxComment(); this._appendChar(char); + this._noLineTerminator = false; } /** @@ -534,13 +539,6 @@ class Printer { return this._indentRepeat * this._indent; } - ensureNoLineTerminator(fn: () => void) { - const { _noLineTerminator } = this; - this._noLineTerminator = true; - fn(); - this._noLineTerminator = _noLineTerminator; - } - printTerminatorless(node: t.Node, parent: t.Node, isLabel: boolean) { /** * Set some state that will be modified if a newline has been inserted before any @@ -558,9 +556,8 @@ class Printer { * `undefined` will be returned and not `foo` due to the terminator. */ if (isLabel) { - this.ensureNoLineTerminator(() => { - this.print(node, parent); - }); + this._noLineTerminator = true; + this.print(node, parent); } else { const terminatorState = { printed: false, @@ -581,9 +578,9 @@ class Printer { print( node: t.Node | null, parent?: t.Node, - noLineTerminator?: boolean, + noLineTerminatorAfter?: boolean, // trailingCommentsLineOffset also used to check if called from printJoin - // it will be ignored if `noLineTerminator||this._noLineTerminator` + // it will be ignored if `noLineTerminatorAfter||this._noLineTerminator` trailingCommentsLineOffset?: number, forceParens?: boolean, ) { @@ -649,16 +646,17 @@ class Printer { this.exactSource(loc, printMethod.bind(this, node, parent)); - if (noLineTerminator && !this._noLineTerminator) { + if (shouldPrintParens) { + this._printTrailingComments(node, parent); + this.token(")"); + this._noLineTerminator = noLineTerminatorAfter; + } else if (noLineTerminatorAfter && !this._noLineTerminator) { this._noLineTerminator = true; this._printTrailingComments(node, parent); - this._noLineTerminator = false; } else { this._printTrailingComments(node, parent, trailingCommentsLineOffset); } - if (shouldPrintParens) this.token(")"); - // end this._printStack.pop(); @@ -929,6 +927,11 @@ class Printer { let val; if (isBlockComment) { + if (this._noLineTerminator && NEWLINE.test(comment.value)) { + this._printedComments.delete(comment); + return; + } + val = `/*${comment.value}*/`; if (this.format.indent.adjustMultilineComment) { const offset = comment.loc?.start.column; @@ -950,6 +953,9 @@ class Printer { } else if (!this._noLineTerminator) { val = `//${comment.value}`; } else { + // It was a single-line comment, so it's guaranteed to not + // contain newlines and it can be safely printed as a block + // comment. val = `/*${comment.value}*/`; } @@ -983,6 +989,7 @@ class Printer { const nodeEndLine = hasLoc ? nodeLoc.end.line : 0; let lastLine = 0; let leadingCommentNewline = 0; + const { _noLineTerminator } = this; for (let i = 0; i < len; i++) { const comment = comments[i]; @@ -1007,10 +1014,10 @@ class Printer { } lastLine = commentEndLine; - this.newline(offset); + if (!_noLineTerminator) this.newline(offset); this._printComment(comment, COMMENT_SKIP_NEWLINE.SKIP_ALL); - if (i + 1 === len) { + if (!_noLineTerminator && i + 1 === len) { this.newline( Math.max(nodeStartLine - lastLine, leadingCommentNewline), ); @@ -1021,10 +1028,10 @@ class Printer { commentStartLine - (i === 0 ? nodeStartLine : lastLine); lastLine = commentEndLine; - this.newline(offset); + if (!_noLineTerminator) this.newline(offset); this._printComment(comment, COMMENT_SKIP_NEWLINE.SKIP_ALL); - if (i + 1 === len) { + if (!_noLineTerminator && i + 1 === len) { this.newline(Math.min(1, nodeEndLine - lastLine)); // TODO: Improve here when inner comments processing is stronger lastLine = nodeEndLine; } @@ -1034,7 +1041,7 @@ class Printer { (i === 0 ? nodeEndLine - lineOffset : lastLine); lastLine = commentEndLine; - this.newline(offset); + if (!_noLineTerminator) this.newline(offset); this._printComment(comment, COMMENT_SKIP_NEWLINE.SKIP_ALL); } } else { diff --git a/packages/babel-generator/test/fixtures/regression/inner-comment-async-arrows/input.js b/packages/babel-generator/test/fixtures/regression/inner-comment-async-arrows/input.js new file mode 100644 index 000000000000..dcb80049846c --- /dev/null +++ b/packages/babel-generator/test/fixtures/regression/inner-comment-async-arrows/input.js @@ -0,0 +1,9 @@ +async /* 1 */ () => {}; +async () /* 2 */ => {}; +async /* 3 */ (param) => {}; +async (param) /* 4 */ => {}; +async ( /* 5 */ ) => {}; +async ( /* + * 6 with newline + */ +) => {}; \ No newline at end of file diff --git a/packages/babel-generator/test/fixtures/regression/inner-comment-async-arrows/output.js b/packages/babel-generator/test/fixtures/regression/inner-comment-async-arrows/output.js new file mode 100644 index 000000000000..b0b92d6c78d0 --- /dev/null +++ b/packages/babel-generator/test/fixtures/regression/inner-comment-async-arrows/output.js @@ -0,0 +1,9 @@ +async /* 1 */ () => {}; +async /* 2 */ () => {}; +async param /* 3 */ => {}; +async param /* 4 */ => {}; +async /* 5 */ () => {}; +async ( /* + * 6 with newline + */ +) => {}; \ No newline at end of file