From b86d0b62156993936bf93169a895a92ad60adf7d Mon Sep 17 00:00:00 2001 From: Friedemann Kleint Date: Fri, 2 Nov 2018 11:27:46 +0100 Subject: uic: Small refactorings - Do not use QString::number() to stream numbers. - Do not use QLatin1String/Char to stream strings or characters. - Add a convenience methods to determine the container page add method for simple containers. - Similarly, extract a method to determine the layout method and simplify the code accordingly. - Fix Clang warnings about else if after return/continue. - Use QString::isEmpty() instead of size() to check emptiness. - Fix QHash-contains()/value() Antipattern Task-number: PYSIDE-797 Change-Id: I9c61d20f46c8d142b947126a27faaf54b41f9e0c Reviewed-by: Cristian Maureira-Fredes Reviewed-by: Jarek Kobus --- src/tools/uic/cpp/cppwriteincludes.cpp | 4 +- src/tools/uic/cpp/cppwriteinitialization.cpp | 167 ++++++++++++--------------- src/tools/uic/customwidgetsinfo.cpp | 21 ++++ src/tools/uic/customwidgetsinfo.h | 1 + src/tools/uic/driver.cpp | 37 +++--- src/tools/uic/main.cpp | 2 +- src/tools/uic/uic.cpp | 4 +- 7 files changed, 116 insertions(+), 120 deletions(-) diff --git a/src/tools/uic/cpp/cppwriteincludes.cpp b/src/tools/uic/cpp/cppwriteincludes.cpp index d51fddffea..74546bb01d 100644 --- a/src/tools/uic/cpp/cppwriteincludes.cpp +++ b/src/tools/uic/cpp/cppwriteincludes.cpp @@ -119,7 +119,7 @@ void WriteIncludes::acceptUI(DomUI *node) writeHeaders(m_globalIncludes, true); writeHeaders(m_localIncludes, false); - m_output << QLatin1Char('\n'); + m_output << '\n'; } void WriteIncludes::acceptWidget(DomWidget *node) @@ -314,7 +314,7 @@ void WriteIncludes::writeHeaders(const OrderedSet &headers, bool global) const QString value = m_oldHeaderToNewHeader.value(header, header); const auto trimmed = QStringRef(&value).trimmed(); if (!trimmed.isEmpty()) - m_output << "#include " << openingQuote << trimmed << closingQuote << QLatin1Char('\n'); + m_output << "#include " << openingQuote << trimmed << closingQuote << '\n'; } } diff --git a/src/tools/uic/cpp/cppwriteinitialization.cpp b/src/tools/uic/cpp/cppwriteinitialization.cpp index 658cdd9567..ab4882d1a0 100644 --- a/src/tools/uic/cpp/cppwriteinitialization.cpp +++ b/src/tools/uic/cpp/cppwriteinitialization.cpp @@ -527,7 +527,8 @@ void WriteInitialization::acceptUI(DomUI *node) qPrintable(m_option.messagePrefix()), b.objName.toLatin1().data()); continue; - } else if (!m_registeredWidgets.contains(b.buddy)) { + } + 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()); @@ -542,7 +543,7 @@ void WriteInitialization::acceptUI(DomUI *node) if (node->elementTabStops()) acceptTabStops(node->elementTabStops()); - if (m_delayedActionInitialization.size()) + if (!m_delayedActionInitialization.isEmpty()) m_output << "\n" << m_delayedActionInitialization; m_output << "\n" << m_indent << "retranslateUi(" << varName << ");\n"; @@ -635,7 +636,8 @@ void WriteInitialization::acceptWidget(DomWidget *node) writeProperties(varName, className, node->elementProperty()); - if (m_uic->customWidgetsInfo()->extends(className, QLatin1String("QMenu")) && parentWidget.size()) { + if (!parentWidget.isEmpty() + && m_uic->customWidgetsInfo()->extends(className, QLatin1String("QMenu"))) { initializeMenu(node, parentWidget); } @@ -677,14 +679,10 @@ void WriteInitialization::acceptWidget(DomWidget *node) } } else if (m_uic->customWidgetsInfo()->extends(className, QLatin1String("QDockWidget"))) { - QString area; - if (DomProperty *pstyle = attributes.value(QLatin1String("dockWidgetArea"))) { - area += QLatin1String("static_cast("); - area += QString::number(pstyle->elementNumber()); - area += QLatin1String("), "); - } - - m_output << m_indent << parentWidget << "->addDockWidget(" << area << varName << ");\n"; + m_output << m_indent << parentWidget << "->addDockWidget("; + if (DomProperty *pstyle = attributes.value(QLatin1String("dockWidgetArea"))) + m_output << "static_cast(" << pstyle->elementNumber() << "), "; + m_output << varName << ");\n"; } else if (m_uic->customWidgetsInfo()->extends(className, QLatin1String("QStatusBar"))) { m_output << m_indent << parentWidget << "->setStatusBar(" << varName << ");\n"; } else { @@ -693,34 +691,21 @@ void WriteInitialization::acceptWidget(DomWidget *node) } // Check for addPageMethod of a custom plugin first - const QString addPageMethod = m_uic->customWidgetsInfo()->customWidgetAddPageMethod(parentClass); + QString addPageMethod = m_uic->customWidgetsInfo()->customWidgetAddPageMethod(parentClass); + if (addPageMethod.isEmpty()) + addPageMethod = m_uic->customWidgetsInfo()->simpleContainerAddPageMethod(parentClass); if (!addPageMethod.isEmpty()) { m_output << m_indent << parentWidget << "->" << addPageMethod << '(' << varName << ");\n"; - } else if (m_uic->customWidgetsInfo()->extends(parentClass, QLatin1String("QStackedWidget"))) { - m_output << m_indent << parentWidget << "->addWidget(" << varName << ");\n"; - } else if (m_uic->customWidgetsInfo()->extends(parentClass, QLatin1String("QToolBar"))) { - m_output << m_indent << parentWidget << "->addWidget(" << varName << ");\n"; - } else if (m_uic->customWidgetsInfo()->extends(parentClass, QLatin1String("QDockWidget"))) { - m_output << m_indent << parentWidget << "->setWidget(" << varName << ");\n"; - } else if (m_uic->customWidgetsInfo()->extends(parentClass, QLatin1String("QScrollArea"))) { - m_output << m_indent << parentWidget << "->setWidget(" << varName << ");\n"; - } else if (m_uic->customWidgetsInfo()->extends(parentClass, QLatin1String("QSplitter"))) { - m_output << m_indent << parentWidget << "->addWidget(" << varName << ");\n"; - } else if (m_uic->customWidgetsInfo()->extends(parentClass, QLatin1String("QMdiArea"))) { - m_output << m_indent << parentWidget << "->addSubWindow(" << varName << ");\n"; } else if (m_uic->customWidgetsInfo()->extends(parentClass, QLatin1String("QWizard"))) { addWizardPage(varName, node, parentWidget); } else if (m_uic->customWidgetsInfo()->extends(parentClass, QLatin1String("QToolBox"))) { - QString icon; - if (const DomProperty *picon = attributes.value(QLatin1String("icon"))) { - icon += QLatin1String(", ") ; - icon += iconCall(picon); - } - const DomProperty *plabel = attributes.value(QLatin1String("label")); DomString *plabelString = plabel ? plabel->elementString() : 0; - m_output << m_indent << parentWidget << "->addItem(" << varName << icon << ", " << noTrCall(plabelString, pageDefaultString) << ");\n"; + m_output << m_indent << parentWidget << "->addItem(" << varName; + if (const DomProperty *picon = attributes.value(QLatin1String("icon"))) + m_output << ", " << iconCall(picon); + m_output << ", " << noTrCall(plabelString, pageDefaultString) << ");\n"; autoTrOutput(plabelString, pageDefaultString) << m_indent << parentWidget << "->setItemText(" << parentWidget << "->indexOf(" << varName << "), " << autoTrCall(plabelString, pageDefaultString) << ");\n"; @@ -734,16 +719,13 @@ void WriteInitialization::acceptWidget(DomWidget *node) << language::closeQtConfig(toolTipConfigKey()); } } else if (m_uic->customWidgetsInfo()->extends(parentClass, QLatin1String("QTabWidget"))) { - QString icon; - if (const DomProperty *picon = attributes.value(QLatin1String("icon"))) { - icon += QLatin1String(", "); - icon += iconCall(picon); - } - const DomProperty *ptitle = attributes.value(QLatin1String("title")); DomString *ptitleString = ptitle ? ptitle->elementString() : 0; - m_output << m_indent << parentWidget << "->addTab(" << varName << icon << ", " << "QString());\n"; + m_output << m_indent << parentWidget << "->addTab(" << varName; + if (const DomProperty *picon = attributes.value(QLatin1String("icon"))) + m_output << ", " << iconCall(picon); + m_output << ", " << "QString());\n"; autoTrOutput(ptitleString, pageDefaultString) << m_indent << parentWidget << "->setTabText(" << parentWidget << "->indexOf(" << varName << "), " << autoTrCall(ptitleString, pageDefaultString) << ");\n"; @@ -989,6 +971,24 @@ static inline QString formLayoutRole(int column, int colspan) return column == 0 ? QLatin1String("QFormLayout::LabelRole") : QLatin1String("QFormLayout::FieldRole"); } +static QString layoutAddMethod(DomLayoutItem::Kind kind, const QString &layoutClass) +{ + const QString methodPrefix = layoutClass == QLatin1String("QFormLayout") + ? QLatin1String("set") : QLatin1String("add"); + switch (kind) { + case DomLayoutItem::Widget: + return methodPrefix + QLatin1String("Widget"); + case DomLayoutItem::Layout: + return methodPrefix + QLatin1String("Layout"); + case DomLayoutItem::Spacer: + return methodPrefix + QLatin1String("Item"); + case DomLayoutItem::Unknown: + Q_ASSERT( false ); + break; + } + Q_UNREACHABLE(); +} + void WriteInitialization::acceptLayoutItem(DomLayoutItem *node) { TreeWalker::acceptLayoutItem(node); @@ -1001,47 +1001,27 @@ void WriteInitialization::acceptLayoutItem(DomLayoutItem *node) const QString layoutName = m_driver->findOrInsertLayout(layout); const QString itemName = m_driver->findOrInsertLayoutItem(node); - QString addArgs; - QString methodPrefix = QLatin1String("add"); //Consistent API-design galore! + m_output << "\n" << m_indent << layoutName << "->" + << layoutAddMethod(node->kind(), layout->attributeClass()) << '('; + if (layout->attributeClass() == QLatin1String("QGridLayout")) { const int row = node->attributeRow(); const int col = node->attributeColumn(); const int rowSpan = node->hasAttributeRowSpan() ? node->attributeRowSpan() : 1; const int colSpan = node->hasAttributeColSpan() ? node->attributeColSpan() : 1; - - addArgs = QString::fromLatin1("%1, %2, %3, %4, %5").arg(itemName).arg(row).arg(col).arg(rowSpan).arg(colSpan); + m_output << itemName << ", " << row << ", " << col << ", " << rowSpan << ", " << colSpan; if (!node->attributeAlignment().isEmpty()) - addArgs += QLatin1String(", ") + node->attributeAlignment(); + m_output << ", " << node->attributeAlignment(); + } else if (layout->attributeClass() == QLatin1String("QFormLayout")) { + const int row = node->attributeRow(); + const int colSpan = node->hasAttributeColSpan() ? node->attributeColSpan() : 1; + const QString role = formLayoutRole(node->attributeColumn(), colSpan); + m_output << row << ", " << role << ", " << itemName; } else { - if (layout->attributeClass() == QLatin1String("QFormLayout")) { - methodPrefix = QLatin1String("set"); - const int row = node->attributeRow(); - const int colSpan = node->hasAttributeColSpan() ? node->attributeColSpan() : 1; - const QString role = formLayoutRole(node->attributeColumn(), colSpan); - addArgs = QString::fromLatin1("%1, %2, %3").arg(row).arg(role, itemName); - } else { - addArgs = itemName; - if (layout->attributeClass().contains(QLatin1String("Box")) && !node->attributeAlignment().isEmpty()) - addArgs += QLatin1String(", 0, ") + node->attributeAlignment(); - } - } - - // figure out "add" method - m_output << "\n" << m_indent << layoutName << "->"; - switch (node->kind()) { - case DomLayoutItem::Widget: - m_output << methodPrefix << "Widget(" << addArgs; - break; - case DomLayoutItem::Layout: - m_output << methodPrefix << "Layout(" << addArgs; - break; - case DomLayoutItem::Spacer: - m_output << methodPrefix << "Item(" << addArgs; - break; - case DomLayoutItem::Unknown: - Q_ASSERT( 0 ); - break; + m_output << itemName; + if (layout->attributeClass().contains(QLatin1String("Box")) && !node->attributeAlignment().isEmpty()) + m_output << ", 0, " << node->attributeAlignment(); } m_output << ");\n\n"; } @@ -1081,16 +1061,16 @@ void WriteInitialization::acceptAction(DomAction *node) void WriteInitialization::acceptActionRef(DomActionRef *node) { QString actionName = node->attributeName(); + if (actionName.isEmpty() || !m_widgetChain.top() + || m_driver->actionGroupByName(actionName)) { + return; + } + + const QString varName = m_driver->findOrInsertWidget(m_widgetChain.top()); const bool isSeparator = actionName == QLatin1String("separator"); bool isMenu = false; - QString varName = m_driver->findOrInsertWidget(m_widgetChain.top()); - - if (actionName.isEmpty() || !m_widgetChain.top()) { - return; - } else if (m_driver->actionGroupByName(actionName)) { - return; - } else if (const DomWidget *w = m_driver->widgetByName(actionName)) { + if (const DomWidget *w = m_driver->widgetByName(actionName)) { isMenu = m_uic->isMenu(w->attributeClass()); } else if (!(m_driver->actionByName(actionName) || isSeparator)) { fprintf(stderr, "%s: Warning: action `%s' not declared\n", @@ -1483,7 +1463,7 @@ void WriteInitialization::writeProperties(const QString &varName, break; } - if (propertyValue.size()) { + if (!propertyValue.isEmpty()) { const QString configKey = configKeyForProperty(propertyName); QTextStream &o = delayProperty ? m_delayedOut : autoTrOutput(p); @@ -1505,11 +1485,9 @@ void WriteInitialization::writeProperties(const QString &varName, } } if (leftMargin != -1 || topMargin != -1 || rightMargin != -1 || bottomMargin != -1) { - m_output << m_indent << varName << QLatin1String("->setContentsMargins(") - << leftMargin << QLatin1String(", ") - << topMargin << QLatin1String(", ") - << rightMargin << QLatin1String(", ") - << bottomMargin << QLatin1String(");\n"); + m_output << m_indent << varName << "->setContentsMargins(" + << leftMargin << ", " << topMargin << ", " + << rightMargin << ", " << bottomMargin << ");\n"; } } @@ -1789,7 +1767,7 @@ void WriteInitialization::writeColorGroup(DomColorGroup *colorGroup, const QStri const DomColor *color = colors.at(i); m_output << m_indent << paletteName << ".setColor(" << group - << ", " << "static_cast(" << QString::number(i) << ')' + << ", " << "static_cast(" << i << ')' << ", " << domColor2QString(color) << ");\n"; } @@ -2055,10 +2033,15 @@ void WriteInitialization::enableSorting(DomWidget *w, const QString &varName, co void WriteInitialization::addInitializer(Item *item, const QString &name, int column, const QString &value, const QString &directive, bool translatable) const { - if (!value.isEmpty()) - item->addSetter(QLatin1String("->set") + name.at(0).toUpper() + name.midRef(1) + - QLatin1Char('(') + (column < 0 ? QString() : QString::number(column) + - QLatin1String(", ")) + value + QLatin1String(");"), directive, translatable); + if (!value.isEmpty()) { + QString setter; + QTextStream str(&setter); + str << "->set" << name.at(0).toUpper() << name.midRef(1) << '('; + if (column >= 0) + str << column << ", "; + str << value << ");"; + item->addSetter(setter, directive, translatable); + } } /*! @@ -2285,7 +2268,7 @@ void WriteInitialization::initializeTableWidget(DomWidget *w) QString itemName = item.writeSetupUi(QString(), Item::ConstructItemAndVariable); item.writeRetranslateUi(varName + QLatin1String("->horizontalHeaderItem(") + QString::number(i) + QLatin1Char(')')); - m_output << m_indent << varName << "->setHorizontalHeaderItem(" << QString::number(i) << ", " << itemName << ");\n"; + m_output << m_indent << varName << "->setHorizontalHeaderItem(" << i << ", " << itemName << ");\n"; } } @@ -2307,7 +2290,7 @@ void WriteInitialization::initializeTableWidget(DomWidget *w) QString itemName = item.writeSetupUi(QString(), Item::ConstructItemAndVariable); item.writeRetranslateUi(varName + QLatin1String("->verticalHeaderItem(") + QString::number(i) + QLatin1Char(')')); - m_output << m_indent << varName << "->setVerticalHeaderItem(" << QString::number(i) << ", " << itemName << ");\n"; + m_output << m_indent << varName << "->setVerticalHeaderItem(" << i << ", " << itemName << ");\n"; } } @@ -2328,7 +2311,7 @@ void WriteInitialization::initializeTableWidget(DomWidget *w) QString itemName = item.writeSetupUi(QString(), Item::ConstructItemAndVariable); item.writeRetranslateUi(varName + QLatin1String("->item(") + QString::number(r) + QLatin1String(", ") + QString::number(c) + QLatin1Char(')')); - m_output << m_indent << varName << "->setItem(" << QString::number(r) << ", " << QString::number(c) << ", " << itemName << ");\n"; + m_output << m_indent << varName << "->setItem(" << r << ", " << c << ", " << itemName << ");\n"; } } enableSorting(w, varName, tempName); diff --git a/src/tools/uic/customwidgetsinfo.cpp b/src/tools/uic/customwidgetsinfo.cpp index 0ac0c2b6a3..4afdf74d08 100644 --- a/src/tools/uic/customwidgetsinfo.cpp +++ b/src/tools/uic/customwidgetsinfo.cpp @@ -31,6 +31,8 @@ #include "ui4.h" #include "utils.h" +#include + QT_BEGIN_NAMESPACE CustomWidgetsInfo::CustomWidgetsInfo() = default; @@ -96,5 +98,24 @@ QString CustomWidgetsInfo::customWidgetAddPageMethod(const QString &name) const return QString(); } +// add page methods for simple containers taking only the widget parameter +QString CustomWidgetsInfo::simpleContainerAddPageMethod(const QString &name) const +{ + using AddPageMethod = std::pair; + + static AddPageMethod addPageMethods[] = { + {"QStackedWidget", "addWidget"}, + {"QToolBar", "addWidget"}, + {"QDockWidget", "setWidget"}, + {"QScrollArea", "setWidget"}, + {"QSplitter", "addWidget"}, + {"QMdiArea", "addSubWindow"} + }; + for (const auto &m : addPageMethods) { + if (extends(name, QLatin1String(m.first))) + return QLatin1String(m.second); + } + return QString(); +} QT_END_NAMESPACE diff --git a/src/tools/uic/customwidgetsinfo.h b/src/tools/uic/customwidgetsinfo.h index 82e86a1266..a4278b1aca 100644 --- a/src/tools/uic/customwidgetsinfo.h +++ b/src/tools/uic/customwidgetsinfo.h @@ -52,6 +52,7 @@ public: { return m_customWidgets.value(name); } QString customWidgetAddPageMethod(const QString &name) const; + QString simpleContainerAddPageMethod(const QString &name) const; QString realClassName(const QString &className) const; diff --git a/src/tools/uic/driver.cpp b/src/tools/uic/driver.cpp index 748be9cece..cf7e67305b 100644 --- a/src/tools/uic/driver.cpp +++ b/src/tools/uic/driver.cpp @@ -33,6 +33,8 @@ #include #include +#include + QT_BEGIN_NAMESPACE Driver::Driver() @@ -136,11 +138,9 @@ QString Driver::findOrInsertName(const QString &name) QString Driver::normalizedName(const QString &name) { QString result = name; - QChar *data = result.data(); - for (int i = name.size(); --i >= 0; ++data) { - if (!data->isLetterOrNumber()) - *data = QLatin1Char('_'); - } + std::replace_if(result.begin(), result.end(), + [] (QChar c) { return !c.isLetterOrNumber(); }, + QLatin1Char('_')); return result; } @@ -149,23 +149,21 @@ QString Driver::unique(const QString &instanceName, const QString &className) QString name; bool alreadyUsed = false; - if (instanceName.size()) { - int id = 1; - name = instanceName; - name = normalizedName(name); + if (!instanceName.isEmpty()) { + name = normalizedName(instanceName); QString base = name; - while (m_nameRepository.contains(name)) { + for (int id = 1; m_nameRepository.contains(name); ++id) { alreadyUsed = true; - name = base + QString::number(id++); + name = base + QString::number(id); } - } else if (className.size()) { + } else if (!className.isEmpty()) { name = unique(qtify(className)); } else { name = unique(QLatin1String("var")); } - if (alreadyUsed && className.size()) { + if (alreadyUsed && !className.isEmpty()) { fprintf(stderr, "%s: Warning: The name '%s' (%s) is already in use, defaulting to '%s'.\n", qPrintable(m_option.messagePrefix()), qPrintable(instanceName), qPrintable(className), @@ -181,17 +179,10 @@ QString Driver::qtify(const QString &name) QString qname = name; if (qname.at(0) == QLatin1Char('Q') || qname.at(0) == QLatin1Char('K')) - qname = qname.mid(1); + qname.remove(0, 1); - int i=0; - while (i < qname.length()) { - if (qname.at(i).toLower() != qname.at(i)) - qname[i] = qname.at(i).toLower(); - else - break; - - ++i; - } + for (int i = 0, size = qname.size(); i < size && qname.at(i).isUpper(); ++i) + qname[i] = qname.at(i).toLower(); return qname; } diff --git a/src/tools/uic/main.cpp b/src/tools/uic/main.cpp index 41bd62bbf4..0516b854ff 100644 --- a/src/tools/uic/main.cpp +++ b/src/tools/uic/main.cpp @@ -131,7 +131,7 @@ int runUic(int argc, char *argv[]) QTextStream *out = 0; QFile f; - if (driver.option().outputFile.size()) { + if (!driver.option().outputFile.isEmpty()) { f.setFileName(driver.option().outputFile); if (!f.open(QIODevice::WriteOnly | QFile::Text)) { fprintf(stderr, "Could not create output file\n"); diff --git a/src/tools/uic/uic.cpp b/src/tools/uic/uic.cpp index 314989fbf9..b426d33e5c 100644 --- a/src/tools/uic/uic.cpp +++ b/src/tools/uic/uic.cpp @@ -106,13 +106,13 @@ bool Uic::printDependencies() void Uic::writeCopyrightHeader(DomUI *ui) { QString comment = ui->elementComment(); - if (comment.size()) + if (!comment.isEmpty()) out << "/*\n" << comment << "\n*/\n\n"; out << "/********************************************************************************\n"; out << "** Form generated from reading UI file '" << QFileInfo(opt.inputFile).fileName() << "'\n"; out << "**\n"; - out << "** Created by: Qt User Interface Compiler version " << QLatin1String(QT_VERSION_STR) << "\n"; + out << "** Created by: Qt User Interface Compiler version " << QT_VERSION_STR << "\n"; out << "**\n"; out << "** WARNING! All changes made in this file will be lost when recompiling UI file!\n"; out << "********************************************************************************/\n\n"; -- cgit v1.2.3