dynamic import is ambiguous with import declaration at module code
authorutatane.tea@gmail.com <utatane.tea@gmail.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sat, 21 Jan 2017 22:10:54 +0000 (22:10 +0000)
committerutatane.tea@gmail.com <utatane.tea@gmail.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sat, 21 Jan 2017 22:10:54 +0000 (22:10 +0000)
https://bugs.webkit.org/show_bug.cgi?id=167098

Reviewed by Darin Adler.

JSTests:

* modules/import-call.js: Added.
(from.string_appeared_here.import.string_appeared_here.then):
* modules/import-call/main.js: Added.
* stress/import-syntax.js:
(async):

Source/JavaScriptCore:

This patch fixes two syntax issues related to dynamic import.

1. Fix member expression parsing with dynamic import results

We should not return import expression immediately after parsing
it in parseMemberExpression. This prohibits us to parse the following
code,

    import("...").then(function () {
    });

2. dynamic import with import declaration under the module context

Before this patch, we always attempt to parse IMPORT as import declaration
under the module context. It means that import call in the top level
expression statement fails to be parsed since the parser attempts to parse
it as import declaration.

    import("...")  // module top level statement.

In this patch, we check the condition `[lookahead != (]` before starting
parsing import declaration. This allows us to put import call in the module
top level statement.

* parser/Parser.cpp:
(JSC::Parser<LexerType>::parseModuleSourceElements):
(JSC::Parser<LexerType>::parseMemberExpression):

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@211017 268f45cc-cd09-0410-ab3c-d52691b4dbfc

JSTests/ChangeLog
JSTests/modules/import-call.js [new file with mode: 0644]
JSTests/modules/import-call/main.js [new file with mode: 0644]
JSTests/stress/import-syntax.js
Source/JavaScriptCore/ChangeLog
Source/JavaScriptCore/parser/Parser.cpp

index 9867bf8..ec48860 100644 (file)
@@ -1,3 +1,16 @@
+2017-01-21  Yusuke Suzuki  <utatane.tea@gmail.com>
+
+        dynamic import is ambiguous with import declaration at module code
+        https://bugs.webkit.org/show_bug.cgi?id=167098
+
+        Reviewed by Darin Adler.
+
+        * modules/import-call.js: Added.
+        (from.string_appeared_here.import.string_appeared_here.then):
+        * modules/import-call/main.js: Added.
+        * stress/import-syntax.js:
+        (async):
+
 2017-01-19  Skachkov Oleksandr  <gskachkov@gmail.com>
 
         "this" missing after await in async arrow function
diff --git a/JSTests/modules/import-call.js b/JSTests/modules/import-call.js
new file mode 100644 (file)
index 0000000..eb523e7
--- /dev/null
@@ -0,0 +1,5 @@
+import { shouldBe } from "./resources/assert.js"
+
+import("./import-call/main.js").then((result) => {
+    shouldBe(result.Cocoa, 42);
+});
diff --git a/JSTests/modules/import-call/main.js b/JSTests/modules/import-call/main.js
new file mode 100644 (file)
index 0000000..6b67122
--- /dev/null
@@ -0,0 +1 @@
+export let Cocoa = 42;
index 8bf78da..934fcf2 100644 (file)
@@ -53,6 +53,11 @@ testSyntaxError(`const import = 42`, `SyntaxError: Cannot use the keyword 'impor
 (async function () {
     await testSyntax(`import("./import-tests/cocoa.js")`);
     await testSyntax(`import("./import-tests/../import-tests/cocoa.js")`);
+    await testSyntax(`import("./import-tests/../import-tests/cocoa.js").then(() => { })`);
+    await testSyntax(`(import("./import-tests/../import-tests/cocoa.js").then(() => { }))`);
+    await testSyntax(`(import("./import-tests/../import-tests/cocoa.js"))`);
+    await testSyntax(`import("./import-tests/../import-tests/cocoa.js").catch(() => { })`);
+    await testSyntax(`(import("./import-tests/../import-tests/cocoa.js").catch(() => { }))`);
 }()).catch((error) => {
     print(String(error));
     abort();
index 9b3f7e3..86a373f 100644 (file)
@@ -1,3 +1,38 @@
+2017-01-21  Yusuke Suzuki  <utatane.tea@gmail.com>
+
+        dynamic import is ambiguous with import declaration at module code
+        https://bugs.webkit.org/show_bug.cgi?id=167098
+
+        Reviewed by Darin Adler.
+
+        This patch fixes two syntax issues related to dynamic import.
+
+        1. Fix member expression parsing with dynamic import results
+
+        We should not return import expression immediately after parsing
+        it in parseMemberExpression. This prohibits us to parse the following
+        code,
+
+            import("...").then(function () {
+            });
+
+        2. dynamic import with import declaration under the module context
+
+        Before this patch, we always attempt to parse IMPORT as import declaration
+        under the module context. It means that import call in the top level
+        expression statement fails to be parsed since the parser attempts to parse
+        it as import declaration.
+
+            import("...")  // module top level statement.
+
+        In this patch, we check the condition `[lookahead != (]` before starting
+        parsing import declaration. This allows us to put import call in the module
+        top level statement.
+
+        * parser/Parser.cpp:
+        (JSC::Parser<LexerType>::parseModuleSourceElements):
+        (JSC::Parser<LexerType>::parseMemberExpression):
+
 2017-01-20  Joseph Pecoraro  <pecoraro@apple.com>
 
         Remove outdated ENABLE(CSP_NEXT) build flag
index c6b0b9c..f4e307b 100644 (file)
@@ -375,30 +375,48 @@ template <class TreeBuilder> TreeSourceElements Parser<LexerType>::parseModuleSo
 
     while (true) {
         TreeStatement statement = 0;
-        if (match(IMPORT)) {
-            statement = parseImportDeclaration(context);
-            if (statement)
-                recordPauseLocation(context.breakpointLocation(statement));
-        } else if (match(EXPORT)) {
+        switch (m_token.m_type) {
+        case EXPORT:
             statement = parseExportDeclaration(context);
             if (statement)
                 recordPauseLocation(context.breakpointLocation(statement));
-        } else {
+            break;
+
+        case IMPORT: {
+            SavePoint savePoint = createSavePoint();
+            next();
+            bool isImportDeclaration = !match(OPENPAREN);
+            restoreSavePoint(savePoint);
+            if (isImportDeclaration) {
+                statement = parseImportDeclaration(context);
+                if (statement)
+                    recordPauseLocation(context.breakpointLocation(statement));
+                break;
+            }
+
+            // This is `import("...")` call case.
+            FALLTHROUGH;
+        }
+
+        default: {
             const Identifier* directive = 0;
             unsigned directiveLiteralLength = 0;
             if (parseMode == SourceParseMode::ModuleAnalyzeMode) {
                 if (!parseStatementListItem(syntaxChecker, directive, &directiveLiteralLength))
-                    break;
+                    goto end;
                 continue;
             }
             statement = parseStatementListItem(context, directive, &directiveLiteralLength);
+            break;
+        }
         }
 
         if (!statement)
-            break;
+            goto end;
         context.appendStatement(sourceElements, statement);
     }
 
+end:
     propagateError();
 
     for (const auto& pair : m_moduleScopeData->exportedBindings()) {
@@ -4395,7 +4413,7 @@ template <class TreeBuilder> TreeExpression Parser<LexerType>::parseMemberExpres
         TreeExpression expr = parseAssignmentExpression(context);
         failIfFalse(expr, "Cannot parse expression");
         consumeOrFail(CLOSEPAREN, "import call expects exactly one argument");
-        return context.createImportExpr(location, expr, expressionStart, expressionEnd, lastTokenEndPosition());
+        base = context.createImportExpr(location, expr, expressionStart, expressionEnd, lastTokenEndPosition());
     } else if (!baseIsNewTarget) {
         const bool isAsync = match(ASYNC);