Implemented new syntax for ImportDeclarations

Also had to split ParseImportsList and ParseExportClause into separate
methods as they have different rules about reserved words and arguments/eval.

Added lots more test cases, including some export cases that were missed before
due to incorrect checking of reserved words.

BUG=v8:1569
LOG=n

Review URL: https://codereview.chromium.org/887843002

Cr-Commit-Position: refs/heads/master@{#26338}

src/parser.cc

@@ -1311,26 +1311,31 @@ Module* Parser::ParseModuleSpecifier(bool* ok) {
 }
 
 
-void* Parser::ParseModuleDeclarationClause(ZoneList<const AstRawString*>* names,
-                                           bool* ok) {
-  // Handles both imports and exports:
-  //
-  // ImportOrExportClause :
+void* Parser::ParseExportClause(ZoneList<const AstRawString*>* names,
+                                Scanner::Location* reserved_loc, bool* ok) {
+  // ExportClause :
   //   '{' '}'
-  //   '{' ImportOrExportsList '}'
-  //   '{' ImportOrExportsList ',' '}'
+  //   '{' ExportsList '}'
+  //   '{' ExportsList ',' '}'
   //
-  // ImportOrExportsList :
-  //   ImportOrExportSpecifier
-  //   ImportOrExportsList ',' ImportOrExportSpecifier
+  // ExportsList :
+  //   ExportSpecifier
+  //   ExportsList ',' ExportSpecifier
   //
-  // ImportOrExportSpecifier :
+  // ExportSpecifier :
   //   IdentifierName
   //   IdentifierName 'as' IdentifierName
 
   Expect(Token::LBRACE, CHECK_OK);
 
-  while (peek() != Token::RBRACE) {
+  Token::Value name_tok;
+  while ((name_tok = peek()) != Token::RBRACE) {
+    // Keep track of the first reserved word encountered in case our
+    // caller needs to report an error.
+    if (!reserved_loc->IsValid() &&
+        !Token::IsIdentifier(name_tok, STRICT, false)) {
+      *reserved_loc = scanner()->location();
+    }
     const AstRawString* name = ParseIdentifierName(CHECK_OK);
     names->Add(name, zone());
     const AstRawString* export_name = NULL;
@@ -1349,30 +1354,124 @@ void* Parser::ParseModuleDeclarationClause(ZoneList<const AstRawString*>* names,
 }
 
 
+void* Parser::ParseNamedImports(ZoneList<const AstRawString*>* names,
+                                bool* ok) {
+  // NamedImports :
+  //   '{' '}'
+  //   '{' ImportsList '}'
+  //   '{' ImportsList ',' '}'
+  //
+  // ImportsList :
+  //   ImportSpecifier
+  //   ImportsList ',' ImportSpecifier
+  //
+  // ImportSpecifier :
+  //   BindingIdentifier
+  //   IdentifierName 'as' BindingIdentifier
+
+  Expect(Token::LBRACE, CHECK_OK);
+
+  Token::Value name_tok;
+  while ((name_tok = peek()) != Token::RBRACE) {
+    const AstRawString* name = ParseIdentifierName(CHECK_OK);
+    const AstRawString* import_name = NULL;
+    // In the presence of 'as', the left-side of the 'as' can
+    // be any IdentifierName. But without 'as', it must be a valid
+    // BindingIdentiifer.
+    if (CheckContextualKeyword(CStrVector("as"))) {
+      import_name = ParseIdentifier(kDontAllowEvalOrArguments, CHECK_OK);
+    } else if (!Token::IsIdentifier(name_tok, STRICT, false)) {
+      *ok = false;
+      ReportMessageAt(scanner()->location(), "unexpected_reserved");
+      return NULL;
+    } else if (IsEvalOrArguments(name)) {
+      *ok = false;
+      ReportMessageAt(scanner()->location(), "strict_eval_arguments");
+      return NULL;
+    }
+    // TODO(ES6): Return the import_name as well as the name.
+    names->Add(name, zone());
+    USE(import_name);
+    if (peek() == Token::RBRACE) break;
+    Expect(Token::COMMA, CHECK_OK);
+  }
+
+  Expect(Token::RBRACE, CHECK_OK);
+
+  return NULL;
+}
+
+
 Statement* Parser::ParseImportDeclaration(bool* ok) {
-  // ImportDeclaration:
-  //    'import' IdentifierName (',' IdentifierName)* 'from' ModuleUrl ';'
+  // ImportDeclaration :
+  //   'import' ImportClause 'from' ModuleSpecifier ';'
+  //   'import' ModuleSpecifier ';'
+  //
+  // ImportClause :
+  //   NameSpaceImport
+  //   NamedImports
+  //   ImportedDefaultBinding
+  //   ImportedDefaultBinding ',' NameSpaceImport
+  //   ImportedDefaultBinding ',' NamedImports
   //
-  // TODO(ES6): implement current syntax
+  // NameSpaceImport :
+  //   '*' 'as' ImportedBinding
 
   int pos = peek_position();
   Expect(Token::IMPORT, CHECK_OK);
+
+  Token::Value tok = peek();
+
+  // 'import' ModuleSpecifier ';'
+  if (tok == Token::STRING) {
+    ParseModuleSpecifier(CHECK_OK);
+    ExpectSemicolon(CHECK_OK);
+    return factory()->NewEmptyStatement(pos);
+  }
+
+  // Parse ImportedDefaultBinding if present.
+  const AstRawString* imported_default_binding = NULL;
+  if (tok != Token::MUL && tok != Token::LBRACE) {
+    imported_default_binding =
+        ParseIdentifier(kDontAllowEvalOrArguments, CHECK_OK);
+  }
+
+  const AstRawString* module_instance_binding = NULL;
   ZoneList<const AstRawString*> names(1, zone());
+  if (imported_default_binding == NULL || Check(Token::COMMA)) {
+    switch (peek()) {
+      case Token::MUL: {
+        Consume(Token::MUL);
+        ExpectContextualKeyword(CStrVector("as"), CHECK_OK);
+        module_instance_binding =
+            ParseIdentifier(kDontAllowEvalOrArguments, CHECK_OK);
+        break;
+      }
 
-  const AstRawString* name = ParseIdentifierName(CHECK_OK);
-  names.Add(name, zone());
-  while (peek() == Token::COMMA) {
-    Consume(Token::COMMA);
-    name = ParseIdentifierName(CHECK_OK);
-    names.Add(name, zone());
+      case Token::LBRACE:
+        ParseNamedImports(&names, CHECK_OK);
+        break;
+
+      default:
+        *ok = false;
+        ReportUnexpectedToken(scanner()->current_token());
+        return NULL;
+    }
   }
 
   ExpectContextualKeyword(CStrVector("from"), CHECK_OK);
   Module* module = ParseModuleSpecifier(CHECK_OK);
+  USE(module);
+
   ExpectSemicolon(CHECK_OK);
 
-  // TODO(ES6): Do something with ParseModuleSpecifier's return value.
-  USE(module);
+  if (module_instance_binding != NULL) {
+    // TODO(ES6): Bind name to the Module Instance Object of module.
+  }
+
+  if (imported_default_binding != NULL) {
+    // TODO(ES6): Add an appropriate declaration.
+  }
 
   for (int i = 0; i < names.length(); ++i) {
     // TODO(ES6): Add an appropriate declaration for each name
@@ -1447,18 +1546,36 @@ Statement* Parser::ParseExportDeclaration(bool* ok) {
       break;
     }
 
-    case Token::LBRACE:
-      ParseModuleDeclarationClause(&names, CHECK_OK);
+    case Token::LBRACE: {
+      // There are two cases here:
+      //
+      // 'export' ExportClause ';'
+      // and
+      // 'export' ExportClause FromClause ';'
+      //
+      // In the first case, the exported identifiers in ExportClause must
+      // not be reserved words, while in the latter they may be. We
+      // pass in a location that gets filled with the first reserved word
+      // encountered, and then throw a SyntaxError if we are in the
+      // non-FromClause case.
+      Scanner::Location reserved_loc = Scanner::Location::invalid();
+      ParseExportClause(&names, &reserved_loc, CHECK_OK);
       if (CheckContextualKeyword(CStrVector("from"))) {
         Module* module = ParseModuleSpecifier(CHECK_OK);
         // TODO(ES6): Do something with the return value
         // of ParseModuleSpecifier.
         USE(module);
         is_export_from = true;
+      } else if (reserved_loc.IsValid()) {
+        // No FromClause, so reserved words are invalid in ExportClause.
+        *ok = false;
+        ReportMessageAt(reserved_loc, "unexpected_reserved");
+        return NULL;
       }
       ExpectSemicolon(CHECK_OK);
       result = factory()->NewEmptyStatement(pos);
       break;
+    }
 
     case Token::FUNCTION:
       result = ParseFunctionDeclaration(&names, CHECK_OK);

src/parser.h

@@ -755,8 +755,9 @@ class Parser : public ParserBase<ParserTraits> {
   Statement* ParseImportDeclaration(bool* ok);
   Statement* ParseExportDeclaration(bool* ok);
   Statement* ParseExportDefault(bool* ok);
-  void* ParseModuleDeclarationClause(ZoneList<const AstRawString*>* names,
-                                     bool* ok);
+  void* ParseExportClause(ZoneList<const AstRawString*>* names,
+                          Scanner::Location* reserved_loc, bool* ok);
+  void* ParseNamedImports(ZoneList<const AstRawString*>* names, bool* ok);
   Statement* ParseStatement(ZoneList<const AstRawString*>* labels, bool* ok);
   Statement* ParseFunctionDeclaration(ZoneList<const AstRawString*>* names,
                                       bool* ok);

test/cctest/test-parsing.cc

@@ -4672,13 +4672,32 @@ TEST(BasicImportExportParsing) {
       "function f() {}; f(); export { f };",
       "var a, b, c; export { a, b as baz, c };",
       "var d, e; export { d as dreary, e, };",
-      "import y from 'http://module.com/foo.js';",
       "export default function f() {}",
       "export default class C {}",
       "export default 42",
       "var x; export default x = 7",
       "export { Q } from 'somemodule.js';",
-      "export * from 'somemodule.js';"
+      "export * from 'somemodule.js';",
+      "var foo; export { foo as for };",
+      "export { arguments } from 'm.js';",
+      "export { for } from 'm.js';",
+      "export { yield } from 'm.js'",
+      "export { static } from 'm.js'",
+      "export { let } from 'm.js'",
+
+      "import 'somemodule.js';",
+      "import { } from 'm.js';",
+      "import { a } from 'm.js';",
+      "import { a, b as d, c, } from 'm.js';",
+      "import * as thing from 'm.js';",
+      "import thing from 'm.js';",
+      "import thing, * as rest from 'm.js';",
+      "import thing, { a, b, c } from 'm.js';",
+      "import { arguments as a } from 'm.js';",
+      "import { for as f } from 'm.js';",
+      "import { yield as y } from 'm.js';",
+      "import { static as s } from 'm.js';",
+      "import { let as l } from 'm.js';",
   };
 
   i::Isolate* isolate = CcTest::i_isolate();
@@ -4711,7 +4730,22 @@ TEST(BasicImportExportParsing) {
       parser.set_allow_harmony_modules(true);
       parser.set_allow_harmony_scoping(true);
       info.MarkAsModule();
-      CHECK(parser.Parse());
+      if (!parser.Parse()) {
+        i::Handle<i::JSObject> exception_handle(
+            i::JSObject::cast(isolate->pending_exception()));
+        i::Handle<i::String> message_string =
+            i::Handle<i::String>::cast(i::Object::GetProperty(
+                  isolate, exception_handle, "message").ToHandleChecked());
+
+        v8::base::OS::Print(
+            "Parser failed on:\n"
+            "\t%s\n"
+            "with error:\n"
+            "\t%s\n"
+            "However, we expected no error.",
+            source->ToCString().get(), message_string->ToCString().get());
+        CHECK(false);
+      }
     }
 
     // And that parsing a script does not.
@@ -4752,6 +4786,38 @@ TEST(ImportExportParsingErrors) {
       "export * from;",
       "export { Q } from;",
       "export default from 'module.js';",
+      "export { for }",
+      "export { for as foo }",
+      "export { arguments }",
+      "export { arguments as foo }",
+
+      "import from;",
+      "import from 'm.js';",
+      "import { };",
+      "import {;",
+      "import };",
+      "import { , };",
+      "import { , } from 'm.js';",
+      "import { a } from;",
+      "import { a } 'm.js';",
+      "import , from 'm.js';",
+      "import a , from 'm.js';",
+      "import a { b, c } from 'm.js';",
+      "import arguments from 'm.js';",
+      "import eval from 'm.js';",
+      "import { arguments } from 'm.js';",
+      "import { eval } from 'm.js';",
+      "import { a as arguments } from 'm.js';",
+      "import { for } from 'm.js';",
+      "import { y as yield } from 'm.js'",
+      "import { s as static } from 'm.js'",
+      "import { l as let } from 'm.js'",
+      "import { x }, def from 'm.js';",
+      "import def, def2 from 'm.js';",
+      "import * as x, def from 'm.js';",
+      "import * as x, * as y from 'm.js';",
+      "import {x}, {y} from 'm.js';",
+      "import * as x, {y} from 'm.js';",
 
       // TODO(ES6): These two forms should be supported
       "export default function() {};",