shiboken2: Refactor property parsing

Store AbstractMetaType instead of TypeEntry in
QPropertySpec for more complete type information and
better parsing.

Introduce a struct TypeSystemProperty to the
typesystem which can be later populated from a newly
introduced typesystem XML element. Rewrite the parser
to first populate a TypeSystemProperty and convert it
to QPropertySpec with type lookup in a second step.

Change the parser algorithm to first look for any of
the READ/WRITE tokens and split the tokens leading up
to them into type name and property name in order to
parse complex types like

Q_PROPERTY(QList<QSpriteSheetItem *> sprites READ sprites..

correctly.

Task-number: PYSIDE-1019
Change-Id: I942bc659cc236d31041cdc22e26a82d270599033
Reviewed-by: Christian Tismer <tismer@stackless.com>

7 files changed, 182 insertions, 64 deletions

diff --git a/sources/shiboken2/ApiExtractor/abstractmetabuilder.cpp b/sources/shiboken2/ApiExtractor/abstractmetabuilder.cpp
index 3c9ef21a7..d9cf69e05 100644
--- a/sources/shiboken2/ApiExtractor/abstractmetabuilder.cpp
+++ b/sources/shiboken2/ApiExtractor/abstractmetabuilder.cpp
@@ -1301,7 +1301,7 @@ void AbstractMetaBuilderPrivate::traverseFunctions(ScopeModelItem scopeItem,
         QPropertySpec *read = nullptr;
         if (!metaFunction->isSignal() && (read = metaClass->propertySpecForRead(metaFunction->name()))) {
             // Property reader must be in the form "<type> name()"
-            if (metaFunction->type() && (read->type() == metaFunction->type()->typeEntry())
+            if (metaFunction->type() && (read->typeEntry() == metaFunction->type()->typeEntry())
                 && metaFunction->arguments().isEmpty()) {
                 *metaFunction += AbstractMetaAttributes::PropertyReader;
                 metaFunction->setPropertySpec(read);
@@ -1310,7 +1310,8 @@ void AbstractMetaBuilderPrivate::traverseFunctions(ScopeModelItem scopeItem,
             // Property setter must be in the form "void name(<type>)"
             // Make sure the function was created with all arguments; some argument can be
             // missing during the parsing because of errors in the typesystem.
-            if ((!metaFunction->type()) && (metaFunction->arguments().size() == 1) && (write->type() == metaFunction->arguments().at(0)->type()->typeEntry())) {
+            if ((!metaFunction->type()) && (metaFunction->arguments().size() == 1)
+                && (write->typeEntry() == metaFunction->arguments().at(0)->type()->typeEntry())) {
                 *metaFunction += AbstractMetaAttributes::PropertyWriter;
                 metaFunction->setPropertySpec(write);
             }
diff --git a/sources/shiboken2/ApiExtractor/abstractmetalang.cpp b/sources/shiboken2/ApiExtractor/abstractmetalang.cpp
index ff1bb53f2..9a512cd44 100644
--- a/sources/shiboken2/ApiExtractor/abstractmetalang.cpp
+++ b/sources/shiboken2/ApiExtractor/abstractmetalang.cpp
@@ -1401,6 +1401,7 @@ AbstractMetaClass::~AbstractMetaClass()
     qDeleteAll(m_functions);
     qDeleteAll(m_fields);
     qDeleteAll(m_enums);
+    qDeleteAll(m_propertySpecs);
     if (hasTemplateBaseClassInstantiations())
         qDeleteAll(templateBaseClassInstantiations());
 }
diff --git a/sources/shiboken2/ApiExtractor/messages.cpp b/sources/shiboken2/ApiExtractor/messages.cpp
index 930cd2c70..031c74324 100644
--- a/sources/shiboken2/ApiExtractor/messages.cpp
+++ b/sources/shiboken2/ApiExtractor/messages.cpp
@@ -368,6 +368,16 @@ QString msgNamespaceToBeExtendedNotFound(const QString &namespaceName, const QSt
         + packageName + QLatin1Char('.');
 }
 
+QString msgPropertyTypeParsingFailed(const QString &name, const QString &typeName,
+                                     const QString &why)
+{
+    QString result;
+    QTextStream str(&result);
+    str << "Unable to decide type of property: \"" << name << "\" (" << typeName
+        << "): " << why;
+    return result;
+}
+
 // docparser.cpp
 
 QString msgCannotFindDocumentation(const QString &fileName,
diff --git a/sources/shiboken2/ApiExtractor/messages.h b/sources/shiboken2/ApiExtractor/messages.h
index 72484050a..191985d3e 100644
--- a/sources/shiboken2/ApiExtractor/messages.h
+++ b/sources/shiboken2/ApiExtractor/messages.h
@@ -127,6 +127,9 @@ QString msgDisallowThread(const AbstractMetaFunction *f);
 
 QString msgNamespaceToBeExtendedNotFound(const QString &namespaceName, const QString &packageName);
 
+QString msgPropertyTypeParsingFailed(const QString &name, const QString &typeName,
+                                     const QString &why);
+
 QString msgCannotFindDocumentation(const QString &fileName,
                                    const char *what, const QString &name,
                                    const QString &query);
diff --git a/sources/shiboken2/ApiExtractor/propertyspec.cpp b/sources/shiboken2/ApiExtractor/propertyspec.cpp
index 682a24473..fcb8286bf 100644
--- a/sources/shiboken2/ApiExtractor/propertyspec.cpp
+++ b/sources/shiboken2/ApiExtractor/propertyspec.cpp
@@ -30,93 +30,171 @@
 #include "abstractmetalang.h"
 #include "abstractmetabuilder_p.h"
 #include "codemodel.h"
+#include "messages.h"
 #include "typesystem.h"
 
+#include <QtCore/QHash>
+
 #ifndef QT_NO_DEBUG_STREAM
 #  include <QtCore/QDebug>
 #endif
 
+#include <algorithm>
+
+QPropertySpec::QPropertySpec(const TypeSystemProperty &ts,
+                             const AbstractMetaType *type) :
+    m_name(ts.name),
+    m_read(ts.read),
+    m_write(ts.write),
+    m_designable(ts.designable),
+    m_reset(ts.reset),
+    m_type(type)
+{
+}
+
+QPropertySpec::~QPropertySpec()
+{
+    delete m_type;
+}
+
 bool QPropertySpec::isValid() const
 {
     return m_type != nullptr && !m_name.isEmpty() && !m_read.isEmpty();
 }
 
-QPropertySpec *QPropertySpec::parseQ_Property(AbstractMetaBuilderPrivate *b,
-                                              AbstractMetaClass *metaClass,
-                                              const QString &declarationIn,
-                                              const QStringList &scopes,
-                                              QString *errorMessage)
+const TypeEntry *QPropertySpec::typeEntry() const
 {
+    return m_type->typeEntry();
+}
+
+// Parse a Q_PROPERTY macro
+// Q_PROPERTY(QString objectName READ objectName WRITE setObjectName NOTIFY objectNameChanged)
+// into a TypeSystemProperty.
+TypeSystemProperty QPropertySpec::typeSystemPropertyFromQ_Property(const QString &declarationIn,
+                                                                   QString *errorMessage)
+{
+    enum class PropertyToken { None, Read, Write, Designable, Reset };
+
+    static const QHash<QString, PropertyToken> tokenLookup = {
+        {QStringLiteral("READ"), PropertyToken::Read},
+        {QStringLiteral("WRITE"), PropertyToken::Write},
+        {QStringLiteral("DESIGNABLE"), PropertyToken::Designable},
+        {QStringLiteral("RESET"), PropertyToken::Reset}
+    };
+
     errorMessage->clear();
 
+    TypeSystemProperty result;
+
     // Q_PROPERTY(QString objectName READ objectName WRITE setObjectName NOTIFY objectNameChanged)
 
     const QString declaration = declarationIn.simplified();
-    auto propertyTokens = declaration.splitRef(QLatin1Char(' '), Qt::SkipEmptyParts);
-    if (propertyTokens.size()  < 4) {
-        *errorMessage = QLatin1String("Insufficient number of tokens");
-        return nullptr;
-    }
+    auto propertyTokens = declaration.split(QLatin1Char(' '), Qt::SkipEmptyParts);
 
-    QString fullTypeName = propertyTokens.takeFirst().toString();
-    QString name = propertyTokens.takeFirst().toString();
-    // Fix errors like "Q_PROPERTY(QXYSeries *series .." to be of type "QXYSeries*"
-    while (name.startsWith(QLatin1Char('*'))) {
-        fullTypeName += name.at(0);
-        name.remove(0, 1);
+    // To properly parse complicated type declarations like
+    // "Q_PROPERTY(const QList<QString > *objectName READ objectName ..."
+    // we first search the first "READ" token, parse the subsequent tokens and
+    // extract type and name from the tokens before "READ".
+    const auto it = std::find_if(propertyTokens.cbegin(), propertyTokens.cend(),
+                                 [](const QString &t) { return tokenLookup.contains(t); });
+    if (it == propertyTokens.cend()) {
+        *errorMessage = QLatin1String("Invalid property specification, READ missing");
+        return result;
     }
 
-    int indirections = 0;
-    QString typeName = fullTypeName;
-    for (; typeName.endsWith(QLatin1Char('*')); ++indirections)
-        typeName.chop(1);
-
-    QScopedPointer<AbstractMetaType> type;
-    QString typeError;
-    for (int j = scopes.size(); j >= 0 && type.isNull(); --j) {
-        QStringList qualifiedName = scopes.mid(0, j);
-        qualifiedName.append(typeName);
-        TypeInfo info;
-        info.setIndirections(indirections);
-        info.setQualifiedName(qualifiedName);
-        type.reset(b->translateType(info, metaClass, {}, &typeError));
+    const int firstToken = int(it - propertyTokens.cbegin());
+    if (firstToken < 2) {
+        *errorMessage = QLatin1String("Insufficient number of tokens in property specification");
+        return result;
     }
 
-    if (!type) {
-        QTextStream str(errorMessage);
-        str << "Unable to decide type of property: \"" << name << "\" ("
-            <<  typeName << "): " << typeError;
-        return nullptr;
+    for (int pos = firstToken; pos + 1 < propertyTokens.size(); pos += 2) {
+        switch (tokenLookup.value(propertyTokens.at(pos))) {
+        case PropertyToken::Read:
+            result.read = propertyTokens.at(pos + 1);
+            break;
+        case PropertyToken::Write:
+            result.write = propertyTokens.at(pos + 1);
+            break;
+        case PropertyToken::Reset:
+            result.reset = propertyTokens.at(pos + 1);
+            break;
+        case PropertyToken::Designable:
+            result.designable = propertyTokens.at(pos + 1);
+            break;
+        case PropertyToken::None:
+            break;
+        }
     }
 
-    QScopedPointer<QPropertySpec> spec(new QPropertySpec(type->typeEntry()));
-    spec->setName(name);
-    spec->setIndirections(indirections);
-
-    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());
-    }
+    const int namePos = firstToken - 1;
+    result.name = propertyTokens.at(namePos);
+
+    result.type = propertyTokens.constFirst();
+    for (int pos = 1; pos < namePos; ++pos)
+        result.type += QLatin1Char(' ') + propertyTokens.at(pos);
 
-    if (!spec->isValid()) {
-        *errorMessage = QLatin1String("Incomplete specification");
-        return nullptr;
+    // Fix errors like "Q_PROPERTY(QXYSeries *series .." to be of type "QXYSeries*"
+    while (!result.name.isEmpty() && !result.name.at(0).isLetter()) {
+        result.type += result.name.at(0);
+        result.name.remove(0, 1);
     }
-    return spec.take();
+    if (!result.isValid())
+        *errorMessage = QLatin1String("Incomplete property specification");
+    return result;
+}
+
+// Create a QPropertySpec from a TypeSystemProperty, determining
+// the AbstractMetaType from the type string.
+QPropertySpec *QPropertySpec::fromTypeSystemProperty(AbstractMetaBuilderPrivate *b,
+                                                     AbstractMetaClass *metaClass,
+                                                     const TypeSystemProperty &ts,
+                                                     const QStringList &scopes,
+                                                     QString *errorMessage)
+ {
+     Q_ASSERT(ts.isValid());
+     QString typeError;
+     TypeInfo info = TypeParser::parse(ts.type, &typeError);
+     if (info.qualifiedName().isEmpty()) {
+         *errorMessage = msgPropertyTypeParsingFailed(ts.name, ts.type, typeError);
+         return nullptr;
+     }
+
+     AbstractMetaType *type = b->translateType(info, metaClass, {}, &typeError);
+     if (!type) {
+         const QStringList qualifiedName = info.qualifiedName();
+         for (int j = scopes.size(); j >= 0 && type == nullptr; --j) {
+             info.setQualifiedName(scopes.mid(0, j) + qualifiedName);
+             type = b->translateType(info, metaClass, {}, &typeError);
+         }
+     }
+
+     if (!type) {
+         *errorMessage = msgPropertyTypeParsingFailed(ts.name, ts.type, typeError);
+         return nullptr;
+     }
+     return new QPropertySpec(ts, type);
+ }
+
+// Convenience to create a QPropertySpec from a Q_PROPERTY macro
+// via TypeSystemProperty.
+QPropertySpec *QPropertySpec::parseQ_Property(AbstractMetaBuilderPrivate *b,
+                                              AbstractMetaClass *metaClass,
+                                              const QString &declarationIn,
+                                              const QStringList &scopes,
+                                              QString *errorMessage)
+{
+    const TypeSystemProperty ts =
+        typeSystemPropertyFromQ_Property(declarationIn, errorMessage);
+    return ts.isValid()
+        ? fromTypeSystemProperty(b, metaClass, ts, scopes, errorMessage)
+        : nullptr;
 }
 
 #ifndef QT_NO_DEBUG_STREAM
 void QPropertySpec::formatDebug(QDebug &d) const
 {
-    d << '#' << m_index << " \"" << m_name << "\" (" << m_type->qualifiedCppName();
-    for (int i = 0; i < m_indirections; ++i)
-        d << '*';
+    d << '#' << m_index << " \"" << m_name << "\" (" << m_type->cppSignature();
     d << "), read=" << m_read;
     if (!m_write.isEmpty())
         d << ", write=" << m_write;
diff --git a/sources/shiboken2/ApiExtractor/propertyspec.h b/sources/shiboken2/ApiExtractor/propertyspec.h
index 9cf9310ec..4abe75c84 100644
--- a/sources/shiboken2/ApiExtractor/propertyspec.h
+++ b/sources/shiboken2/ApiExtractor/propertyspec.h
@@ -33,8 +33,11 @@
 
 class AbstractMetaClass;
 class AbstractMetaBuilderPrivate;
+class AbstractMetaType;
 class TypeEntry;
 
+struct TypeSystemProperty;
+
 QT_FORWARD_DECLARE_CLASS(QDebug)
 
 class QPropertySpec
@@ -42,7 +45,18 @@ class QPropertySpec
 public:
     Q_DISABLE_COPY_MOVE(QPropertySpec)
 
-    explicit QPropertySpec(const TypeEntry *type) : m_type(type) {}
+    explicit QPropertySpec(const TypeSystemProperty &ts,
+                           const AbstractMetaType *type);
+    ~QPropertySpec();
+
+    static TypeSystemProperty typeSystemPropertyFromQ_Property(const QString &declarationIn,
+                                                               QString *errorMessage);
+
+    static QPropertySpec *fromTypeSystemProperty(AbstractMetaBuilderPrivate *b,
+                                                 AbstractMetaClass *metaClass,
+                                                 const TypeSystemProperty &ts,
+                                                 const QStringList &scopes,
+                                                 QString *errorMessage);
 
     static QPropertySpec *parseQ_Property(AbstractMetaBuilderPrivate *b,
                                           AbstractMetaClass *metaClass,
@@ -52,10 +66,10 @@ public:
 
     bool isValid() const;
 
-    const TypeEntry *type() const { return m_type; }
+    const AbstractMetaType *type() const { return m_type; }
+    void setType(AbstractMetaType *t) { m_type = t; }
 
-    int indirections() const { return m_indirections; }
-    void setIndirections(int indirections) { m_indirections = indirections; }
+    const TypeEntry *typeEntry() const;
 
     QString name() const { return m_name; }
     void setName(const QString &name) { m_name = name; }
@@ -85,8 +99,7 @@ private:
     QString m_write;
     QString m_designable;
     QString m_reset;
-    const TypeEntry *m_type;
-    int m_indirections = 0;
+    const AbstractMetaType *m_type = nullptr;
     int m_index = -1;
 };
 
diff --git a/sources/shiboken2/ApiExtractor/typesystem.h b/sources/shiboken2/ApiExtractor/typesystem.h
index f351699d8..08d4cf311 100644
--- a/sources/shiboken2/ApiExtractor/typesystem.h
+++ b/sources/shiboken2/ApiExtractor/typesystem.h
@@ -553,6 +553,18 @@ private:
 class CustomConversion;
 class TypeSystemTypeEntry;
 
+struct TypeSystemProperty
+{
+    bool isValid() const { return !name.isEmpty() && !read.isEmpty() && !type.isEmpty(); }
+
+    QString type;
+    QString name;
+    QString read;
+    QString write;
+    QString reset;
+    QString designable;
+};
+
 class TypeEntry
 {
     Q_GADGET