1
Fork 0
mirror of https://github.com/RGBCube/serenity synced 2025-05-31 08:58:11 +00:00

LibJS: Follow the spec with storing im- and export entries

Because we can have arbitrary in- and export names with strings we can
have '*' and '' which means using '*' as an indicating namespace imports
failed / behaved incorrectly for string imports '*'.
We now use more specific types to indicate these special states instead
of these 'magic' string values.

Do note that 'default' is not actually a magic string value but one
specified by the spec. And you can in fact export the default value by
doing: `export { 1 as default }`.
This commit is contained in:
davidot 2022-01-27 01:54:47 +01:00 committed by Linus Groh
parent 8473f6caee
commit e0e4ead2c8
11 changed files with 190 additions and 96 deletions

View file

@ -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) {

View file

@ -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<ImportEntry> 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<ASTNode> statement, Vector<ExportEntry> entries, bool is_default_export)
ExportStatement(SourceRange source_range, RefPtr<ASTNode> statement, Vector<ExportEntry> 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<ASTNode> m_statement;
Vector<ExportEntry> m_entries;
bool m_is_default_export { false };
ModuleRequest m_module_request;
};
class Program final : public ScopeNode {

View file

@ -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<ImportStatement> Parser::parse_import_statement(Program& program)
@ -4133,7 +4132,7 @@ NonnullRefPtr<ImportStatement> 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<ImportStatement> Parser::parse_import_statement(Program& program)
NonnullRefPtr<ExportStatement> 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<ExportStatement> 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<EntryAndLocation> entries_with_location;
RefPtr<ASTNode> expression = {};
bool is_default = false;
ModuleRequest from_specifier;
if (match_default()) {
is_default = true;
@ -4353,7 +4348,7 @@ NonnullRefPtr<ExportStatement> 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<ExportStatement> 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<ExportStatement> Parser::parse_export_statement(Program& program)
m_state.current_scope_pusher->add_declaration(declaration);
if (is<FunctionDeclaration>(*declaration)) {
auto& func = static_cast<FunctionDeclaration&>(*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<ClassDeclaration>(*declaration)) {
auto& class_declaration = static_cast<ClassDeclaration&>(*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<VariableDeclaration>(*declaration));
auto& variables = static_cast<VariableDeclaration&>(*declaration);
@ -4395,11 +4390,11 @@ NonnullRefPtr<ExportStatement> Parser::parse_export_statement(Program& program)
for (auto& decl : variables.declarations()) {
decl.target().visit(
[&](NonnullRefPtr<Identifier> 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<BindingPattern> 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<ExportStatement> Parser::parse_export_statement(Program& program)
for (auto& decl : variable_declaration->declarations()) {
decl.target().visit(
[&](NonnullRefPtr<Identifier> 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<BindingPattern> 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<ExportStatement> 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<ExportStatement> 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<ExportStatement> Parser::parse_export_statement(Program& program)
entries.append(move(entry.entry));
}
return create_ast_node<ExportStatement>({ m_state.current_token.filename(), rule_start.position(), position() }, move(expression), move(entries), is_default);
return create_ast_node<ExportStatement>({ m_state.current_token.filename(), rule_start.position(), position() }, move(expression), move(entries), is_default, move(from_specifier));
}
}

View file

@ -37,9 +37,9 @@ static Vector<FlyString> 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<NonnullRefPtr<SourceTextModule>, Vector<Parser::Error>> 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<NonnullRefPtr<SourceTextModule>, Vector<Parser::Error>> 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<NonnullRefPtr<SourceTextModule>, Vector<Parser::Error>> 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<NonnullRefPtr<SourceTextModule>, Vector<Parser::Error>> 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<Vector<FlyString>> 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<ResolvedBinding> 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<ResolvedBinding> 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));

View file

@ -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", () => {

View file

@ -0,0 +1,2 @@
// This is an all-but-default export and should thus only provide '*'.
export * from "./default-and-star-export.mjs";

View file

@ -0,0 +1,6 @@
export default "defaultValue";
const star = "starExportValue";
const empty = "empty";
export { star as "*", empty as "" };

View file

@ -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";

View file

@ -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";

View file

@ -0,0 +1,6 @@
import * as indirectNs from "./default-and-star-export-indirect.mjs";
export const passed =
indirectNs["*"] === "starExportValue" &&
indirectNs[""] === "empty" &&
indirectNs.default === undefined;

View file

@ -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";