Skip to content

Attach importer sourceURL/line/column to module link SyntaxErrors #378

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
robobun wants to merge 2 commits into
base: main
Choose a base branch
from
+61 −8
Open

Attach importer sourceURL/line/column to module link SyntaxErrors #378

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
b5a21f5
Attach importer sourceURL/line/column to module link SyntaxErrors
robobun Aug 1, 2026
aac0d8c
Match the lexer's line-terminator set when scanning for line/column
robobun Aug 1, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions Source/JavaScriptCore/parser/NodesAnalyzeModule.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -113,6 +113,9 @@ bool ImportDeclarationNode::analyzeModule(ModuleAnalyzer& analyzer)
m_moduleName->moduleName(),
specifier->importedName(),
specifier->localName(),
#if USE(BUN_JSC_ADDITIONS)
static_cast<unsigned>(std::max(0, specifier->startOffset())),
#endif
});
}
return true;
Expand Down
3 changes: 3 additions & 0 deletions Source/JavaScriptCore/runtime/AbstractModuleRecord.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -111,6 +111,9 @@ class AbstractModuleRecord : public JSInternalFieldObjectImpl<2> {
Identifier moduleRequest;
Identifier importName;
Identifier localName;
#if USE(BUN_JSC_ADDITIONS)
unsigned sourceOffset { 0 };
#endif
};

using OrderedIdentifierSet = OrderedHashSet<RefPtr<UniquedStringImpl>, IdentifierRepHash>;
Expand Down
60 changes: 52 additions & 8 deletions Source/JavaScriptCore/runtime/CyclicModuleRecord.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,50 @@
ASSERT(jsModule);
#endif

#if USE(BUN_JSC_ADDITIONS)
auto throwLinkSyntaxError = [&](String&& message, unsigned sourceOffset = 0) {
JSObject* error = createSyntaxError(globalObject, message);
auto attrs = static_cast<unsigned>(PropertyAttribute::DontEnum);
unsigned line = 0;
unsigned column = 0;
String sourceURL;
if (jsModule && jsModule->sourceCode().provider()) {
auto& sourceCode = jsModule->sourceCode();
sourceURL = sourceCode.provider()->sourceURL();
if (sourceOffset) {
StringView source = sourceCode.provider()->source();
unsigned end = std::min<unsigned>(sourceOffset, source.length());
unsigned lineStart = 0;
line = sourceCode.firstLine().oneBasedInt();
for (unsigned i = 0; i < end; ++i) {
if (source[i] == '\n') {
++line;
lineStart = i + 1;
}
}

Check warning on line 112 in Source/JavaScriptCore/runtime/CyclicModuleRecord.cpp

View check run for this annotation

Claude / Claude Code Review

Line-count scan only recognizes '\n', diverges from JSC lexer's line-terminator set 

nit: the line/column scan only counts `'\n'`, but the JSC lexer also treats `'\r'`, U+2028, and U+2029 as line terminators (see `Lexer<T>::isLineTerminator` / `shiftLineTerminator`). Sources using lone `\r` or Unicode line separators will report `line = firstLine` and a column equal to the absolute offset + 1. CRLF happens to work; matching the lexer's terminator set is a ~3-line change if you want the diagnostic to be exact.
Comment thread
robobun marked this conversation as resolved.
column = end - lineStart + 1;
}
}
if (sourceURL.isEmpty())
sourceURL = moduleKey().string();
if (auto* errorInstance = dynamicDowncast<ErrorInstance>(error)) {
if (!sourceURL.isEmpty())
errorInstance->setSourceURL(sourceURL);
if (line) {
errorInstance->setLine(line);
errorInstance->setColumn(column);
}
}
if (!sourceURL.isEmpty())
error->putDirect(vm, vm.propertyNames->sourceURL, jsString(vm, WTF::move(sourceURL)), attrs);

@Jarred-Sumner Jarred-Sumner Aug 1, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@robobun don't set both here. just set the one.

if (line) {
error->putDirect(vm, vm.propertyNames->line, jsNumber(line), attrs);
error->putDirect(vm, vm.propertyNames->column, jsNumber(column), attrs);
}
throwException(globalObject, scope, error);
};
#endif

ModuleProgramExecutable* moduleProgramExecutable = nullptr;
JSModuleEnvironment* env = nullptr;

Expand All @@ -106,23 +150,23 @@
case Resolution::Type::NotFound:
#if USE(BUN_JSC_ADDITIONS)
if (m_isTypeScript) break;
throwSyntaxError(globalObject, scope, makeString("export '"_s, StringView(e.exportName.impl()), "' not found in '"_s, StringView(e.moduleName.impl()), "'"_s));
throwLinkSyntaxError(makeString("export '"_s, StringView(e.exportName.impl()), "' not found in '"_s, StringView(e.moduleName.impl()), "'"_s));
#else
throwSyntaxError(globalObject, scope, makeString("Indirectly exported binding name '"_s, StringView(e.exportName.impl()), "' is not found."_s));
#endif
return;

case Resolution::Type::Ambiguous:
#if USE(BUN_JSC_ADDITIONS)
throwSyntaxError(globalObject, scope, makeString("Cannot export '"_s, StringView(e.exportName.impl()), "' multiple times in '"_s, StringView(e.moduleName.impl()), "'"_s));
throwLinkSyntaxError(makeString("Cannot export '"_s, StringView(e.exportName.impl()), "' multiple times in '"_s, StringView(e.moduleName.impl()), "'"_s));
#else
throwSyntaxError(globalObject, scope, makeString("Indirectly exported binding name '"_s, StringView(e.exportName.impl()), "' cannot be resolved due to ambiguous multiple bindings."_s));
#endif
return;

case Resolution::Type::Error:
#if USE(BUN_JSC_ADDITIONS)
throwSyntaxError(globalObject, scope, "export default cannot be used with export *"_s);
throwLinkSyntaxError("export default cannot be used with export *"_s);
#else
throwSyntaxError(globalObject, scope, "Indirectly exported binding name 'default' cannot be resolved by star export entries."_s);
#endif
Expand Down Expand Up @@ -209,19 +253,19 @@
Resolution otherResolution = importedModule->resolveExport(globalObject, vm.propertyNames->defaultKeyword);
RETURN_IF_EXCEPTION(scope, void());
if (otherResolution.type == Resolution::Type::Resolved && otherResolution.localName == in.localName) {
throwSyntaxError(globalObject, scope, makeString("Export named '"_s, in.importName.string(), "' not found in module '"_s, importedModule->moduleKey().string(), "'. Did you mean to import default?"_s));
throwLinkSyntaxError(makeString("Export named '"_s, in.importName.string(), "' not found in module '"_s, importedModule->moduleKey().string(), "'. Did you mean to import default?"_s), in.sourceOffset);
return;
}
}
throwSyntaxError(globalObject, scope, makeString("Export named '"_s, in.importName.string(), "' not found in module '"_s, importedModule->moduleKey().string(), "'."_s));
throwLinkSyntaxError(makeString("Export named '"_s, in.importName.string(), "' not found in module '"_s, importedModule->moduleKey().string(), "'."_s), in.sourceOffset);
#else
throwSyntaxError(globalObject, scope, makeString("Importing binding name '"_s, StringView(in.importName.impl()), "' is not found."_s));
#endif
return;

case Resolution::Type::Ambiguous:
#if USE(BUN_JSC_ADDITIONS)
throwSyntaxError(globalObject, scope, makeString("Export named '"_s, in.importName.string(), "' cannot be resolved due to ambiguous multiple bindings in module '"_s, importedModule->moduleKey().string(), "'."_s));
throwLinkSyntaxError(makeString("Export named '"_s, in.importName.string(), "' cannot be resolved due to ambiguous multiple bindings in module '"_s, importedModule->moduleKey().string(), "'."_s), in.sourceOffset);
#else
throwSyntaxError(globalObject, scope, makeString("Importing binding name '"_s, StringView(in.importName.impl()), "' cannot be resolved due to ambiguous multiple bindings."_s));
#endif
Expand All @@ -233,11 +277,11 @@
Resolution otherResolution = importedModule->resolveExport(globalObject, in.localName);
RETURN_IF_EXCEPTION(scope, void());
if (otherResolution.type == Resolution::Type::Resolved) {
throwSyntaxError(globalObject, scope, makeString("module '"_s, importedModule->moduleKey().string(), "' does not have an export named 'default'. Did you mean '"_s, String(in.localName.impl()), "'?"_s));
throwLinkSyntaxError(makeString("module '"_s, importedModule->moduleKey().string(), "' does not have an export named 'default'. Did you mean '"_s, String(in.localName.impl()), "'?"_s), in.sourceOffset);
return;
}
}
throwSyntaxError(globalObject, scope, makeString("Missing 'default' export in module '"_s, importedModule->moduleKey().string(), "'."_s));
throwLinkSyntaxError(makeString("Missing 'default' export in module '"_s, importedModule->moduleKey().string(), "'."_s), in.sourceOffset);
#else
throwSyntaxError(globalObject, scope, "Importing binding name 'default' cannot be resolved by star export entries."_s);
#endif
Expand Down
Loading