Implement missing check for allowed values in PropertyOptions

========== Performance data for Resolving ==========
    Old instruction count: 10195378481
    New instruction count: 10238464294
    Relative change: 0 %

[ChangeLog] Qbs now checks string and stringList values according to the
allowedValues property in PropertyOptions

Change-Id: Ide88987c74b35f4172ffaf71aacd991536131ee5
Reviewed-by: Richard Weickelt <richard@weickelt.de>
Reviewed-by: Christian Kandeler <christian.kandeler@qt.io>

4 files changed, 91 insertions, 1 deletions

diff --git a/tests/auto/blackbox/testdata/allowed-values/allowed-values.qbs b/tests/auto/blackbox/testdata/allowed-values/allowed-values.qbs
new file mode 100644
index 000000000..699713770
--- /dev/null
+++ b/tests/auto/blackbox/testdata/allowed-values/allowed-values.qbs
@@ -0,0 +1,19 @@
+Product {
+    Depends { name: "a" }
+    // tests VariantValue
+    property string prop
+    PropertyOptions {
+        name: "prop"
+        description: "Some prop"
+        allowedValues: "foo"
+    }
+    // tests JSValue
+    property string prop2 // setter for otherProp
+    property string otherProp: prop2
+    PropertyOptions {
+        name: "otherProp"
+        description: "Some other prop"
+        allowedValues: "foo"
+    }
+    name: "p"
+}
diff --git a/tests/auto/blackbox/testdata/allowed-values/modules/a/a.qbs b/tests/auto/blackbox/testdata/allowed-values/modules/a/a.qbs
new file mode 100644
index 000000000..2bbcde525
--- /dev/null
+++ b/tests/auto/blackbox/testdata/allowed-values/modules/a/a.qbs
@@ -0,0 +1,18 @@
+Module {
+    // tests VariantValue
+    property stringList prop
+    PropertyOptions {
+        name: "prop"
+        description: "Some prop"
+        allowedValues: ["foo", "bar"]
+    }
+    // tests JSValue
+    property stringList prop2 // setter for otherProp
+    property stringList otherProp: prop2
+    PropertyOptions {
+        name: "otherProp"
+        description: "Some other prop"
+        allowedValues: ["foo", "bar"]
+    }
+}
+
diff --git a/tests/auto/blackbox/tst_blackbox.cpp b/tests/auto/blackbox/tst_blackbox.cpp
index 721796b19..20116ff56 100644
--- a/tests/auto/blackbox/tst_blackbox.cpp
+++ b/tests/auto/blackbox/tst_blackbox.cpp
@@ -363,6 +363,57 @@ TestBlackbox::TestBlackbox() : TestBlackboxBase (SRCDIR "/testdata", "blackbox")
 {
 }
 
+void TestBlackbox::allowedValues()
+{
+    QFETCH(QString, property);
+    QFETCH(QString, value);
+    QFETCH(QString, invalidValue);
+
+    QDir::setCurrent(testDataDir + "/allowed-values");
+    rmDirR(relativeBuildDir());
+
+    QbsRunParameters params;
+    if (!property.isEmpty() && !value.isEmpty()) {
+        params.arguments << QStringLiteral("%1:%2").arg(property, value);
+    }
+
+    params.expectFailure = !invalidValue.isEmpty();
+    QCOMPARE(runQbs(params) == 0, !params.expectFailure);
+    if (params.expectFailure) {
+        const auto errorString =
+                QStringLiteral("Value '%1' is not allowed for property").arg(invalidValue);
+        QVERIFY2(m_qbsStderr.contains(errorString.toUtf8()), m_qbsStderr.constData());
+    }
+}
+
+void TestBlackbox::allowedValues_data()
+{
+    QTest::addColumn<QString>("property");
+    QTest::addColumn<QString>("value");
+    QTest::addColumn<QString>("invalidValue");
+
+    QTest::newRow("default") << QString() << QString() << QString();
+
+    QTest::newRow("allowed (product, CLI)") << "products.p.prop" << "foo" << QString();
+    QTest::newRow("not allowed (product, CLI)") << "products.p.prop" << "bar" << "bar";
+    QTest::newRow("allowed (product, JS)") << "products.p.prop2" << "foo" << QString();
+    QTest::newRow("not allowed (product, JS)") << "products.p.prop2" << "bar" << "bar";
+
+    QTest::newRow("allowed single (module, CLI)") << "modules.a.prop" << "foo" << QString();
+    QTest::newRow("not allowed single (module, CLI)") << "modules.a.prop" << "baz" << "baz";
+    QTest::newRow("allowed mult (module, CLI)") << "modules.a.prop" << "foo,bar" << QString();
+    QTest::newRow("not allowed mult (module, CLI)") << "modules.a.prop" << "foo,baz" << "baz";
+
+    QTest::newRow("allowed single (module, JS)") << "modules.a.prop2" << "foo" << QString();
+    QTest::newRow("not allowed single (module, JS)") << "modules.a.prop2" << "baz" << "baz";
+    QTest::newRow("allowed mult (module, JS)") << "modules.a.prop2" << "foo,bar" << QString();
+    QTest::newRow("not allowed mult (module, JS)") << "modules.a.prop2" << "foo,baz" << "baz";
+
+    // undefined should always be allowed
+    QTest::newRow("undefined (product)") << "products.p.prop" << "undefined" << QString();
+    QTest::newRow("undefined (module)") << "modules.a.prop" << "undefined" << QString();
+}
+
 void TestBlackbox::addFileTagToGeneratedArtifact()
 {
     QDir::setCurrent(testDataDir + "/add-filetag-to-generated-artifact");
@@ -3943,7 +3994,7 @@ void TestBlackbox::exportsQbs()
 
     // Trying to build with an unsupported build variant must fail.
     paramsExternalBuild.arguments = QStringList{"-f", "consumer.qbs",
-            "modules.qbs.buildVariant:unknown"};
+            "modules.qbs.buildVariant:profiling"};
     paramsExternalBuild.buildDirectory = QDir::currentPath() + "/external-consumer-profile";
     paramsExternalBuild.expectFailure = true;
     QVERIFY(runQbs(paramsExternalBuild) != 0);
diff --git a/tests/auto/blackbox/tst_blackbox.h b/tests/auto/blackbox/tst_blackbox.h
index 4b87e45a8..8cc21378a 100644
--- a/tests/auto/blackbox/tst_blackbox.h
+++ b/tests/auto/blackbox/tst_blackbox.h
@@ -40,6 +40,8 @@ public:
     TestBlackbox();
 
 private slots:
+    void allowedValues();
+    void allowedValues_data();
     void addFileTagToGeneratedArtifact();
     void alwaysRun();
     void alwaysRun_data();