shiboken2: Refactor the parsing of Q_PROPERTY()

Split the function and add more error handling, add the source
location to the message

Task-number: PYSIDE-1019
Change-Id: I74276b2f163ff52db6792e3f3b8907ad392f1055
Reviewed-by: Cristian Maureira-Fredes <cristian.maureira-fredes@qt.io>

4 files changed, 73 insertions, 38 deletions

diff --git a/sources/shiboken2/ApiExtractor/abstractmetabuilder.cpp b/sources/shiboken2/ApiExtractor/abstractmetabuilder.cpp
index 2364cec7a..f9e34ba5a 100644
--- a/sources/shiboken2/ApiExtractor/abstractmetabuilder.cpp
+++ b/sources/shiboken2/ApiExtractor/abstractmetabuilder.cpp
@@ -1069,7 +1069,7 @@ AbstractMetaClass *AbstractMetaBuilderPrivate::traverseClass(const FileModelItem
     }
     metaClass->setTemplateArguments(template_args);
 
-    parseQ_Property(metaClass, classItem->propertyDeclarations());
+    parseQ_Properties(metaClass, classItem->propertyDeclarations());
 
     traverseEnums(classItem, metaClass, classItem->enumsDeclarations());
 
@@ -2782,51 +2782,77 @@ bool AbstractMetaBuilderPrivate::inheritTemplate(AbstractMetaClass *subclass,
     return true;
 }
 
-void AbstractMetaBuilderPrivate::parseQ_Property(AbstractMetaClass *metaClass,
-                                                 const QStringList &declarations)
+void AbstractMetaBuilderPrivate::parseQ_Properties(AbstractMetaClass *metaClass,
+                                                   const QStringList &declarations)
 {
     const QStringList scopes = currentScope()->qualifiedName();
-
+    QString errorMessage;
     for (int i = 0; i < declarations.size(); ++i) {
-        const auto propertyTokens = declarations.at(i).splitRef(QLatin1Char(' '));
-
-        AbstractMetaType *type = nullptr;
-        for (int j = scopes.size(); j >= 0; --j) {
-            QStringList qualifiedName = scopes.mid(0, j);
-            qualifiedName.append(propertyTokens.at(0).toString());
-            TypeInfo info;
-            info.setQualifiedName(qualifiedName);
-
-            type = translateType(info, metaClass);
-            if (type)
-                break;
+        if (auto spec = parseQ_Property(metaClass, declarations.at(i), scopes, &errorMessage)) {
+            spec->setIndex(i);
+            metaClass->addPropertySpec(spec);
+        } else {
+            QString message;
+            QTextStream str(&message);
+            str << metaClass->sourceLocation() << errorMessage;
+            qCWarning(lcShiboken, "%s", qPrintable(message));
         }
+    }
+}
 
-        if (!type) {
-            qCWarning(lcShiboken).noquote().nospace()
-                << QStringLiteral("Unable to decide type of property: '%1' in class '%2'")
-                                  .arg(propertyTokens.at(0).toString(), metaClass->name());
-            continue;
-        }
+QPropertySpec *AbstractMetaBuilderPrivate::parseQ_Property(AbstractMetaClass *metaClass,
+                                                           const QString &declaration,
+                                                           const QStringList &scopes,
+                                                           QString *errorMessage)
+{
+    errorMessage->clear();
 
-        auto *spec = new QPropertySpec(type->typeEntry());
-        spec->setName(propertyTokens.at(1).toString());
-        spec->setIndex(i);
+    // Q_PROPERTY(QString objectName READ objectName WRITE setObjectName NOTIFY objectNameChanged)
 
-        for (int pos = 2; pos + 1 < propertyTokens.size(); pos += 2) {
-            if (propertyTokens.at(pos) == QLatin1String("READ"))
-                spec->setRead(propertyTokens.at(pos + 1).toString());
-            else if (propertyTokens.at(pos) == QLatin1String("WRITE"))
-                spec->setWrite(propertyTokens.at(pos + 1).toString());
-            else if (propertyTokens.at(pos) == QLatin1String("DESIGNABLE"))
-                spec->setDesignable(propertyTokens.at(pos + 1).toString());
-            else if (propertyTokens.at(pos) == QLatin1String("RESET"))
-                spec->setReset(propertyTokens.at(pos + 1).toString());
-        }
+    auto propertyTokens = declaration.splitRef(QLatin1Char(' '), Qt::SkipEmptyParts);
+    if (propertyTokens.size()  < 4) {
+        *errorMessage = QLatin1String("Insufficient number of tokens");
+        return nullptr;
+    }
 
-        metaClass->addPropertySpec(spec);
-        delete type;
+    const QString typeName = propertyTokens.takeFirst().toString();
+    const QString name = propertyTokens.takeFirst().toString();
+
+    QScopedPointer<AbstractMetaType> type;
+    for (int j = scopes.size(); j >= 0 && type.isNull(); --j) {
+        QStringList qualifiedName = scopes.mid(0, j);
+        qualifiedName.append(typeName);
+        TypeInfo info;
+        info.setQualifiedName(qualifiedName);
+        type.reset(translateType(info, metaClass));
+    }
+
+    if (!type) {
+        QTextStream str(errorMessage);
+        str << "Unable to decide type of property: \"" << name << "\" ("
+            <<  typeName << ')';
+        return nullptr;
+    }
+
+    QScopedPointer<QPropertySpec> spec(new QPropertySpec(type->typeEntry()));
+    spec->setName(name);
+
+    for (int pos = 0; pos + 1 < propertyTokens.size(); pos += 2) {
+        if (propertyTokens.at(pos) == QLatin1String("READ"))
+            spec->setRead(propertyTokens.at(pos + 1).toString());
+        else if (propertyTokens.at(pos) == QLatin1String("WRITE"))
+            spec->setWrite(propertyTokens.at(pos + 1).toString());
+        else if (propertyTokens.at(pos) == QLatin1String("DESIGNABLE"))
+            spec->setDesignable(propertyTokens.at(pos + 1).toString());
+        else if (propertyTokens.at(pos) == QLatin1String("RESET"))
+            spec->setReset(propertyTokens.at(pos + 1).toString());
+    }
+
+    if (!spec->isValid()) {
+        *errorMessage = QLatin1String("Incomplete specification");
+        return nullptr;
     }
+    return spec.take();
 }
 
 static AbstractMetaFunction* findCopyCtor(AbstractMetaClass* cls)
diff --git a/sources/shiboken2/ApiExtractor/abstractmetabuilder_p.h b/sources/shiboken2/ApiExtractor/abstractmetabuilder_p.h
index 2686ebacb..2867e5f74 100644
--- a/sources/shiboken2/ApiExtractor/abstractmetabuilder_p.h
+++ b/sources/shiboken2/ApiExtractor/abstractmetabuilder_p.h
@@ -125,7 +125,9 @@ public:
      */
     void fixReturnTypeOfConversionOperator(AbstractMetaFunction *metaFunction);
 
-    void parseQ_Property(AbstractMetaClass *metaClass, const QStringList &declarations);
+    void parseQ_Properties(AbstractMetaClass *metaClass, const QStringList &declarations);
+    QPropertySpec *parseQ_Property(AbstractMetaClass *metaClass, const QString &declaration,
+                                   const QStringList &scopes, QString *errorMessage);
     void setupEquals(AbstractMetaClass *metaClass);
     void setupComparable(AbstractMetaClass *metaClass);
     void setupClonable(AbstractMetaClass *cls);
diff --git a/sources/shiboken2/ApiExtractor/abstractmetalang.cpp b/sources/shiboken2/ApiExtractor/abstractmetalang.cpp
index a202c42d5..8e1f0d7e5 100644
--- a/sources/shiboken2/ApiExtractor/abstractmetalang.cpp
+++ b/sources/shiboken2/ApiExtractor/abstractmetalang.cpp
@@ -2735,6 +2735,11 @@ QString AbstractMetaEnum::package() const
     return m_typeEntry->targetLangPackage();
 }
 
+bool QPropertySpec::isValid() const
+{
+    return m_type != nullptr && !m_name.isEmpty() && !m_read.isEmpty();
+}
+
 #ifndef QT_NO_DEBUG_STREAM
 void QPropertySpec::formatDebug(QDebug &d) const
 {
diff --git a/sources/shiboken2/ApiExtractor/abstractmetalang.h b/sources/shiboken2/ApiExtractor/abstractmetalang.h
index 8169c4a30..d99a54fc2 100644
--- a/sources/shiboken2/ApiExtractor/abstractmetalang.h
+++ b/sources/shiboken2/ApiExtractor/abstractmetalang.h
@@ -1742,6 +1742,8 @@ class QPropertySpec
 public:
     explicit QPropertySpec(const TypeEntry *type) : m_type(type) {}
 
+    bool isValid() const;
+
     const TypeEntry *type() const
     {
         return m_type;