diff --git a/Userland/Libraries/LibJS/AST.cpp b/Userland/Libraries/LibJS/AST.cpp index 41ab8ca398..ae68e31e90 100644 --- a/Userland/Libraries/LibJS/AST.cpp +++ b/Userland/Libraries/LibJS/AST.cpp @@ -4231,12 +4231,17 @@ void ExportStatement::dump(int indent) const for (auto& entry : m_entries) { print_indent(indent + 2); - out("ModuleRequest: {}", entry.module_request.module_specifier); - dump_assert_clauses(entry.module_request); - outln(", ImportName: {}, LocalName: {}, ExportName: {}", - entry.kind == ExportEntry::Kind::ModuleRequest ? string_or_null(entry.local_or_import_name) : "null", - entry.kind != ExportEntry::Kind::ModuleRequest ? string_or_null(entry.local_or_import_name) : "null", - string_or_null(entry.export_name)); + out("ExportName: {}, ImportName: {}, LocalName: {}, ModuleRequest: ", + string_or_null(entry.export_name), + entry.is_module_request() ? string_or_null(entry.local_or_import_name) : "null", + entry.is_module_request() ? "null" : string_or_null(entry.local_or_import_name)); + if (entry.is_module_request()) { + out("{}", entry.m_module_request->module_specifier); + dump_assert_clauses(*entry.m_module_request); + outln(); + } else { + outln("null"); + } } if (m_statement) { diff --git a/Userland/Libraries/LibJS/AST.h b/Userland/Libraries/LibJS/AST.h index 52382b14f0..1403878a9d 100644 --- a/Userland/Libraries/LibJS/AST.h +++ b/Userland/Libraries/LibJS/AST.h @@ -268,19 +268,18 @@ struct ModuleRequest { class ImportStatement final : public Statement { public: + // ImportEntry Record, https://tc39.es/ecma262/#table-importentry-record-fields struct ImportEntry { - FlyString import_name; - FlyString local_name; + FlyString import_name; // [[ImportName]] if a String + FlyString local_name; // [[LocalName]] + bool is_namespace { false }; // [[ImportName]] if `namespace-object` - ImportEntry(FlyString import_name_, FlyString local_name_) + ImportEntry(FlyString import_name_, FlyString local_name_, bool is_namespace_ = false) : import_name(move(import_name_)) , local_name(move(local_name_)) + , is_namespace(is_namespace_) { - } - - bool is_namespace() const - { - return import_name == "*"sv; + VERIFY(!is_namespace || import_name.is_null()); } ModuleRequest const& module_request() const @@ -291,7 +290,7 @@ public: private: friend ImportStatement; - ModuleRequest* m_module_request; + ModuleRequest* m_module_request; // [[ModuleRequest]] }; explicit ImportStatement(SourceRange source_range, ModuleRequest from_module, Vector entries = {}) @@ -320,52 +319,74 @@ class ExportStatement final : public Statement { public: static FlyString local_name_for_default; + // ExportEntry Record, https://tc39.es/ecma262/#table-exportentry-records struct ExportEntry { enum class Kind { - ModuleRequest, - LocalExport + NamedExport, + ModuleRequestAll, + ModuleRequestAllButDefault, } kind; - // Can always have - FlyString export_name; - // Only if module request - ModuleRequest module_request; + FlyString export_name; // [[ExportName]] + FlyString local_or_import_name; // Either [[ImportName]] or [[LocalName]] - // Has just one of ones below - FlyString local_or_import_name; - - ExportEntry(FlyString export_name, FlyString local_name) - : kind(Kind::LocalExport) - , export_name(move(export_name)) - , local_or_import_name(move(local_name)) - { - } - - ExportEntry(ModuleRequest module_request_, FlyString import_name, FlyString export_name_) - : kind(Kind::ModuleRequest) + ExportEntry(Kind export_kind, FlyString export_name_, FlyString local_or_import_name_) + : kind(export_kind) , export_name(move(export_name_)) - , module_request(move(module_request_)) - , local_or_import_name(move(import_name)) + , local_or_import_name(move(local_or_import_name_)) { } - bool is_all_but_default() const + bool is_module_request() const { - return kind == Kind::ModuleRequest && local_or_import_name == "*"sv && export_name.is_null(); + return m_module_request != nullptr; } - bool is_all() const + static ExportEntry indirect_export_entry(ModuleRequest const& module_request, FlyString export_name, FlyString import_name) { - return kind == Kind::ModuleRequest && local_or_import_name == "*"sv && !export_name.is_empty(); + ExportEntry entry { Kind::NamedExport, move(export_name), move(import_name) }; + entry.m_module_request = &module_request; + return entry; + } + + ModuleRequest const& module_request() const + { + VERIFY(m_module_request); + return *m_module_request; + } + + private: + ModuleRequest const* m_module_request { nullptr }; // [[ModuleRequest]] + friend ExportStatement; + + public: + static ExportEntry named_export(FlyString export_name, FlyString local_name) + { + return ExportEntry { Kind::NamedExport, move(export_name), move(local_name) }; + } + + static ExportEntry all_but_default_entry() + { + return ExportEntry { Kind::ModuleRequestAllButDefault, {}, {} }; + } + + static ExportEntry all_module_request(FlyString export_name) + { + return ExportEntry { Kind::ModuleRequestAll, move(export_name), {} }; } }; - explicit ExportStatement(SourceRange source_range, RefPtr statement, Vector entries, bool is_default_export) + ExportStatement(SourceRange source_range, RefPtr statement, Vector entries, bool is_default_export, ModuleRequest module_request) : Statement(source_range) , m_statement(move(statement)) , m_entries(move(entries)) , m_is_default_export(is_default_export) + , m_module_request(move(module_request)) { + if (!m_module_request.module_specifier.is_null()) { + for (auto& entry : m_entries) + entry.m_module_request = &m_module_request; + } } virtual Completion execute(Interpreter&, GlobalObject&) const override; @@ -389,6 +410,7 @@ private: RefPtr m_statement; Vector m_entries; bool m_is_default_export { false }; + ModuleRequest m_module_request; }; class Program final : public ScopeNode { diff --git a/Userland/Libraries/LibJS/Parser.cpp b/Userland/Libraries/LibJS/Parser.cpp index 999dddbbb9..702623132f 100644 --- a/Userland/Libraries/LibJS/Parser.cpp +++ b/Userland/Libraries/LibJS/Parser.cpp @@ -544,7 +544,7 @@ void Parser::parse_module(Program& program) if (export_statement.has_statement()) continue; for (auto& entry : export_statement.entries()) { - if (entry.kind == ExportStatement::ExportEntry::Kind::ModuleRequest) + if (entry.is_module_request()) return; auto const& exported_name = entry.local_or_import_name; @@ -4058,7 +4058,6 @@ ModuleRequest Parser::parse_module_request() return request; } -static FlyString namespace_string_value = "*"; static FlyString default_string_value = "default"; NonnullRefPtr Parser::parse_import_statement(Program& program) @@ -4133,7 +4132,7 @@ NonnullRefPtr Parser::parse_import_statement(Program& program) if (match_imported_binding()) { auto namespace_position = position(); auto namespace_name = consume().value(); - entries_with_location.append({ { namespace_string_value, namespace_name }, namespace_position }); + entries_with_location.append({ ImportStatement::ImportEntry({}, namespace_name, true), namespace_position }); } else { syntax_error(String::formatted("Unexpected token: {}", m_state.current_token.name())); } @@ -4223,6 +4222,8 @@ NonnullRefPtr Parser::parse_import_statement(Program& program) NonnullRefPtr Parser::parse_export_statement(Program& program) { + using ExportEntry = ExportStatement::ExportEntry; + // We use the extended syntax which adds: // ExportDeclaration: // export ExportFromClause FromClause [no LineTerminator here] AssertClause ; @@ -4247,21 +4248,15 @@ NonnullRefPtr Parser::parse_export_statement(Program& program) consume(TokenType::Export); struct EntryAndLocation { - ExportStatement::ExportEntry entry; + ExportEntry entry; Position position; - - void to_module_request(ModuleRequest from_module) - { - entry.kind = ExportStatement::ExportEntry::Kind::ModuleRequest; - entry.module_request = move(from_module); - } }; Vector entries_with_location; RefPtr expression = {}; - bool is_default = false; + ModuleRequest from_specifier; if (match_default()) { is_default = true; @@ -4353,7 +4348,7 @@ NonnullRefPtr Parser::parse_export_statement(Program& program) local_name = ExportStatement::local_name_for_default; } - entries_with_location.append({ { default_string_value, move(local_name) }, default_position }); + entries_with_location.append({ ExportEntry::named_export(default_string_value, move(local_name)), default_position }); } else { enum FromSpecifier { NotAllowed, @@ -4370,12 +4365,12 @@ NonnullRefPtr Parser::parse_export_statement(Program& program) if (match_identifier_name()) { auto namespace_position = position(); auto exported_name = consume().value(); - entries_with_location.append({ { exported_name, namespace_string_value }, namespace_position }); + entries_with_location.append({ ExportEntry::all_module_request(exported_name), namespace_position }); } else { expected("identifier"); } } else { - entries_with_location.append({ { {}, namespace_string_value }, asterisk_position }); + entries_with_location.append({ ExportEntry::all_but_default_entry(), asterisk_position }); } check_for_from = Required; } else if (match_declaration()) { @@ -4384,10 +4379,10 @@ NonnullRefPtr Parser::parse_export_statement(Program& program) m_state.current_scope_pusher->add_declaration(declaration); if (is(*declaration)) { auto& func = static_cast(*declaration); - entries_with_location.append({ { func.name(), func.name() }, func.source_range().start }); + entries_with_location.append({ ExportEntry::named_export(func.name(), func.name()), func.source_range().start }); } else if (is(*declaration)) { auto& class_declaration = static_cast(*declaration); - entries_with_location.append({ { class_declaration.name(), class_declaration.name() }, class_declaration.source_range().start }); + entries_with_location.append({ ExportEntry::named_export(class_declaration.name(), class_declaration.name()), class_declaration.source_range().start }); } else { VERIFY(is(*declaration)); auto& variables = static_cast(*declaration); @@ -4395,11 +4390,11 @@ NonnullRefPtr Parser::parse_export_statement(Program& program) for (auto& decl : variables.declarations()) { decl.target().visit( [&](NonnullRefPtr const& identifier) { - entries_with_location.append({ { identifier->string(), identifier->string() }, identifier->source_range().start }); + entries_with_location.append({ ExportEntry::named_export(identifier->string(), identifier->string()), identifier->source_range().start }); }, [&](NonnullRefPtr const& binding) { binding->for_each_bound_name([&](auto& name) { - entries_with_location.append({ { name, name }, decl_position }); + entries_with_location.append({ ExportEntry::named_export(name, name), decl_position }); }); }); } @@ -4412,11 +4407,11 @@ NonnullRefPtr Parser::parse_export_statement(Program& program) for (auto& decl : variable_declaration->declarations()) { decl.target().visit( [&](NonnullRefPtr const& identifier) { - entries_with_location.append({ { identifier->string(), identifier->string() }, identifier->source_range().start }); + entries_with_location.append({ ExportEntry::named_export(identifier->string(), identifier->string()), identifier->source_range().start }); }, [&](NonnullRefPtr const& binding) { binding->for_each_bound_name([&](auto& name) { - entries_with_location.append({ { name, name }, variable_position }); + entries_with_location.append({ ExportEntry::named_export(name, name), variable_position }); }); }); } @@ -4444,18 +4439,13 @@ NonnullRefPtr Parser::parse_export_statement(Program& program) auto identifier_position = position(); auto identifier = parse_export_specifier(true); - if (identifier.is_empty()) - break; - if (match_as()) { consume(TokenType::Identifier); auto export_name = parse_export_specifier(false); - if (export_name.is_empty()) - break; - entries_with_location.append({ { move(export_name), move(identifier) }, identifier_position }); + entries_with_location.append({ ExportEntry::named_export(move(export_name), move(identifier)), identifier_position }); } else { - entries_with_location.append({ { identifier, identifier }, identifier_position }); + entries_with_location.append({ ExportEntry::named_export(identifier, identifier), identifier_position }); } if (!match(TokenType::Comma)) @@ -4472,12 +4462,7 @@ NonnullRefPtr Parser::parse_export_statement(Program& program) if (check_for_from != NotAllowed && match_from()) { consume(TokenType::Identifier); - auto from_specifier = parse_module_request(); - - // FIXME: We can probably store only one module request - // per ExportStatement like we do with ImportStatement. - for (auto& entry : entries_with_location) - entry.to_module_request(from_specifier); + from_specifier = parse_module_request(); } else if (check_for_from == Required) { expected("from"); } @@ -4503,6 +4488,6 @@ NonnullRefPtr Parser::parse_export_statement(Program& program) entries.append(move(entry.entry)); } - return create_ast_node({ m_state.current_token.filename(), rule_start.position(), position() }, move(expression), move(entries), is_default); + return create_ast_node({ m_state.current_token.filename(), rule_start.position(), position() }, move(expression), move(entries), is_default, move(from_specifier)); } } diff --git a/Userland/Libraries/LibJS/SourceTextModule.cpp b/Userland/Libraries/LibJS/SourceTextModule.cpp index 62324b1479..e198690cab 100644 --- a/Userland/Libraries/LibJS/SourceTextModule.cpp +++ b/Userland/Libraries/LibJS/SourceTextModule.cpp @@ -37,9 +37,9 @@ static Vector module_requests(Program const& program) for (auto const& export_statement : program.exports()) { for (auto const& export_entry : export_statement.entries()) { - if (export_entry.kind != ExportStatement::ExportEntry::Kind::ModuleRequest) + if (!export_entry.is_module_request()) continue; - requested_modules_with_indices.append({ export_entry.module_request.module_specifier.view(), + requested_modules_with_indices.append({ export_entry.module_request().module_specifier.view(), export_statement.source_range().start.offset }); } } @@ -115,7 +115,8 @@ Result, Vector> SourceTextModule: VERIFY(export_statement.has_statement()); auto const& entry = export_statement.entries()[0]; - VERIFY(entry.kind == ExportStatement::ExportEntry::Kind::LocalExport); + VERIFY(entry.kind == ExportStatement::ExportEntry::Kind::NamedExport); + VERIFY(!entry.is_module_request()); VERIFY(import_entries.find_if( [&](ImportEntry const& import_entry) { return import_entry.local_name == entry.local_or_import_name; @@ -127,7 +128,7 @@ Result, Vector> SourceTextModule: for (auto const& export_entry : export_statement.entries()) { // a. If ee.[[ModuleRequest]] is null, then - if (export_entry.kind == ExportStatement::ExportEntry::Kind::LocalExport) { + if (!export_entry.is_module_request()) { auto in_imported_bound_names = import_entries.find_if( [&](ImportEntry const& import_entry) { @@ -145,8 +146,10 @@ Result, Vector> SourceTextModule: auto& import_entry = *in_imported_bound_names; // 2. If ie.[[ImportName]] is namespace-object, then - if (import_entry.is_namespace()) { + if (import_entry.is_namespace) { // a. NOTE: This is a re-export of an imported module namespace object. + VERIFY(export_entry.is_module_request() && export_entry.kind != ExportStatement::ExportEntry::Kind::NamedExport); + // b. Append ee to localExportEntries. local_export_entries.empend(export_entry); } @@ -154,12 +157,12 @@ Result, Vector> SourceTextModule: else { // a. NOTE: This is a re-export of a single name. // b. Append the ExportEntry Record { [[ModuleRequest]]: ie.[[ModuleRequest]], [[ImportName]]: ie.[[ImportName]], [[LocalName]]: null, [[ExportName]]: ee.[[ExportName]] } to indirectExportEntries. - indirect_export_entries.empend(import_entry.module_request(), import_entry.import_name, export_entry.export_name); + indirect_export_entries.empend(ExportEntry::indirect_export_entry(import_entry.module_request(), import_entry.import_name, export_entry.export_name)); } } } // b. Else if ee.[[ImportName]] is all-but-default, then - else if (export_entry.is_all_but_default()) { + else if (export_entry.kind == ExportStatement::ExportEntry::Kind::ModuleRequestAllButDefault) { // i. Assert: ee.[[ExportName]] is null. VERIFY(export_entry.export_name.is_null()); // ii. Append ee to starExportEntries. @@ -230,7 +233,7 @@ ThrowCompletionOr> SourceTextModule::get_exported_names(VM& vm // 7. For each ExportEntry Record e of module.[[StarExportEntries]], do for (auto& entry : m_star_export_entries) { // a. Let requestedModule be ? HostResolveImportedModule(module, e.[[ModuleRequest]]). - auto requested_module = TRY(vm.host_resolve_imported_module(this, entry.module_request)); + auto requested_module = TRY(vm.host_resolve_imported_module(this, entry.module_request())); // b. Let starNames be ? requestedModule.GetExportedNames(exportStarSet). auto star_names = TRY(requested_module->get_exported_names(vm, export_star_set)); @@ -293,7 +296,7 @@ Completion SourceTextModule::initialize_environment(VM& vm) // b. NOTE: The above call cannot fail because imported module requests are a subset of module.[[RequestedModules]], and these have been resolved earlier in this algorithm. // c. If in.[[ImportName]] is namespace-object, then - if (import_entry.is_namespace()) { + if (import_entry.is_namespace) { // i. Let namespace be ? GetModuleNamespace(importedModule). auto* namespace_ = TRY(imported_module->get_module_namespace(vm)); @@ -489,10 +492,10 @@ ThrowCompletionOr SourceTextModule::resolve_export(VM& vm, FlyS continue; // i. Let importedModule be ? HostResolveImportedModule(module, e.[[ModuleRequest]]). - auto imported_module = TRY(vm.host_resolve_imported_module(this, entry.module_request)); + auto imported_module = TRY(vm.host_resolve_imported_module(this, entry.module_request())); // ii. If e.[[ImportName]] is all, then - if (entry.is_all()) { + if (entry.kind == ExportStatement::ExportEntry::Kind::ModuleRequestAll) { // 1. Assert: module does not provide the direct binding for this export. // FIXME: What does this mean? / How do we check this @@ -529,7 +532,7 @@ ThrowCompletionOr SourceTextModule::resolve_export(VM& vm, FlyS // 8. For each ExportEntry Record e of module.[[StarExportEntries]], do for (auto& entry : m_star_export_entries) { // a. Let importedModule be ? HostResolveImportedModule(module, e.[[ModuleRequest]]). - auto imported_module = TRY(vm.host_resolve_imported_module(this, entry.module_request)); + auto imported_module = TRY(vm.host_resolve_imported_module(this, entry.module_request())); // b. Let resolution be ? importedModule.ResolveExport(exportName, resolveSet). auto resolution = TRY(imported_module->resolve_export(vm, export_name, resolve_set)); diff --git a/Userland/Libraries/LibJS/Tests/modules/basic-modules.js b/Userland/Libraries/LibJS/Tests/modules/basic-modules.js index 7693b1df14..eb927ab63c 100644 --- a/Userland/Libraries/LibJS/Tests/modules/basic-modules.js +++ b/Userland/Libraries/LibJS/Tests/modules/basic-modules.js @@ -1,25 +1,22 @@ // Because you can't easily load modules directly we load them via here and check // if they passed by checking the result -function expectModulePassed(filename) { +function validTestModule(filename) { if (!filename.endsWith(".mjs") || !filename.startsWith("./")) { throw new ExpectationError( - "Expected module name to start with './' " + - "and end with '.mjs' but got '" + - filename + - "'" + `Expected module name to start with './' and end with '.mjs' but got '${filename}'` ); } +} - async function getModule() { - return import(filename); - } +function expectModulePassed(filename) { + validTestModule(filename); let moduleLoaded = false; let moduleResult = null; let thrownError = null; - getModule() + import(filename) .then(result => { moduleLoaded = true; moduleResult = result; @@ -36,10 +33,36 @@ function expectModulePassed(filename) { } expect(moduleLoaded).toBeTrue(); - return moduleResult; } +function expectedModuleToThrowSyntaxError(filename, message) { + validTestModule(filename); + + let moduleLoaded = false; + let thrownError = null; + + import(filename) + .then(() => { + moduleLoaded = true; + }) + .catch(error => { + thrownError = error; + }); + + runQueuedPromiseJobs(); + + if (thrownError) { + expect(() => { + throw thrownError; + }).toThrowWithMessage(SyntaxError, message); + } else { + throw new ExpectationError( + `Expected module: '${filename}' to fail to load with a syntax error but did not throw.` + ); + } +} + describe("testing behavior", () => { // To ensure the other tests are interpreter correctly we first test the underlying // mechanisms so these tests don't use expectModulePassed. @@ -131,6 +154,25 @@ describe("in- and exports", () => { test("declaration exports which can be used in the module it self", () => { expectModulePassed("./declarations-tests.mjs"); }); + + test("string '*' is not a full namespace import", () => { + expectModulePassed("./string-import-names.mjs"); + }); + + test("can combine string and default exports", () => { + expectModulePassed("./string-import-namespace.mjs"); + }); + + test("can re export string names", () => { + expectModulePassed("./string-import-namespace-indirect.mjs"); + }); + + test("re exporting all-but-default does not export a default value", () => { + expectedModuleToThrowSyntaxError( + "./indirect-export-without-default.mjs", + "Invalid or ambiguous export entry 'default'" + ); + }); }); describe("loops", () => { diff --git a/Userland/Libraries/LibJS/Tests/modules/default-and-star-export-indirect.mjs b/Userland/Libraries/LibJS/Tests/modules/default-and-star-export-indirect.mjs new file mode 100644 index 0000000000..658425f847 --- /dev/null +++ b/Userland/Libraries/LibJS/Tests/modules/default-and-star-export-indirect.mjs @@ -0,0 +1,2 @@ +// This is an all-but-default export and should thus only provide '*'. +export * from "./default-and-star-export.mjs"; diff --git a/Userland/Libraries/LibJS/Tests/modules/default-and-star-export.mjs b/Userland/Libraries/LibJS/Tests/modules/default-and-star-export.mjs new file mode 100644 index 0000000000..ffe97ffedb --- /dev/null +++ b/Userland/Libraries/LibJS/Tests/modules/default-and-star-export.mjs @@ -0,0 +1,6 @@ +export default "defaultValue"; + +const star = "starExportValue"; +const empty = "empty"; + +export { star as "*", empty as "" }; diff --git a/Userland/Libraries/LibJS/Tests/modules/indirect-export-without-default.mjs b/Userland/Libraries/LibJS/Tests/modules/indirect-export-without-default.mjs new file mode 100644 index 0000000000..e6aa386731 --- /dev/null +++ b/Userland/Libraries/LibJS/Tests/modules/indirect-export-without-default.mjs @@ -0,0 +1,3 @@ +// Since 'default-and-star-export-indirect.mjs' only contains an all-but-default re export of +// 'default-and-star-export.mjs' it should not have a default value. +import defaultExportIndirect from "./default-and-star-export-indirect.mjs"; diff --git a/Userland/Libraries/LibJS/Tests/modules/string-import-names.mjs b/Userland/Libraries/LibJS/Tests/modules/string-import-names.mjs new file mode 100644 index 0000000000..d61158f85a --- /dev/null +++ b/Userland/Libraries/LibJS/Tests/modules/string-import-names.mjs @@ -0,0 +1,12 @@ +import { "*" as starImport, "" as emptyImport } from "./default-and-star-export.mjs"; + +import { + "*" as starImportIndirect, + "" as emptyImportIndirect, +} from "./default-and-star-export-indirect.mjs"; + +export const passed = + starImport === "starExportValue" && + starImportIndirect === "starExportValue" && + emptyImport === "empty" && + emptyImportIndirect === "empty"; diff --git a/Userland/Libraries/LibJS/Tests/modules/string-import-namespace-indirect.mjs b/Userland/Libraries/LibJS/Tests/modules/string-import-namespace-indirect.mjs new file mode 100644 index 0000000000..f203dd2d07 --- /dev/null +++ b/Userland/Libraries/LibJS/Tests/modules/string-import-namespace-indirect.mjs @@ -0,0 +1,6 @@ +import * as indirectNs from "./default-and-star-export-indirect.mjs"; + +export const passed = + indirectNs["*"] === "starExportValue" && + indirectNs[""] === "empty" && + indirectNs.default === undefined; diff --git a/Userland/Libraries/LibJS/Tests/modules/string-import-namespace.mjs b/Userland/Libraries/LibJS/Tests/modules/string-import-namespace.mjs new file mode 100644 index 0000000000..c06777aae3 --- /dev/null +++ b/Userland/Libraries/LibJS/Tests/modules/string-import-namespace.mjs @@ -0,0 +1,8 @@ +import * as ns from "./default-and-star-export.mjs"; +import defaultExport from "./default-and-star-export.mjs"; + +export const passed = + ns.default === "defaultValue" && + ns["*"] === "starExportValue" && + ns[""] === "empty" && + defaultExport === "defaultValue";