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>

7 files changed, 79 insertions, 10 deletions

diff --git a/src/lib/corelib/language/moduleloader.cpp b/src/lib/corelib/language/moduleloader.cpp
index 4eefe9d47..5c4033c3f 100644
--- a/src/lib/corelib/language/moduleloader.cpp
+++ b/src/lib/corelib/language/moduleloader.cpp
@@ -90,14 +90,6 @@ namespace Internal {
 using MultiplexConfigurationByIdTable = QThreadStorage<QHash<QString, QVariantMap> >;
 Q_GLOBAL_STATIC(MultiplexConfigurationByIdTable, multiplexConfigurationsById);
 
-static void handlePropertyError(const ErrorInfo &error, const SetupProjectParameters &params,
-                                Logger &logger)
-{
-    if (params.propertyCheckingMode() == ErrorHandlingMode::Strict)
-        throw error;
-    logger.printWarning(error);
-}
-
 static bool multiplexConfigurationIntersects(const QVariantMap &lhs, const QVariantMap &rhs)
 {
     QBS_CHECK(!lhs.isEmpty() && !rhs.isEmpty());
@@ -1767,6 +1759,9 @@ void ModuleLoader::handlePropertyOptions(Item *optionsItem)
                 optionsItem, StringConstants::descriptionProperty());
     const auto removalVersion = Version::fromString(m_evaluator->stringValue(optionsItem,
             StringConstants::removalVersionProperty()));
+    const auto allowedValues = m_evaluator->stringListValue(
+                optionsItem, StringConstants::allowedValuesProperty());
+
     PropertyDeclaration decl = optionsItem->parent()->propertyDeclaration(name);
     if (!decl.isValid()) {
         decl.setName(name);
@@ -1777,6 +1772,7 @@ void ModuleLoader::handlePropertyOptions(Item *optionsItem)
         DeprecationInfo di(removalVersion, description);
         decl.setDeprecationInfo(di);
     }
+    decl.setAllowedValues(allowedValues);
     const ValuePtr property = optionsItem->parent()->property(name);
     if (!property && !decl.isExpired()) {
         throw ErrorInfo(Tr::tr("PropertyOptions item refers to non-existing property '%1'")
diff --git a/src/lib/corelib/language/projectresolver.cpp b/src/lib/corelib/language/projectresolver.cpp
index 4e8e94b4e..570b3513f 100644
--- a/src/lib/corelib/language/projectresolver.cpp
+++ b/src/lib/corelib/language/projectresolver.cpp
@@ -1778,6 +1778,7 @@ void ProjectResolver::evaluateProperty(const Item *item, const QString &propName
         } else if (pd.type() == PropertyDeclaration::VariantList) {
             v = v.toList();
         }
+        checkAllowedValues(v, propValue->location(), pd, propName);
         result[propName] = v;
         break;
     }
@@ -1788,15 +1789,52 @@ void ProjectResolver::evaluateProperty(const Item *item, const QString &propName
         VariantValuePtr vvp = std::static_pointer_cast<VariantValue>(propValue);
         QVariant v = vvp->value();
 
-        if (v.isNull() && !item->propertyDeclaration(propName).isScalar()) // QTBUG-51237
+        const PropertyDeclaration pd = item->propertyDeclaration(propName);
+        if (v.isNull() && !pd.isScalar()) // QTBUG-51237
             v = QStringList();
 
+        checkAllowedValues(v, propValue->location(), pd, propName);
         result[propName] = v;
         break;
     }
     }
 }
 
+void ProjectResolver::checkAllowedValues(
+        const QVariant &value, const CodeLocation &loc, const PropertyDeclaration &decl,
+        const QString &key) const
+{
+    const auto type = decl.type();
+    if (type != PropertyDeclaration::String && type != PropertyDeclaration::StringList)
+        return;
+
+    if (value.isNull())
+        return;
+
+    const auto &allowedValues = decl.allowedValues();
+    if (allowedValues.isEmpty())
+        return;
+
+    const auto checkValue = [this, &loc, &allowedValues, &key](const QString &value)
+    {
+        if (!allowedValues.contains(value)) {
+            const auto message = Tr::tr("Value '%1' is not allowed for property '%2'.")
+                    .arg(value, key);
+            ErrorInfo error(message, loc);
+            handlePropertyError(error, m_setupParams, m_logger);
+        }
+    };
+
+    if (type == PropertyDeclaration::StringList) {
+        const auto strings = value.toStringList();
+        for (const auto &string: strings) {
+            checkValue(string);
+        }
+    } else if (type == PropertyDeclaration::String) {
+        checkValue(value.toString());
+    }
+}
+
 void ProjectResolver::collectPropertiesForExportItem(const QualifiedId &moduleName,
          const ValuePtr &value, Item *moduleInstance, QVariantMap &moduleProps)
 {
diff --git a/src/lib/corelib/language/projectresolver.h b/src/lib/corelib/language/projectresolver.h
index a1e24a555..73f3442ac 100644
--- a/src/lib/corelib/language/projectresolver.h
+++ b/src/lib/corelib/language/projectresolver.h
@@ -132,6 +132,9 @@ private:
                                    bool checkErrors);
     void evaluateProperty(const Item *item, const QString &propName, const ValuePtr &propValue,
                           QVariantMap &result, bool checkErrors);
+    void checkAllowedValues(
+            const QVariant &v, const CodeLocation &loc, const PropertyDeclaration &decl,
+            const QString &key) const;
     void createProductConfig(ResolvedProduct *product);
     ProjectContext createProjectContext(ProjectContext *parentProjectContext) const;
     void adaptExportedPropertyValues(const Item *shadowProductItem);
diff --git a/src/lib/corelib/language/propertydeclaration.cpp b/src/lib/corelib/language/propertydeclaration.cpp
index abe6a1626..14ed52d1e 100644
--- a/src/lib/corelib/language/propertydeclaration.cpp
+++ b/src/lib/corelib/language/propertydeclaration.cpp
@@ -62,6 +62,7 @@ public:
     QString name;
     PropertyDeclaration::Type type;
     PropertyDeclaration::Flags flags;
+    QStringList allowedValues;
     QString description;
     QString initialValueSource;
     QStringList functionArgumentNames;
@@ -182,6 +183,16 @@ void PropertyDeclaration::setFlags(Flags f)
     d->flags = f;
 }
 
+const QStringList &PropertyDeclaration::allowedValues() const
+{
+    return d->allowedValues;
+}
+
+void PropertyDeclaration::setAllowedValues(const QStringList &v)
+{
+    d->allowedValues = v;
+}
+
 const QString &PropertyDeclaration::description() const
 {
     return d->description;
diff --git a/src/lib/corelib/language/propertydeclaration.h b/src/lib/corelib/language/propertydeclaration.h
index 874275bde..5ced7eccb 100644
--- a/src/lib/corelib/language/propertydeclaration.h
+++ b/src/lib/corelib/language/propertydeclaration.h
@@ -100,6 +100,9 @@ public:
     Flags flags() const;
     void setFlags(Flags f);
 
+    const QStringList &allowedValues() const;
+    void setAllowedValues(const QStringList &v);
+
     const QString &description() const;
     void setDescription(const QString &str);
 
diff --git a/src/lib/corelib/tools/error.cpp b/src/lib/corelib/tools/error.cpp
index ff7ce46cf..fe5027313 100644
--- a/src/lib/corelib/tools/error.cpp
+++ b/src/lib/corelib/tools/error.cpp
@@ -42,6 +42,8 @@
 #include "persistence.h"
 #include "qttools.h"
 #include "stringconstants.h"
+#include "setupprojectparameters.h"
+#include "logging/logger.h"
 
 #include <QtCore/qjsonarray.h>
 #include <QtCore/qjsonobject.h>
@@ -325,4 +327,12 @@ void appendError(ErrorInfo &dst, const ErrorInfo &src)
         dst.append(item);
 }
 
+void handlePropertyError(
+        const ErrorInfo &error, const SetupProjectParameters &params, Internal::Logger &logger)
+{
+    if (params.propertyCheckingMode() == ErrorHandlingMode::Strict)
+        throw error;
+    logger.printWarning(error);
+}
+
 } // namespace qbs
diff --git a/src/lib/corelib/tools/error.h b/src/lib/corelib/tools/error.h
index 36cf5e0ea..3ae9399f1 100644
--- a/src/lib/corelib/tools/error.h
+++ b/src/lib/corelib/tools/error.h
@@ -54,9 +54,14 @@ class QStringList;
 QT_END_NAMESPACE
 
 namespace qbs {
-namespace Internal { class PersistentPool; }
+namespace Internal {
+class PersistentPool;
+class Logger;
+} // namespace Internal
 class CodeLocation;
 
+class SetupProjectParameters;
+
 class QBS_EXPORT ErrorItem
 {
     friend class ErrorInfo;
@@ -120,6 +125,9 @@ private:
 };
 
 void appendError(ErrorInfo &dst, const ErrorInfo &src);
+void handlePropertyError(
+        const ErrorInfo &error, const SetupProjectParameters &params, Internal::Logger &logger);
+
 inline uint qHash(const ErrorInfo &e) { return qHash(e.toString()); }
 inline bool operator==(const ErrorInfo &e1, const ErrorInfo &e2) {
     return e1.toString() == e2.toString();