From bf48a8b5938ecc64962809cb9bf20b2947213bef Mon Sep 17 00:00:00 2001 From: Yoshi Cheng-Hao Huang Date: Mon, 18 Mar 2024 14:04:14 +0000 Subject: [PATCH] Bug 1873610 - Part 2: Improve modules import/export error messages. r=jonco Refactor the error message with the adding the information from the imported module, for example: // a.mjs import { a } from "./b.mjs"; // b.mjs const b = 1; Originally the error message is "import not found: default", now it becomes "The requested module: b.mjs doesn't provide an export named: a" // c.mjs import { c } from "./d.mjs"; // d.mjs export * from "./d1.mjs"; export * from "./d2.mjs"; // d1.mjs export const c = 1; // d2.mjs export const c = 2; Originally the error message is "ambiguous import: c", now it becomes "The requested module d.mjs contains ambiguous star export: c from d1.mjs and d2.mjs" Add another error message for circular import: // e.mjs export { e } from "f.mjs"; // f.mjs export { e } from "e.mjs"; Originally it was "indirect export not found: e", now it is "The request module: f.mjs contains circular import: e". Differential Revision: https://phabricator.services.mozilla.com/D200345 --- .eslintignore | 4 + dom/base/test/jsmodules/chrome.toml | 12 ++ .../test/jsmodules/export_star_ambiguous.mjs | 1 + .../jsmodules/import_ambiguous_export.mjs | 1 + .../import_ambiguous_export_star.mjs | 1 + dom/base/test/jsmodules/import_circular.mjs | 1 + dom/base/test/jsmodules/import_circular_1.mjs | 1 + dom/base/test/jsmodules/import_no_export.mjs | 2 +- .../jsmodules/import_no_indirect_export.mjs | 2 +- dom/base/test/jsmodules/module_a.mjs | 2 + dom/base/test/jsmodules/module_b.mjs | 2 + dom/base/test/jsmodules/module_c.mjs | 3 + dom/base/test/jsmodules/module_d.mjs | 1 + dom/base/test/jsmodules/module_e.mjs | 1 + .../jsmodules/test_import_errorMessage.html | 40 ++--- .../jsmodules/test_import_errorMessage2.html | 35 ++++ js/public/friend/ErrorNumbers.msg | 7 +- js/src/vm/Modules.cpp | 168 ++++++++++++------ js/src/vm/Modules.h | 27 ++- .../tests/unit/test_import_global_current.js | 2 +- .../module/instantiation-error-3.html.ini | 5 - .../module/instantiation-error-4.html.ini | 5 - .../module/instantiation-error-5.html.ini | 5 - 23 files changed, 225 insertions(+), 103 deletions(-) create mode 100644 dom/base/test/jsmodules/export_star_ambiguous.mjs create mode 100644 dom/base/test/jsmodules/import_ambiguous_export.mjs create mode 100644 dom/base/test/jsmodules/import_ambiguous_export_star.mjs create mode 100644 dom/base/test/jsmodules/import_circular.mjs create mode 100644 dom/base/test/jsmodules/import_circular_1.mjs create mode 100644 dom/base/test/jsmodules/module_a.mjs create mode 100644 dom/base/test/jsmodules/module_b.mjs create mode 100644 dom/base/test/jsmodules/module_c.mjs create mode 100644 dom/base/test/jsmodules/module_d.mjs create mode 100644 dom/base/test/jsmodules/module_e.mjs create mode 100644 dom/base/test/jsmodules/test_import_errorMessage2.html delete mode 100644 testing/web-platform/meta/html/semantics/scripting-1/the-script-element/module/instantiation-error-3.html.ini delete mode 100644 testing/web-platform/meta/html/semantics/scripting-1/the-script-element/module/instantiation-error-4.html.ini delete mode 100644 testing/web-platform/meta/html/semantics/scripting-1/the-script-element/module/instantiation-error-5.html.ini diff --git a/.eslintignore b/.eslintignore index 552d20ec4237..e862d501e869 100644 --- a/.eslintignore +++ b/.eslintignore @@ -295,3 +295,7 @@ browser/extensions/translations/extension/ # "scaffolding" used by uniffi which isn't valid JS in its original form. toolkit/components/uniffi-bindgen-gecko-js/src/templates/js/ toolkit/components/uniffi-bindgen-gecko-js/components/generated/* + +# Test files for circular import in modules. +dom/base/test/jsmodules/import_circular.mjs +dom/base/test/jsmodules/import_circular_1.mjs diff --git a/dom/base/test/jsmodules/chrome.toml b/dom/base/test/jsmodules/chrome.toml index 82d02ad4df8b..8b8a614bfbf0 100644 --- a/dom/base/test/jsmodules/chrome.toml +++ b/dom/base/test/jsmodules/chrome.toml @@ -3,11 +3,21 @@ support-files = [ "ambiguous_export.mjs", "import_ambiguous.mjs", "import_ambiguous_indirect_export.mjs", + "import_ambiguous_export.mjs", + "import_ambiguous_export_star.mjs", + "import_circular.mjs", + "import_circular_1.mjs", "import_no_export.mjs", "import_no_indirect_export.mjs", "exportA1.mjs", "exportA2.mjs", "export_ambiguous.mjs", + "export_star_ambiguous.mjs", + "module_a.mjs", + "module_b.mjs", + "module_c.mjs", + "module_d.mjs", + "module_e.mjs", "module_setRan.mjs", "module_testSyntax.mjs", "module_badSyntax.mjs", @@ -47,6 +57,8 @@ support-files = [ ["test_import_errorMessage.html"] +["test_import_errorMessage2.html"] + ["test_import_meta_resolve.html"] ["test_importedModuleMemoization.html"] diff --git a/dom/base/test/jsmodules/export_star_ambiguous.mjs b/dom/base/test/jsmodules/export_star_ambiguous.mjs new file mode 100644 index 000000000000..35cc979dee64 --- /dev/null +++ b/dom/base/test/jsmodules/export_star_ambiguous.mjs @@ -0,0 +1 @@ +export * from "./ambiguous_export.mjs"; diff --git a/dom/base/test/jsmodules/import_ambiguous_export.mjs b/dom/base/test/jsmodules/import_ambiguous_export.mjs new file mode 100644 index 000000000000..f5c12ff08684 --- /dev/null +++ b/dom/base/test/jsmodules/import_ambiguous_export.mjs @@ -0,0 +1 @@ +import { a } from "./ambiguous_export.mjs"; diff --git a/dom/base/test/jsmodules/import_ambiguous_export_star.mjs b/dom/base/test/jsmodules/import_ambiguous_export_star.mjs new file mode 100644 index 000000000000..1ee2a56d379b --- /dev/null +++ b/dom/base/test/jsmodules/import_ambiguous_export_star.mjs @@ -0,0 +1 @@ +import { a } from "./export_star_ambiguous.mjs"; diff --git a/dom/base/test/jsmodules/import_circular.mjs b/dom/base/test/jsmodules/import_circular.mjs new file mode 100644 index 000000000000..bb3a46bd5ec3 --- /dev/null +++ b/dom/base/test/jsmodules/import_circular.mjs @@ -0,0 +1 @@ +export { a } from "./import_circular_1.mjs"; diff --git a/dom/base/test/jsmodules/import_circular_1.mjs b/dom/base/test/jsmodules/import_circular_1.mjs new file mode 100644 index 000000000000..eb7c038c773e --- /dev/null +++ b/dom/base/test/jsmodules/import_circular_1.mjs @@ -0,0 +1 @@ +export { a } from "./import_circular.mjs"; diff --git a/dom/base/test/jsmodules/import_no_export.mjs b/dom/base/test/jsmodules/import_no_export.mjs index 47cabac557be..d989498c88aa 100644 --- a/dom/base/test/jsmodules/import_no_export.mjs +++ b/dom/base/test/jsmodules/import_no_export.mjs @@ -1 +1 @@ -import x from "./no_export.mjs"; +import { x } from "./no_export.mjs"; diff --git a/dom/base/test/jsmodules/import_no_indirect_export.mjs b/dom/base/test/jsmodules/import_no_indirect_export.mjs index dd1ca847fc9e..bfcdcedbc5c0 100644 --- a/dom/base/test/jsmodules/import_no_indirect_export.mjs +++ b/dom/base/test/jsmodules/import_no_indirect_export.mjs @@ -1,2 +1,2 @@ /* eslint-disable import/default */ -import x from "./no_indirect_export.mjs"; +import { a } from "./no_indirect_export.mjs"; diff --git a/dom/base/test/jsmodules/module_a.mjs b/dom/base/test/jsmodules/module_a.mjs new file mode 100644 index 000000000000..0b0c3304ff37 --- /dev/null +++ b/dom/base/test/jsmodules/module_a.mjs @@ -0,0 +1,2 @@ +export var a = 1; +export var b = 2; diff --git a/dom/base/test/jsmodules/module_b.mjs b/dom/base/test/jsmodules/module_b.mjs new file mode 100644 index 000000000000..79da84736d49 --- /dev/null +++ b/dom/base/test/jsmodules/module_b.mjs @@ -0,0 +1,2 @@ +export var b = 3; +export var c = 4; diff --git a/dom/base/test/jsmodules/module_c.mjs b/dom/base/test/jsmodules/module_c.mjs new file mode 100644 index 000000000000..5a2a7e3e09ac --- /dev/null +++ b/dom/base/test/jsmodules/module_c.mjs @@ -0,0 +1,3 @@ +/* eslint-disable import/export */ +export * from "./module_a.mjs"; +export * from "./module_b.mjs"; diff --git a/dom/base/test/jsmodules/module_d.mjs b/dom/base/test/jsmodules/module_d.mjs new file mode 100644 index 000000000000..04dc02d27aac --- /dev/null +++ b/dom/base/test/jsmodules/module_d.mjs @@ -0,0 +1 @@ +import { a } from "./module_c.mjs"; diff --git a/dom/base/test/jsmodules/module_e.mjs b/dom/base/test/jsmodules/module_e.mjs new file mode 100644 index 000000000000..544c424fcba5 --- /dev/null +++ b/dom/base/test/jsmodules/module_e.mjs @@ -0,0 +1 @@ +import { b } from "./module_c.mjs"; diff --git a/dom/base/test/jsmodules/test_import_errorMessage.html b/dom/base/test/jsmodules/test_import_errorMessage.html index 6ab0b1dd74d2..2330b46dd921 100644 --- a/dom/base/test/jsmodules/test_import_errorMessage.html +++ b/dom/base/test/jsmodules/test_import_errorMessage.html @@ -8,40 +8,38 @@ let count = 0; window.onerror = function (event, src, lineno, colno, error) { - info("window.onerror :" + error.message); + info("window.onerror: message: " + error.message); + info("window.onerror: src: " + src); ok(error instanceof SyntaxError, "Should be a SyntaxError."); - // import_no_indirect_export.mjs and import_ambiguous_indirect_export.mjs - // are related to indirect import/export. - if (count < 2) { - ok(error.message.match("indirect"), "Should contain 'indirect'"); - } - - // import_ambiguous_indirect_export.mjs and import_ambiguous.mjs both - // have ambiguous import/export. - if (count % 2 === 1) { - ok(error.message.match("ambiguous"), "Should contain 'ambiguous'"); - } - - if (count === 2) { - ok(!error.message.match("ambiguous") && !error.message.match("indirect"), - "Should NOT contain 'indirect' nor 'ambiguous'"); + if (src.match("no_indirect_export.mjs") || + src.match("import_no_export.mjs")) { + ok(error.message.match("doesn't provide an export named")); + } else if(src.match("export_ambiguous.mjs") || + src.match("import_ambiguous_export_star.mjs") || + src.match("import_ambiguous_export.mjs") || + src.match("import_ambiguous.mjs")) { + ok(error.message.match("contains ambiguous star export")); + } else if (src.match("import_circular_1.mjs")) { + ok(error.message.match("contains circular import")); + } else { + ok(false, "unknown src " + src); } count++; }; function testLoaded() { - ok(count === 4, "Should have 4 SynaxErrors thrown."); + ok(count === 7, "Should have 7 SynaxErrors thrown."); SimpleTest.finish(); } + - + + + diff --git a/dom/base/test/jsmodules/test_import_errorMessage2.html b/dom/base/test/jsmodules/test_import_errorMessage2.html new file mode 100644 index 000000000000..ed4227362e4d --- /dev/null +++ b/dom/base/test/jsmodules/test_import_errorMessage2.html @@ -0,0 +1,35 @@ + + +Test to get the filename of the requested module after it has been evaluated + + + + + + + + + diff --git a/js/public/friend/ErrorNumbers.msg b/js/public/friend/ErrorNumbers.msg index e75e7b973cf1..12b54ee4d511 100644 --- a/js/public/friend/ErrorNumbers.msg +++ b/js/public/friend/ErrorNumbers.msg @@ -714,10 +714,9 @@ MSG_DEF(JSMSG_CANT_DELETE_SUPER, 0, JSEXN_REFERENCEERR, "invalid delete involvin MSG_DEF(JSMSG_REINIT_THIS, 0, JSEXN_REFERENCEERR, "super() called twice in derived class constructor") // Modules -MSG_DEF(JSMSG_MISSING_INDIRECT_EXPORT, 0, JSEXN_SYNTAXERR, "indirect export not found") -MSG_DEF(JSMSG_AMBIGUOUS_INDIRECT_EXPORT, 0, JSEXN_SYNTAXERR, "ambiguous indirect export") -MSG_DEF(JSMSG_MISSING_IMPORT, 0, JSEXN_SYNTAXERR, "import not found") -MSG_DEF(JSMSG_AMBIGUOUS_IMPORT, 0, JSEXN_SYNTAXERR, "ambiguous import") +MSG_DEF(JSMSG_MODULE_NO_EXPORT, 2, JSEXN_SYNTAXERR, "The requested module '{0}' doesn't provide an export named: '{1}'") +MSG_DEF(JSMSG_MODULE_CIRCULAR_IMPORT, 2, JSEXN_SYNTAXERR, "The requested module '{0}' contains circular import: '{1}'") +MSG_DEF(JSMSG_MODULE_AMBIGUOUS, 4, JSEXN_SYNTAXERR, "The requested module '{0}' contains ambiguous star export: '{1}' from '{2}' and '{3}'") MSG_DEF(JSMSG_MISSING_EXPORT, 1, JSEXN_SYNTAXERR, "local binding for export '{0}' not found") MSG_DEF(JSMSG_BAD_MODULE_STATUS, 1, JSEXN_INTERNALERR, "module record has unexpected status: {0}") MSG_DEF(JSMSG_DYNAMIC_IMPORT_FAILED, 1, JSEXN_TYPEERR, "error loading dynamically imported module: {0}") diff --git a/js/src/vm/Modules.cpp b/js/src/vm/Modules.cpp index f461e7bec198..5230c6dec17b 100644 --- a/js/src/vm/Modules.cpp +++ b/js/src/vm/Modules.cpp @@ -359,7 +359,8 @@ static ModuleObject* HostResolveImportedModule( static bool ModuleResolveExport(JSContext* cx, Handle module, Handle exportName, MutableHandle resolveSet, - MutableHandle result); + MutableHandle result, + ModuleErrorInfo* errorInfoOut = nullptr); static bool SyntheticModuleResolveExport(JSContext* cx, Handle module, Handle exportName, @@ -577,7 +578,8 @@ static ModuleObject* HostResolveImportedModule( // bool js::ModuleResolveExport(JSContext* cx, Handle module, Handle exportName, - MutableHandle result) { + MutableHandle result, + ModuleErrorInfo* errorInfoOut = nullptr) { if (module->hasSyntheticModuleFields()) { return ::SyntheticModuleResolveExport(cx, module, exportName, result); } @@ -585,7 +587,8 @@ bool js::ModuleResolveExport(JSContext* cx, Handle module, // Step 1. If resolveSet is not present, set resolveSet to a new empty List. Rooted resolveSet(cx); - return ::ModuleResolveExport(cx, module, exportName, &resolveSet, result); + return ::ModuleResolveExport(cx, module, exportName, &resolveSet, result, + errorInfoOut); } static bool CreateResolvedBindingObject(JSContext* cx, @@ -605,7 +608,8 @@ static bool CreateResolvedBindingObject(JSContext* cx, static bool ModuleResolveExport(JSContext* cx, Handle module, Handle exportName, MutableHandle resolveSet, - MutableHandle result) { + MutableHandle result, + ModuleErrorInfo* errorInfoOut) { // Step 2. For each Record { [[Module]], [[ExportName]] } r of resolveSet, do: for (const auto& entry : resolveSet) { // Step 2.a. If module and r.[[Module]] are the same Module Record and @@ -614,6 +618,9 @@ static bool ModuleResolveExport(JSContext* cx, Handle module, // Step 2.a.i. Assert: This is a circular import request. // Step 2.a.ii. Return null. result.setNull(); + if (errorInfoOut) { + errorInfoOut->setCircularImport(cx, module); + } return true; } } @@ -669,8 +676,8 @@ static bool ModuleResolveExport(JSContext* cx, Handle module, // importedModule.ResolveExport(e.[[ImportName]], // resolveSet). name = e.importName(); - return ModuleResolveExport(cx, importedModule, name, resolveSet, - result); + return ModuleResolveExport(cx, importedModule, name, resolveSet, result, + errorInfoOut); } } } @@ -683,6 +690,9 @@ static bool ModuleResolveExport(JSContext* cx, Handle module, // Step 6.c. NOTE: A default export cannot be provided by an export * from // "mod" declaration. result.setNull(); + if (errorInfoOut) { + errorInfoOut->setImportedModule(cx, module); + } return true; } @@ -705,7 +715,7 @@ static bool ModuleResolveExport(JSContext* cx, Handle module, // Step 8.b. Let resolution be ? importedModule.ResolveExport(exportName, // resolveSet). if (!ModuleResolveExport(cx, importedModule, exportName, resolveSet, - &resolution)) { + &resolution, errorInfoOut)) { return false; } @@ -744,6 +754,12 @@ static bool ModuleResolveExport(JSContext* cx, Handle module, if (binding->module() != starResolution->module() || binding->bindingName() != starResolution->bindingName()) { result.set(StringValue(cx->names().ambiguous)); + + if (errorInfoOut) { + Rooted module1(cx, starResolution->module()); + Rooted module2(cx, binding->module()); + errorInfoOut->setForAmbiguousImport(cx, module, module1, module2); + } return true; } } @@ -752,6 +768,9 @@ static bool ModuleResolveExport(JSContext* cx, Handle module, // Step 9. Return starResolution. result.setObjectOrNull(starResolution); + if (!starResolution && errorInfoOut) { + errorInfoOut->setImportedModule(cx, module); + } return true; } @@ -923,63 +942,93 @@ static ModuleNamespaceObject* ModuleNamespaceCreate( return ns; } +void ModuleErrorInfo::setImportedModule(JSContext* cx, + ModuleObject* importedModule) { + imported = importedModule->filename(); +} + +void ModuleErrorInfo::setCircularImport(JSContext* cx, + ModuleObject* importedModule) { + setImportedModule(cx, importedModule); + isCircular = true; +} + +void ModuleErrorInfo::setForAmbiguousImport(JSContext* cx, + ModuleObject* importedModule, + ModuleObject* module1, + ModuleObject* module2) { + setImportedModule(cx, importedModule); + entry1 = module1->filename(); + entry2 = module2->filename(); +} + +static void CreateErrorNumberMessageUTF8(JSContext* cx, unsigned errorNumber, + JSErrorReport* reportOut, ...) { + va_list ap; + va_start(ap, reportOut); + AutoReportFrontendContext fc(cx); + if (!ExpandErrorArgumentsVA(&fc, GetErrorMessage, nullptr, errorNumber, + ArgumentsAreUTF8, reportOut, ap)) { + ReportOutOfMemory(cx); + return; + } + + va_end(ap); +} + static void ThrowResolutionError(JSContext* cx, Handle module, - Handle resolution, bool isDirectImport, - Handle name, uint32_t line, - JS::ColumnNumberOneOrigin column) { - MOZ_ASSERT(line != 0); + Handle resolution, Handle name, + ModuleErrorInfo* errorInfo) { + MOZ_ASSERT(errorInfo); + auto chars = StringToNewUTF8CharsZ(cx, *name); + if (!chars) { + ReportOutOfMemory(cx); + return; + } bool isAmbiguous = resolution == StringValue(cx->names().ambiguous); - // ErrorNumbers: - // | MISSING | AMBIGUOUS | - // ---------+---------+-----------+ - // INDIRECT | - // DIRECT | - static constexpr unsigned ErrorNumbers[2][2] = { - {JSMSG_MISSING_INDIRECT_EXPORT, JSMSG_AMBIGUOUS_INDIRECT_EXPORT}, - {JSMSG_MISSING_IMPORT, JSMSG_AMBIGUOUS_IMPORT}}; - unsigned errorNumber = ErrorNumbers[isDirectImport][isAmbiguous]; - - const JSErrorFormatString* errorString = - GetErrorMessage(nullptr, errorNumber); - MOZ_ASSERT(errorString); - - MOZ_ASSERT(errorString->argCount == 0); - Rooted message(cx, JS_NewStringCopyZ(cx, errorString->format)); - if (!message) { - return; - } - - Rooted separator(cx, JS_NewStringCopyZ(cx, ": ")); - if (!separator) { - return; - } - - message = ConcatStrings(cx, message, separator); - if (!message) { - return; - } - - message = ConcatStrings(cx, message, name); - if (!message) { - return; - } - - RootedString filename(cx); - if (const char* chars = module->script()->filename()) { - filename = - JS_NewStringCopyUTF8Z(cx, JS::ConstUTF8CharsZ(chars, strlen(chars))); + unsigned errorNumber; + if (errorInfo->isCircular) { + errorNumber = JSMSG_MODULE_CIRCULAR_IMPORT; + } else if (isAmbiguous) { + errorNumber = JSMSG_MODULE_AMBIGUOUS; } else { - filename = cx->names().empty_; + errorNumber = JSMSG_MODULE_NO_EXPORT; } + + JSErrorReport report; + report.isWarning_ = false; + report.errorNumber = errorNumber; + + if (errorNumber == JSMSG_MODULE_AMBIGUOUS) { + CreateErrorNumberMessageUTF8(cx, errorNumber, &report, errorInfo->imported, + chars.get(), errorInfo->entry1, + errorInfo->entry2); + } else { + CreateErrorNumberMessageUTF8(cx, errorNumber, &report, errorInfo->imported, + chars.get()); + } + + Rooted message(cx, report.newMessageString(cx)); + if (!message) { + ReportOutOfMemory(cx); + return; + } + + const char* file = module->filename(); + RootedString filename( + cx, JS_NewStringCopyUTF8Z(cx, JS::ConstUTF8CharsZ(file, strlen(file)))); if (!filename) { + ReportOutOfMemory(cx); return; } RootedValue error(cx); - if (!JS::CreateError(cx, JSEXN_SYNTAXERR, nullptr, filename, line, column, - nullptr, message, JS::NothingHandleValue, &error)) { + if (!JS::CreateError(cx, JSEXN_SYNTAXERR, nullptr, filename, + errorInfo->lineNumber, errorInfo->columnNumber, nullptr, + message, JS::NothingHandleValue, &error)) { + ReportOutOfMemory(cx); return; } @@ -1002,15 +1051,15 @@ bool js::ModuleInitializeEnvironment(JSContext* cx, // Step 1.b. Let resolution be ? module.ResolveExport(e.[[ExportName]]). exportName = e.exportName(); - if (!ModuleResolveExport(cx, module, exportName, &resolution)) { + ModuleErrorInfo errorInfo{e.lineNumber(), e.columnNumber()}; + if (!ModuleResolveExport(cx, module, exportName, &resolution, &errorInfo)) { return false; } // Step 1.c. If resolution is either null or AMBIGUOUS, throw a SyntaxError // exception. if (!IsResolvedBinding(cx, resolution)) { - ThrowResolutionError(cx, module, resolution, false, exportName, - e.lineNumber(), e.columnNumber()); + ThrowResolutionError(cx, module, resolution, exportName, &errorInfo); return false; } } @@ -1059,15 +1108,16 @@ bool js::ModuleInitializeEnvironment(JSContext* cx, // Step 7.d. Else: // Step 7.d.i. Let resolution be ? // importedModule.ResolveExport(in.[[ImportName]]). - if (!ModuleResolveExport(cx, importedModule, importName, &resolution)) { + ModuleErrorInfo errorInfo{in.lineNumber(), in.columnNumber()}; + if (!ModuleResolveExport(cx, importedModule, importName, &resolution, + &errorInfo)) { return false; } // Step 7.d.ii. If resolution is null or ambiguous, throw a SyntaxError // exception. if (!IsResolvedBinding(cx, resolution)) { - ThrowResolutionError(cx, module, resolution, true, importName, - in.lineNumber(), in.columnNumber()); + ThrowResolutionError(cx, module, resolution, importName, &errorInfo); return false; } diff --git a/js/src/vm/Modules.h b/js/src/vm/Modules.h index 21aff46b90b4..c3e337f86ef6 100644 --- a/js/src/vm/Modules.h +++ b/js/src/vm/Modules.h @@ -20,9 +20,34 @@ namespace js { using ModuleVector = GCVector; +// A struct with detailed error information when import/export failed. +struct ModuleErrorInfo { + ModuleErrorInfo(uint32_t lineNumber_, JS::ColumnNumberOneOrigin columnNumber_) + : lineNumber(lineNumber_), columnNumber(columnNumber_) {} + + void setImportedModule(JSContext* cx, ModuleObject* importedModule); + void setCircularImport(JSContext* cx, ModuleObject* importedModule); + void setForAmbiguousImport(JSContext* cx, ModuleObject* importedModule, + ModuleObject* module1, ModuleObject* module2); + + uint32_t lineNumber; + JS::ColumnNumberOneOrigin columnNumber; + + // The filename of the imported module. + const char* imported; + + // The filenames of the ambiguous entries. + const char* entry1; + const char* entry2; + + // A bool to indicate the error is a circular import when it's true. + bool isCircular = false; +}; + bool ModuleResolveExport(JSContext* cx, Handle module, Handle exportName, - MutableHandle result); + MutableHandle result, + ModuleErrorInfo* errorInfoOut); ModuleNamespaceObject* GetOrCreateModuleNamespace(JSContext* cx, Handle module); diff --git a/js/xpconnect/tests/unit/test_import_global_current.js b/js/xpconnect/tests/unit/test_import_global_current.js index cf466a7391bf..59037512f3bf 100644 --- a/js/xpconnect/tests/unit/test_import_global_current.js +++ b/js/xpconnect/tests/unit/test_import_global_current.js @@ -204,7 +204,7 @@ ChromeUtils.importESModule("resource://test/es6module_import_error.js", { `, sb); } catch (e) { caught = true; - Assert.stringMatches(e.message, /import not found/); + Assert.stringMatches(e.message, /doesn't provide an export named/); } Assert.ok(caught); }); diff --git a/testing/web-platform/meta/html/semantics/scripting-1/the-script-element/module/instantiation-error-3.html.ini b/testing/web-platform/meta/html/semantics/scripting-1/the-script-element/module/instantiation-error-3.html.ini deleted file mode 100644 index b7eb7b18b933..000000000000 --- a/testing/web-platform/meta/html/semantics/scripting-1/the-script-element/module/instantiation-error-3.html.ini +++ /dev/null @@ -1,5 +0,0 @@ -[instantiation-error-3.html] - expected: - if (os == "android") and fission: [OK, TIMEOUT] - [Test that unresolvable cycles lead to SyntaxError events on window and load events on script] - expected: FAIL diff --git a/testing/web-platform/meta/html/semantics/scripting-1/the-script-element/module/instantiation-error-4.html.ini b/testing/web-platform/meta/html/semantics/scripting-1/the-script-element/module/instantiation-error-4.html.ini deleted file mode 100644 index af7b9cd3d183..000000000000 --- a/testing/web-platform/meta/html/semantics/scripting-1/the-script-element/module/instantiation-error-4.html.ini +++ /dev/null @@ -1,5 +0,0 @@ -[instantiation-error-4.html] - expected: - if (os == "android") and fission: [OK, TIMEOUT] - [Test that loading a graph in which a module is already errored results in an error.] - expected: FAIL diff --git a/testing/web-platform/meta/html/semantics/scripting-1/the-script-element/module/instantiation-error-5.html.ini b/testing/web-platform/meta/html/semantics/scripting-1/the-script-element/module/instantiation-error-5.html.ini deleted file mode 100644 index ac84583c4e23..000000000000 --- a/testing/web-platform/meta/html/semantics/scripting-1/the-script-element/module/instantiation-error-5.html.ini +++ /dev/null @@ -1,5 +0,0 @@ -[instantiation-error-5.html] - expected: - if (os == "android") and fission: [OK, TIMEOUT] - [Test that loading a graph in which a module is already errored results an error.] - expected: FAIL