Fix error reporting when imports or re-exports in modules fail

Collect the location of the import/export statement and include it in
the exception thrown.

Change-Id: I7966dfd53ed67d2d7087acde2dd8ff67c64cb044
Reviewed-by: Lars Knoll <lars.knoll@qt.io>

9 files changed, 50 insertions, 5 deletions

diff --git a/src/qml/compiler/qv4compileddata.cpp b/src/qml/compiler/qv4compileddata.cpp
index c82f3d1bc8..244e762faf 100644
--- a/src/qml/compiler/qv4compileddata.cpp
+++ b/src/qml/compiler/qv4compileddata.cpp
@@ -410,7 +410,7 @@ Heap::Module *CompilationUnit::instantiate(ExecutionEngine *engine)
         if (!valuePtr) {
             QString referenceErrorMessage = QStringLiteral("Unable to resolve import reference ");
             referenceErrorMessage += importName->toQString();
-            engine->throwReferenceError(referenceErrorMessage, fileName(), /*### line*/1, /*### column*/1);
+            engine->throwReferenceError(referenceErrorMessage, fileName(), entry.location.line, entry.location.column);
             return nullptr;
         }
         imports[i] = valuePtr;
@@ -426,7 +426,7 @@ Heap::Module *CompilationUnit::instantiate(ExecutionEngine *engine)
         if (!dependentModuleUnit->resolveExport(importName)) {
             QString referenceErrorMessage = QStringLiteral("Unable to resolve re-export reference ");
             referenceErrorMessage += importName->toQString();
-            engine->throwReferenceError(referenceErrorMessage, fileName(), /*### line*/1, /*### column*/1);
+            engine->throwReferenceError(referenceErrorMessage, fileName(), entry.location.line, entry.location.column);
             return nullptr;
         }
     }
@@ -949,6 +949,13 @@ bool Unit::verifyHeader(QDateTime expectedSourceTimeStamp, QString *errorString)
 #endif
 }
 
+Location &Location::operator=(const QQmlJS::AST::SourceLocation &astLocation)
+{
+    line = astLocation.startLine;
+    column = astLocation.startColumn;
+    return *this;
+}
+
 }
 
 }
diff --git a/src/qml/compiler/qv4compileddata_p.h b/src/qml/compiler/qv4compileddata_p.h
index 1f6c591ada..c2ece459c8 100644
--- a/src/qml/compiler/qv4compileddata_p.h
+++ b/src/qml/compiler/qv4compileddata_p.h
@@ -63,6 +63,7 @@
 #include <private/qv4identifier_p.h>
 #include <private/qflagpointer_p.h>
 #include <private/qendian_p.h>
+#include <private/qqmljsastfwd_p.h>
 #ifndef V4_BOOTSTRAP
 #include <private/qqmltypenamecache_p.h>
 #include <private/qqmlpropertycache_p.h>
@@ -127,6 +128,8 @@ struct Location
 
     Location() : _dummy(0) { }
 
+    Location &operator=(const QQmlJS::AST::SourceLocation &astLocation);
+
     inline bool operator<(const Location &other) const {
         return line < other.line ||
                (line == other.line && column < other.column);
@@ -376,15 +379,16 @@ struct ExportEntry
     quint32_le moduleRequest;
     quint32_le importName;
     quint32_le localName;
+    Location location;
 };
-static_assert(sizeof(ExportEntry) == 16, "ExportEntry structure needs to have the expected size to be binary compatible on disk when generated by host compiler and loaded by target");
+static_assert(sizeof(ExportEntry) == 20, "ExportEntry structure needs to have the expected size to be binary compatible on disk when generated by host compiler and loaded by target");
 
 struct ImportEntry
 {
     quint32_le moduleRequest;
     quint32_le importName;
     quint32_le localName;
-    quint32_le padding;
+    Location location;
 };
 static_assert(sizeof(ImportEntry) == 16, "ImportEntry structure needs to have the expected size to be binary compatible on disk when generated by host compiler and loaded by target");
 
diff --git a/src/qml/compiler/qv4compiler.cpp b/src/qml/compiler/qv4compiler.cpp
index f2e3aaedb5..7636baa1e6 100644
--- a/src/qml/compiler/qv4compiler.cpp
+++ b/src/qml/compiler/qv4compiler.cpp
@@ -342,6 +342,7 @@ QV4::CompiledData::Unit *QV4::Compiler::JSUnitGenerator::generateUnit(GeneratorO
                 entryToWrite->moduleRequest = getStringId(entry.moduleRequest);
                 entryToWrite->importName = getStringId(entry.importName);
                 entryToWrite->localName = getStringId(entry.localName);
+                entryToWrite->location = entry.location;
                 entryToWrite++;
             }
         };
@@ -356,6 +357,7 @@ QV4::CompiledData::Unit *QV4::Compiler::JSUnitGenerator::generateUnit(GeneratorO
             entryToWrite->moduleRequest = getStringId(entry.moduleRequest);
             entryToWrite->importName = getStringId(entry.importName);
             entryToWrite->localName = getStringId(entry.localName);
+            entryToWrite->location = entry.location;
             entryToWrite++;
         }
     }
diff --git a/src/qml/compiler/qv4compilercontext_p.h b/src/qml/compiler/qv4compilercontext_p.h
index a29f4ea4cb..796d5e274c 100644
--- a/src/qml/compiler/qv4compilercontext_p.h
+++ b/src/qml/compiler/qv4compilercontext_p.h
@@ -104,6 +104,7 @@ struct ExportEntry
     QString moduleRequest;
     QString importName;
     QString localName;
+    CompiledData::Location location;
 
     static bool lessThan(const ExportEntry &lhs, const ExportEntry &rhs)
     { return lhs.exportName < rhs.exportName; }
@@ -114,6 +115,7 @@ struct ImportEntry
     QString moduleRequest;
     QString importName;
     QString localName;
+    CompiledData::Location location;
 };
 
 struct Module {
diff --git a/src/qml/compiler/qv4compilerscanfunctions.cpp b/src/qml/compiler/qv4compilerscanfunctions.cpp
index 4e18af2227..6a56b7d1a0 100644
--- a/src/qml/compiler/qv4compilerscanfunctions.cpp
+++ b/src/qml/compiler/qv4compilerscanfunctions.cpp
@@ -176,6 +176,7 @@ bool ScanFunctions::visit(ExportDeclaration *declaration)
         Compiler::ExportEntry entry;
         entry.moduleRequest = declaration->fromClause->moduleSpecifier.toString();
         entry.importName = QStringLiteral("*");
+        entry.location = declaration->firstSourceLocation();
         _context->exportEntries << entry;
     } else if (declaration->exportClause) {
         for (ExportsList *it = declaration->exportClause->exportsList; it; it = it->next) {
@@ -188,6 +189,7 @@ bool ScanFunctions::visit(ExportDeclaration *declaration)
 
             entry.moduleRequest = module;
             entry.exportName = spec->exportedIdentifier.toString();
+            entry.location = it->firstSourceLocation();
 
             _context->exportEntries << entry;
         }
@@ -202,6 +204,7 @@ bool ScanFunctions::visit(ExportDeclaration *declaration)
             Compiler::ExportEntry entry;
             entry.localName = name;
             entry.exportName = name;
+            entry.location = vstmt->firstSourceLocation();
             _context->exportEntries << entry;
         }
     } else if (auto *classDecl = AST::cast<AST::ClassDeclaration*>(declaration->variableStatementOrDeclaration)) {
@@ -210,6 +213,7 @@ bool ScanFunctions::visit(ExportDeclaration *declaration)
             Compiler::ExportEntry entry;
             entry.localName = name;
             entry.exportName = name;
+            entry.location = classDecl->firstSourceLocation();
             _context->exportEntries << entry;
             if (declaration->exportDefault)
                 localNameForDefaultExport = entry.localName;
@@ -228,6 +232,7 @@ bool ScanFunctions::visit(ExportDeclaration *declaration)
             Compiler::ExportEntry entry;
             entry.localName = functionName;
             entry.exportName = functionName;
+            entry.location = fdef->firstSourceLocation();
             _context->exportEntries << entry;
             if (declaration->exportDefault)
                 localNameForDefaultExport = entry.localName;
@@ -239,6 +244,7 @@ bool ScanFunctions::visit(ExportDeclaration *declaration)
         entry.localName = localNameForDefaultExport;
         _context->localNameForDefaultExport = localNameForDefaultExport;
         entry.exportName = QStringLiteral("default");
+        entry.location = declaration->firstSourceLocation();
         _context->exportEntries << entry;
     }
 
@@ -263,6 +269,7 @@ bool ScanFunctions::visit(ImportDeclaration *declaration)
             entry.moduleRequest = module;
             entry.importName = QStringLiteral("default");
             entry.localName = import->importedDefaultBinding.toString();
+            entry.location = declaration->firstSourceLocation();
             _context->importEntries << entry;
         }
 
@@ -271,6 +278,7 @@ bool ScanFunctions::visit(ImportDeclaration *declaration)
             entry.moduleRequest = module;
             entry.importName = QStringLiteral("*");
             entry.localName = import->nameSpaceImport->importedBinding.toString();
+            entry.location = declaration->firstSourceLocation();
             _context->importEntries << entry;
         }
 
@@ -283,6 +291,7 @@ bool ScanFunctions::visit(ImportDeclaration *declaration)
                     entry.importName = it->importSpecifier->identifier.toString();
                 else
                     entry.importName = entry.localName;
+                entry.location = declaration->firstSourceLocation();
                 _context->importEntries << entry;
             }
         }
diff --git a/tests/auto/qml/qjsengine/exporterror1.mjs b/tests/auto/qml/qjsengine/exporterror1.mjs
new file mode 100644
index 0000000000..9ce9aa3b83
--- /dev/null
+++ b/tests/auto/qml/qjsengine/exporterror1.mjs
@@ -0,0 +1,2 @@
+let dummy = 0;
+export { nothere } from "./testmodule.mjs";
diff --git a/tests/auto/qml/qjsengine/importerror1.mjs b/tests/auto/qml/qjsengine/importerror1.mjs
new file mode 100644
index 0000000000..be33753c56
--- /dev/null
+++ b/tests/auto/qml/qjsengine/importerror1.mjs
@@ -0,0 +1,2 @@
+let dummy = 0;
+import { nothere } from "./testmodule.mjs";
diff --git a/tests/auto/qml/qjsengine/qjsengine.pro b/tests/auto/qml/qjsengine/qjsengine.pro
index 1fb2d55b31..b412e37727 100644
--- a/tests/auto/qml/qjsengine/qjsengine.pro
+++ b/tests/auto/qml/qjsengine/qjsengine.pro
@@ -4,6 +4,6 @@ QT +=  qml qml-private widgets testlib gui-private
 macx:CONFIG -= app_bundle
 SOURCES += tst_qjsengine.cpp
 RESOURCES += qjsengine.qrc
-RESOURCES += testmodule.mjs modulewithlexicals.mjs
+RESOURCES += testmodule.mjs modulewithlexicals.mjs importerror1.mjs exporterror1.mjs
 
 TESTDATA = script/*
diff --git a/tests/auto/qml/qjsengine/tst_qjsengine.cpp b/tests/auto/qml/qjsengine/tst_qjsengine.cpp
index be62908772..6bc0359483 100644
--- a/tests/auto/qml/qjsengine/tst_qjsengine.cpp
+++ b/tests/auto/qml/qjsengine/tst_qjsengine.cpp
@@ -227,6 +227,7 @@ private slots:
     void importModule();
     void importModuleRelative();
     void importModuleWithLexicallyScopedVars();
+    void importExportErrors();
 
 public:
     Q_INVOKABLE QJSValue throwingCppMethod();
@@ -4404,6 +4405,22 @@ void tst_QJSEngine::importModuleWithLexicallyScopedVars()
     QCOMPARE(ns.property("main").call().toInt(), 10);
 }
 
+void tst_QJSEngine::importExportErrors()
+{
+    {
+        QJSEngine engine;
+        QJSValue result = engine.importModule(QStringLiteral(":/importerror1.mjs"));
+        QVERIFY(result.isError());
+        QCOMPARE(result.property("lineNumber").toInt(), 2);
+    }
+    {
+        QJSEngine engine;
+        QJSValue result = engine.importModule(QStringLiteral(":/exporterror1.mjs"));
+        QVERIFY(result.isError());
+        QCOMPARE(result.property("lineNumber").toInt(), 2);
+    }
+}
+
 QTEST_MAIN(tst_QJSEngine)
 
 #include "tst_qjsengine.moc"