Designer: Insert into correct class for "Go to slot"

Make use of LookupContext to find the right class.

Task-number: QTCREATORBUG-10348

Change-Id: I7f8ec769ff2239d5123726e562a1bd430f8c4567
Reviewed-by: Friedemann Kleint <Friedemann.Kleint@digia.com>
Reviewed-by: Erik Verbruggen <erik.verbruggen@digia.com>

src/plugins/designer/gotoslot_test.cpp

@@ -59,6 +59,9 @@ namespace {
 
 class MyTestDataDir : public Core::Internal::Tests::TestDataDir {
 public:
+    MyTestDataDir()
+        : TestDataDir(QString())
+    {}
     MyTestDataDir(const QString &dir)
         : TestDataDir(QLatin1String(SRCDIR "/../../../tests/designer/") + dir)
     {}
@@ -248,7 +251,7 @@ private:
 #endif
 
 /// Check: Executes "Go To Slot..." on a QPushButton in a *.ui file and checks if the respective
-/// header and source files are updated.
+/// header and source files are correctly updated.
 void Designer::Internal::FormEditorPlugin::test_gotoslot()
 {
 #if QT_VERSION >= 0x050000
@@ -267,11 +270,39 @@ void Designer::Internal::FormEditorPlugin::test_gotoslot_data()
     typedef QLatin1String _;
     QTest::addColumn<QStringList>("files");
 
-    MyTestDataDir testData(QLatin1String("gotoslot_withoutProject"));
+    MyTestDataDir testDataDirWithoutProject(_("gotoslot_withoutProject"));
     QTest::newRow("withoutProject")
         << (QStringList()
-            << testData.file(_("form.cpp"))
+            << testDataDirWithoutProject.file(_("form.cpp"))
-            << testData.file(_("form.h"))
+            << testDataDirWithoutProject.file(_("form.h"))
-            << testData.file(_("form.ui")));
+            << testDataDirWithoutProject.file(_("form.ui")));
+
+    // Finding the right class for inserting definitions/declarations is based on
+    // finding a class with a member whose type is the class from the "ui_xxx.h" header.
+    // In the following test data the header files contain an extra class referencing
+    // the same class name.
+
+    MyTestDataDir testDataDir;
+
+    testDataDir = MyTestDataDir(_("gotoslot_insertIntoCorrectClass_pointer"));
+    QTest::newRow("insertIntoCorrectClass_pointer")
+        << (QStringList()
+            << testDataDir.file(_("form.cpp"))
+            << testDataDir.file(_("form.h"))
+            << testDataDirWithoutProject.file(_("form.ui"))); // reuse
+
+    testDataDir = MyTestDataDir(_("gotoslot_insertIntoCorrectClass_non-pointer"));
+    QTest::newRow("insertIntoCorrectClass_non-pointer")
+        << (QStringList()
+            << testDataDir.file(_("form.cpp"))
+            << testDataDir.file(_("form.h"))
+            << testDataDirWithoutProject.file(_("form.ui"))); // reuse
+
+    testDataDir = MyTestDataDir(_("gotoslot_insertIntoCorrectClass_pointer_ns_using"));
+    QTest::newRow("insertIntoCorrectClass_pointer_ns_using")
+        << (QStringList()
+            << testDataDir.file(_("form.cpp"))
+            << testDataDir.file(_("form.h"))
+            << testDataDir.file(_("form.ui")));
 #endif
 }

src/plugins/designer/qtcreatorintegration.cpp

@@ -160,27 +160,26 @@ static bool inherits(const Overview &o, const Class *klass, const QString &baseC
     return false;
 }
 
-// Check for a class name where haystack is a member class of an object.
+QString fullyQualifiedName(const LookupContext &context, const Name *name, Scope *scope)
-// So, haystack can be shorter (can have some namespaces omitted because of a
-// "using namespace" declaration, for example, comparing
-// "foo::Ui::form", against "using namespace foo; Ui::form".
-
-static bool matchMemberClassName(const QString &needle, const QString &hayStack)
 {
-    if (needle == hayStack)
+    if (!name || !scope)
-        return true;
+        return QString();
-    if (!needle.endsWith(hayStack))
+
-        return false;
+    const QList<LookupItem> items = context.lookup(name, scope);
-    // Check if there really is a separator "::"
+    if (items.isEmpty()) { // "ui_xxx.h" might not be generated and nothing is forward declared.
-    const int separatorPos = needle.size() - hayStack.size() - 1;
+        return Overview().prettyName(name);
-    return separatorPos > 1 && needle.at(separatorPos) == QLatin1Char(':');
+    } else {
+        Symbol *symbol = items.first().declaration();
+        return Overview().prettyName(LookupContext::fullyQualifiedName(symbol));
+    }
+    return QString();
 }
 
 // Find class definition in namespace (that is, the outer class
 // containing a member of the desired class type) or inheriting the desired class
 // in case of forms using the Multiple Inheritance approach
-static const Class *findClass(const Namespace *parentNameSpace,
+static const Class *findClass(const Namespace *parentNameSpace, const LookupContext &context,
-                                         const QString &className, QString *namespaceName)
+                              const QString &className, QString *namespaceName)
 {
     if (Designer::Constants::Internal::debug)
         qDebug() << Q_FUNC_INFO << className;
@@ -194,16 +193,22 @@ static const Class *findClass(const Namespace *parentNameSpace,
             // 1) we go through class members
             const unsigned classMemberCount = cl->memberCount();
             for (unsigned j = 0; j < classMemberCount; ++j)
-                if (const Declaration *decl = cl->memberAt(j)->asDeclaration()) {
+                if (Declaration *decl = cl->memberAt(j)->asDeclaration()) {
                 // we want to know if the class contains a member (so we look into
                 // a declaration) of uiClassName type
-                    const NamedType *nt = decl->type()->asNamedType();
+                    QString nameToMatch;
+                    if (const NamedType *nt = decl->type()->asNamedType()) {
+                        nameToMatch = fullyQualifiedName(context, nt->name(),
+                                                         decl->enclosingScope());
                     // handle pointers to member variables
-                    if (PointerType *pt = decl->type()->asPointerType())
+                    } else if (PointerType *pt = decl->type()->asPointerType()) {
-                        nt = pt->elementType()->asNamedType();
+                        if (NamedType *nt = pt->elementType()->asNamedType()) {
-
+                            nameToMatch = fullyQualifiedName(context, nt->name(),
-                    if (nt && matchMemberClassName(className, o.prettyName(nt->name())))
+                                                             decl->enclosingScope());
-                            return cl;
+                        }
+                    }
+                    if (!nameToMatch.isEmpty() && className == nameToMatch)
+                        return cl;
                 } // decl
             // 2) does it inherit the desired class
             if (inherits(o, cl, className))
@@ -214,7 +219,7 @@ static const Class *findClass(const Namespace *parentNameSpace,
                 QString tempNS = *namespaceName;
                 tempNS += o.prettyName(ns->name());
                 tempNS += QLatin1String("::");
-                if (const Class *cl = findClass(ns, className, &tempNS)) {
+                if (const Class *cl = findClass(ns, context, className, &tempNS)) {
                     *namespaceName = tempNS;
                     return cl;
                 }
@@ -445,14 +450,15 @@ static QString addParameterNames(const QString &functionSignature, const QString
 typedef QPair<const Class *, Document::Ptr> ClassDocumentPtrPair;
 
 static ClassDocumentPtrPair
-        findClassRecursively(const Snapshot &docTable,
+        findClassRecursively(const LookupContext &context, const QString &className,
-                             const Document::Ptr &doc, const QString &className,
                              unsigned maxIncludeDepth, QString *namespaceName)
 {
+    const Document::Ptr doc = context.thisDocument();
+    const Snapshot docTable = context.snapshot();
     if (Designer::Constants::Internal::debug)
         qDebug() << Q_FUNC_INFO << doc->fileName() << className << maxIncludeDepth;
     // Check document
-    if (const Class *cl = findClass(doc->globalNamespace(), className, namespaceName))
+    if (const Class *cl = findClass(doc->globalNamespace(), context, className, namespaceName))
         return ClassDocumentPtrPair(cl, doc);
     if (maxIncludeDepth) {
         // Check the includes
@@ -461,7 +467,9 @@ static ClassDocumentPtrPair
             const Snapshot::const_iterator it = docTable.find(include);
             if (it != docTable.end()) {
                 const Document::Ptr includeDoc = it.value();
-                const ClassDocumentPtrPair irc = findClassRecursively(docTable, it.value(), className, recursionMaxIncludeDepth, namespaceName);
+                LookupContext context(includeDoc, docTable);
+                const ClassDocumentPtrPair irc = findClassRecursively(context, className,
+                    recursionMaxIncludeDepth, namespaceName);
                 if (irc.first)
                     return irc;
             }
@@ -588,7 +596,8 @@ bool QtCreatorIntegration::navigateToSlot(const QString &objectName,
     Document::Ptr doc;
 
     foreach (const Document::Ptr &d, docMap) {
-        const ClassDocumentPtrPair cd = findClassRecursively(docTable, d, uiClass, 1u , &namespaceName);
+        LookupContext context(d, docTable);
+        const ClassDocumentPtrPair cd = findClassRecursively(context, uiClass, 1u , &namespaceName);
         if (cd.first) {
             cl = cd.first;
             doc = cd.second;

tests/designer/gotoslot_insertIntoCorrectClass_non-pointer/form.cpp

@@ -0,0 +1,9 @@
+// Copyright header
+
+#include "form.h"
+
+Form::Form(QWidget *parent) :
+    QWidget(parent)
+{
+    ui.setupUi(this);
+}

tests/designer/gotoslot_insertIntoCorrectClass_non-pointer/form.h

@@ -0,0 +1,27 @@
+// Copyright header
+
+#ifndef FORM_H
+#define FORM_H
+
+#include "ui_form.h"
+
+#include <QWidget>
+
+class Form;
+struct MyClass
+{
+    Form *form;
+};
+
+class Form : public QWidget
+{
+    Q_OBJECT
+
+public:
+    explicit Form(QWidget *parent = 0);
+
+private:
+    Ui::Form ui;
+};
+
+#endif // FORM_H

tests/designer/gotoslot_insertIntoCorrectClass_pointer/form.cpp

@@ -0,0 +1,16 @@
+// Copyright header
+
+#include "form.h"
+#include "ui_form.h"
+
+Form::Form(QWidget *parent) :
+    QWidget(parent),
+    ui(new Ui::Form)
+{
+    ui->setupUi(this);
+}
+
+Form::~Form()
+{
+    delete ui;
+}

tests/designer/gotoslot_insertIntoCorrectClass_pointer/form.h

@@ -0,0 +1,30 @@
+// Copyright header
+
+#ifndef FORM_H
+#define FORM_H
+
+#include <QWidget>
+
+namespace Ui {
+class Form;
+}
+
+class Form;
+struct MyClass
+{
+    Form *form;
+};
+
+class Form : public QWidget
+{
+    Q_OBJECT
+
+public:
+    explicit Form(QWidget *parent = 0);
+    ~Form();
+
+private:
+    Ui::Form *ui;
+};
+
+#endif // FORM_H

tests/designer/gotoslot_insertIntoCorrectClass_pointer_ns_using/form.cpp

@@ -0,0 +1,16 @@
+// Copyright header
+
+#include "form.h"
+#include "ui_form.h"
+
+Form::Form(QWidget *parent) :
+    QWidget(parent),
+    ui(new Ui::Form)
+{
+    ui->setupUi(this);
+}
+
+Form::~Form()
+{
+    delete ui;
+}

tests/designer/gotoslot_insertIntoCorrectClass_pointer_ns_using/form.h

@@ -0,0 +1,28 @@
+// Copyright header
+
+#ifndef N_FORM_H
+#define N_FORM_H
+
+#include <QWidget>
+
+namespace N {
+namespace Ui {
+class Form;
+}
+}
+
+using namespace N;
+
+class Form : public QWidget
+{
+    Q_OBJECT
+
+public:
+    explicit Form(QWidget *parent = 0);
+    ~Form();
+
+private:
+    Ui::Form *ui;
+};
+
+#endif // N_FORM_H

tests/designer/gotoslot_insertIntoCorrectClass_pointer_ns_using/form.ui

@@ -0,0 +1,32 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<ui version="4.0">
+ <class>N::Form</class>
+ <widget class="QWidget" name="N::Form">
+  <property name="geometry">
+   <rect>
+    <x>0</x>
+    <y>0</y>
+    <width>400</width>
+    <height>300</height>
+   </rect>
+  </property>
+  <property name="windowTitle">
+   <string>Form</string>
+  </property>
+  <widget class="QPushButton" name="pushButton">
+   <property name="geometry">
+    <rect>
+     <x>60</x>
+     <y>60</y>
+     <width>80</width>
+     <height>21</height>
+    </rect>
+   </property>
+   <property name="text">
+    <string>PushButton</string>
+   </property>
+  </widget>
+ </widget>
+ <resources/>
+ <connections/>
+</ui>