From 5c62a537065b0186bd07cc008a47405607b2ba92 Mon Sep 17 00:00:00 2001 From: Simon Hausmann Date: Fri, 31 Jan 2014 15:09:46 +0100 Subject: [PATCH] [new compiler] Fix error messages for binding vs. property declarations Separate binding errors from property declaration errors - they were accidentally mixed up. Change-Id: Id2d5134dc98ee3e1d7ce0c3d356f165e144e0d82 Reviewed-by: Lars Knoll --- src/qml/compiler/qqmlcodegenerator.cpp | 113 +++++++++++++++++++-------------- src/qml/compiler/qqmlcodegenerator_p.h | 13 ++-- src/qml/compiler/qqmltypecompiler.cpp | 4 +- 3 files changed, 76 insertions(+), 54 deletions(-) diff --git a/src/qml/compiler/qqmlcodegenerator.cpp b/src/qml/compiler/qqmlcodegenerator.cpp index 33476e6..5d31235 100644 --- a/src/qml/compiler/qqmlcodegenerator.cpp +++ b/src/qml/compiler/qqmlcodegenerator.cpp @@ -123,18 +123,29 @@ QString QmlObject::appendSignal(Signal *signal) return QString(); // no error } -bool QmlObject::appendProperty(QmlProperty *prop, bool isDefaultProperty) +QString QmlObject::appendProperty(QmlProperty *prop, const QString &propertyName, bool isDefaultProperty, const AST::SourceLocation &defaultToken, AST::SourceLocation *errorLocation) { QmlObject *target = declarationsOverride; if (!target) target = this; + + if (target->propertyNames.contains(prop->nameIndex)) + return tr("Duplicate property name"); + + if (propertyName.constData()->isUpper()) + return tr("Property names cannot begin with an upper case letter"); + + target->propertyNames.insert(prop->nameIndex); + const int index = target->properties->append(prop); if (isDefaultProperty) { - if (target->indexOfDefaultProperty != -1) - return false; + if (target->indexOfDefaultProperty != -1) { + *errorLocation = defaultToken; + return tr("Duplicate default property"); + } target->indexOfDefaultProperty = index; } - return true; + return QString(); // no error } void QmlObject::appendFunction(Function *f) @@ -145,6 +156,20 @@ void QmlObject::appendFunction(Function *f) target->functions->append(f); } +QString QmlObject::appendBinding(Binding *b, bool isListBinding, bool bindToDefaultProperty) +{ + if (!isListBinding && !bindToDefaultProperty + && b->type != QV4::CompiledData::Binding::Type_GroupProperty + && b->type != QV4::CompiledData::Binding::Type_AttachedProperty + && !(b->flags & QV4::CompiledData::Binding::IsOnAssignment)) { + if (bindingNames.contains(b->propertyNameIndex)) + return tr("Property value set multiple times"); + bindingNames.insert(b->propertyNameIndex); + } + bindings->append(b); + return QString(); // no error +} + QStringList Signal::parameterStringList(const QStringList &stringPool) const { QStringList result; @@ -702,7 +727,8 @@ bool QQmlCodeGenerator::visit(AST::UiPublicMember *node) else property->customTypeNameIndex = emptyStringIndex; - property->nameIndex = registerString(name.toString()); + const QString propName = name.toString(); + property->nameIndex = registerString(propName); AST::SourceLocation loc = node->firstSourceLocation(); property->location.line = loc.startLine; @@ -756,13 +782,23 @@ bool QQmlCodeGenerator::visit(AST::UiPublicMember *node) qSwap(_propertyDeclaration, property); } - if (!_object->appendProperty(property, node->isDefaultMember)) { - Q_ASSERT(node->isDefaultMember); - QQmlError error; - error.setDescription(QCoreApplication::translate("QQmlParser","Duplicate default property")); - error.setLine(node->defaultToken.startLine); - error.setColumn(node->defaultToken.startColumn); - errors << error; + AST::SourceLocation errorLocation; + QString error; + + if (illegalNames.contains(propName)) + error = tr("Illegal property name"); + else + error = _object->appendProperty(property, propName, node->isDefaultMember, node->defaultToken, &errorLocation); + + if (!error.isEmpty()) { + if (errorLocation.startLine == 0) + errorLocation = node->identifierToken; + + QQmlError qmlError; + qmlError.setDescription(error); + qmlError.setLine(errorLocation.startLine); + qmlError.setColumn(errorLocation.startColumn); + errors << qmlError; return false; } @@ -907,35 +943,36 @@ void QQmlCodeGenerator::appendBinding(AST::UiQualifiedId *name, int objectIndex, qSwap(_object, object); } -void QQmlCodeGenerator::appendBinding(const AST::SourceLocation &nameLocation, int propertyNameIndex, AST::Statement *value) +void QQmlCodeGenerator::appendBinding(const AST::SourceLocation &nameLocation, quint32 propertyNameIndex, AST::Statement *value) { - if (!sanityCheckPropertyName(nameLocation, propertyNameIndex)) - return; - if (stringAt(propertyNameIndex) == QStringLiteral("id")) { setId(value); return; } + const bool bindingToDefaultProperty = (propertyNameIndex == emptyStringIndex); + Binding *binding = New(); binding->propertyNameIndex = propertyNameIndex; binding->location.line = nameLocation.startLine; binding->location.column = nameLocation.startColumn; binding->flags = 0; setBindingValue(binding, value); - bindingsTarget()->appendBinding(binding); + QString error = bindingsTarget()->appendBinding(binding, /*isListBinding*/false, bindingToDefaultProperty); + if (!error.isEmpty()) { + recordError(nameLocation, error); + } } -void QQmlCodeGenerator::appendBinding(const AST::SourceLocation &nameLocation, int propertyNameIndex, int objectIndex, bool isListItem, bool isOnAssignment) +void QQmlCodeGenerator::appendBinding(const AST::SourceLocation &nameLocation, quint32 propertyNameIndex, int objectIndex, bool isListItem, bool isOnAssignment) { - if (!sanityCheckPropertyName(nameLocation, propertyNameIndex, isListItem | isOnAssignment)) - return; - if (stringAt(propertyNameIndex) == QStringLiteral("id")) { recordError(nameLocation, tr("Invalid component id specification")); return; } + const bool bindingToDefaultProperty = (propertyNameIndex == emptyStringIndex); + Binding *binding = New(); binding->propertyNameIndex = propertyNameIndex; binding->location.line = nameLocation.startLine; @@ -959,7 +996,10 @@ void QQmlCodeGenerator::appendBinding(const AST::SourceLocation &nameLocation, i binding->flags |= QV4::CompiledData::Binding::IsOnAssignment; binding->value.objectIndex = objectIndex; - bindingsTarget()->appendBinding(binding); + QString error = bindingsTarget()->appendBinding(binding, isListItem, bindingToDefaultProperty); + if (!error.isEmpty()) { + recordError(nameLocation, error); + } } QmlObject *QQmlCodeGenerator::bindingsTarget() const @@ -1055,7 +1095,11 @@ bool QQmlCodeGenerator::resolveQualifiedId(AST::UiQualifiedId **nameToResolve, Q int objIndex = defineQMLObject(0, AST::SourceLocation(), 0, 0); binding->value.objectIndex = objIndex; - (*object)->appendBinding(binding); + QString error = (*object)->appendBinding(binding, /*isListBinding*/false, /*bindingToDefaultProperty*/false); + if (!error.isEmpty()) { + recordError(qualifiedIdElement->identifierToken, error); + return false; + } *object = _objects[objIndex]; qualifiedIdElement = qualifiedIdElement->next; @@ -1066,29 +1110,6 @@ bool QQmlCodeGenerator::resolveQualifiedId(AST::UiQualifiedId **nameToResolve, Q return true; } -bool QQmlCodeGenerator::sanityCheckPropertyName(const AST::SourceLocation &nameLocation, int nameIndex, bool isListItemOnOrAssignment) -{ - const QString &name = jsGenerator->strings.at(nameIndex); - if (name.isEmpty()) - return true; - - // List items are implement by multiple bindings to the same name, so allow duplicates. - if (!isListItemOnOrAssignment) { - if (_object->propertyNames.contains(nameIndex)) - COMPILE_EXCEPTION(nameLocation, tr("Duplicate property name")); - - _object->propertyNames.insert(nameIndex); - } - - if (name.at(0).isUpper()) - COMPILE_EXCEPTION(nameLocation, tr("Property names cannot begin with an upper case letter")); - - if (illegalNames.contains(name)) - COMPILE_EXCEPTION(nameLocation, tr("Illegal property name")); - - return true; -} - void QQmlCodeGenerator::recordError(const AST::SourceLocation &location, const QString &description) { QQmlError error; diff --git a/src/qml/compiler/qqmlcodegenerator_p.h b/src/qml/compiler/qqmlcodegenerator_p.h index c687444..eae3594 100644 --- a/src/qml/compiler/qqmlcodegenerator_p.h +++ b/src/qml/compiler/qqmlcodegenerator_p.h @@ -188,18 +188,19 @@ public: QString sanityCheckFunctionNames(const QList &allFunctions, const QSet &illegalNames, AST::SourceLocation *errorLocation); QString appendSignal(Signal *signal); - bool appendProperty(QmlProperty *prop, bool isDefaultProperty); + QString appendProperty(QmlProperty *prop, const QString &propertyName, bool isDefaultProperty, const AST::SourceLocation &defaultToken, AST::SourceLocation *errorLocation); void appendFunction(Function *f); - void appendBinding(Binding *b) { bindings->append(b); } + QString appendBinding(Binding *b, bool isListBinding, bool bindToDefaultProperty); - QSet propertyNames; private: PoolList *properties; PoolList *qmlSignals; PoolList *bindings; PoolList *functions; + QSet propertyNames; + QSet bindingNames; QSet signalNames; }; @@ -287,8 +288,8 @@ public: void appendBinding(AST::UiQualifiedId *name, AST::Statement *value); void appendBinding(AST::UiQualifiedId *name, int objectIndex, bool isOnAssignment = false); - void appendBinding(const AST::SourceLocation &nameLocation, int propertyNameIndex, AST::Statement *value); - void appendBinding(const AST::SourceLocation &nameLocation, int propertyNameIndex, int objectIndex, bool isListItem = false, bool isOnAssignment = false); + void appendBinding(const AST::SourceLocation &nameLocation, quint32 propertyNameIndex, AST::Statement *value); + void appendBinding(const AST::SourceLocation &nameLocation, quint32 propertyNameIndex, int objectIndex, bool isListItem = false, bool isOnAssignment = false); QmlObject *bindingsTarget() const; @@ -298,8 +299,6 @@ public: // with the object any right-hand-side of a binding should apply to. bool resolveQualifiedId(AST::UiQualifiedId **nameToResolve, QmlObject **object); - bool sanityCheckPropertyName(const AST::SourceLocation &nameLocation, int nameIndex, bool isListItemOnOrAssignment = false); - void recordError(const AST::SourceLocation &location, const QString &description); void collectTypeReferences(); diff --git a/src/qml/compiler/qqmltypecompiler.cpp b/src/qml/compiler/qqmltypecompiler.cpp index b831f7c..81b3e34 100644 --- a/src/qml/compiler/qqmltypecompiler.cpp +++ b/src/qml/compiler/qqmltypecompiler.cpp @@ -1039,7 +1039,9 @@ void QQmlComponentAndAliasResolver::findAndRegisterImplicitComponents(const QtQm QtQml::Binding *syntheticBinding = pool->New(); *syntheticBinding = *binding; syntheticBinding->type = QV4::CompiledData::Binding::Type_Object; - syntheticComponent->appendBinding(syntheticBinding); + QString error = syntheticComponent->appendBinding(syntheticBinding, /*isListBinding*/false, /*bindingToDefaultProperty*/false); + Q_ASSERT(error.isEmpty()); + Q_UNUSED(error); binding->value.objectIndex = componentIndex; -- 2.7.4