From d90924f4e94a9da53e1ec2348ddb134bca0a86bf Mon Sep 17 00:00:00 2001
From: Christian Kandeler <christian.kandeler@qt.io>
Date: Fri, 19 Oct 2018 13:40:09 +0200
Subject: Do not try to evaluate the properties of non-present modules

It's unnecessary and potentially harmful.

Change-Id: Iff5a9a52a7aeb518617104c5eaa41cfeededbea5
Reviewed-by: Nikolai Kosjar <nikolai.kosjar@qt.io>
---
 src/lib/corelib/language/projectresolver.cpp       |  2 ++
 .../testdata/eval-error-in-non-present-module.qbs  |  7 +++++
 .../language/testdata/modules/broken/broken.qbs    | 19 ++++++++++++
 tests/auto/language/tst_language.cpp               | 34 ++++++++++++++++++++++
 tests/auto/language/tst_language.h                 |  2 ++
 5 files changed, 64 insertions(+)
 create mode 100644 tests/auto/language/testdata/eval-error-in-non-present-module.qbs
 create mode 100644 tests/auto/language/testdata/modules/broken/broken.qbs

diff --git a/src/lib/corelib/language/projectresolver.cpp b/src/lib/corelib/language/projectresolver.cpp
index 97015dae4..59b4efc34 100644
--- a/src/lib/corelib/language/projectresolver.cpp
+++ b/src/lib/corelib/language/projectresolver.cpp
@@ -1620,6 +1620,8 @@ QVariantMap ProjectResolver::evaluateModuleValues(Item *item, bool lookupPrototy
                                        ? &m_elapsedTimeModPropEval : nullptr);
     QVariantMap moduleValues;
     for (const Item::Module &module : item->modules()) {
+        if (!module.item->isPresentModule())
+            continue;
         const QString fullName = module.name.toString();
         moduleValues[fullName] = evaluateProperties(module.item, lookupPrototype, true);
     }
diff --git a/tests/auto/language/testdata/eval-error-in-non-present-module.qbs b/tests/auto/language/testdata/eval-error-in-non-present-module.qbs
new file mode 100644
index 000000000..c112bc6d4
--- /dev/null
+++ b/tests/auto/language/testdata/eval-error-in-non-present-module.qbs
@@ -0,0 +1,7 @@
+import qbs
+
+Product {
+    name: "p"
+    property bool moduleRequired
+    Depends { name: "broken"; required: moduleRequired }
+}
diff --git a/tests/auto/language/testdata/modules/broken/broken.qbs b/tests/auto/language/testdata/modules/broken/broken.qbs
new file mode 100644
index 000000000..b960117cf
--- /dev/null
+++ b/tests/auto/language/testdata/modules/broken/broken.qbs
@@ -0,0 +1,19 @@
+import qbs
+
+Module {
+    Probe {
+        id: theProbe
+
+        property stringList broken
+        property stringList fine
+
+        configure: {
+            broken = [["x"]];
+            fine = ["x"]
+            found = true;
+        }
+    }
+
+    property stringList broken: theProbe.broken
+    property stringList fine: theProbe.fine.filter(function(incl) { return incl != "y"; });
+}
diff --git a/tests/auto/language/tst_language.cpp b/tests/auto/language/tst_language.cpp
index 3a9325c29..ab5aaf759 100644
--- a/tests/auto/language/tst_language.cpp
+++ b/tests/auto/language/tst_language.cpp
@@ -665,6 +665,40 @@ void TestLanguage::enumerateProjectProperties()
     QCOMPARE(exceptionCaught, false);
 }
 
+void TestLanguage::evalErrorInNonPresentModule_data()
+{
+    QTest::addColumn<bool>("moduleRequired");
+    QTest::addColumn<QString>("errorMessage");
+
+    QTest::newRow("module required")
+            << true << "broken.qbs:4:5 Element at index 0 of list property 'broken' "
+                       "does not have string type";
+    QTest::newRow("module not required") << false << QString();
+}
+
+void TestLanguage::evalErrorInNonPresentModule()
+{
+    QFETCH(bool, moduleRequired);
+    QFETCH(QString, errorMessage);
+    try {
+        SetupProjectParameters params = defaultParameters;
+        params.setProjectFilePath(testProject("eval-error-in-non-present-module.qbs"));
+        QVariantMap overridden{std::make_pair("products.p.moduleRequired", moduleRequired)};
+        params.setOverriddenValues(overridden);
+        TopLevelProjectPtr project = loader->loadProject(params);
+        QVERIFY(errorMessage.isEmpty());
+        QVERIFY(!!project);
+        QHash<QString, ResolvedProductPtr> products = productsFromProject(project);
+        QCOMPARE(products.size(), 1);
+        const ResolvedProductPtr product = products.value("p");
+        QVERIFY(!!product);
+    }
+    catch (const ErrorInfo &e) {
+        QVERIFY(!errorMessage.isEmpty());
+        QVERIFY2(e.toString().contains(errorMessage), qPrintable(e.toString()));
+    }
+}
+
 void TestLanguage::defaultValue()
 {
     bool exceptionCaught = false;
diff --git a/tests/auto/language/tst_language.h b/tests/auto/language/tst_language.h
index d724e5fc6..660c1b9b6 100644
--- a/tests/auto/language/tst_language.h
+++ b/tests/auto/language/tst_language.h
@@ -95,6 +95,8 @@ private slots:
     void dottedNames();
     void emptyJsFile();
     void enumerateProjectProperties();
+    void evalErrorInNonPresentModule_data();
+    void evalErrorInNonPresentModule();
     void environmentVariable();
     void errorInDisabledProduct();
     void erroneousFiles_data();
-- 
cgit v1.2.3