Fix importing a collection of JavaScript files.

The old implementation only made the contents of a single file in the
directory available (namely the last one it happened to read).

Change-Id: Ib88cb738bef161bd1c2c5ad842cf7ff33ca171b6
Reviewed-by: Joerg Bornemann <joerg.bornemann@theqtcompany.com>

8 files changed, 65 insertions, 26 deletions

diff --git a/src/lib/corelib/language/scriptengine.cpp b/src/lib/corelib/language/scriptengine.cpp
index b555ffe28..811b935fa 100644
--- a/src/lib/corelib/language/scriptengine.cpp
+++ b/src/lib/corelib/language/scriptengine.cpp
@@ -124,23 +124,20 @@ void ScriptEngine::import(const JsImport &jsImport, QScriptValue scope, QScriptV
     QBS_ASSERT(targetObject.engine() == this, return);
 
     if (debugJSImports)
-        m_logger.qbsDebug() << "[ENGINE] import into " << jsImport.scopeName;
-
-    foreach (const QString &filePath, jsImport.filePaths) {
-        QScriptValue jsImportValue;
-        jsImportValue = m_jsImportCache.value(filePath);
-        if (jsImportValue.isValid()) {
-            if (debugJSImports)
-                m_logger.qbsDebug() << "[ENGINE] " << filePath << " (cache hit)";
-            targetObject.setProperty(jsImport.scopeName, jsImportValue);
-        } else {
-            if (debugJSImports)
-                m_logger.qbsDebug() << "[ENGINE] " << filePath << " (cache miss)";
-            jsImportValue = importFile(filePath, scope);
-            targetObject.setProperty(jsImport.scopeName, jsImportValue);
-            m_jsImportCache.insert(filePath, jsImportValue);
-        }
+        qDebug() << "[ENGINE] import into " << jsImport.scopeName;
+
+    QScriptValue jsImportValue = m_jsImportCache.value(jsImport);
+    if (jsImportValue.isValid()) {
+        if (debugJSImports)
+            qDebug() << "[ENGINE] " << jsImport.filePaths << " (cache hit)";
+    } else {
+        if (debugJSImports)
+            qDebug() << "[ENGINE] " << jsImport.filePaths << " (cache miss)";
+        foreach (const QString &filePath, jsImport.filePaths)
+            importFile(filePath, scope, &jsImportValue);
+        m_jsImportCache.insert(jsImport, jsImportValue);
     }
+    targetObject.setProperty(jsImport.scopeName, jsImportValue);
 }
 
 void ScriptEngine::clearImportsCache()
@@ -237,7 +234,8 @@ void ScriptEngine::setEnvironment(const QProcessEnvironment &env)
     m_environment = env;
 }
 
-QScriptValue ScriptEngine::importFile(const QString &filePath, const QScriptValue &scope)
+QScriptValue ScriptEngine::importFile(const QString &filePath, const QScriptValue &scope,
+                                      QScriptValue *targetObject)
 {
     QFile file(filePath);
     if (Q_UNLIKELY(!file.open(QFile::ReadOnly)))
@@ -245,11 +243,13 @@ QScriptValue ScriptEngine::importFile(const QString &filePath, const QScriptValu
     const QString sourceCode = QTextStream(&file).readAll();
     file.close();
     QScriptProgram program(sourceCode, filePath);
-    QScriptValue obj = newObject();
+    QScriptValue obj;
+    if (!targetObject)
+        obj = newObject();
     m_currentDirPathStack.push(FileInfo::path(filePath));
-    importProgram(program, scope, obj);
+    importProgram(program, scope, targetObject ? *targetObject : obj);
     m_currentDirPathStack.pop();
-    return obj;
+    return targetObject ? *targetObject : obj;
 }
 
 void ScriptEngine::importProgram(const QScriptProgram &program, const QScriptValue &scope,
@@ -289,7 +289,7 @@ void ScriptEngine::importProgram(const QScriptProgram &program, const QScriptVal
         while (it.hasNext()) {
             it.next();
             if (debugJSImports)
-                m_logger.qbsDebug() << "[ENGINE] Copying property " << it.name();
+                qDebug() << "[ENGINE] Copying property " << it.name();
             targetObject.setProperty(it.name(), it.value());
         }
     }
@@ -304,8 +304,8 @@ void ScriptEngine::importProgram(const QScriptProgram &program, const QScriptVal
             continue;
 
         if (debugJSImports) {
-            m_logger.qbsDebug() << "[ENGINE] inserting global property "
-                                << it.name() << " " << it.value().toString();
+            qDebug() << "[ENGINE] inserting global property "
+                     << it.name() << " " << it.value().toString();
         }
 
         targetObject.setProperty(it.name(), it.value());
@@ -453,7 +453,12 @@ void ScriptEngine::addFileLastModifiedResult(const QString &filePath, const File
 
 QSet<QString> ScriptEngine::imports() const
 {
-    return QSet<QString>::fromList(m_jsImportCache.keys());
+    QSet<QString> filePaths;
+    foreach (const JsImport &jsImport, m_jsImportCache.keys()) {
+        foreach (const QString &filePath, jsImport.filePaths)
+            filePaths << filePath;
+    }
+    return filePaths;
 }
 
 QScriptValueList ScriptEngine::argumentList(const QStringList &argumentNames,
diff --git a/src/lib/corelib/language/scriptengine.h b/src/lib/corelib/language/scriptengine.h
index d0b0ec9d9..765682add 100644
--- a/src/lib/corelib/language/scriptengine.h
+++ b/src/lib/corelib/language/scriptengine.h
@@ -124,7 +124,8 @@ private:
     void installQbsFunction(const QString &name, FunctionSignature f);
     void installImportFunctions();
     void uninstallImportFunctions();
-    QScriptValue importFile(const QString &filePath, const QScriptValue &scope);
+    QScriptValue importFile(const QString &filePath, const QScriptValue &scope,
+                            QScriptValue *targetObject = nullptr);
     void importProgram(const QScriptProgram &program, const QScriptValue &scope,
                        QScriptValue &targetObject);
     static QScriptValue js_loadExtension(QScriptContext *context, QScriptEngine *qtengine);
@@ -148,7 +149,7 @@ private:
     friend bool operator==(const PropertyCacheKey &lhs, const PropertyCacheKey &rhs);
     friend uint qHash(const ScriptEngine::PropertyCacheKey &k, uint seed);
 
-    QHash<QString, QScriptValue> m_jsImportCache;
+    QHash<JsImport, QScriptValue> m_jsImportCache;
     bool m_propertyCacheEnabled;
     QHash<PropertyCacheKey, QVariant> m_propertyCache;
     PropertySet m_propertiesRequestedInScript;
diff --git a/src/lib/corelib/language/testdata/import-collection/imports/Collection/file1.js b/src/lib/corelib/language/testdata/import-collection/imports/Collection/file1.js
new file mode 100644
index 000000000..9f4785b2e
--- /dev/null
+++ b/src/lib/corelib/language/testdata/import-collection/imports/Collection/file1.js
@@ -0,0 +1 @@
+function f1() { return "f1"; }
diff --git a/src/lib/corelib/language/testdata/import-collection/imports/Collection/file2.js b/src/lib/corelib/language/testdata/import-collection/imports/Collection/file2.js
new file mode 100644
index 000000000..887fd6fdb
--- /dev/null
+++ b/src/lib/corelib/language/testdata/import-collection/imports/Collection/file2.js
@@ -0,0 +1 @@
+function f2() { return "f2"; }
diff --git a/src/lib/corelib/language/testdata/import-collection/product.qbs b/src/lib/corelib/language/testdata/import-collection/product.qbs
new file mode 100644
index 000000000..af240ddca
--- /dev/null
+++ b/src/lib/corelib/language/testdata/import-collection/product.qbs
@@ -0,0 +1,6 @@
+import Collection
+
+Product {
+    name: "da product"
+    targetName: Collection.f1() + Collection.f2()
+}
diff --git a/src/lib/corelib/language/testdata/import-collection/project.qbs b/src/lib/corelib/language/testdata/import-collection/project.qbs
new file mode 100644
index 000000000..ee34869b4
--- /dev/null
+++ b/src/lib/corelib/language/testdata/import-collection/project.qbs
@@ -0,0 +1,6 @@
+import qbs
+
+Project {
+    references: ["product.qbs"]
+}
+
diff --git a/src/lib/corelib/language/tst_language.cpp b/src/lib/corelib/language/tst_language.cpp
index 1044db3bd..d07af317a 100644
--- a/src/lib/corelib/language/tst_language.cpp
+++ b/src/lib/corelib/language/tst_language.cpp
@@ -805,6 +805,24 @@ void TestLanguage::idUsage()
     QVERIFY(!exceptionCaught);
 }
 
+void TestLanguage::importCollection()
+{
+    bool exceptionCaught = false;
+    try {
+        defaultParameters.setProjectFilePath(testProject("import-collection/project.qbs"));
+        const TopLevelProjectPtr project = loader->loadProject(defaultParameters);
+        QVERIFY(project);
+        QHash<QString, ResolvedProductPtr> products = productsFromProject(project);
+        const ResolvedProductConstPtr product = products.value("da product");
+        QCOMPARE(product->productProperties.value("targetName").toString(), QLatin1String("f1f2"));
+    }
+    catch (const ErrorInfo &e) {
+        exceptionCaught = true;
+        qDebug() << e.toString();
+    }
+    QVERIFY(!exceptionCaught);
+}
+
 void TestLanguage::invalidBindingInDisabledItem()
 {
     bool exceptionCaught = false;
diff --git a/src/lib/corelib/language/tst_language.h b/src/lib/corelib/language/tst_language.h
index 4707e8722..624f5428b 100644
--- a/src/lib/corelib/language/tst_language.h
+++ b/src/lib/corelib/language/tst_language.h
@@ -87,6 +87,7 @@ private slots:
     void identifierSearch_data();
     void identifierSearch();
     void idUsage();
+    void importCollection();
     void invalidBindingInDisabledItem();
     void itemPrototype();
     void itemScope();