diff options
author | Christian Kandeler <christian.kandeler@qt.io> | 2016-08-24 17:34:53 +0200 |
---|---|---|
committer | Christian Kandeler <christian.kandeler@qt.io> | 2016-10-06 15:10:09 +0000 |
commit | 2c75537141283895da9b80e249b391bc38d63505 (patch) | |
tree | 02c826f75b808de3a532df3d53e006ecb42115a6 /src | |
parent | d5c52a0b517505d10a090dd47dc04f9a09ca5c2a (diff) |
Do not auto-convert values to property types
The JavaScript-style conversions are often unexpected and mask user
errors. They also make a mockery of our type system.
The exceptions are as follows:
- Boolean properties are still auto-converted according to
the JS rules, because otherwise user code would get tedious to
write.
- Lists of strings can still be assigned a single string, so
the convenience pattern "files: 'main.cpp'" still works.
This patch also fixes some bugs in our own modules that were uncovered
by the change.
[ChangeLog] Added stricter type checking for properties.
Task-number: QBS-1013
Change-Id: I282c23eca3b3877c5551a49ef5c6dd8da91488fe
Reviewed-by: Jake Petroules <jake.petroules@qt.io>
Diffstat (limited to 'src')
-rw-r--r-- | src/lib/corelib/language/evaluator.cpp | 16 | ||||
-rw-r--r-- | src/lib/corelib/language/evaluatorscriptclass.cpp | 69 | ||||
-rw-r--r-- | src/lib/corelib/language/propertydeclaration.cpp | 15 | ||||
-rw-r--r-- | src/lib/corelib/language/propertydeclaration.h | 1 | ||||
-rw-r--r-- | src/lib/corelib/language/tst_language.cpp | 4 |
5 files changed, 70 insertions, 35 deletions
diff --git a/src/lib/corelib/language/evaluator.cpp b/src/lib/corelib/language/evaluator.cpp index 13f69a5f1..a801311bb 100644 --- a/src/lib/corelib/language/evaluator.cpp +++ b/src/lib/corelib/language/evaluator.cpp @@ -113,8 +113,7 @@ QString Evaluator::stringValue(const Item *item, const QString &name, return v.toString(); } -static QStringList toStringList(const QScriptValue &scriptValue, - const Item *item, const QString &propertyName) +static QStringList toStringList(const QScriptValue &scriptValue) { if (scriptValue.isString()) { return QStringList(scriptValue.toString()); @@ -125,17 +124,6 @@ static QStringList toStringList(const QScriptValue &scriptValue, QScriptValue elem = scriptValue.property(i++); if (!elem.isValid()) break; - if (elem.isUndefined()) { - throw ErrorInfo(Tr::tr("Array element at index %1 is undefined. String expected.") - .arg(i - 1), - item->property(propertyName)->location()); - } - if (elem.isArray() || elem.isObject()) { - // Let's assume all other JS types are convertible to string. - throw ErrorInfo(Tr::tr("Expected array element of type String at index %1.") - .arg(i - 1), - item->property(propertyName)->location()); - } lst.append(elem.toString()); } return lst; @@ -149,7 +137,7 @@ QStringList Evaluator::stringListValue(const Item *item, const QString &name, bo if (propertyWasSet) *propertyWasSet = v.isValid() && !v.isUndefined(); handleEvaluationError(item, name, v); - return toStringList(v, item, name); + return toStringList(v); } QScriptValue Evaluator::scriptValue(const Item *item) diff --git a/src/lib/corelib/language/evaluatorscriptclass.cpp b/src/lib/corelib/language/evaluatorscriptclass.cpp index bf14b2772..7ca472364 100644 --- a/src/lib/corelib/language/evaluatorscriptclass.cpp +++ b/src/lib/corelib/language/evaluatorscriptclass.cpp @@ -46,6 +46,7 @@ #include "scriptengine.h" #include "propertydeclaration.h" #include "value.h" +#include <logging/translator.h> #include <tools/architectures.h> #include <tools/fileinfo.h> #include <tools/hostosinfo.h> @@ -406,12 +407,27 @@ static QString overriddenSourceDirectory(const Item *item) return v ? v->value().toString() : QString(); } -inline void convertToPropertyType(const Item *item, const PropertyDeclaration::Type t, - QScriptValue &v) +static void makeTypeError(const ErrorInfo &error, QScriptValue &v) +{ + v = v.engine()->currentContext()->throwError(QScriptContext::TypeError, + error.toString()); +} + +static void makeTypeError(const PropertyDeclaration &decl, const CodeLocation &location, + QScriptValue &v) +{ + const ErrorInfo error(Tr::tr("Value assigned to property '%1' does not have type '%2'.") + .arg(decl.name(), decl.typeString()), location); + makeTypeError(error, v); +} + +static void convertToPropertyType(const Item *item, const PropertyDeclaration& decl, + const CodeLocation &location, QScriptValue &v) { if (v.isUndefined() || v.isError()) return; - switch (t) { + QString srcDir; + switch (decl.type()) { case PropertyDeclaration::UnknownType: case PropertyDeclaration::Variant: break; @@ -421,12 +437,14 @@ inline void convertToPropertyType(const Item *item, const PropertyDeclaration::T break; case PropertyDeclaration::Integer: if (!v.isNumber()) - v = v.toNumber(); + makeTypeError(decl, location, v); break; case PropertyDeclaration::Path: { - if (!v.isString()) - v = v.toString(); + if (!v.isString()) { + makeTypeError(decl, location, v); + break; + } const QString srcDir = overriddenSourceDirectory(item); if (!srcDir.isEmpty()) v = v.engine()->toScriptValue(FileInfo::resolvePath(srcDir, v.toString())); @@ -434,33 +452,46 @@ inline void convertToPropertyType(const Item *item, const PropertyDeclaration::T } case PropertyDeclaration::String: if (!v.isString()) - v = v.toString(); + makeTypeError(decl, location, v); break; case PropertyDeclaration::PathList: + srcDir = overriddenSourceDirectory(item); + // Fall-through. + case PropertyDeclaration::StringList: { if (!v.isArray()) { QScriptValue x = v.engine()->newArray(1); - x.setProperty(0, v.isString() ? v : v.toString()); + x.setProperty(0, v); v = x; } - const QString srcDir = overriddenSourceDirectory(item); - if (srcDir.isEmpty()) - break; const quint32 c = v.property(QLatin1String("length")).toUInt32(); for (quint32 i = 0; i < c; ++i) { QScriptValue elem = v.property(i); + if (elem.isUndefined()) { + ErrorInfo error(Tr::tr("Element at index %1 of list property '%2' is undefined. " + "String expected.").arg(i).arg(decl.name()), location); + makeTypeError(error, v); + break; + } + if (elem.isNull()) { + ErrorInfo error(Tr::tr("Element at index %1 of list property '%2' is null. " + "String expected.").arg(i).arg(decl.name()), location); + makeTypeError(error, v); + break; + } + if (!elem.isString()) { + ErrorInfo error(Tr::tr("Element at index %1 of list property '%2' does not have " + "string type.").arg(i).arg(decl.name()), location); + makeTypeError(error, v); + break; + } + if (srcDir.isEmpty()) + continue; elem = v.engine()->toScriptValue(FileInfo::resolvePath(srcDir, elem.toString())); v.setProperty(i, elem); } break; } - case PropertyDeclaration::StringList: - if (!v.isArray()) { - QScriptValue x = v.engine()->newArray(1); - x.setProperty(0, v.isString() ? v : v.toString()); - v = x; - } - break; } } @@ -506,7 +537,7 @@ QScriptValue EvaluatorScriptClass::property(const QScriptValue &object, const QS converter.start(); const PropertyDeclaration decl = data->item->propertyDeclaration(name.toString()); - convertToPropertyType(data->item, decl.type(), result); + convertToPropertyType(data->item, decl, value->location(), result); } if (debugProperties) diff --git a/src/lib/corelib/language/propertydeclaration.cpp b/src/lib/corelib/language/propertydeclaration.cpp index dd4920ea3..14b43e734 100644 --- a/src/lib/corelib/language/propertydeclaration.cpp +++ b/src/lib/corelib/language/propertydeclaration.cpp @@ -126,6 +126,21 @@ PropertyDeclaration::Type PropertyDeclaration::propertyTypeFromString(const QStr return PropertyDeclaration::UnknownType; } +QString PropertyDeclaration::typeString() const +{ + switch (type()) { + case Boolean: return QLatin1String("bool"); + case Integer: return QLatin1String("int"); + case Path: return QLatin1String("path"); + case PathList: return QLatin1String("pathList"); + case String: return QLatin1String("string"); + case StringList: return QLatin1String("stringList"); + case Variant: return QLatin1String("variant"); + case UnknownType: return QLatin1String("unknown"); + } + Q_UNREACHABLE(); // For stupid compilers. +} + const QString &PropertyDeclaration::name() const { return d->name; diff --git a/src/lib/corelib/language/propertydeclaration.h b/src/lib/corelib/language/propertydeclaration.h index 711766b99..5c72dd0cb 100644 --- a/src/lib/corelib/language/propertydeclaration.h +++ b/src/lib/corelib/language/propertydeclaration.h @@ -87,6 +87,7 @@ public: bool isScalar() const; static Type propertyTypeFromString(const QString &typeName); + QString typeString() const; const QString &name() const; void setName(const QString &name); diff --git a/src/lib/corelib/language/tst_language.cpp b/src/lib/corelib/language/tst_language.cpp index a27d8c9d1..6647089ee 100644 --- a/src/lib/corelib/language/tst_language.cpp +++ b/src/lib/corelib/language/tst_language.cpp @@ -509,9 +509,9 @@ void TestLanguage::erroneousFiles_data() QTest::newRow("subproject_cycle") << "Cycle detected while loading subproject file 'subproject_cycle.qbs'."; QTest::newRow("invalid_stringlist_element") - << "Expected array element of type String at index 1."; + << "Element at index 1 of list property 'files' does not have string type."; QTest::newRow("undefined_stringlist_element") - << "Array element at index 1 is undefined. String expected."; + << "Element at index 1 of list property 'files' is undefined. String expected."; QTest::newRow("undeclared_item") << "Item 'cpp' is not declared."; QTest::newRow("undeclared_property_wrapper") |