Merge lp:~ubuntu-sdk-team/ubuntu-sdk-api/listsAndNameSpaces into lp:ubuntu-sdk-api

[Cris Dywan]

More robust list and namespace handling in apicheck

Forward port of the same commit to the apicheck that lives in the toolkit:

https://code.launchpad.net/~ubuntu-sdk-team/ubuntu-ui-toolkit/listsAndNameSpaces/+merge/299048

[ubuntu-sdk-build-bot (ubuntu-sdk-build-bot)]

PASSED: Continuous integration, rev:4
https://jenkins.ubuntu.com/ubuntu-sdk/job/ubuntu-sdk-api-ci-amd64/9/
Executed test runs:
    None: https://jenkins.ubuntu.com/ubuntu-sdk/job/generic-update-mp/6663/console

Click here to trigger a rebuild:
https://jenkins.ubuntu.com/ubuntu-sdk/job/ubuntu-sdk-api-ci-amd64/9/rebuild

review: Approve (continuous-integration)

[Benjamin Zeller (zeller-benjamin)]

In the Animals test at the bottom, seems there is information lost
before you had Extinct.Animals.Europe.ModernContinent: Enum and Extinct.Animals.Pangaea.ModernContinent: Enum.
Now you have Extinct.Animals.ModernContinent: Enum twice
The Europe vs Pangaea information is lost.

Also in line 40 of the patch you do : QString newQmlType(QString(types[0]->qmlTypeName()).split("/")[1].toUtf8());.
Are you sure there are always at least 2 items in the list after splitting the string?

review: Needs Fixing

[Cris Dywan (kalikiana)]

> In the Animals test at the bottom, seems there is information lost
> before you had Extinct.Animals.Europe.ModernContinent: Enum and
> Extinct.Animals.Pangaea.ModernContinent: Enum.
> Now you have Extinct.Animals.ModernContinent: Enum twice
> The Europe vs Pangaea information is lost.

If filed bug 1628129 to keep track of this: basically the upcoming new format will supersede this, and it's generally going to be more robust and universal. So fixing this in the original QML-ish code would only be interesting if real code were to hit it - Ubuntu UI Toolkit, currently the only project relying on this in real life, isn't affected.

> Also in line 40 of the patch you do : QString
> newQmlType(QString(types[0]->qmlTypeName()).split("/")[1].toUtf8());.
> Are you sure there are always at least 2 items in the list after splitting the
> string?

The code has been thoroughly reviewed before with Ubuntu UI Toolkit in mind, in that sense I'm pretty confident it works in production.

[Benjamin Zeller (zeller-benjamin)]

Ok then

review: Approve

[ubuntu-sdk-build-bot (ubuntu-sdk-build-bot)]

PASSED: Continuous integration, rev:4
https://jenkins.ubuntu.com/ubuntu-sdk/job/ubuntu-sdk-api-ci-amd64/12/
Executed test runs:
    None: https://jenkins.ubuntu.com/ubuntu-sdk/job/generic-update-mp/7081/console

Click here to trigger a rebuild:
https://jenkins.ubuntu.com/ubuntu-sdk/job/ubuntu-sdk-api-ci-amd64/12/rebuild

review: Approve (continuous-integration)

=== modified file 'apicheck/apicheck.cpp'
--- apicheck/apicheck.cpp	2016-09-21 16:04:42 +0000
+++ apicheck/apicheck.cpp	2016-09-21 16:04:42 +0000
@@ -140,30 +140,37 @@
 */
 QByteArray convertToId(const QString &cppName)
 {
-    QString qmlType(cppName);
+    QString qmlType(QString(qPrintable(cppName)).replace(QRegExp("(QQmlListProperty|QList)<(.+)>"), "\\2"));
+    QString typeFormat(qmlType != cppName ? "list<%1>" : "%1");
+
+    QList<const QQmlType*>types(qmlTypesByCppName[qPrintable(qmlType)].toList());
+    std::sort(types.begin(), types.end(), typeNameSort);
+
     if (qmlType.contains("::")) {
         QStringList parts(qmlType.split("::"));
-        return qPrintable(convertToId(parts[0]) + "." + convertToId(parts[1]));
+        // Enum or Flags
+        if (parts.length() > 2) {
+            qmlType = convertToId(parts[0] + "::" + parts[1]) + "." + parts[2];
+        } else {
+            // Namespace
+            qmlType = parts[1];
+            // FIXME: Namespaces in properties?
+            if (parts[0] == "Qt" || parts[0] == "RenderTimer")
+                return qPrintable(typeFormat.arg(parts[0] + "." + qmlType));
+        }
     }
 
-    QList<const QQmlType*>types(qmlTypesByCppName[qPrintable(cppName)].toList());
-    std::sort(types.begin(), types.end(), typeNameSort);
-    if (!types.isEmpty())
-        qmlType = QString(types[0]->qmlTypeName()).split("/")[1].toUtf8();
-    else
-        qmlType = cppToId.value(qPrintable(qmlType), qPrintable(cppName));
+    if (types.isEmpty())
+        qmlType = cppToId.value(qPrintable(qmlType), qPrintable(qmlType));
+    else {
+        // Cache QML type name
+        QString newQmlType(QString(types[0]->qmlTypeName()).split("/")[1].toUtf8());
+        cppToId.insert(qPrintable(qmlType), qPrintable(newQmlType));
+        qmlType = newQmlType;
+    }
     // Strip internal _QMLTYPE_xy suffix
     qmlType = qmlType.split("_")[0];
-    // List type
-    if (qmlType.startsWith("QQmlListProperty<")) {
-        QString subType(qmlType.mid(17, qmlType.size() - 18));
-        qmlType = "list<" + convertToId(subType) + ">";
-    }
-    else if (qmlType.startsWith("QList<")) {
-        QString subType(qmlType.mid(6, qmlType.size() - 7));
-        qmlType = "list<" + convertToId(subType) + ">";
-    }
-    return qPrintable(qmlType.replace("QTestRootObject", "QtObject"));
+    return qPrintable(typeFormat.arg(qmlType));
 }
 
 QByteArray convertToId(const QMetaObject *mo)
@@ -626,7 +633,7 @@
         // Two leading underscores: internal API
         if (name.startsWith("__"))
             return;
-        const QString typeName = meth.typeName();
+        QString typeName = meth.typeName();
 
         if (implicitSignals.contains(name)
                 && !meth.revision()
@@ -652,6 +659,8 @@
         if (revision && object->contains("#version"))
             method["version"] = object->value("#version").toString();
 
+        if (typeName.endsWith("*"))
+            typeName.truncate(typeName.size() - 1);
         if (typeName != QLatin1String("void"))
             method["returns"] = typeName;
 
@@ -850,6 +859,7 @@
     cppToId.insert("QPoint", "Qt.point");
     cppToId.insert("QColor", "color");
     cppToId.insert("QQmlEasingValueType::Type", "Type");
+    cppToId.insert("QTestRootObject", "QtObject");
 
     // find a valid QtQuick import
     QByteArray importCode;

=== modified file 'tests/qml/Extinct.Animals.api'
--- tests/qml/Extinct.Animals.api	2016-03-03 11:11:58 +0000
+++ tests/qml/Extinct.Animals.api	2016-09-21 16:04:42 +0000
@@ -11,21 +11,21 @@
 Extinct.Animals.Europe 4.2: Europe
     readonly property ushort era 4.2
     function ushort era(uchar year)
-Extinct.Animals.Europe.ModernContinent: Enum
-    America
-    Europe
 Extinct.Animals.Excavator 1.0 1.1
 Extinct.Animals.Gigantophis 4.3: QQuickItem
     property bool extinct
     function var first()
     function var fourth()
+Extinct.Animals.ModernContinent: Enum
+    America
+    Europe
+Extinct.Animals.ModernContinent: Enum
+    America
+    Europe
 Extinct.Animals.Pangaea 1.0: QtObject singleton
 Extinct.Animals.Pangaea 4.0: Pangaea singleton
     readonly property ushort era 4.0
     function ushort era(uchar year)
-Extinct.Animals.Pangaea.ModernContinent: Enum
-    America
-    Europe
 Extinct.Animals.Smilodon 1.0 ScimitarCat 0.1 EASmilodon: QtObject
     function string paw(int i)
     property string name