fix scalar property lookup some more

Let there be a product P that depends on module A. Module A depends on
module B.A sets B.foo to 1. Default value of B.foo is 0.  The value of
product.moduleProperty("B", "foo") is 1.

Now add the direct dependency B to P. P does not overwrite the default
value of B.foo. The overwritten value of B.foo from A takes
precedence.

This semantics change is needed to make sure that adding/removing
modules to a product does not change property values in surprising
ways.

Task-number: QBS-736
Change-Id: I73152a24649bc670a9075e82998ef2b0d8709a84
Reviewed-by: Christian Kandeler <christian.kandeler@theqtcompany.com>

4 files changed, 59 insertions, 10 deletions

diff --git a/src/lib/corelib/language/projectresolver.cpp b/src/lib/corelib/language/projectresolver.cpp
index 0bf8b5ed3..3117edd80 100644
--- a/src/lib/corelib/language/projectresolver.cpp
+++ b/src/lib/corelib/language/projectresolver.cpp
@@ -1079,6 +1079,21 @@ QVariantMap ProjectResolver::evaluateModuleValues(Item *item, bool lookupPrototy
     return result;
 }
 
+static QStringList ownPropertiesSet(Item *item)
+{
+    QStringList names;
+    do {
+        names += item->properties().keys();
+        item = item->prototype();
+    } while (item && item->isModuleInstance());
+
+    std::sort(names.begin(), names.end());
+    QStringList::iterator lastIt = std::unique(names.begin(), names.end());
+    if (lastIt != names.end())
+        names.erase(lastIt);
+    return names;
+}
+
 void ProjectResolver::evaluateModuleValues(Item *item, QVariantMap *modulesMap,
         bool lookupPrototype) const
 {
@@ -1097,7 +1112,9 @@ void ProjectResolver::evaluateModuleValues(Item *item, QVariantMap *modulesMap,
         evaluateModuleValues(module.item, &depmods, lookupPrototype);
         QVariantMap dep = evaluateProperties(module.item, lookupPrototype);
         dep.insert(QLatin1String("modules"), depmods);
-        modulesMap->insert(ModuleLoader::fullModuleName(module.name), dep);
+        const QString fullName = ModuleLoader::fullModuleName(module.name);
+        modulesMap->insert(fullName, dep);
+        modulesMap->insert(QLatin1Char('@') + fullName, ownPropertiesSet(module.item));
     }
 }
 
diff --git a/src/lib/corelib/tools/propertyfinder.cpp b/src/lib/corelib/tools/propertyfinder.cpp
index b1f89592e..3941228b0 100644
--- a/src/lib/corelib/tools/propertyfinder.cpp
+++ b/src/lib/corelib/tools/propertyfinder.cpp
@@ -31,6 +31,8 @@
 
 #include "qbsassert.h"
 
+#include <QQueue>
+
 namespace qbs {
 namespace Internal {
 
@@ -40,7 +42,7 @@ QVariantList PropertyFinder::propertyValues(const QVariantMap &properties,
     m_moduleName = moduleName;
     m_key = key;
     m_values.clear();
-    findModuleValues(properties, true);
+    findModuleValues(properties);
     if (mergeType == DoMergeLists)
         mergeLists(&m_values);
     return m_values;
@@ -52,13 +54,13 @@ QVariant PropertyFinder::propertyValue(const QVariantMap &properties, const QStr
     m_moduleName = moduleName;
     m_key = key;
     m_values.clear();
-    findModuleValues(properties, false);
+    findScalarModuleValue(properties);
 
     QBS_ASSERT(m_values.count() <= 1, return QVariant());
     return m_values.isEmpty() ? QVariant() : m_values.first();
 }
 
-void PropertyFinder::findModuleValues(const QVariantMap &properties, bool searchRecursively)
+void PropertyFinder::findModuleValues(const QVariantMap &properties)
 {
     QVariantMap moduleProperties = properties.value(QLatin1String("modules")).toMap();
 
@@ -71,13 +73,43 @@ void PropertyFinder::findModuleValues(const QVariantMap &properties, bool search
         moduleProperties.erase(modIt);
     }
 
-    if (!m_values.isEmpty() && !searchRecursively)
-        return;
-
     // These are the non-matching modules.
     for (QVariantMap::ConstIterator it = moduleProperties.constBegin();
          it != moduleProperties.constEnd(); ++it) {
-        findModuleValues(it->toMap(), searchRecursively);
+        findModuleValues(it->toMap());
+    }
+}
+
+void PropertyFinder::findScalarModuleValue(const QVariantMap &properties)
+{
+    QQueue<QVariantMap> q;
+    q.enqueue(properties.value(QLatin1String("modules")).toMap());
+
+    while (!q.isEmpty()) {
+        QVariantMap moduleProperties = q.takeFirst();
+        const QVariantMap::Iterator modIt = moduleProperties.find(m_moduleName);
+        if (modIt != moduleProperties.end()) {
+            const QVariantMap moduleMap = modIt->toMap();
+            const QVariant property = moduleMap.value(m_key);
+            const QStringList ownPropertiesSet
+                    = moduleProperties.value(QLatin1Char('@') + m_moduleName).toStringList();
+            if (std::binary_search(ownPropertiesSet.constBegin(), ownPropertiesSet.constEnd(),
+                                   m_key)) {
+                // this is the one!
+                m_values.clear();
+                addToList(property);
+                return;
+            }
+            addToList(property);
+            moduleProperties.erase(modIt);
+        }
+
+        // These are the non-matching modules.
+        for (QVariantMap::ConstIterator it = moduleProperties.constBegin();
+             it != moduleProperties.constEnd(); ++it) {
+            if (!it.key().startsWith(QLatin1Char('@')))
+                q.enqueue(it->toMap().value(QLatin1String("modules")).toMap());
+        }
     }
 }
 
diff --git a/src/lib/corelib/tools/propertyfinder.h b/src/lib/corelib/tools/propertyfinder.h
index 9c559b464..66322952a 100644
--- a/src/lib/corelib/tools/propertyfinder.h
+++ b/src/lib/corelib/tools/propertyfinder.h
@@ -48,7 +48,8 @@ public:
                          const QString &key);
 
 private:
-    void findModuleValues(const QVariantMap &properties, bool searchRecursively);
+    void findModuleValues(const QVariantMap &properties);
+    void findScalarModuleValue(const QVariantMap &properties);
     void addToList(const QVariant &value);
     static void mergeLists(QVariantList *values);
 
diff --git a/tests/auto/blackbox/tst_blackbox.cpp b/tests/auto/blackbox/tst_blackbox.cpp
index 753558ffb..d1e3353ab 100644
--- a/tests/auto/blackbox/tst_blackbox.cpp
+++ b/tests/auto/blackbox/tst_blackbox.cpp
@@ -1923,7 +1923,6 @@ void TestBlackbox::nestedProperties()
 {
     QDir::setCurrent(testDataDir + "/nested-properties");
     QCOMPARE(runQbs(), 0);
-    QEXPECT_FAIL(0, "QBS-736", Abort);
     QVERIFY2(m_qbsStdout.contains("value in higherlevel"), m_qbsStdout.constData());
 }