From 5acd507e853fe67f1065dc69933d81e2564fb002 Mon Sep 17 00:00:00 2001 From: Ivan Komissarov Date: Wed, 6 Oct 2021 11:14:27 +0300 Subject: Fix setting stringlist properties in module providers ...when using foo,bar,baz syntax Change-Id: I013a55f02c5d6d4bbbccf809b9524bed3c486df4 Reviewed-by: Christian Kandeler --- src/lib/corelib/language/moduleloader.cpp | 58 ++-------------------- src/lib/corelib/language/moduleproviderloader.cpp | 7 ++- src/lib/corelib/language/propertydeclaration.cpp | 56 +++++++++++++++++++++ src/lib/corelib/language/propertydeclaration.h | 7 +++ .../module-providers/provider_a.qbs | 4 +- .../module-providers/provider_b.qbs | 4 +- .../providers-properties/providers-properties.qbs | 5 +- .../testdata/qbs-module-providers-helpers.js | 9 +++- tests/auto/blackbox/tst_blackbox.cpp | 7 +-- 9 files changed, 92 insertions(+), 65 deletions(-) diff --git a/src/lib/corelib/language/moduleloader.cpp b/src/lib/corelib/language/moduleloader.cpp index 72e248433..3ffd0736e 100644 --- a/src/lib/corelib/language/moduleloader.cpp +++ b/src/lib/corelib/language/moduleloader.cpp @@ -3194,55 +3194,6 @@ QStringList &ModuleLoader::getModuleFileNames(const QString &dirPath) return moduleFileNames; } -// returns QMetaType::UnknownType for types that do not need conversion -static QMetaType::Type variantType(PropertyDeclaration::Type t) -{ - switch (t) { - case PropertyDeclaration::UnknownType: - break; - case PropertyDeclaration::Boolean: - return QMetaType::Bool; - case PropertyDeclaration::Integer: - return QMetaType::Int; - case PropertyDeclaration::Path: - return QMetaType::QString; - case PropertyDeclaration::PathList: - return QMetaType::QStringList; - case PropertyDeclaration::String: - return QMetaType::QString; - case PropertyDeclaration::StringList: - return QMetaType::QStringList; - case PropertyDeclaration::VariantList: - return QMetaType::QVariantList; - case PropertyDeclaration::Variant: - break; - } - return QMetaType::UnknownType; -} - -static QVariant convertToPropertyType(const QVariant &v, PropertyDeclaration::Type t, - const QStringList &namePrefix, const QString &key) -{ - if (v.isNull() || !v.isValid()) - return v; - const auto vt = variantType(t); - if (vt == QMetaType::UnknownType) - return v; - - // Handle the foo,bar,bla stringlist syntax. - if (t == PropertyDeclaration::StringList && v.userType() == QMetaType::QString) - return v.toString().split(QLatin1Char(',')); - - QVariant c = v; - if (!qVariantConvert(c, vt)) { - QStringList name = namePrefix; - name << key; - throw ErrorInfo(Tr::tr("Value '%1' of property '%2' has incompatible type.") - .arg(v.toString(), name.join(QLatin1Char('.')))); - } - return c; -} - static Item *findDeepestModuleInstance(Item *instance) { while (instance->prototype() && instance->prototype()->type() == ItemType::ModuleInstance) @@ -3324,8 +3275,9 @@ std::pair ModuleLoader::getModulePrototype(ProductContext *product continue; } const PropertyDeclaration decl = module->propertyDeclaration(it.key()); - VariantValuePtr v = VariantValue::create(convertToPropertyType(it.value(), decl.type(), - QStringList(fullModuleName), it.key())); + VariantValuePtr v = VariantValue::create( + PropertyDeclaration::convertToPropertyType(it.value(), decl.type(), + QStringList(fullModuleName), it.key())); module->setProperty(it.key(), v); } @@ -3813,8 +3765,8 @@ void ModuleLoader::overrideItemProperties(Item *item, const QString &buildConfig continue; } item->setProperty(it.key(), - VariantValue::create(convertToPropertyType(it.value(), decl.type(), - QStringList(buildConfigKey), it.key()))); + VariantValue::create(PropertyDeclaration::convertToPropertyType( + it.value(), decl.type(), QStringList(buildConfigKey), it.key()))); } } diff --git a/src/lib/corelib/language/moduleproviderloader.cpp b/src/lib/corelib/language/moduleproviderloader.cpp index f036e36ab..d8acad651 100644 --- a/src/lib/corelib/language/moduleproviderloader.cpp +++ b/src/lib/corelib/language/moduleproviderloader.cpp @@ -302,13 +302,18 @@ QStringList ModuleProviderLoader::getProviderSearchPaths( BuiltinDeclarations::instance().nameForType(ItemType::ModuleProvider))); } providerItem->setParent(product.item); + + // TODO: this looks like ModuleLoader::overrideItemProperties(), merge them? for (auto it = moduleConfig.begin(); it != moduleConfig.end(); ++it) { const PropertyDeclaration decl = providerItem->propertyDeclaration(it.key()); if (!decl.isValid()) { throw ErrorInfo(Tr::tr("No such property '%1' in module provider '%2'.") .arg(it.key(), name.toString())); } - providerItem->setProperty(it.key(), VariantValue::create(it.value())); + VariantValuePtr v = VariantValue::create( + PropertyDeclaration::convertToPropertyType(it.value(), decl.type(), + QStringList(name), it.key())); + providerItem->setProperty(it.key(), v); } EvalContextSwitcher contextSwitcher(m_evaluator->engine(), EvalContext::ModuleProvider); return m_evaluator->stringListValue(providerItem, QStringLiteral("searchPaths")); diff --git a/src/lib/corelib/language/propertydeclaration.cpp b/src/lib/corelib/language/propertydeclaration.cpp index 14ed52d1e..2dbe41afd 100644 --- a/src/lib/corelib/language/propertydeclaration.cpp +++ b/src/lib/corelib/language/propertydeclaration.cpp @@ -42,14 +42,46 @@ #include "deprecationinfo.h" #include +#include + +#include +#include #include +#include #include #include +#include namespace qbs { namespace Internal { +// returns QMetaType::UnknownType for types that do not need conversion +static QMetaType::Type variantType(PropertyDeclaration::Type t) +{ + switch (t) { + case PropertyDeclaration::UnknownType: + break; + case PropertyDeclaration::Boolean: + return QMetaType::Bool; + case PropertyDeclaration::Integer: + return QMetaType::Int; + case PropertyDeclaration::Path: + return QMetaType::QString; + case PropertyDeclaration::PathList: + return QMetaType::QStringList; + case PropertyDeclaration::String: + return QMetaType::QString; + case PropertyDeclaration::StringList: + return QMetaType::QStringList; + case PropertyDeclaration::VariantList: + return QMetaType::QVariantList; + case PropertyDeclaration::Variant: + break; + } + return QMetaType::UnknownType; +} + class PropertyDeclarationData : public QSharedData { public: @@ -243,5 +275,29 @@ void PropertyDeclaration::setDeprecationInfo(const DeprecationInfo &deprecationI d->deprecationInfo = deprecationInfo; } +// see also: EvaluatorScriptClass::convertToPropertyType() +QVariant PropertyDeclaration::convertToPropertyType(const QVariant &v, Type t, + const QStringList &namePrefix, const QString &key) +{ + if (v.isNull() || !v.isValid()) + return v; + const auto vt = variantType(t); + if (vt == QMetaType::UnknownType) + return v; + + // Handle the foo,bar,bla stringlist syntax. + if (t == PropertyDeclaration::StringList && v.userType() == QMetaType::QString) + return v.toString().split(QLatin1Char(',')); + + QVariant c = v; + if (!qVariantConvert(c, vt)) { + QStringList name = namePrefix; + name << key; + throw ErrorInfo(Tr::tr("Value '%1' of property '%2' has incompatible type.") + .arg(v.toString(), name.join(QLatin1Char('.')))); + } + return c; +} + } // namespace Internal } // namespace qbs diff --git a/src/lib/corelib/language/propertydeclaration.h b/src/lib/corelib/language/propertydeclaration.h index 77b6837f5..137315d14 100644 --- a/src/lib/corelib/language/propertydeclaration.h +++ b/src/lib/corelib/language/propertydeclaration.h @@ -43,6 +43,10 @@ #include #include +QT_BEGIN_NAMESPACE +class QVariant; +QT_END_NAMESPACE + namespace qbs { namespace Internal { class DeprecationInfo; @@ -113,6 +117,9 @@ public: const DeprecationInfo &deprecationInfo() const; void setDeprecationInfo(const DeprecationInfo &deprecationInfo); + static QVariant convertToPropertyType( + const QVariant &v, Type t, const QStringList &namePrefix, const QString &key); + private: QSharedDataPointer d; }; diff --git a/tests/auto/blackbox/testdata/providers-properties/module-providers/provider_a.qbs b/tests/auto/blackbox/testdata/providers-properties/module-providers/provider_a.qbs index 7985199c1..ab9d475d8 100644 --- a/tests/auto/blackbox/testdata/providers-properties/module-providers/provider_a.qbs +++ b/tests/auto/blackbox/testdata/providers-properties/module-providers/provider_a.qbs @@ -1,9 +1,9 @@ import "../../qbs-module-providers-helpers.js" as Helpers ModuleProvider { - property string someProp: "provider_a" + property stringList someProp: "provider_a" relativeSearchPaths: { - Helpers.writeModule(outputBaseDir, "qbsmetatestmodule", someProp); + Helpers.writeModule(outputBaseDir, "qbsmetatestmodule", undefined, someProp); return ""; } } diff --git a/tests/auto/blackbox/testdata/providers-properties/module-providers/provider_b.qbs b/tests/auto/blackbox/testdata/providers-properties/module-providers/provider_b.qbs index 58042d8d3..1b2a79979 100644 --- a/tests/auto/blackbox/testdata/providers-properties/module-providers/provider_b.qbs +++ b/tests/auto/blackbox/testdata/providers-properties/module-providers/provider_b.qbs @@ -1,9 +1,9 @@ import "../../qbs-module-providers-helpers.js" as Helpers ModuleProvider { - property string someProp: "provider_b" + property stringList someProp: "provider_b" relativeSearchPaths: { - Helpers.writeModule(outputBaseDir, "qbsothermodule", someProp); + Helpers.writeModule(outputBaseDir, "qbsothermodule", undefined, someProp); return ""; } } diff --git a/tests/auto/blackbox/testdata/providers-properties/providers-properties.qbs b/tests/auto/blackbox/testdata/providers-properties/providers-properties.qbs index e29536c17..258a973fa 100644 --- a/tests/auto/blackbox/testdata/providers-properties/providers-properties.qbs +++ b/tests/auto/blackbox/testdata/providers-properties/providers-properties.qbs @@ -5,7 +5,8 @@ Product { Depends { name: "qbsothermodule" } moduleProviders.provider_a.someProp: "someValue" property bool dummy: { - console.info("p.qbsmetatestmodule.prop: " + qbsmetatestmodule.prop); - console.info("p.qbsothermodule.prop: " + qbsothermodule.prop); + console.info("p.qbsmetatestmodule.listProp: " + + JSON.stringify(qbsmetatestmodule.listProp)); + console.info("p.qbsothermodule.listProp: " + JSON.stringify(qbsothermodule.listProp)); } } diff --git a/tests/auto/blackbox/testdata/qbs-module-providers-helpers.js b/tests/auto/blackbox/testdata/qbs-module-providers-helpers.js index 2a2acfdcb..b33b87329 100644 --- a/tests/auto/blackbox/testdata/qbs-module-providers-helpers.js +++ b/tests/auto/blackbox/testdata/qbs-module-providers-helpers.js @@ -1,14 +1,19 @@ var File = require("qbs.File"); var FileInfo = require("qbs.FileInfo"); var TextFile = require("qbs.TextFile"); +var ModUtils = require("qbs.ModUtils"); -function writeModule(outputBaseDir, name, prop) { +function writeModule(outputBaseDir, name, prop, listProp) { console.info("Running setup script for " + name); var moduleDir = FileInfo.joinPaths(outputBaseDir, "modules", name); File.makePath(moduleDir); var module = new TextFile(FileInfo.joinPaths(moduleDir, "module.qbs"), TextFile.WriteOnly); module.writeLine("Module {"); - module.writeLine(" property string prop: '" + prop + "'"); + module.writeLine(" property string prop: " + ModUtils.toJSLiteral(prop)); + if (listProp) { + module.writeLine(" property stringList listProp: " + + ModUtils.toJSLiteral(listProp)); + } module.writeLine("}"); module.close(); } diff --git a/tests/auto/blackbox/tst_blackbox.cpp b/tests/auto/blackbox/tst_blackbox.cpp index 75dec843b..3cacd67e9 100644 --- a/tests/auto/blackbox/tst_blackbox.cpp +++ b/tests/auto/blackbox/tst_blackbox.cpp @@ -5852,10 +5852,11 @@ void TestBlackbox::providersProperties() QDir::setCurrent(testDataDir + "/providers-properties"); QbsRunParameters params("build"); - params.arguments = QStringList("moduleProviders.provider_b.someProp: \"otherValue\""); + params.arguments = QStringList("moduleProviders.provider_b.someProp: \"first,second\""); QCOMPARE(runQbs(params), 0); - QVERIFY2(m_qbsStdout.contains("p.qbsmetatestmodule.prop: someValue"), m_qbsStdout); - QVERIFY2(m_qbsStdout.contains("p.qbsothermodule.prop: otherValue"), m_qbsStdout); + QVERIFY2(m_qbsStdout.contains("p.qbsmetatestmodule.listProp: [\"someValue\"]"), m_qbsStdout); + QVERIFY2(m_qbsStdout.contains( + "p.qbsothermodule.listProp: [\"first\",\"second\"]"), m_qbsStdout); } void TestBlackbox::pseudoMultiplexing() -- cgit v1.2.3