From 3b1c3e574f51e1ca4efc725813a1b10bba19fc63 Mon Sep 17 00:00:00 2001 From: davidot Date: Mon, 29 Aug 2022 22:12:25 +0200 Subject: [PATCH] LibJS: Handle empty named export This is an export which looks like `export {} from "module"`, and although it doesn't have any real export entries it should still add "module" to the required modules to load. --- .prettierignore | 1 + Userland/Libraries/LibJS/AST.cpp | 3 +++ Userland/Libraries/LibJS/AST.h | 10 +++++++++ Userland/Libraries/LibJS/Parser.cpp | 11 ++++++++-- Userland/Libraries/LibJS/SourceTextModule.cpp | 7 ++++++ .../LibJS/Tests/modules/basic-modules.js | 22 +++++++++++++++++++ .../Tests/modules/exporting-from-failing.mjs | 1 + .../exporting-nothing-from-failing.mjs | 4 ++++ .../Libraries/LibJS/Tests/modules/failing.mjs | 2 ++ .../modules/importing-failing-module.mjs | 1 + 10 files changed, 60 insertions(+), 2 deletions(-) create mode 100644 Userland/Libraries/LibJS/Tests/modules/exporting-from-failing.mjs create mode 100644 Userland/Libraries/LibJS/Tests/modules/exporting-nothing-from-failing.mjs create mode 100644 Userland/Libraries/LibJS/Tests/modules/failing.mjs create mode 100644 Userland/Libraries/LibJS/Tests/modules/importing-failing-module.mjs diff --git a/.prettierignore b/.prettierignore index 95fa7d51cfd..4930eee75a8 100644 --- a/.prettierignore +++ b/.prettierignore @@ -1,3 +1,4 @@ Base/home/anon/Source/js Userland/Libraries/LibJS/Tests/invalid-lhs-in-assignment.js Userland/Libraries/LibJS/Tests/unicode-identifier-escape.js +Userland/Libraries/LibJS/Tests/modules/failing.mjs diff --git a/Userland/Libraries/LibJS/AST.cpp b/Userland/Libraries/LibJS/AST.cpp index 734eea302ab..461281f1b1c 100644 --- a/Userland/Libraries/LibJS/AST.cpp +++ b/Userland/Libraries/LibJS/AST.cpp @@ -4497,6 +4497,9 @@ void ImportStatement::dump(int indent) const bool ExportStatement::has_export(FlyString const& export_name) const { return any_of(m_entries.begin(), m_entries.end(), [&](auto& entry) { + // Make sure that empty exported names does not overlap with anything + if (entry.kind == ExportEntry::Kind::EmptyNamedExport) + return false; return entry.export_name == export_name; }); } diff --git a/Userland/Libraries/LibJS/AST.h b/Userland/Libraries/LibJS/AST.h index 13cf6f55d92..6debd070b8c 100644 --- a/Userland/Libraries/LibJS/AST.h +++ b/Userland/Libraries/LibJS/AST.h @@ -360,6 +360,11 @@ public: NamedExport, ModuleRequestAll, ModuleRequestAllButDefault, + // EmptyNamedExport is a special type for export {} from "module", + // which should import the module without getting any of the exports + // however we don't want give it a fake export name which may get + // duplicates + EmptyNamedExport, } kind; FlyString export_name; // [[ExportName]] @@ -409,6 +414,11 @@ public: { return ExportEntry { Kind::ModuleRequestAll, move(export_name), {} }; } + + static ExportEntry empty_named_export() + { + return ExportEntry { Kind::EmptyNamedExport, {}, {} }; + } }; ExportStatement(SourceRange source_range, RefPtr statement, Vector entries, bool is_default_export, ModuleRequest module_request) diff --git a/Userland/Libraries/LibJS/Parser.cpp b/Userland/Libraries/LibJS/Parser.cpp index f6964e0f3e8..e0d01ea0884 100644 --- a/Userland/Libraries/LibJS/Parser.cpp +++ b/Userland/Libraries/LibJS/Parser.cpp @@ -550,7 +550,7 @@ void Parser::parse_module(Program& program) if (export_statement.has_statement()) continue; for (auto& entry : export_statement.entries()) { - if (entry.is_module_request()) + if (entry.is_module_request() || entry.kind == ExportStatement::ExportEntry::Kind::EmptyNamedExport) return; auto const& exported_name = entry.local_or_import_name; @@ -4489,6 +4489,7 @@ NonnullRefPtr Parser::parse_export_statement(Program& program) consume(TokenType::CurlyOpen); check_for_from = FromSpecifier::Optional; + // FIXME: Even when empty should add module to requiredModules! while (!done() && !match(TokenType::CurlyClose)) { auto identifier_position = position(); auto identifier = parse_module_export_name(true); @@ -4508,6 +4509,12 @@ NonnullRefPtr Parser::parse_export_statement(Program& program) consume(TokenType::Comma); } + if (entries_with_location.is_empty()) { + // export {} from "module"; Since this will never be a + // duplicate we can give a slightly wrong location. + entries_with_location.append({ ExportEntry::empty_named_export(), position() }); + } + consume(TokenType::CurlyClose); } else { @@ -4535,7 +4542,7 @@ NonnullRefPtr Parser::parse_export_statement(Program& program) } for (auto& new_entry : entries) { - if (new_entry.export_name == entry.entry.export_name) + if (new_entry.kind != ExportStatement::ExportEntry::Kind::EmptyNamedExport && new_entry.export_name == entry.entry.export_name) syntax_error(String::formatted("Duplicate export with name: '{}'", entry.entry.export_name), entry.position); } diff --git a/Userland/Libraries/LibJS/SourceTextModule.cpp b/Userland/Libraries/LibJS/SourceTextModule.cpp index f2509a1f4fb..b86340312a1 100644 --- a/Userland/Libraries/LibJS/SourceTextModule.cpp +++ b/Userland/Libraries/LibJS/SourceTextModule.cpp @@ -174,6 +174,13 @@ Result, Vector> SourceTextModule: for (auto const& export_entry : export_statement.entries()) { + // Special case, export {} from "module" should add "module" to + // required_modules but not any import or export so skip here. + if (export_entry.kind == ExportStatement::ExportEntry::Kind::EmptyNamedExport) { + VERIFY(export_statement.entries().size() == 1); + break; + } + // a. If ee.[[ModuleRequest]] is null, then if (!export_entry.is_module_request()) { diff --git a/Userland/Libraries/LibJS/Tests/modules/basic-modules.js b/Userland/Libraries/LibJS/Tests/modules/basic-modules.js index 23efa91826f..d0806c5e823 100644 --- a/Userland/Libraries/LibJS/Tests/modules/basic-modules.js +++ b/Userland/Libraries/LibJS/Tests/modules/basic-modules.js @@ -193,3 +193,25 @@ describe("loops", () => { expectModulePassed("./loop-entry.mjs"); }); }); + +describe("failing modules cascade", () => { + let failingModuleError = "Left-hand side of postfix"; + test("importing a file with a SyntaxError results in a SyntaxError", () => { + expectedModuleToThrowSyntaxError("./failing.mjs", failingModuleError); + }); + + test("importing a file without a syntax error which imports a file with a syntax error fails", () => { + expectedModuleToThrowSyntaxError("./importing-failing-module.mjs", failingModuleError); + }); + + test("importing a file which re exports a file with a syntax error fails", () => { + expectedModuleToThrowSyntaxError("./exporting-from-failing.mjs", failingModuleError); + }); + + test("importing a file re exports nothing from a file with a syntax error fails", () => { + expectedModuleToThrowSyntaxError( + "./exporting-nothing-from-failing.mjs", + failingModuleError + ); + }); +}); diff --git a/Userland/Libraries/LibJS/Tests/modules/exporting-from-failing.mjs b/Userland/Libraries/LibJS/Tests/modules/exporting-from-failing.mjs new file mode 100644 index 00000000000..5387b9efc22 --- /dev/null +++ b/Userland/Libraries/LibJS/Tests/modules/exporting-from-failing.mjs @@ -0,0 +1 @@ +export * as shouldFail from "./failing.mjs"; diff --git a/Userland/Libraries/LibJS/Tests/modules/exporting-nothing-from-failing.mjs b/Userland/Libraries/LibJS/Tests/modules/exporting-nothing-from-failing.mjs new file mode 100644 index 00000000000..6a0c0d4fdf6 --- /dev/null +++ b/Userland/Libraries/LibJS/Tests/modules/exporting-nothing-from-failing.mjs @@ -0,0 +1,4 @@ +export {} from "./failing.mjs"; +export {}; +// This should not be a duplicate! +export {} from "./failing.mjs"; diff --git a/Userland/Libraries/LibJS/Tests/modules/failing.mjs b/Userland/Libraries/LibJS/Tests/modules/failing.mjs new file mode 100644 index 00000000000..6b078479237 --- /dev/null +++ b/Userland/Libraries/LibJS/Tests/modules/failing.mjs @@ -0,0 +1,2 @@ +// Should produce a SyntaxError! +0++; diff --git a/Userland/Libraries/LibJS/Tests/modules/importing-failing-module.mjs b/Userland/Libraries/LibJS/Tests/modules/importing-failing-module.mjs new file mode 100644 index 00000000000..c1bdcaa3e56 --- /dev/null +++ b/Userland/Libraries/LibJS/Tests/modules/importing-failing-module.mjs @@ -0,0 +1 @@ +import "./failing.mjs";