From ec5741d793a6c7e239e83325838c9559ea151986 Mon Sep 17 00:00:00 2001 From: Closure Team Date: Fri, 15 Sep 2023 15:31:53 -0700 Subject: [PATCH] For instance members, declare the computed field keys with side effects as separate statements added before the class declaration. PiperOrigin-RevId: 565794428 --- .../jscomp/RewriteClassMembers.java | 53 ++++++------- .../jscomp/RewriteClassMembersTest.java | 75 ++++++++++++------- 2 files changed, 72 insertions(+), 56 deletions(-) diff --git a/src/com/google/javascript/jscomp/RewriteClassMembers.java b/src/com/google/javascript/jscomp/RewriteClassMembers.java index 48af1b7a22c..c9763eca2d1 100644 --- a/src/com/google/javascript/jscomp/RewriteClassMembers.java +++ b/src/com/google/javascript/jscomp/RewriteClassMembers.java @@ -21,6 +21,7 @@ import com.google.common.collect.ImmutableSet; import com.google.javascript.jscomp.parsing.parser.FeatureSet; import com.google.javascript.jscomp.parsing.parser.FeatureSet.Feature; +import com.google.javascript.rhino.IR; import com.google.javascript.rhino.Node; import java.util.ArrayDeque; import java.util.Deque; @@ -253,11 +254,10 @@ private void moveComputedFieldsWithSideEffectsInsideComputedFunction( * becomes * * 
-   * var $jscomp$computedfield$0;
+   * var $jscomp$computedfield$0 = bar('str');
    * class Foo {
    *   [$jscomp$computedfield$0] = 4;
    * }
-   * $jscomp$computedfield$0 = bar('str');
    *
*/ private void extractExpressionFromCompField( @@ -271,16 +271,12 @@ private void extractExpressionFromCompField( Node compExpression = memberField.getFirstChild().detach(); Node compFieldVar = - astFactory.createSingleVarNameDeclaration(generateUniqueCompFieldVarName(t)); + astFactory.createSingleVarNameDeclaration( + generateUniqueCompFieldVarName(t), compExpression); Node compFieldName = compFieldVar.getFirstChild(); memberField.addChildToFront(compFieldName.cloneNode()); compFieldVar.insertBefore(record.insertionPointBeforeClass); compFieldVar.srcrefTreeIfMissing(record.classNode); - Node exprResult = - astFactory.exprResult(astFactory.createAssign(compFieldName.cloneNode(), compExpression)); - exprResult.insertAfter(record.insertionPointAfterClass); - record.insertionPointAfterClass = exprResult; - exprResult.srcrefTreeIfMissing(record.classNode); } /** Returns $jscomp$compfield$[FILE_ID]$[number] */ @@ -298,7 +294,7 @@ private void rewriteInstanceMembers(NodeTraversal t, ClassRecord record) { ctorCreator.synthesizeClassConstructorIfMissing(t, record.classNode); Node ctor = NodeUtil.getEs6ClassConstructorMemberFunctionDef(record.classNode); Node ctorBlock = ctor.getFirstChild().getLastChild(); - Node insertionPoint = findInitialInstanceInsertionPoint(ctorBlock); + Node insertionPoint = addTemporaryInsertionPoint(ctorBlock); while (!instanceMembers.isEmpty()) { Node instanceMember = instanceMembers.pop(); @@ -314,14 +310,14 @@ private void rewriteInstanceMembers(NodeTraversal t, ClassRecord record) { instanceMember.isMemberFieldDef() ? convNonCompFieldToGetProp(thisNode, instanceMember.detach()) : convCompFieldToGetElem(thisNode, instanceMember.detach()); - if (insertionPoint == ctorBlock) { // insert the field at the beginning of the block, no super - ctorBlock.addChildToFront(transpiledNode); - } else { - transpiledNode.insertAfter(insertionPoint); - } - t.reportCodeChange(); // we moved the field from the class body - t.reportCodeChange(ctorBlock); // to the constructor, so we need both + + transpiledNode.insertBefore(insertionPoint); } + + insertionPoint.detach(); + + t.reportCodeChange(); // we moved the field from the class body + t.reportCodeChange(ctorBlock); // to the constructor, so we need both } /** Rewrites and moves all static blocks and fields */ @@ -399,25 +395,22 @@ private Node convCompFieldToGetElem(Node receiver, Node computedField) { } /** - * Finds the location in the constructor to put the transpiled instance fields - * - *

Returns the constructor body if there is no super() call so the field can be put at the - * beginning of the class + * Finds the location of super() call in the constructor and add a temporary empty node after the + * super() call. If there is no super() call, add a temporary empty node at the beginning of the + * constructor body. * - *

Returns the super() call otherwise so the field can be put after the super() call + *

Returns the added temporary empty node */ - private Node findInitialInstanceInsertionPoint(Node ctorBlock) { - if (NodeUtil.referencesSuper(ctorBlock)) { - // will use the fact that if there is super in the constructor, the first appearance of - // super - // must be the super call + private Node addTemporaryInsertionPoint(Node ctorBlock) { + Node tempNode = IR.empty(); for (Node stmt = ctorBlock.getFirstChild(); stmt != null; stmt = stmt.getNext()) { if (NodeUtil.isExprCall(stmt) && stmt.getFirstFirstChild().isSuper()) { - return stmt; + tempNode.insertAfter(stmt); + return tempNode; } } - } - return ctorBlock; // in case the super loop doesn't work, insert at beginning of block + ctorBlock.addChildToFront(tempNode); + return tempNode; } /** @@ -489,7 +482,7 @@ void enterField(Node field) { if (field.isStaticMember()) { staticMembers.push(field); } else { - instanceMembers.push(field); + instanceMembers.offer(field); } } diff --git a/test/com/google/javascript/jscomp/RewriteClassMembersTest.java b/test/com/google/javascript/jscomp/RewriteClassMembersTest.java index e70d978bae1..029849e1fb7 100644 --- a/test/com/google/javascript/jscomp/RewriteClassMembersTest.java +++ b/test/com/google/javascript/jscomp/RewriteClassMembersTest.java @@ -283,17 +283,11 @@ public void testSuperInStaticField() { "class Bar {", " static a = { method1() {} };", " static b = { method2() { super.method1(); } };", - "}", - "Object.setPrototypeOf = function(c, d) {}", - "Object.setPrototypeOf(Foo.b, Foo.a);", - "Foo.b.method2();"), + "}"), lines( "class Bar {}", "Bar.a = { method1() {} };", - "Bar.b = { method2() { super.method1(); } };", - "Object.setPrototypeOf = function(c, d) {}", - "Object.setPrototypeOf(Foo.b, Foo.a);", - "Foo.b.method2();")); + "Bar.b = { method2() { super.method1(); } };")); } @Test @@ -302,15 +296,17 @@ public void testComputedPropInNonStaticField() { lines( "/** @unrestricted */", "class C {", // - " ['x'];", - " ['y'] = 3;", + " [x+=1];", + " [x+=2] = 3;", "}"), lines( - "class C {", // + "var $jscomp$compfield$m1146332801$0 = x += 1;", + "var $jscomp$compfield$m1146332801$1 = x += 2;", + "class C {", " constructor() {", - " this['x'];", - " this['y'] = 3;", - " }", + " this[$jscomp$compfield$m1146332801$0];", + " this[$jscomp$compfield$m1146332801$1] = 3;", + " }", "}")); test( @@ -492,14 +488,13 @@ public void testSideEffectsInComputedField() { lines( "function bar() {", " this.x = 3;", - " var COMPFIELD$0;", + " var COMPFIELD$0 = this.x;", " class Foo {", " constructor() {", " this.y;", " this[COMPFIELD$0] = 2;", " }", " }", - " COMPFIELD$0 = this.x;", "}"))); computedFieldTest( @@ -522,13 +517,12 @@ public void testSideEffectsInComputedField() { "}", "class F extends E {", " x() {", - " var COMPFIELD$0;", + " var COMPFIELD$0 = super.y();", " const testcode$classdecl$var0 = class {", " constructor() {", " this[COMPFIELD$0] = 4;", " }", " };", - " COMPFIELD$0 = super.y();", " return testcode$classdecl$var0;", " }", "}"))); @@ -546,15 +540,13 @@ public void testSideEffectsInComputedField() { expected( lines( "function bar(num) {}", - "var COMPFIELD$0;", - "var COMPFIELD$1;", + "var COMPFIELD$0 = bar(1);", + "var COMPFIELD$1 = bar(2);", "class Foo {", " constructor() {", " this[COMPFIELD$0] = 'a'", " }", "}", - "COMPFIELD$0 = bar(1);", - "COMPFIELD$1 = bar(2);", "Foo.b = bar(3);", "Foo[COMPFIELD$1] = bar(4);"))); @@ -569,9 +561,8 @@ public void testSideEffectsInComputedField() { expected( lines( "let x = 'hello';", - "var COMPFIELD$0;", + "var COMPFIELD$0 = x;", "class Foo {}", - "COMPFIELD$0 = x;", "Foo.n = x = 5;", "Foo[COMPFIELD$0] = 'world';"))); @@ -843,16 +834,20 @@ public void testStaticNoncomputed() { public void testInstanceNoncomputedWithNonemptyConstructor() { test( lines( - "class C {", // + "class C extends Object {", // " x = 1;", + " z = 3;", " constructor() {", + " super();", " this.y = 2;", " }", "}"), lines( - "class C {", // + "class C extends Object{", // " constructor() {", + " super();", " this.x = 1", + " this.z = 3", " this.y = 2;", " }", "}")); @@ -962,6 +957,34 @@ public void testInstanceNoncomputedWithNonemptyConstructor() { "}")); } + @Test + public void testInstanceComputedWithNonemptyConstructorAndSuper() { + + computedFieldTest( + srcs( + lines( + "class A { constructor() { alert(1); } }", + "/** @unrestricted */ class C extends A {", // + " ['x'] = 1;", + " constructor() {", + " super();", + " this['y'] = 2;", + " this['z'] = 3;", + " }", + "}")), + expected( + lines( + "class A { constructor() { alert(1); } }", + "class C extends A {", // + " constructor() {", + " super()", + " this['x'] = 1", + " this['y'] = 2;", + " this['z'] = 3;", + " }", + "}"))); + } + @Test public void testInstanceNoncomputedWithNonemptyConstructorAndSuper() { test(