From 3fcc79d6fb6d2e2196b52eee3e0123120ba30d62 Mon Sep 17 00:00:00 2001 From: bradfordcsmith Date: Tue, 27 Jun 2017 16:13:47 -0700 Subject: [PATCH] Correct transpilation of async arrow function in non-async function. ------------- Created by MOE: https://github.com/google/moe MOE_MIGRATED_REVID=160341741 --- .../jscomp/RewriteAsyncFunctions.java | 30 +++++++++++------ .../jscomp/RewriteAsyncFunctionsTest.java | 32 +++++++++++++++++-- .../runtime_tests/async_function_test.js | 18 +++++++++++ 3 files changed, 68 insertions(+), 12 deletions(-) diff --git a/src/com/google/javascript/jscomp/RewriteAsyncFunctions.java b/src/com/google/javascript/jscomp/RewriteAsyncFunctions.java index 7ba55274719..9a13115c9ee 100644 --- a/src/com/google/javascript/jscomp/RewriteAsyncFunctions.java +++ b/src/com/google/javascript/jscomp/RewriteAsyncFunctions.java @@ -58,8 +58,8 @@ public final class RewriteAsyncFunctions implements NodeTraversal.Callback, HotS private static final class LexicalContext { final Optional function; // absent for top level final LexicalContext thisAndArgumentsContext; - boolean asyncThisReplacementWasDone = false; - boolean asyncArgumentsReplacementWasDone = false; + boolean mustAddAsyncThisVariable = false; + boolean mustAddAsyncArgumentsVariable = false; /** Creates root-level context. */ LexicalContext() { @@ -79,17 +79,29 @@ boolean isAsyncContext() { } boolean mustReplaceThisAndArguments() { - return thisAndArgumentsContext.isAsyncContext(); + return isAsyncContext() || thisAndArgumentsContext.isAsyncContext(); } void recordAsyncThisReplacementWasDone() { - checkState(thisAndArgumentsContext.isAsyncContext()); - thisAndArgumentsContext.asyncThisReplacementWasDone = true; + if (thisAndArgumentsContext.isAsyncContext()) { + thisAndArgumentsContext.mustAddAsyncThisVariable = true; + } else { + // The current context is an async arrow function within a non-async function, + // so it must define its own replacement variable. + checkState(isAsyncContext()); + mustAddAsyncThisVariable = true; + } } void recordAsyncArgumentsReplacementWasDone() { - checkState(thisAndArgumentsContext.isAsyncContext()); - thisAndArgumentsContext.asyncArgumentsReplacementWasDone = true; + if (thisAndArgumentsContext.isAsyncContext()) { + thisAndArgumentsContext.mustAddAsyncArgumentsVariable = true; + } else { + // The current context is an async arrow function within a non-async function, + // so it must define its own replacement variable. + checkState(isAsyncContext()); + mustAddAsyncArgumentsVariable = true; + } } } @@ -176,10 +188,10 @@ private void convertAsyncFunction(LexicalContext functionContext) { Node newBody = IR.block().useSourceInfoIfMissingFrom(originalBody); originalFunction.replaceChild(originalBody, newBody); - if (functionContext.asyncThisReplacementWasDone) { + if (functionContext.mustAddAsyncThisVariable) { newBody.addChildToBack(IR.constNode(IR.name(ASYNC_THIS), IR.thisNode())); } - if (functionContext.asyncArgumentsReplacementWasDone) { + if (functionContext.mustAddAsyncArgumentsVariable) { newBody.addChildToBack(IR.constNode(IR.name(ASYNC_ARGUMENTS), IR.name("arguments"))); } diff --git a/test/com/google/javascript/jscomp/RewriteAsyncFunctionsTest.java b/test/com/google/javascript/jscomp/RewriteAsyncFunctionsTest.java index 830db70281e..29e45944f88 100644 --- a/test/com/google/javascript/jscomp/RewriteAsyncFunctionsTest.java +++ b/test/com/google/javascript/jscomp/RewriteAsyncFunctionsTest.java @@ -149,12 +149,12 @@ public void testClassMethod() { "}")); } - public void testClassMethodWithAsyncArrow() { + public void testAsyncClassMethodWithAsyncArrow() { test( LINE_JOINER.join( "class A {", " async f() {", - " let g = async () => { console.log(this); };", + " let g = async () => { console.log(this, arguments); };", " g();", " }", "}"), @@ -162,10 +162,11 @@ public void testClassMethodWithAsyncArrow() { "class A {", " f() {", " const $jscomp$async$this = this;", + " const $jscomp$async$arguments = arguments;", " function *$jscomp$async$generator() {", " let g = () => {", " function *$jscomp$async$generator() {", - " console.log($jscomp$async$this);", + " console.log($jscomp$async$this, $jscomp$async$arguments);", " }", " return $jscomp.executeAsyncGenerator($jscomp$async$generator());", " };", @@ -176,6 +177,31 @@ public void testClassMethodWithAsyncArrow() { "}")); } + public void testNonAsyncClassMethodWithAsyncArrow() { + test( + LINE_JOINER.join( + "class A {", + " f() {", + " let g = async () => { console.log(this, arguments); };", + " g();", + " }", + "}"), + LINE_JOINER.join( + "class A {", + " f() {", + " let g = () => {", + " const $jscomp$async$this = this;", + " const $jscomp$async$arguments = arguments;", + " function *$jscomp$async$generator() {", + " console.log($jscomp$async$this, $jscomp$async$arguments);", + " }", + " return $jscomp.executeAsyncGenerator($jscomp$async$generator());", + " };", + " g();", + " }", + "}")); + } + public void testArrowFunctionExpressionBody() { test( "let f = async () => 1;", diff --git a/test/com/google/javascript/jscomp/runtime_tests/async_function_test.js b/test/com/google/javascript/jscomp/runtime_tests/async_function_test.js index 8fb05a924bc..65c3104e689 100644 --- a/test/com/google/javascript/jscomp/runtime_tests/async_function_test.js +++ b/test/com/google/javascript/jscomp/runtime_tests/async_function_test.js @@ -162,6 +162,24 @@ testSuite({ }); }, + testNonAsyncMemberFunctionUsingThisInAsyncArrowFunction() { + class C { + constructor() { + this.value = 0; + } + + delayedIncrementAndReturnThis() { + const nestedArrow = async () => { this.value++; return this; }; + return nestedArrow(); + } + } + const c = new C(); + return c.delayedIncrementAndReturnThis().then(result => { + assertEquals(c, result); + assertEquals(1, c.value); + }); + }, + testArgumentsHandledCorrectly() { const expected1 = {}; const expected2 = 2;