Skip to content

Commit

Permalink
fix: ) position with createParenthesizedExpressions (#15515)
Browse files Browse the repository at this point in the history 
* fix

* fix jest

* revert and add a test
  • Loading branch information
@liuxingbaoyu
liuxingbaoyu committed Apr 24, 2023
1 parent 978ea31 commit 1c2311a
Show file tree
Hide file tree
Showing 28 changed files with 315 additions and 69 deletions.
2 changes: 1 addition & 1 deletion packages/babel-core/test/fixtures/transformation/source-maps/class/source-map.json
View file
Expand Up @@ -17,5 +17,5 @@
"sourcesContent": [
"class Test {\n get bar() {\n throw new Error(\"wow\");\n }\n}\n\nvar test = new Test;\ntest.bar;"
],
"mappings": "IAAMA,IAAI;EAAA;;EAAA,SAAAA,KAAA;IAAAC,YAAA,CAAAC,cAAA,OAAAF,IAAA;EAAA;EAAAC,YAAA,CAAAE,WAAA,CAAAH,IAAA;IAAAI,GAAA;IAAAC,GAAA,EACR,SAAAA,CAAA,EAAU;MACR,MAAM,IAAIC,KAAK,CAAC,KAAK,CAAC;IACxB;EAAC;EAAA,OAAAN,IAAA;AAAA;AAGH,IAAIO,IAAI,GAAG,IAAIP,IAAI;AACnBO,IAAI,CAACC,GAAG"
"mappings": "IAAMA,IAAI;EAAA;;EAAA,SAAAA,KAAA;IAAAC,YAAA,CAAAC,cAAA,OAAAF,IAAA;EAAA;EAAAC,YAAA,CAAAE,WAAA,CAAAH,IAAA;IAAAI,GAAA;IAAAC,GAAA,EACR,SAAAA,CAAA,EAAU;MACR,MAAM,IAAIC,KAAK,CAAC,KAAK,CAAC;IACxB;EAAC;EAAA,OAAAN,IAAA;AAAA;AAGH,IAAIO,IAAI,GAAG,IAAIP,IAAI,CAAD,CAAC;AACnBO,IAAI,CAACC,GAAG"
}
4 changes: 1 addition & 3 deletions packages/babel-generator/src/generators/base.ts
View file
Expand Up @@ -48,9 +48,7 @@ export function BlockStatement(this: Printer, node: t.BlockStatement) {

this.printSequence(node.body, node, { indent: true });

this.sourceWithOffset("end", node.loc, 0, -1);

this.rightBrace();
this.rightBrace(node);
}

export function Directive(this: Printer, node: t.Directive) {
Expand Down
9 changes: 2 additions & 7 deletions packages/babel-generator/src/generators/classes.ts
View file
Expand Up @@ -78,9 +78,7 @@ export function ClassBody(this: Printer, node: t.ClassBody) {

if (!this.endsWith(charCodes.lineFeed)) this.newline();

this.sourceWithOffset("end", node.loc, 0, -1);

this.rightBrace();
this.rightBrace(node);
}
}

Expand Down Expand Up @@ -224,9 +222,6 @@ export function StaticBlock(this: Printer, node: t.StaticBlock) {
this.printSequence(node.body, node, {
indent: true,
});

this.sourceWithOffset("end", node.loc, 0, -1);

this.rightBrace();
this.rightBrace(node);
}
}
40 changes: 21 additions & 19 deletions packages/babel-generator/src/generators/expressions.ts
View file
Expand Up @@ -9,17 +9,18 @@ import type * as t from "@babel/types";
import * as n from "../node";

export function UnaryExpression(this: Printer, node: t.UnaryExpression) {
const { operator } = node;
if (
node.operator === "void" ||
node.operator === "delete" ||
node.operator === "typeof" ||
operator === "void" ||
operator === "delete" ||
operator === "typeof" ||
// throwExpressions
node.operator === "throw"
operator === "throw"
) {
this.word(node.operator);
this.word(operator);
this.space();
} else {
this.token(node.operator);
this.token(operator);
}

this.print(node.argument, node);
Expand All @@ -41,7 +42,7 @@ export function ParenthesizedExpression(
) {
this.token("(");
this.print(node.expression, node);
this.token(")");
this.rightParens(node);
}

export function UpdateExpression(this: Printer, node: t.UpdateExpression) {
Expand Down Expand Up @@ -97,7 +98,7 @@ export function NewExpression(
}
this.token("(");
this.printList(node.arguments, node);
this.token(")");
this.rightParens(node);
}

export function SequenceExpression(this: Printer, node: t.SequenceExpression) {
Expand Down Expand Up @@ -169,30 +170,32 @@ export function OptionalMemberExpression(
this: Printer,
node: t.OptionalMemberExpression,
) {
let { computed } = node;
const { optional, property } = node;

this.print(node.object, node);

if (!node.computed && isMemberExpression(node.property)) {
if (!computed && isMemberExpression(property)) {
throw new TypeError("Got a MemberExpression for MemberExpression property");
}

let computed = node.computed;
// @ts-expect-error todo(flow->ts) maybe instead of typeof check specific literal types?
if (isLiteral(node.property) && typeof node.property.value === "number") {
if (isLiteral(property) && typeof property.value === "number") {
computed = true;
}
if (node.optional) {
if (optional) {
this.token("?.");
}

if (computed) {
this.token("[");
this.print(node.property, node);
this.print(property, node);
this.token("]");
} else {
if (!node.optional) {
if (!optional) {
this.token(".");
}
this.print(node.property, node);
this.print(property, node);
}
}

Expand All @@ -212,7 +215,7 @@ export function OptionalCallExpression(

this.token("(");
this.printList(node.arguments, node);
this.token(")");
this.rightParens(node);
}

export function CallExpression(this: Printer, node: t.CallExpression) {
Expand All @@ -222,7 +225,7 @@ export function CallExpression(this: Printer, node: t.CallExpression) {
this.print(node.typeParameters, node); // TS
this.token("(");
this.printList(node.arguments, node);
this.token(")");
this.rightParens(node);
}

export function Import(this: Printer) {
Expand Down Expand Up @@ -377,6 +380,5 @@ export function ModuleExpression(this: Printer, node: t.ModuleExpression) {
}
this.print(body, node);
this.dedent();
this.sourceWithOffset("end", node.loc, 0, -1);
this.rightBrace();
this.rightBrace(node);
}
2 changes: 1 addition & 1 deletion packages/babel-generator/src/generators/statements.ts
View file
Expand Up @@ -224,7 +224,7 @@ export function SwitchStatement(this: Printer, node: t.SwitchStatement) {
},
});

this.token("}");
this.rightBrace(node);
}

export function SwitchCase(this: Printer, node: t.SwitchCase) {
Expand Down
4 changes: 1 addition & 3 deletions packages/babel-generator/src/generators/typescript.ts
View file
Expand Up @@ -317,9 +317,7 @@ function tsPrintBraced(printer: Printer, members: t.Node[], node: t.Node) {
printer.dedent();
}

printer.sourceWithOffset("end", node.loc, 0, -1);

printer.rightBrace();
printer.rightBrace(node);
}

export function TSArrayType(this: Printer, node: t.TSArrayType) {
Expand Down
43 changes: 21 additions & 22 deletions packages/babel-generator/src/printer.ts
View file
Expand Up @@ -174,13 +174,19 @@ class Printer {
* Add a right brace to the buffer.
*/

rightBrace(): void {
rightBrace(node: t.Node): void {
if (this.format.minified) {
this._buf.removeLastSemicolon();
}
this.sourceWithOffset("end", node.loc, 0, -1);
this.token("}");
}

rightParens(node: t.Node): void {
this.sourceWithOffset("end", node.loc, 0, -1);

This comment has been minimized.

Sign in to view

Copy link
@StNekroman

StNekroman May 11, 2023

@liuxingbaoyu after this line, which arrived within "@babel/generator": "7.21.5",
I started to face a webpack build failure with error message:

ModuleBuildError: Module build failed (from ./node_modules/babel-loader/lib/index.js):
Error: <path>\node_modules\@ux-aspects\ux-aspects\fesm2020\ux-aspects-ux-aspects.mjs: `column` must be greater than or equal to 0 (columns start at column 0)

After downgrade it back to 7.21.4 - everything works.
As far as I debuged, looks like it fails on processing Angular's component, which has simple template: "{{ dateNumber }}\n"

Not going to submit an issue, because I don't have standalone reproduce. But may be you, as author, can imagine what could go wrong?
In my case error somes from @jridgewell/trace-mapping where it tries to wire source map to source location {line:32, column: -1}, so -1 is unacceptable.

This comment has been minimized.

Sign in to view

Copy link
@liuxingbaoyu

liuxingbaoyu May 11, 2023

Author Member

Thank you for your report! Can you provide the full error stack?

This comment has been minimized.

Sign in to view

Copy link
@StNekroman

StNekroman May 11, 2023

@liuxingbaoyu
But that isn't very helpful anyway.
In my case error comes exactly from commented line.
-1 makes column = -1

This comment has been minimized.

Sign in to view

Copy link
@StNekroman

StNekroman May 11, 2023

MicrosoftTeams-image (2)
this.token(")");
}

/**
* Add a space to the buffer unless it is compact.
*/
Expand Down Expand Up @@ -240,7 +246,6 @@ class Printer {
/**
* Writes a simple token.
*/

token(str: string, maybeNewline = false): void {
this._maybePrintInnerComments();

Expand Down Expand Up @@ -270,8 +275,6 @@ class Printer {
tokenChar(char: number): void {
this._maybePrintInnerComments();

// space is mandatory to avoid outputting <!--
// http://javascript.spec.whatwg.org/#comment-syntax
const lastChar = this.getLastChar();
if (
// Need spaces for operators of the same kind to avoid: `a+++b`
Expand Down Expand Up @@ -553,9 +556,9 @@ class Printer {
if (!this.format.retainLines) return;

// catch up to this nodes newline if we're behind
const pos = loc ? loc[prop] : null;
if (pos?.line != null) {
const count = pos.line - this._buf.getCurrentLine();
const line = loc?.[prop]?.line;
if (line != null) {
const count = line - this._buf.getCurrentLine();

for (let i = 0; i < count; i++) {
this._newline();
Expand Down Expand Up @@ -655,19 +658,13 @@ class Printer {
this._insideAux = node.loc == undefined;
this._maybeAddAuxComment(this._insideAux && !oldInAux);

let shouldPrintParens = false;
if (forceParens) {
shouldPrintParens = true;
} else if (
format.retainFunctionParens &&
nodeType === "FunctionExpression" &&
node.extra &&
node.extra.parenthesized
) {
shouldPrintParens = true;
} else {
shouldPrintParens = needsParens(node, parent, this._printStack);
}
const shouldPrintParens =
forceParens ||
(format.retainFunctionParens &&
nodeType === "FunctionExpression" &&
node.extra?.parenthesized) ||
needsParens(node, parent, this._printStack);

if (shouldPrintParens) {
this.token("(");
this._endsWithInnerRaw = false;
Expand Down Expand Up @@ -895,12 +892,14 @@ class Printer {
}

_printNewline(newLine: boolean, opts: AddNewlinesOptions) {
const format = this.format;

// Fast path since 'this.newline' does nothing when not tracking lines.
if (this.format.retainLines || this.format.compact) return;
if (format.retainLines || format.compact) return;

// Fast path for concise since 'this.newline' just inserts a space when
// concise formatting is in use.
if (this.format.concise) {
if (format.concise) {
this.space();
return;
}
Expand Down
2 changes: 1 addition & 1 deletion ...ression/comment-before-parentheses-return-arg-createParenthesizedExpressions/options.json
View file
@@ -1,4 +1,4 @@
{
"retainLines": true,
"createParenthesizedExpressions": true
"parserOpts": { "createParenthesizedExpressions": true }
}
13 changes: 7 additions & 6 deletions ...regression/comment-before-parentheses-return-arg-createParenthesizedExpressions/output.js
View file
@@ -1,16 +1,17 @@
function assertElement(assertFn, shouldBeElement, opt_message) {
return (/** @type {!Ele ment} */
return (/** @type {!Ele ment} */(
assertType_(
assertFn,
shouldBeElement,
isElement(shouldBeElement),
'Element expected',
opt_message));


opt_message
)
));
}

const slot = /** @type {!HTMLSlotElement} */e.target;
const slot = /** @type {!HTMLSlotElement} */(e.target);

assertElement(
/** @type {Element} */el);
/** @type {Element} */(el)
);
2 changes: 1 addition & 1 deletion packages/babel-generator/test/fixtures/sourcemaps/call-identifiers/source-map.json
View file
Expand Up @@ -11,5 +11,5 @@
"sourcesContent": [
"foo;\nfoo();\nfoo().bar;\nobj.foo;\nobj.foo();\nobj.foo.bar;\nobj.foo().bar;\n{\n foo;\n foo();\n foo().bar;\n obj.foo;\n obj.foo();\n obj.foo.bar;\n obj.foo().bar;\n}"
],
"mappings": "AAAAA,GAAG;AACHA,GAAG,EAAE;AACLA,GAAG,EAAE,CAACC,GAAG;AACTC,GAAG,CAACF,GAAG;AACPE,GAAG,CAACF,GAAG,EAAE;AACTE,GAAG,CAACF,GAAG,CAACC,GAAG;AACXC,GAAG,CAACF,GAAG,EAAE,CAACC,GAAG;AACb;EACED,GAAG;EACHA,GAAG,EAAE;EACLA,GAAG,EAAE,CAACC,GAAG;EACTC,GAAG,CAACF,GAAG;EACPE,GAAG,CAACF,GAAG,EAAE;EACTE,GAAG,CAACF,GAAG,CAACC,GAAG;EACXC,GAAG,CAACF,GAAG,EAAE,CAACC,GAAG;AACf"
"mappings": "AAAAA,GAAG;AACHA,GAAG,CAAC,CAAC;AACLA,GAAG,CAAC,CAAC,CAACC,GAAG;AACTC,GAAG,CAACF,GAAG;AACPE,GAAG,CAACF,GAAG,CAAC,CAAC;AACTE,GAAG,CAACF,GAAG,CAACC,GAAG;AACXC,GAAG,CAACF,GAAG,CAAC,CAAC,CAACC,GAAG;AACb;EACED,GAAG;EACHA,GAAG,CAAC,CAAC;EACLA,GAAG,CAAC,CAAC,CAACC,GAAG;EACTC,GAAG,CAACF,GAAG;EACPE,GAAG,CAACF,GAAG,CAAC,CAAC;EACTE,GAAG,CAACF,GAAG,CAACC,GAAG;EACXC,GAAG,CAACF,GAAG,CAAC,CAAC,CAACC,GAAG;AACf"
}
17 changes: 17 additions & 0 deletions .../sourcemaps/comment-before-parentheses-return-arg-createParenthesizedExpressions/input.js
View file
@@ -0,0 +1,17 @@
function assertElement(assertFn, shouldBeElement, opt_message) {
return /** @type {!Ele ment} */ (
assertType_(
assertFn,
shouldBeElement,
isElement(shouldBeElement),
'Element expected',
opt_message
)
);
}

const slot = /** @type {!HTMLSlotElement} */ (e.target);

assertElement(
/** @type {Element} */ (el),
);
5 changes: 5 additions & 0 deletions ...rcemaps/comment-before-parentheses-return-arg-createParenthesizedExpressions/options.json
View file
@@ -0,0 +1,5 @@
{
"retainLines": true,
"parserOpts": { "createParenthesizedExpressions": true },
"sourceMaps": true
}
17 changes: 17 additions & 0 deletions ...sourcemaps/comment-before-parentheses-return-arg-createParenthesizedExpressions/output.js
View file
@@ -0,0 +1,17 @@
function assertElement(assertFn, shouldBeElement, opt_message) {
return (/** @type {!Ele ment} */(
assertType_(
assertFn,
shouldBeElement,
isElement(shouldBeElement),
'Element expected',
opt_message
)
));
}

const slot = /** @type {!HTMLSlotElement} */(e.target);

assertElement(
/** @type {Element} */(el)
);
22 changes: 22 additions & 0 deletions ...maps/comment-before-parentheses-return-arg-createParenthesizedExpressions/source-map.json
View file
@@ -0,0 +1,22 @@
{
"version": 3,
"names": [
"assertElement",
"assertFn",
"shouldBeElement",
"opt_message",
"assertType_",
"isElement",
"slot",
"e",
"target",
"el"
],
"sources": [
"fixtures/sourcemaps/comment-before-parentheses-return-arg-createParenthesizedExpressions/input.js"
],
"sourcesContent": [
"function assertElement(assertFn, shouldBeElement, opt_message) {\n return /** @type {!Ele\tment} */ (\n\t assertType_(\n\t assertFn,\n\t\t shouldBeElement,\n\t\t isElement(shouldBeElement),\n\t\t 'Element expected',\n\t\t opt_message\n\t )\n\t);\n}\n\nconst slot = /** @type {!HTMLSlotElement} */ (e.target);\n\nassertElement(\n /** @type {Element} */ (el),\n);"
],
"mappings": "AAAA,SAASA,aAAaA,CAACC,QAAQ,EAAEC,eAAe,EAAEC,WAAW,EAAE;EAC7D,OAAO,yBAAyB;IAC/BC,WAAW;IACTH,QAAQ;IACTC,eAAe;IACfG,SAAS,CAACH,eAAe,CAAC;IAC1B,kBAAkB;IAClBC;IACD;IACF,CAAC;AACF;;AAEA,MAAMG,IAAI,GAAG,+BAAgC,CAACC,CAAC,CAACC,MAAM,CAAC;;AAEvDR,aAAa;AACX,sBAAuB,CAACS,EAAE;AAC5B,CAAC"
}
17 changes: 17 additions & 0 deletions ...s/babel-generator/test/fixtures/sourcemaps/comment-before-parentheses-return-arg/input.js
View file
@@ -0,0 +1,17 @@
function assertElement(assertFn, shouldBeElement, opt_message) {
return /** @type {!Ele ment} */ (
assertType_(
assertFn,
shouldBeElement,
isElement(shouldBeElement),
'Element expected',
opt_message
)
);
}

const slot = /** @type {!HTMLSlotElement} */ (e.target);

assertElement(
/** @type {Element} */ (el),
);
4 changes: 4 additions & 0 deletions ...bel-generator/test/fixtures/sourcemaps/comment-before-parentheses-return-arg/options.json
View file
@@ -0,0 +1,4 @@
{
"retainLines": true,
"sourceMaps": true
}

0 comments on commit 1c2311a

Please sign in to comment.