Skip to content

Commit 09f2d9b

Browse files
rishipalcopybara-github
authored andcommitted
Place $jscomp$super$this after any hoisted function declarations to preserve normalization when rewriting classes
Currently, the Es6ConvertSuperConstructorCalls generates var declarations as first child of the constructor. This breaks normalization as normalization expects all function declarations to stay hoisted. This CL ensures that. PiperOrigin-RevId: 623222665
1 parent 8eaf354 commit 09f2d9b

File tree

2 files changed

+71
-6
lines changed

2 files changed

+71
-6
lines changed

src/com/google/javascript/jscomp/Es6ConvertSuperConstructorCalls.java

+41-6
Original file line numberDiff line numberDiff line change
@@ -223,10 +223,18 @@ private void visitSuper(NodeTraversal t, ConstructorData constructorData) {
223223
// `this` -> `$jscomp$super$this` throughout the constructor body,
224224
// except for super() calls.
225225
updateThisToSuperThis(typeOfThis, constructorBody, superCalls, uniqueSuperThisName);
226-
// Start constructor with `var $jscomp$super$this;`
227-
constructorBody.addChildToFront(
226+
// Start constructor with `var $jscomp$super$this;`, but place it after any hoisted
227+
// function declarations
228+
Node declaration =
228229
IR.var(astFactory.createName(uniqueSuperThisName, typeOfThis))
229-
.srcrefTree(constructorBody));
230+
.srcrefTree(constructorBody);
231+
Node insertBeforePoint = getInsertBeforePoint(constructorBody);
232+
if (insertBeforePoint != null) {
233+
declaration.insertBefore(insertBeforePoint);
234+
} else {
235+
// functionBody only contains hoisted function declarations
236+
constructorBody.addChildToBack(declaration);
237+
}
230238
// End constructor with `return $jscomp$super$this;`
231239
constructorBody.addChildToBack(
232240
IR.returnNode(astFactory.createName(uniqueSuperThisName, typeOfThis))
@@ -249,6 +257,24 @@ private void visitSuper(NodeTraversal t, ConstructorData constructorData) {
249257
}
250258
}
251259

260+
/**
261+
* Gets an insertion point in the function body before which we want to insert the new let
262+
* declaration. If there's not good insertion point (e.g. the function is empty or only contains
263+
* inner function declarations), return null.
264+
*/
265+
private Node getInsertBeforePoint(Node functionBody) {
266+
checkState(functionBody.getParent().isFunction());
267+
Node current = functionBody.getFirstChild();
268+
269+
// Do not insert the let declaration before any hoisted function declarations in this function
270+
// body as those function declarations are hoisted by normalization. We must maintain
271+
// normalization.
272+
while (current != null && NodeUtil.isFunctionDeclaration(current)) {
273+
current = current.getNext();
274+
}
275+
return current;
276+
}
277+
252278
/**
253279
* Change calls to `super` to use `$jscomp.construct` instead.
254280
*
@@ -299,11 +325,20 @@ private void convertSuperCallsToJsCompConstructCalls(
299325
// `this` -> `$jscomp$super$this` throughout the constructor body,
300326
// except for super() calls.
301327
updateThisToSuperThis(typeOfThis, constructorBody, superCalls, uniqueSuperThisName);
302-
// Start constructor with `var $jscomp$super$this;`
303-
constructorBody.addChildToFront(
328+
// Start constructor with `var $jscomp$super$this;`, but place it after any hoisted
329+
// function declarations
330+
Node declaration =
304331
astFactory
305332
.createSingleVarNameDeclaration(uniqueSuperThisName)
306-
.srcrefTree(constructorBody));
333+
.srcrefTree(constructorBody);
334+
Node insertBeforePoint = getInsertBeforePoint(constructorBody);
335+
if (insertBeforePoint != null) {
336+
declaration.insertBefore(insertBeforePoint);
337+
} else {
338+
// functionBody only contains hoisted function declarations
339+
constructorBody.addChildToBack(declaration);
340+
}
341+
307342
// End constructor with `return $jscomp$super$this;`
308343
constructorBody.addChildToBack(
309344
astFactory

test/com/google/javascript/jscomp/Es6RewriteClassTest.java

+30
Original file line numberDiff line numberDiff line change
@@ -1444,6 +1444,36 @@ public void testExtendNativeClass_withExplicitConstructor() {
14441444
""));
14451445
}
14461446

1447+
@Test
1448+
public void testExtendNativeClass_withExplicitConstructor_withInnerFunction() {
1449+
test(
1450+
lines(
1451+
"class FooPromise extends Promise {",
1452+
" /** @param {string} msg */",
1453+
" constructor(callbackArg, msg) {",
1454+
" function inner() {}", // hoisted, normalized
1455+
" super(callbackArg);",
1456+
" this.msg = msg;",
1457+
" }",
1458+
"}"),
1459+
lines(
1460+
"/**",
1461+
" * @constructor",
1462+
" */",
1463+
"let FooPromise = function(callbackArg, msg) {",
1464+
// stays hoisted
1465+
" function inner() {}",
1466+
// declaration created after function declaration
1467+
" var $jscomp$super$this$m1146332801$0;",
1468+
" $jscomp$super$this$m1146332801$0 =",
1469+
" $jscomp.construct(Promise, [callbackArg], this.constructor);",
1470+
" $jscomp$super$this$m1146332801$0.msg = msg;",
1471+
" return $jscomp$super$this$m1146332801$0;",
1472+
"}",
1473+
"$jscomp.inherits(FooPromise, Promise);",
1474+
""));
1475+
}
1476+
14471477
@Test
14481478
public void testExtendNativeClass_withImplicitConstructor() {
14491479
test(

0 commit comments

Comments
 (0)