Skip to content

Commit

Permalink
LibJS: Handle empty named export
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
davidot authored and linusg committed Sep 2, 2022
1 parent f75c51b commit 3b1c3e5
Show file tree
Hide file tree
Showing 10 changed files with 60 additions and 2 deletions.
1 change: 1 addition & 0 deletions .prettierignore
Original file line number Diff line number Diff line change
@@ -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
3 changes: 3 additions & 0 deletions Userland/Libraries/LibJS/AST.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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;
});
}
Expand Down
10 changes: 10 additions & 0 deletions Userland/Libraries/LibJS/AST.h
Original file line number Diff line number Diff line change
Expand Up @@ -360,6 +360,11 @@ class ExportStatement final : public Statement {
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]]
Expand Down Expand Up @@ -409,6 +414,11 @@ class ExportStatement final : public Statement {
{
return ExportEntry { Kind::ModuleRequestAll, move(export_name), {} };
}

static ExportEntry empty_named_export()
{
return ExportEntry { Kind::EmptyNamedExport, {}, {} };
}
};

ExportStatement(SourceRange source_range, RefPtr<ASTNode> statement, Vector<ExportEntry> entries, bool is_default_export, ModuleRequest module_request)
Expand Down
11 changes: 9 additions & 2 deletions Userland/Libraries/LibJS/Parser.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -4489,6 +4489,7 @@ NonnullRefPtr<ExportStatement> 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);
Expand All @@ -4508,6 +4509,12 @@ NonnullRefPtr<ExportStatement> 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 {
Expand Down Expand Up @@ -4535,7 +4542,7 @@ NonnullRefPtr<ExportStatement> 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);
}

Expand Down
7 changes: 7 additions & 0 deletions Userland/Libraries/LibJS/SourceTextModule.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -174,6 +174,13 @@ Result<NonnullRefPtr<SourceTextModule>, Vector<Parser::Error>> 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()) {

Expand Down
22 changes: 22 additions & 0 deletions Userland/Libraries/LibJS/Tests/modules/basic-modules.js
Original file line number Diff line number Diff line change
Expand Up @@ -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
);
});
});
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
export * as shouldFail from "./failing.mjs";
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
export {} from "./failing.mjs";
export {};
// This should not be a duplicate!
export {} from "./failing.mjs";
2 changes: 2 additions & 0 deletions Userland/Libraries/LibJS/Tests/modules/failing.mjs
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
// Should produce a SyntaxError!
0++;
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
import "./failing.mjs";

0 comments on commit 3b1c3e5

Please sign in to comment.