From 5f82d52c47dfff64cb1572dae5c71242131102c7 Mon Sep 17 00:00:00 2001 From: Closure Team Date: Thu, 22 Jun 2023 15:41:48 -0700 Subject: [PATCH] Update RewriteAsyncIteration to work on a normalized AST. Arrow functions now have a block with a return in them. The for loop initializer has been moved outside the for loop. Unit tests were modified to work on normalization. PiperOrigin-RevId: 542687541 --- .../jscomp/RewriteAsyncIteration.java | 26 ++--- .../jscomp/RewriteAsyncIterationTest.java | 97 +++++++++++-------- 2 files changed, 73 insertions(+), 50 deletions(-) diff --git a/src/com/google/javascript/jscomp/RewriteAsyncIteration.java b/src/com/google/javascript/jscomp/RewriteAsyncIteration.java index 0aa8549b0d4..a2fec9a6def 100644 --- a/src/com/google/javascript/jscomp/RewriteAsyncIteration.java +++ b/src/com/google/javascript/jscomp/RewriteAsyncIteration.java @@ -69,7 +69,7 @@ public final class RewriteAsyncIteration implements NodeTraversal.Callback, Comp "$jscomp.AsyncGeneratorWrapper$ActionEnum.YIELD_VALUE"; private static final String ACTION_ENUM_YIELD_STAR = "$jscomp.AsyncGeneratorWrapper$ActionEnum.YIELD_STAR"; - + // Variables with these names get created when rewriting for-await-of loops private static final String FOR_AWAIT_ITERATOR_TEMP_NAME = "$jscomp$forAwait$tempIterator"; private static final String FOR_AWAIT_RESULT_TEMP_NAME = "$jscomp$forAwait$tempResult"; @@ -412,7 +412,7 @@ private void convertYieldOfAsyncGenerator(LexicalContext ctx, Node yieldNode) { newActionRecord.addChildToBack(astFactory.createQName(this.namespace, ACTION_ENUM_YIELD)); newActionRecord.addChildToBack(expression); } - + newActionRecord.srcrefTreeIfMissing(yieldNode); yieldNode.addChildToFront(newActionRecord); yieldNode.putBooleanProp(Node.YIELD_ALL, false); @@ -440,18 +440,18 @@ private void convertReturnOfAsyncGenerator(LexicalContext ctx, Node returnNode) checkState(returnNode.isReturn()); checkState(ctx != null && ctx.function != null); checkState(ctx.function.isAsyncGeneratorFunction()); - + Node expression = returnNode.removeFirstChild(); Node newActionRecord = astFactory.createNewNode(astFactory.createQName(this.namespace, ACTION_RECORD_NAME)); - + if (expression == null) { expression = NodeUtil.newUndefinedNode(null); } // return expression becomes new ActionRecord(YIELD, expression) newActionRecord.addChildToBack(astFactory.createQName(this.namespace, ACTION_ENUM_YIELD)); newActionRecord.addChildToBack(expression); - + newActionRecord.srcrefTreeIfMissing(returnNode); returnNode.addChildToFront(newActionRecord); } @@ -598,7 +598,7 @@ private void replaceForAwaitOf(LexicalContext ctx, Node forAwaitOf) { Node newForLoop = astFactory.createFor( - initializer, + astFactory.createEmpty(), astFactory.createEmpty(), astFactory.createEmpty(), astFactory.createBlock( @@ -607,9 +607,10 @@ private void replaceForAwaitOf(LexicalContext ctx, Node forAwaitOf) { if (replacementPoint.isLabel()) { newForLoop = astFactory.createLabel(replacementPoint.getFirstChild().cloneNode(), newForLoop); } - + // Generates code `try { .. newForLoop .. }` Node tryNode = createOuterTry(newForLoop); + initializer.insertBefore(newForLoop); // Generate code `catch(e) { errorRes = { error: e }; }` Node catchNode = createOuterCatch(catchErrorParamTempName, errorResultTempName); @@ -631,7 +632,7 @@ private void replaceForAwaitOf(LexicalContext ctx, Node forAwaitOf) { errorResDecl.insertBefore(tryCatchFinally); tempResultDecl.insertBefore(tryCatchFinally); returnFuncDecl.insertBefore(tryCatchFinally); - + compiler.reportChangeToEnclosingScope(tryCatchFinally); } @@ -936,15 +937,16 @@ private void prependTempVarDeclarations(LexicalContext ctx, NodeTraversal t) { } private Node createSuperMethodReferenceGetter(Node replacedMethodReference, NodeTraversal t) { - - // const super$get$x = () => super.x; + // const super$get$x = () => { return super.x; }; AstFactory.Type typeOfSuper = type(replacedMethodReference.getFirstChild()); Node superReference = astFactory.createSuper(typeOfSuper); String replacedMethodName = replacedMethodReference.getString(); Node arrowFunction = astFactory.createZeroArgArrowFunctionForExpression( - astFactory.createGetProp( - superReference, replacedMethodName, type(replacedMethodReference))); + astFactory.createBlock( + astFactory.createReturn( + astFactory.createGetProp( + superReference, replacedMethodName, type(replacedMethodReference))))); compiler.reportChangeToChangeScope(arrowFunction); NodeUtil.addFeatureToScript(t.getCurrentScript(), Feature.ARROW_FUNCTIONS, compiler); String superReplacementName = SUPER_PROP_GETTER_PREFIX + replacedMethodName; diff --git a/test/com/google/javascript/jscomp/RewriteAsyncIterationTest.java b/test/com/google/javascript/jscomp/RewriteAsyncIterationTest.java index 82eba407eca..a1b10f91a64 100644 --- a/test/com/google/javascript/jscomp/RewriteAsyncIterationTest.java +++ b/test/com/google/javascript/jscomp/RewriteAsyncIterationTest.java @@ -49,6 +49,7 @@ public RewriteAsyncIterationTest() { @Before public void enableTypeCheckBeforePass() { + enableNormalize(); enableTypeCheck(); enableTypeInfoValidation(); allowExternsChanges(); @@ -102,8 +103,8 @@ public void testForAwaitWithThrow() { " var $jscomp$forAwait$tempResult0;", " var $jscomp$forAwait$retFn0;", " try {", - " for (var $jscomp$forAwait$tempIterator0 = $jscomp.makeAsyncIterator(source());;)" - + " {", + "var $jscomp$forAwait$tempIterator0 = $jscomp.makeAsyncIterator(source());", + " for (;;)" + " {", " $jscomp$forAwait$tempResult0 = await $jscomp$forAwait$tempIterator0.next();", " if ($jscomp$forAwait$tempResult0.done) {", " break;", @@ -139,7 +140,8 @@ public void testBug173319540() { srcs( lines( "", // - "let key, value;", + "let key;", + "let value;", "window.onload = async function() {", " for await ([key,value] of window[\"unknownAsyncIterable\"]) {", " alert(key,value);", @@ -149,14 +151,16 @@ public void testBug173319540() { expected( lines( "", // - "let key, value;", + "let key;", + "let value;", "window.onload = async function() {", " var $jscomp$forAwait$errResult0;", " var $jscomp$forAwait$tempResult0;", " var $jscomp$forAwait$retFn0;", " try {", - " for (var $jscomp$forAwait$tempIterator0 =" - + " $jscomp.makeAsyncIterator(window[\"unknownAsyncIterable\"]);;) {", + "var $jscomp$forAwait$tempIterator0 =" + + " $jscomp.makeAsyncIterator(window[\"unknownAsyncIterable\"]);", + " for (;;) {", " $jscomp$forAwait$tempResult0 = await $jscomp$forAwait$tempIterator0.next();", " if ($jscomp$forAwait$tempResult0.done) {", " break;", @@ -199,15 +203,16 @@ public void testBug173319540() { " var $jscomp$forAwait$tempResult0;", " var $jscomp$forAwait$retFn0;", " try {", - " for (var $jscomp$forAwait$tempIterator0 =" - + " $jscomp.makeAsyncIterator(window[\"unknownAsyncIterable\"]);;) {", + "var $jscomp$forAwait$tempIterator0 =" + + " $jscomp.makeAsyncIterator(window[\"unknownAsyncIterable\"]);", + " for (;;) {", " $jscomp$forAwait$tempResult0 = await $jscomp$forAwait$tempIterator0.next();", " if ($jscomp$forAwait$tempResult0.done) {", " break;", " }", - " const [key, value] = $jscomp$forAwait$tempResult0.value;", + " const [key, value$jscomp$3] = $jscomp$forAwait$tempResult0.value;", " {", - " alert(key, value);", + " alert(key, value$jscomp$3);", " }", " }", " } catch ($jscomp$forAwait$catchErrParam0) {", @@ -439,7 +444,8 @@ public void testThisInArrowNestedInAsyncGenerator() { " return new $jscomp.AsyncGeneratorWrapper((function*() {", " return new $jscomp.AsyncGeneratorWrapper$ActionRecord(", " $jscomp.AsyncGeneratorWrapper$ActionEnum.YIELD_VALUE,", - " (t = $jscomp$asyncIter$this) => t || $jscomp$asyncIter$this);", + " (t = $jscomp$asyncIter$this) => {", + " return t || $jscomp$asyncIter$this});", " })());", "}", "")); @@ -468,7 +474,9 @@ public void testThisInFunctionNestedInAsyncGenerator() { " (function*() {", " return new $jscomp.AsyncGeneratorWrapper$ActionRecord(", " $jscomp.AsyncGeneratorWrapper$ActionEnum.YIELD_VALUE,", - " () => $jscomp$asyncIter$this);", + " () => { ", + " return $jscomp$asyncIter$this;", + " });", " })());", "}")); } @@ -496,8 +504,9 @@ public void testInnerSuperReferenceInAsyncGenerator() { "}", "class X extends A {", " m() {", - " const $jscomp$asyncIter$super$get$m =", - " () => super.m;", + " const $jscomp$asyncIter$super$get$m = () => {", + " return super.m; ", + " };", " return new $jscomp.AsyncGeneratorWrapper(", " function* () {", " const tmp = $jscomp$asyncIter$super$get$m();", @@ -532,8 +541,9 @@ public void testInnerSuperCallInAsyncGenerator() { "class X extends A {", " m() {", " const $jscomp$asyncIter$this = this;", - " const $jscomp$asyncIter$super$get$m =", - " () => super.m;", + " const $jscomp$asyncIter$super$get$m = () => {", + " return super.m;", + " };", " return new $jscomp.AsyncGeneratorWrapper(", " function* () {", " return new $jscomp.AsyncGeneratorWrapper$ActionRecord(", @@ -611,7 +621,8 @@ public void testForAwaitOfDeclarations() { " var $jscomp$forAwait$tempResult0;", " var $jscomp$forAwait$retFn0;", " try {", - " for (var $jscomp$forAwait$tempIterator0 = $jscomp.makeAsyncIterator(foo());;) {", + " var $jscomp$forAwait$tempIterator0 = $jscomp.makeAsyncIterator(foo());", + " for (;;) {", " $jscomp$forAwait$tempResult0 = await $jscomp$forAwait$tempIterator0.next();", " if ($jscomp$forAwait$tempResult0.done) {", " break;", @@ -639,14 +650,21 @@ public void testForAwaitOfDeclarations() { "function foo() {", "}")); - Node forNode = - findFunctionDefinition(getLastCompiler(), "abc") - .getRootNode() + Node abcFunction = findFunctionDefinition(getLastCompiler(), "abc").getRootNode(); + Node firstTry = + abcFunction .getLastChild() // block - .getLastChild() // try - .getFirstFirstChild(); // for + .getLastChild(); // try + Node forNode = + firstTry + .getFirstChild() // block + .getSecondChild(); // for assertNode(forNode).hasToken(Token.FOR); - Node tempIterator0 = forNode.getFirstFirstChild(); + Node tempIterator0 = + firstTry + .getFirstFirstChild() // var + .getFirstChild(); // name + assertNode(tempIterator0).hasToken(Token.NAME); // Find $jscomp.makeAsyncIterator(foo()) Node makeAsyncIteratorCall = tempIterator0.getFirstChild(); Node block = forNode.getLastChild(); @@ -665,15 +683,12 @@ public void testForAwaitOfDeclarations() { .getChildAtIndex(2) // exprResult .getFirstChild() // assign .getLastChild(); // getprop - assertNode(tempIterator0) .hasColorThat() .isEqualTo(getGlobalColor(StandardColors.ASYNC_ITERATOR_ITERABLE_ID)); assertNode(makeAsyncIteratorCall) .hasColorThat() .isEqualTo(getGlobalColor(StandardColors.ASYNC_ITERATOR_ITERABLE_ID)); - assertNode(tempResult0).hasColorThat().isEqualTo(StandardColors.TOP_OBJECT); // IIterableResult - assertNode(await).hasColorThat().isEqualTo(StandardColors.TOP_OBJECT); // IIterableResult assertNode(nextCall).hasColorThat().isEqualTo(getGlobalColor(StandardColors.PROMISE_ID)); assertNode(done).hasColorThat().isEqualTo(StandardColors.BOOLEAN); assertNode(value).hasColorThat().isEqualTo(StandardColors.NUMBER); @@ -682,16 +697,18 @@ public void testForAwaitOfDeclarations() { lines("async function abc() { for await (var a of foo()) { bar(); } }"), lines( "async function abc() {", + " var a$jscomp$3;", " var $jscomp$forAwait$errResult0;", " var $jscomp$forAwait$tempResult0;", " var $jscomp$forAwait$retFn0;", " try {", - " for (var $jscomp$forAwait$tempIterator0 = $jscomp.makeAsyncIterator(foo());;) {", + " var $jscomp$forAwait$tempIterator0 = $jscomp.makeAsyncIterator(foo()); ", + " for (;;) {", " $jscomp$forAwait$tempResult0 = await $jscomp$forAwait$tempIterator0.next();", " if ($jscomp$forAwait$tempResult0.done) {", " break;", " }", - " var a = $jscomp$forAwait$tempResult0.value;", + " a$jscomp$3 = $jscomp$forAwait$tempResult0.value;", " {", " bar();", " }", @@ -720,12 +737,13 @@ public void testForAwaitOfDeclarations() { " var $jscomp$forAwait$tempResult0;", " var $jscomp$forAwait$retFn0;", " try {", - " for (var $jscomp$forAwait$tempIterator0 = $jscomp.makeAsyncIterator(foo());;) {", + " var $jscomp$forAwait$tempIterator0 = $jscomp.makeAsyncIterator(foo());", + " for (;;) {", " $jscomp$forAwait$tempResult0 = await $jscomp$forAwait$tempIterator0.next();", " if ($jscomp$forAwait$tempResult0.done) {", " break;", " }", - " let a = $jscomp$forAwait$tempResult0.value;", + " let a$jscomp$3 = $jscomp$forAwait$tempResult0.value;", " {", " bar();", " }", @@ -754,12 +772,13 @@ public void testForAwaitOfDeclarations() { " var $jscomp$forAwait$tempResult0;", " var $jscomp$forAwait$retFn0;", " try {", - " for (var $jscomp$forAwait$tempIterator0 = $jscomp.makeAsyncIterator(foo());;) {", + " var $jscomp$forAwait$tempIterator0 = $jscomp.makeAsyncIterator(foo());", + " for (;;) {", " $jscomp$forAwait$tempResult0 = await $jscomp$forAwait$tempIterator0.next();", " if ($jscomp$forAwait$tempResult0.done) {", " break;", " }", - " const a = $jscomp$forAwait$tempResult0.value;", + " const a$jscomp$3 = $jscomp$forAwait$tempResult0.value;", " {", " bar();", " }", @@ -791,12 +810,13 @@ public void testForAwaitOfInAsyncArrow() { " var $jscomp$forAwait$tempResult0;", " var $jscomp$forAwait$retFn0;", " try {", - " for (var $jscomp$forAwait$tempIterator0 = $jscomp.makeAsyncIterator(foo());;) {", + "var $jscomp$forAwait$tempIterator0 = $jscomp.makeAsyncIterator(foo());", + " for (;;) {", " $jscomp$forAwait$tempResult0 = await $jscomp$forAwait$tempIterator0.next();", " if ($jscomp$forAwait$tempResult0.done) {", " break;", " }", - " let a = $jscomp$forAwait$tempResult0.value;", + " let a$jscomp$3 = $jscomp$forAwait$tempResult0.value;", " {", " bar();", " }", @@ -835,13 +855,13 @@ public void testLabelledForAwaitOf() { " var $jscomp$forAwait$retFn0;", " try {", // rewriting does not lose the label with the for await of statement - " label: for (var $jscomp$forAwait$tempIterator0 =" - + " $jscomp.makeAsyncIterator(foo());;) {", + " var $jscomp$forAwait$tempIterator0 = $jscomp.makeAsyncIterator(foo());", + " label: for (;;) {", " $jscomp$forAwait$tempResult0 = await $jscomp$forAwait$tempIterator0.next();", " if ($jscomp$forAwait$tempResult0.done) {", " break;", " }", - " let a = $jscomp$forAwait$tempResult0.value;", + " let a$jscomp$3 = $jscomp$forAwait$tempResult0.value;", " {", " bar();", " }", @@ -879,7 +899,8 @@ public void testForAwaitOfInAsyncGenerator() { " var $jscomp$forAwait$tempResult0;", " var $jscomp$forAwait$retFn0;", " try {", - " for (var $jscomp$forAwait$tempIterator0 = $jscomp.makeAsyncIterator(bar());;) {", + "var $jscomp$forAwait$tempIterator0 = $jscomp.makeAsyncIterator(bar());", + " for (;;) {", " $jscomp$forAwait$tempResult0 = yield new" + " $jscomp.AsyncGeneratorWrapper$ActionRecord($jscomp.AsyncGeneratorWrapper$ActionEnum.AWAIT_VALUE," + " $jscomp$forAwait$tempIterator0.next());",