uic/Python: Fix tab stop/Z-Order and buddy handling

The code compared the attribute names of the properties against the
m_registeredWidgets hash which contained the qualified names
(self.widget) and thus reported errors without actually generating
anything.

Replace the m_registeredWidgets hash by a lookup of the attribute name
in the m_widgets hash and add a function widgetVariableName()
returning the qualified variable name for an attribute name and use
that for the checks.

Remove unused m_registeredActions hash and rename some variables to
make it clearer.

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

src/tools/uic/cpp/cppwriteinitialization.cpp

@@ -490,7 +490,6 @@ void WriteInitialization::acceptUI(DomUI *node)
 
     const QString varName = m_driver->findOrInsertWidget(node->elementWidget());
     m_mainFormVarName = varName;
-    m_registeredWidgets.insert(varName, node->elementWidget()); // register the main widget
 
     const QString widgetClassName = node->elementWidget()->attributeClass();
 
@@ -515,21 +514,16 @@ void WriteInitialization::acceptUI(DomUI *node)
     if (!m_buddies.empty())
         m_output << language::openQtConfig(shortcutConfigKey());
     for (const Buddy &b : qAsConst(m_buddies)) {
-        if (!m_registeredWidgets.contains(b.objName)) {
+        const QString buddyVarName = m_driver->widgetVariableName(b.buddyAttributeName);
+        if (buddyVarName.isEmpty()) {
             fprintf(stderr, "%s: Warning: Buddy assignment: '%s' is not a valid widget.\n",
                     qPrintable(m_option.messagePrefix()),
-                    b.objName.toLatin1().data());
+                    qPrintable(b.buddyAttributeName));
-            continue;
-        }
-        if (!m_registeredWidgets.contains(b.buddy)) {
-            fprintf(stderr, "%s: Warning: Buddy assignment: '%s' is not a valid widget.\n",
-                    qPrintable(m_option.messagePrefix()),
-                    b.buddy.toLatin1().data());
             continue;
         }
 
-        m_output << m_indent << b.objName << language::derefPointer
+        m_output << m_indent << b.labelVarName << language::derefPointer
-            << "setBuddy(" << b.buddy << ')' << language::eol;
+            << "setBuddy(" << buddyVarName << ')' << language::eol;
     }
     if (!m_buddies.empty())
         m_output << language::closeQtConfig(shortcutConfigKey());
@@ -602,7 +596,6 @@ void WriteInitialization::acceptWidget(DomWidget *node)
     m_layoutMarginType = m_widgetChain.count() == 1 ? TopLevelMargin : ChildMargin;
     const QString className = node->attributeClass();
     const QString varName = m_driver->findOrInsertWidget(node);
-    m_registeredWidgets.insert(varName, node); // register the current widget
 
     QString parentWidget, parentClass;
     if (m_widgetChain.top()) {
@@ -828,15 +821,13 @@ void WriteInitialization::acceptWidget(DomWidget *node)
 
     const QStringList zOrder = node->elementZOrder();
     for (const QString &name : zOrder) {
-        if (!m_registeredWidgets.contains(name)) {
+        const QString varName = m_driver->widgetVariableName(name);
+        if (varName.isEmpty()) {
             fprintf(stderr, "%s: Warning: Z-order assignment: '%s' is not a valid widget.\n",
                     qPrintable(m_option.messagePrefix()),
                     name.toLatin1().data());
-            continue;
+        } else {
-        }
+            m_output << m_indent << varName << language::derefPointer << "raise()"
-
-        if (!name.isEmpty()) {
-            m_output << m_indent << name << language::derefPointer << "raise()"
                 << language::eol;
         }
     }
@@ -1080,7 +1071,6 @@ void WriteInitialization::acceptAction(DomAction *node)
         return;
 
     const QString actionName = m_driver->findOrInsertAction(node);
-    m_registeredActions.insert(actionName, node);
     QString varName = m_driver->findOrInsertWidget(m_widgetChain.top());
 
     if (m_actionGroupChain.top())
@@ -2007,12 +1997,11 @@ void WriteInitialization::acceptTabStops(DomTabStops *tabStops)
 
     const QStringList l = tabStops->elementTabStop();
     for (int i=0; i<l.size(); ++i) {
-        const QString &name = l.at(i);
+        const QString name = m_driver->widgetVariableName(l.at(i));
 
-        if (!m_registeredWidgets.contains(name)) {
+        if (name.isEmpty()) {
             fprintf(stderr, "%s: Warning: Tab-stop assignment: '%s' is not a valid widget.\n",
-                    qPrintable(m_option.messagePrefix()),
+                    qPrintable(m_option.messagePrefix()), qPrintable(l.at(i)));
-                    name.toLatin1().data());
             continue;
         }
 
@@ -2023,7 +2012,8 @@ void WriteInitialization::acceptTabStops(DomTabStops *tabStops)
         if (name.isEmpty() || lastName.isEmpty())
             continue;
 
-        m_output << m_indent << "QWidget::setTabOrder(" << lastName << ", " << name << ");\n";
+        m_output << m_indent << "QWidget" << language::qualifier << "setTabOrder("
+            << lastName << ", " << name << ')' << language::eol;
 
         lastName = name;
     }

src/tools/uic/cpp/cppwriteinitialization.h

@@ -248,8 +248,8 @@ private:
 
     struct Buddy
     {
-        QString objName;
+        QString labelVarName;
-        QString buddy;
+        QString buddyAttributeName;
     };
     friend class QTypeInfo<Buddy>;
 
@@ -259,8 +259,6 @@ private:
     QVector<Buddy> m_buddies;
 
     QSet<QString> m_buttonGroups;
-    QHash<QString, DomWidget*> m_registeredWidgets;
-    QHash<QString, DomAction*> m_registeredActions;
     typedef QHash<uint, QString> ColorBrushHash;
     ColorBrushHash m_colorBrushHash;
     // Map from font properties to  font variable name for reuse

src/tools/uic/driver.cpp

@@ -52,15 +52,25 @@ static inline QString actionGroupClass() { return QStringLiteral("QActionGroup")
 static inline QString actionClass()      { return QStringLiteral("QAction"); }
 static inline QString buttonGroupClass() { return QStringLiteral("QButtonGroup"); }
 
+template <class DomClass>
+Driver::DomObjectHashConstIt<DomClass>
+    Driver::findByAttributeNameIt(const DomObjectHash<DomClass> &domHash,
+                                  const QString &name) const
+{
+    const auto end = domHash.cend();
+    for (auto it = domHash.cbegin(); it != end; ++it) {
+        if (it.key()->attributeName() == name)
+            return it;
+    }
+    return end;
+}
+
 template <class DomClass>
 const DomClass *Driver::findByAttributeName(const DomObjectHash<DomClass> &domHash,
                                             const QString &name) const
 {
-    for (auto it = domHash.cbegin(), end = domHash.cend(); it != end; ++it) {
+    auto it = findByAttributeNameIt(domHash, name);
-        if (it.key()->attributeName() == name)
+    return it != domHash.cend() ? it.key() : nullptr;
-            return it.key();
-   }
-    return nullptr;
 }
 
 template <class DomClass>
@@ -299,19 +309,25 @@ bool Driver::uic(const QString &fileName, QTextStream *out)
     return rtn;
 }
 
-const DomWidget *Driver::widgetByName(const QString &name) const
+const DomWidget *Driver::widgetByName(const QString &attributeName) const
 {
-    return findByAttributeName(m_widgets, name);
+    return findByAttributeName(m_widgets, attributeName);
 }
 
-const DomActionGroup *Driver::actionGroupByName(const QString &name) const
+QString Driver::widgetVariableName(const QString &attributeName) const
 {
-    return findByAttributeName(m_actionGroups, name);
+    auto it = findByAttributeNameIt(m_widgets, attributeName);
+    return it != m_widgets.cend() ? it.value() : QString();
 }
 
-const DomAction *Driver::actionByName(const QString &name) const
+const DomActionGroup *Driver::actionGroupByName(const QString &attributeName) const
 {
-    return findByAttributeName(m_actions, name);
+    return findByAttributeName(m_actionGroups, attributeName);
+}
+
+const DomAction *Driver::actionByName(const QString &attributeName) const
+{
+    return findByAttributeName(m_actions, attributeName);
 }
 
 QT_END_NAMESPACE

src/tools/uic/driver.h

@@ -84,17 +84,23 @@ public:
     // Find a group by its non-uniqified name
     const DomButtonGroup *findButtonGroup(const QString &attributeName) const;
 
-    const DomWidget *widgetByName(const QString &name) const;
+    const DomWidget *widgetByName(const QString &attributeName) const;
-    const DomActionGroup *actionGroupByName(const QString &name) const;
+    QString widgetVariableName(const QString &attributeName) const;
-    const DomAction *actionByName(const QString &name) const;
+    const DomActionGroup *actionGroupByName(const QString &attributeName) const;
+    const DomAction *actionByName(const QString &attributeName) const;
 
     bool useIdBasedTranslations() const { return m_idBasedTranslations; }
     void setUseIdBasedTranslations(bool u) { m_idBasedTranslations = u; }
 
 private:
     template <class DomClass> using DomObjectHash = QHash<const DomClass *, QString>;
+    template <class DomClass> using DomObjectHashConstIt =
+        typename DomObjectHash<DomClass>::ConstIterator;
 
     template <class DomClass>
+    DomObjectHashConstIt<DomClass> findByAttributeNameIt(const DomObjectHash<DomClass> &domHash,
+                                                         const QString &name) const;
+    template <class DomClass>
     const DomClass *findByAttributeName(const DomObjectHash<DomClass> &domHash,
                                         const QString &name) const;
     template <class DomClass>