Skip to content

Commit

Permalink
Update RewriteAsyncIteration to work on a normalized AST.
Browse files Browse the repository at this point in the history 
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
  • Loading branch information
@copybara-github
Closure Team authored and copybara-github committed Jun 22, 2023
1 parent f396cad commit 5f82d52
Show file tree
Hide file tree
Showing 2 changed files with 73 additions and 50 deletions.
26 changes: 14 additions & 12 deletions src/com/google/javascript/jscomp/RewriteAsyncIteration.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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";
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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);
}
Expand Down Expand Up @@ -598,7 +598,7 @@ private void replaceForAwaitOf(LexicalContext ctx, Node forAwaitOf) {

Node newForLoop =
astFactory.createFor(
initializer,
astFactory.createEmpty(),
astFactory.createEmpty(),
astFactory.createEmpty(),
astFactory.createBlock(
Expand All @@ -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);
Expand All @@ -631,7 +632,7 @@ private void replaceForAwaitOf(LexicalContext ctx, Node forAwaitOf) {
errorResDecl.insertBefore(tryCatchFinally);
tempResultDecl.insertBefore(tryCatchFinally);
returnFuncDecl.insertBefore(tryCatchFinally);

compiler.reportChangeToEnclosingScope(tryCatchFinally);
}

Expand Down Expand Up @@ -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;
Expand Down
97 changes: 59 additions & 38 deletions test/com/google/javascript/jscomp/RewriteAsyncIterationTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@ public RewriteAsyncIterationTest() {

@Before
public void enableTypeCheckBeforePass() {
enableNormalize();
enableTypeCheck();
enableTypeInfoValidation();
allowExternsChanges();
Expand Down Expand Up @@ -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;",
Expand Down Expand Up @@ -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);",
Expand All @@ -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;",
Expand Down Expand Up @@ -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) {",
Expand Down Expand Up @@ -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});",
" })());",
"}",
""));
Expand Down Expand Up @@ -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;",
" });",
" })());",
"}"));
}
Expand Down Expand Up @@ -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();",
Expand Down Expand Up @@ -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(",
Expand Down Expand Up @@ -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;",
Expand Down Expand Up @@ -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();
Expand All @@ -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);
Expand All @@ -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();",
" }",
Expand Down Expand Up @@ -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();",
" }",
Expand Down Expand Up @@ -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();",
" }",
Expand Down Expand Up @@ -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();",
" }",
Expand Down Expand Up @@ -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();",
" }",
Expand Down Expand Up @@ -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());",
Expand Down

0 comments on commit 5f82d52

Please sign in to comment.