uic: use qOverload for ambiguous slots

qOverload was used only for ambiguous signals and slots with no
arguments. Connections to ambiguous slots made in widget designer's
signal/slot editor, lead to an unresolved ambiguity when compiling
the generated code in ui_xxx.h. One example is QLabel::setNum(), which
can either take a double or an int.

Always using qOverload is not an alternative, because signatures are
normalised. E.g. const QString & / QString can't be distinguished.

Add a map with all ambiguous signals and slots to customwidgetsinfo.cpp.
Hardcode the map, because uic lives in QtCore and can't access meta
objects of gui/widget classes.

Use this map in isAmbiguousSignal() and add isAmbiguousSlot().

Change OverloadUse enum into an enum class in language.cpp. Change
enum values to reflect the new condition for slots to use qOverload.

Rename existing enum values and remove default argument in
language::formatMemberFnPtr() to improve code readability.
Apply const T& syntax for all types starting with Q, except QSize and
QPoint.

Revert 94c16517b3f8f01309a89598e698931ef77d60db, which was a hack
forcing for QLCDNumber::display() to a string based connection.

Add a new baseline to tst_uic, to test the above.

Fixes: QTBUG-93413
Task-number: QTBUG-124241
Pick-to: 6.7 6.6 6.5 6.2
Change-Id: I49ccc1688cfc08970cce3e56adf18e5ac49a77e1
Reviewed-by: Friedemann Kleint <Friedemann.Kleint@qt.io>

4 files changed, 144 insertions, 29 deletions

diff --git a/src/tools/uic/cpp/cppwriteinitialization.cpp b/src/tools/uic/cpp/cppwriteinitialization.cpp
index c1e3e98f43..893cc5b8ec 100644
--- a/src/tools/uic/cpp/cppwriteinitialization.cpp
+++ b/src/tools/uic/cpp/cppwriteinitialization.cpp
@@ -2698,10 +2698,6 @@ ConnectionSyntax WriteInitialization::connectionSyntax(const language::SignalSlo
         return ConnectionSyntax::StringBased;
     }
 
-    // QTBUG-110952, ambiguous overloads of display()
-    if (receiver.className == u"QLCDNumber" && receiver.signature.startsWith(u"display("))
-        return ConnectionSyntax::StringBased;
-
     if ((sender.name == m_mainFormVarName && m_customSignals.contains(sender.signature))
          || (receiver.name == m_mainFormVarName && m_customSlots.contains(receiver.signature))) {
         return ConnectionSyntax::StringBased;
@@ -2729,14 +2725,21 @@ void WriteInitialization::acceptConnection(DomConnection *connection)
         return;
     }
     const QString senderSignature = connection->elementSignal();
+    const QString slotSignature = connection->elementSlot();
+    const bool senderAmbiguous = m_uic->customWidgetsInfo()->isAmbiguousSignal(senderDecl.className,
+                                                                               senderSignature);
+    const bool slotAmbiguous = m_uic->customWidgetsInfo()->isAmbiguousSlot(receiverDecl.className,
+                                                                           slotSignature);
+
     language::SignalSlotOptions signalOptions;
-    if (m_uic->customWidgetsInfo()->isAmbiguousSignal(senderDecl.className, senderSignature))
-        signalOptions.setFlag(language::SignalSlotOption::Ambiguous);
+    signalOptions.setFlag(language::SignalSlotOption::Ambiguous, senderAmbiguous);
+    language::SignalSlotOptions slotOptions;
+    slotOptions.setFlag(language::SignalSlotOption::Ambiguous, slotAmbiguous);
 
     language::SignalSlot theSignal{senderDecl.name, senderSignature,
                                    senderDecl.className, signalOptions};
-    language::SignalSlot theSlot{receiverDecl.name, connection->elementSlot(),
-                                 receiverDecl.className, {}};
+    language::SignalSlot theSlot{receiverDecl.name, slotSignature,
+                                 receiverDecl.className, slotOptions};
 
     m_output << m_indent;
     language::formatConnection(m_output, theSignal, theSlot,
diff --git a/src/tools/uic/customwidgetsinfo.cpp b/src/tools/uic/customwidgetsinfo.cpp
index 169cdad618..6ec418634c 100644
--- a/src/tools/uic/customwidgetsinfo.cpp
+++ b/src/tools/uic/customwidgetsinfo.cpp
@@ -78,19 +78,89 @@ bool CustomWidgetsInfo::isCustomWidgetContainer(const QString &className) const
     return false;
 }
 
+// FIXME in 7.0 - QTBUG-124241
+// Remove isAmbiguous logic when widget slots have been disambiguated.
+bool CustomWidgetsInfo::isAmbiguous(const QString &className, const QString &signature,
+                                    QMetaMethod::MethodType type) const
+{
+    using TypeMap = QHash<QString, QMetaMethod::MethodType>;
+    struct AmbiguousInClass {
+        QLatin1StringView className;
+        TypeMap methodMap;
+    };
+
+    static const QList<AmbiguousInClass> ambiguousList = {
+
+        {"QAction"_L1, {{"triggered"_L1, QMetaMethod::Signal}}},
+        {"QCommandLinkButton"_L1, {{"triggered"_L1, QMetaMethod::Signal},
+                                   {"clicked"_L1, QMetaMethod::Signal}}},
+        {"QPushButton"_L1, {{"triggered"_L1, QMetaMethod::Signal},
+                            {"clicked"_L1, QMetaMethod::Signal}}},
+        {"QCheckBox"_L1, {{"triggered"_L1, QMetaMethod::Signal},
+                          {"clicked"_L1, QMetaMethod::Signal}}},
+        {"QRadioButton"_L1, {{"triggered"_L1, QMetaMethod::Signal},
+                             {"clicked"_L1, QMetaMethod::Signal}}},
+        {"QToolButton"_L1,  {{"triggered"_L1, QMetaMethod::Signal},
+                            {"clicked"_L1, QMetaMethod::Signal}}},
+        {"QLabel"_L1, {{"setNum"_L1, QMetaMethod::Slot}}},
+        {"QGraphicsView"_L1, {{"invalidateScene"_L1, QMetaMethod::Slot}}},
+        {"QListView"_L1, {{"dataChanged"_L1, QMetaMethod::Slot}}},
+        {"QColumnView"_L1, {{"dataChanged"_L1, QMetaMethod::Slot}}},
+        {"QListWidget"_L1, {{"dataChanged"_L1, QMetaMethod::Slot},
+                           {"scrollToItem"_L1, QMetaMethod::Slot}}},
+        {"QTableView"_L1, {{"dataChanged"_L1, QMetaMethod::Slot}}},
+        {"QTableWidget"_L1, {{"dataChanged"_L1, QMetaMethod::Slot},
+                            {"scrollToItem"_L1, QMetaMethod::Slot}}},
+        {"QTreeView"_L1, {{"dataChanged"_L1, QMetaMethod::Slot},
+                         {"verticalScrollbarValueChanged"_L1, QMetaMethod::Slot},
+                         {"expandRecursively"_L1, QMetaMethod::Slot}}},
+        {"QTreeWidget"_L1, {{"dataChanged"_L1, QMetaMethod::Slot},
+                           {"verticalScrollbarValueChanged"_L1, QMetaMethod::Slot}
+                           ,{"expandRecursively"_L1, QMetaMethod::Slot}
+                           ,{"scrollToItem"_L1, QMetaMethod::Slot}}},
+        {"QUndoView"_L1, {{"dataChanged"_L1, QMetaMethod::Slot}}},
+        {"QLCDNumber"_L1, {{"display"_L1, QMetaMethod::Slot}}},
+        {"QMenuBar"_L1, {{"setVisible"_L1, QMetaMethod::Slot}}},
+        {"QTextBrowser"_L1, {{"setSource"_L1, QMetaMethod::Slot}}},
+
+        /*
+        The following widgets with ambiguities are not used in the widget designer:
+
+        {"QSplashScreen"_L1, {{"showMessage"_L1, QMetaMethod::Slot}}},
+        {"QCompleter"_L1, {{"activated"_L1, QMetaMethod::Signal},
+                          {"highlighted"_L1, QMetaMethod::Signal}}},
+        {"QSystemTrayIcon"_L1, {{"showMessage"_L1, QMetaMethod::Slot}}},
+        {"QStyledItemDelegate"_L1, {{"closeEditor"_L1, QMetaMethod::Signal}}},
+        {"QErrorMessage"_L1, {{"showMessage"_L1, QMetaMethod::Slot}}},
+        {"QGraphicsDropShadowEffect"_L1, {{"setOffset"_L1, QMetaMethod::Slot}}},
+        {"QGraphicsScene"_L1, {{"invalidate"_L1, QMetaMethod::Slot}}},
+        {"QItemDelegate"_L1, {{"closeEditor"_L1, QMetaMethod::Signal}}}
+        */
+    };
+
+    for (auto it = ambiguousList.constBegin(); it != ambiguousList.constEnd(); ++it) {
+        if (extends(className, it->className)) {
+            const qsizetype pos = signature.indexOf(u'(');
+            const QString method = signature.left(pos);
+            const auto methodIterator = it->methodMap.find(method);
+            return methodIterator != it->methodMap.constEnd() && type == methodIterator.value();
+        }
+    }
+    return false;
+}
+
 // Is it ambiguous, resulting in different signals for Python
 // "QAbstractButton::clicked(checked=false)"
 bool CustomWidgetsInfo::isAmbiguousSignal(const QString &className,
                                           const QString &signalSignature) const
 {
-    if (signalSignature.startsWith(u"triggered") && extends(className, "QAction"))
-        return true;
-    if (signalSignature.startsWith(u"clicked(")
-        && extendsOneOf(className, {u"QCommandLinkButton"_s, u"QCheckBox"_s,
-                                    u"QPushButton"_s, u"QRadioButton"_s, u"QToolButton"_s})) {
-        return true;
-    }
-    return false;
+    return isAmbiguous(className, signalSignature, QMetaMethod::Signal);
+}
+
+bool CustomWidgetsInfo::isAmbiguousSlot(const QString &className,
+                                        const QString &signalSignature) const
+{
+    return isAmbiguous(className, signalSignature, QMetaMethod::Slot);
 }
 
 QString CustomWidgetsInfo::realClassName(const QString &className) const
diff --git a/src/tools/uic/customwidgetsinfo.h b/src/tools/uic/customwidgetsinfo.h
index 4bd004bdc7..f336292f2a 100644
--- a/src/tools/uic/customwidgetsinfo.h
+++ b/src/tools/uic/customwidgetsinfo.h
@@ -7,6 +7,7 @@
 #include "treewalker.h"
 #include <qstringlist.h>
 #include <qmap.h>
+#include <QtCore/qmetaobject.h>
 
 QT_BEGIN_NAMESPACE
 
@@ -38,10 +39,14 @@ public:
 
     bool isAmbiguousSignal(const QString &className,
                            const QString &signalSignature) const;
+    bool isAmbiguousSlot(const QString &className,
+                         const QString &slotSignature) const;
 
 private:
     using NameCustomWidgetMap = QMap<QString, DomCustomWidget*>;
     NameCustomWidgetMap m_customWidgets;
+    bool isAmbiguous(const QString &className, const QString &signature,
+                     QMetaMethod::MethodType type) const;
 };
 
 QT_END_NAMESPACE
diff --git a/src/tools/uic/shared/language.cpp b/src/tools/uic/shared/language.cpp
index bfdce0bc4c..d59688e346 100644
--- a/src/tools/uic/shared/language.cpp
+++ b/src/tools/uic/shared/language.cpp
@@ -4,6 +4,7 @@
 #include "language.h"
 
 #include <QtCore/qtextstream.h>
+#include <QtCore/QList>
 
 namespace language {
 
@@ -370,17 +371,40 @@ void _formatStackVariable(QTextStream &str, const char *className, QStringView v
     }
 }
 
-enum OverloadUse {
-    UseOverload,
-    UseOverloadWhenNoArguments, // Use overload only when the argument list is empty,
-                                // in this case there is no chance of connecting
-                                // mismatching T against const T &
-    DontUseOverload
+enum class OverloadUse {
+    Always,
+    WhenAmbiguousOrEmpty, // Use overload if
+                          // - signal/slot is ambiguous
+                          // - argument list is empty (chance of connecting mismatching T against const T &)
+    Never,
 };
 
 // Format a member function for a signal slot connection
-static void formatMemberFnPtr(QTextStream &str, const SignalSlot &s,
-                              OverloadUse useQOverload = DontUseOverload)
+static bool isConstRef(const QStringView &arg)
+{
+    return arg.startsWith(u'Q') && arg != "QPoint"_L1 && arg != "QSize"_L1;
+}
+
+static QString formatOverload(const QStringView &parameters)
+{
+    QString result = "qOverload<"_L1;
+    const auto args = QStringView{parameters}.split(u',');
+    for (qsizetype i = 0, size = args.size(); i < size; ++i) {
+        const auto &arg = args.at(i);
+        if (i > 0)
+            result += u',';
+        const bool constRef = isConstRef(arg);
+        if (constRef)
+            result += "const "_L1;
+        result += arg;
+        if (constRef)
+            result += u'&';
+    }
+    result += u'>';
+    return result;
+}
+
+static void formatMemberFnPtr(QTextStream &str, const SignalSlot &s, OverloadUse useQOverload)
 {
     const qsizetype parenPos = s.signature.indexOf(u'(');
     Q_ASSERT(parenPos >= 0);
@@ -388,11 +412,24 @@ static void formatMemberFnPtr(QTextStream &str, const SignalSlot &s,
 
     const auto parameters = QStringView{s.signature}.mid(parenPos + 1,
                                                s.signature.size() - parenPos - 2);
-    const bool withOverload = useQOverload == UseOverload ||
-            (useQOverload == UseOverloadWhenNoArguments && parameters.isEmpty());
+
+    const bool isAmbiguous = s.options.testFlag(SignalSlotOption::Ambiguous);
+    bool withOverload = false; // just to silence the compiler
+
+    switch (useQOverload) {
+    case OverloadUse::Always:
+        withOverload = true;
+        break;
+    case OverloadUse::Never:
+        withOverload = false;
+        break;
+    case OverloadUse::WhenAmbiguousOrEmpty:
+        withOverload = parameters.empty() || isAmbiguous;
+        break;
+    }
 
     if (withOverload)
-        str << "qOverload<" << parameters << ">(";
+        str << formatOverload(parameters) << '(';
 
     str << '&' << s.className << "::" << functionName;
 
@@ -405,9 +442,9 @@ static void formatMemberFnPtrConnection(QTextStream &str,
                                         const SignalSlot &receiver)
 {
     str << "QObject::connect(" << sender.name << ", ";
-    formatMemberFnPtr(str, sender);
+    formatMemberFnPtr(str, sender, OverloadUse::Never);
     str << ", " << receiver.name << ", ";
-    formatMemberFnPtr(str, receiver, UseOverloadWhenNoArguments);
+    formatMemberFnPtr(str, receiver, OverloadUse::WhenAmbiguousOrEmpty);
     str << ')';
 }