Loader: Fix dependency parameter extraction

The implementation was strangely convoluted and unnecessarily limiting
regarding what users can do with Depends items.

Fixes: QBS-1762
Change-Id: Id633a3b790fde5fb478ef1145e97535682f39703
Reviewed-by: Ivan Komissarov <ABBAPOH@gmail.com>

2 files changed, 13 insertions, 27 deletions

diff --git a/src/lib/corelib/loader/dependenciesresolver.cpp b/src/lib/corelib/loader/dependenciesresolver.cpp
index 38cba0aa3..8c7a69e08 100644
--- a/src/lib/corelib/loader/dependenciesresolver.cpp
+++ b/src/lib/corelib/loader/dependenciesresolver.cpp
@@ -195,7 +195,6 @@ private:
 };
 
 static bool haveSameSubProject(const ProductContext &p1, const ProductContext &p2);
-static Item::PropertyMap filterItemProperties(const Item::PropertyMap &properties);
 static QVariantMap safeToVariant(JSContext *ctx, const JSValue &v);
 
 } // namespace
@@ -930,26 +929,23 @@ DependenciesResolver::multiplexDependency(const EvaluatedDependsItem &dependency
 
 QVariantMap DependenciesResolver::extractParameters(Item *dependsItem) const
 {
-    QVariantMap result;
-    const Item::PropertyMap &itemProperties = filterItemProperties(dependsItem->properties());
-    if (itemProperties.empty())
-        return result;
-    auto origProperties = dependsItem->properties();
-
-    // TODO: This is not exception-safe. Also, can't we do the item value check along the
-    //       way, without allocationg an extra map and exchanging the list of children?
-    dependsItem->setProperties(itemProperties);
-
-    JSValue sv = m_loaderState.evaluator().scriptValue(dependsItem);
     try {
-        result = safeToVariant(m_loaderState.evaluator().engine()->context(), sv);
+        QVariantMap result;
+        const auto &properties = dependsItem->properties();
+        Evaluator &evaluator = m_loaderState.evaluator();
+        for (auto it = properties.begin(); it != properties.end(); ++it) {
+            if (it.value()->type() != Value::ItemValueType)
+                continue;
+            const JSValue sv = evaluator.scriptValue(
+                        std::static_pointer_cast<ItemValue>(it.value())->item());
+            result.insert(it.key(), safeToVariant(evaluator.engine()->context(), sv));
+        }
+        return result;
     } catch (const ErrorInfo &exception) {
         auto ei = exception;
         ei.prepend(Tr::tr("Error in dependency parameter."), dependsItem->location());
         throw ei;
     }
-    dependsItem->setProperties(origProperties);
-    return result;
 }
 
 void DependenciesResolver::forwardParameterDeclarations(const Item *dependsItem,
@@ -1086,16 +1082,6 @@ bool haveSameSubProject(const ProductContext &p1, const ProductContext &p2)
     return false;
 }
 
-Item::PropertyMap filterItemProperties(const Item::PropertyMap &properties)
-{
-    Item::PropertyMap result;
-    for (auto it = properties.begin(); it != properties.end(); ++it) {
-        if (it.value()->type() == Value::ItemValueType)
-            result.insert(it.key(), it.value());
-    }
-    return result;
-}
-
 QVariantMap safeToVariant(JSContext *ctx, const JSValue &v)
 {
     QVariantMap result;
diff --git a/tests/auto/blackbox/testdata/plugin-dependency/plugin-dependency.qbs b/tests/auto/blackbox/testdata/plugin-dependency/plugin-dependency.qbs
index 30b7f4ace..752673b78 100644
--- a/tests/auto/blackbox/testdata/plugin-dependency/plugin-dependency.qbs
+++ b/tests/auto/blackbox/testdata/plugin-dependency/plugin-dependency.qbs
@@ -11,8 +11,8 @@ Project {
         Depends { name: "plugin2" }                     // not to be linked
         Depends {
             name: "plugin3"                             // supposed to be linked
-            //property bool theCondition: true
-            cpp.link: /*theCondition && */product.name === "myapp"  // TODO: Make this work
+            property bool theCondition: true
+            cpp.link: theCondition && product.name === "myapp"
         }
         Depends { name: "plugin4" }                     // supposed to be linked
         Depends { name: "helper1" }                     // supposed to be linked