Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: Types containing comments generate invalid code #14762

Merged
merged 2 commits into from Jul 18, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
6 changes: 3 additions & 3 deletions packages/babel-generator/src/generators/flow.ts
Expand Up @@ -11,7 +11,7 @@ export function ArrayTypeAnnotation(
this: Printer,
node: t.ArrayTypeAnnotation,
) {
this.print(node.elementType, node);
this.print(node.elementType, node, true);
this.token("[");
this.token("]");
}
Expand Down Expand Up @@ -357,7 +357,7 @@ export function FunctionTypeParam(this: Printer, node: t.FunctionTypeParam) {

export function InterfaceExtends(this: Printer, node: t.InterfaceExtends) {
this.print(node.id, node);
this.print(node.typeParameters, node);
this.print(node.typeParameters, node, true);
}

export {
Expand Down Expand Up @@ -758,7 +758,7 @@ export function VoidTypeAnnotation(this: Printer) {
}

export function IndexedAccessType(this: Printer, node: t.IndexedAccessType) {
this.print(node.objectType, node);
this.print(node.objectType, node, true);
this.token("[");
this.print(node.indexType, node);
this.token("]");
Expand Down
10 changes: 1 addition & 9 deletions packages/babel-generator/src/generators/methods.ts
Expand Up @@ -11,15 +11,7 @@ export function _params(
this._parameters(node.params, node);
this.token(")");

if (node.returnType) {
if (node.type === "ArrowFunctionExpression") {
this._noLineTerminator = true;
this.print(node.returnType, node);
this._noLineTerminator = false;
} else {
this.print(node.returnType, node);
}
}
this.print(node.returnType, node, node.type === "ArrowFunctionExpression");
}

export function _parameters(
Expand Down
9 changes: 5 additions & 4 deletions packages/babel-generator/src/generators/typescript.ts
Expand Up @@ -259,8 +259,8 @@ export function tsPrintFunctionOrConstructorType(
}

export function TSTypeReference(this: Printer, node: t.TSTypeReference) {
this.print(node.typeName, node);
this.print(node.typeParameters, node);
this.print(node.typeName, node, true);
this.print(node.typeParameters, node, true);
}

export function TSTypePredicate(this: Printer, node: t.TSTypePredicate) {
Expand Down Expand Up @@ -317,7 +317,8 @@ function tsPrintBraced(printer: Printer, members: t.Node[], node: t.Node) {
}

export function TSArrayType(this: Printer, node: t.TSArrayType) {
this.print(node.elementType, node);
this.print(node.elementType, node, true);

this.token("[]");
}

Expand Down Expand Up @@ -408,7 +409,7 @@ export function TSIndexedAccessType(
this: Printer,
node: t.TSIndexedAccessType,
) {
this.print(node.objectType, node);
this.print(node.objectType, node, true);
this.token("[");
this.print(node.indexType, node);
this.token("]");
Expand Down
38 changes: 23 additions & 15 deletions packages/babel-generator/src/printer.ts
@@ -1,7 +1,6 @@
import Buffer from "./buffer";
import type { Loc } from "./buffer";
import * as n from "./node";
import { isProgram, isFile, isEmptyStatement } from "@babel/types";
import type * as t from "@babel/types";
import type {
RecordAndTuplePluginOptions,
Expand Down Expand Up @@ -504,45 +503,48 @@ class Printer {
}
}

print(node: t.Node | null, parent?: t.Node) {
print(node: t.Node | null, parent?: t.Node, noLineTerminator?: boolean) {
if (!node) return;

const oldConcise = this.format.concise;
const nodeType = node.type;
const format = this.format;

const oldConcise = format.concise;
if (
// @ts-expect-error document _compact AST properties
node._compact
) {
this.format.concise = true;
format.concise = true;
}

const printMethod =
this[
node.type as Exclude<
nodeType as Exclude<
t.Node["type"],
// removed
| "Noop"
// renamed
| t.DeprecatedAliases["type"]
>
];
if (!printMethod) {
if (printMethod === undefined) {
throw new ReferenceError(
`unknown node of type ${JSON.stringify(
node.type,
)} with constructor ${JSON.stringify(node?.constructor.name)}`,
nodeType,
)} with constructor ${JSON.stringify(node.constructor.name)}`,
);
}

this._printStack.push(node);

const oldInAux = this._insideAux;
this._insideAux = !node.loc;
this._insideAux = node.loc == undefined;
this._maybeAddAuxComment(this._insideAux && !oldInAux);

let shouldPrintParens: boolean;
if (
this.format.retainFunctionParens &&
node.type === "FunctionExpression" &&
format.retainFunctionParens &&
nodeType === "FunctionExpression" &&
node.extra &&
node.extra.parenthesized
) {
Expand All @@ -554,18 +556,24 @@ class Printer {

this._printLeadingComments(node);

const loc = isProgram(node) || isFile(node) ? null : node.loc;
const loc = nodeType === "Program" || nodeType === "File" ? null : node.loc;

this.withSource("start", loc, printMethod.bind(this, node, parent));

this._printTrailingComments(node);
if (noLineTerminator && !this._noLineTerminator) {
this._noLineTerminator = true;
this._printTrailingComments(node);
this._noLineTerminator = false;
} else {
this._printTrailingComments(node);
}

if (shouldPrintParens) this.token(")");

// end
this._printStack.pop();

this.format.concise = oldConcise;
format.concise = oldConcise;
this._insideAux = oldInAux;
}

Expand Down Expand Up @@ -667,7 +675,7 @@ class Printer {
printBlock(parent: Extract<t.Node, { body: t.Statement }>) {
const node = parent.body;

if (!isEmptyStatement(node)) {
if (node.type !== "EmptyStatement") {
this.space();
}

Expand Down
@@ -1 +1,3 @@
export default (): void /* hi! */ => {};

const foo: any /* Hi! */[] = [];
@@ -0,0 +1,2 @@
export default ((): void /* hi! */ => {});
const foo: any /* Hi! */[] = [];
@@ -0,0 +1,3 @@
export default (): void /* hi! */ => {};

const foo: any /* Hi! */[] = [];
@@ -0,0 +1,2 @@
export default ((): void /* hi! */ => {});
const foo: any /* Hi! */[] = [];

This file was deleted.

@@ -0,0 +1,23 @@
type T = U.
/* 1 */
C /* 2 */ [
/* 3 */
0];

type T2 = U.
/* 1 */
C /* 2 */ [
/* 3 */
];

type T3 = U.
/* 1 */
C /* 2 */ <
/* 3 */
0>;

let f = (x)
/* 1 */
:
/* 2 */
void /* 3 */ => 1
@@ -0,0 +1,3 @@
{
"retainLines": true
}
@@ -0,0 +1,23 @@
type T = U.
/* 1 */
C /* 2 */[
/* 3 */
0];

type T2 = U.
/* 1 */
C /* 2 */[];


Comment on lines +6 to +11
Copy link
Member Author

Choose a reason for hiding this comment

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

There seems to be a bug here.

update: maybe not

Copy link
Member

Choose a reason for hiding this comment

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

Yes, the /* 3 */ comment should be printed somewhere. It's a bug that we already have regardless of this PR.


type T3 = U.
/* 1 */
C /* 2 */<
/* 3 */
0>;

let f = (x)
/* 1 */
:
/* 2 */
void /* 3 */ => 1;