CppEditor: Selecting an override of a virtual function jumps to definition

...instead declaration (F2 on a virtual function call).

Task-number: QTCREATORBUG-10287
Change-Id: Ib913bd4e777c7253659458ae17584354c7416d23
Reviewed-by: Orgad Shaneh <orgads@gmail.com>
Reviewed-by: Erik Verbruggen <erik.verbruggen@digia.com>

3 files changed, 84 insertions, 22 deletions

diff --git a/src/plugins/cppeditor/cppeditorplugin.h b/src/plugins/cppeditor/cppeditorplugin.h
index 3a4eaa19f4..c9e6f5e873 100644
--- a/src/plugins/cppeditor/cppeditorplugin.h
+++ b/src/plugins/cppeditor/cppeditorplugin.h
@@ -138,6 +138,8 @@ private slots:
     void test_FollowSymbolUnderCursor_virtualFunctionCall_allOverrides();
     void test_FollowSymbolUnderCursor_virtualFunctionCall_possibleOverrides1();
     void test_FollowSymbolUnderCursor_virtualFunctionCall_possibleOverrides2();
+    void test_FollowSymbolUnderCursor_virtualFunctionCall_fallbackToDeclaration();
+    void test_FollowSymbolUnderCursor_virtualFunctionCall_itemOrder();
     void test_FollowSymbolUnderCursor_virtualFunctionCall_onDotMemberAccessOfReferenceTypes();
     void test_FollowSymbolUnderCursor_virtualFunctionCall_notOnDotMemberAccessOfNonReferenceType();
     void test_FollowSymbolUnderCursor_virtualFunctionCall_notOnQualified();
diff --git a/src/plugins/cppeditor/cppvirtualfunctionassistprovider.cpp b/src/plugins/cppeditor/cppvirtualfunctionassistprovider.cpp
index a9668bc3fc..5c86bf455b 100644
--- a/src/plugins/cppeditor/cppvirtualfunctionassistprovider.cpp
+++ b/src/plugins/cppeditor/cppvirtualfunctionassistprovider.cpp
@@ -40,6 +40,8 @@
 #include <coreplugin/actionmanager/actionmanager.h>
 #include <coreplugin/actionmanager/command.h>
 
+#include <cpptools/symbolfinder.h>
+
 #include <texteditor/codeassist/basicproposalitemlistmodel.h>
 #include <texteditor/codeassist/genericproposal.h>
 #include <texteditor/codeassist/genericproposalwidget.h>
@@ -124,7 +126,7 @@ public:
         hintItem->setOrder(-1000);
 
         QList<BasicProposalItem *> items;
-        items << itemFromSymbol(m_function, m_function);
+        items << itemFromSymbol(maybeDefinitionFor(m_function));
         items << hintItem;
         return new VirtualFunctionProposal(interface->position(),
                                            new BasicProposalItemListModel(items),
@@ -142,16 +144,28 @@ public:
 
         const QList<Symbol *> overrides = FunctionHelper::overrides(m_startClass, m_function,
                                                                     m_snapshot);
+        if (overrides.isEmpty())
+            return 0;
+
         QList<BasicProposalItem *> items;
         foreach (Symbol *symbol, overrides)
-            items << itemFromSymbol(symbol, m_function);
+            items << itemFromSymbol(maybeDefinitionFor(symbol));
+        items.first()->setOrder(1000); // Ensure top position for function of static type
 
         return new VirtualFunctionProposal(interface->position(),
                                            new BasicProposalItemListModel(items),
                                            m_openInNextSplit);
     }
 
-    BasicProposalItem *itemFromSymbol(Symbol *symbol, Symbol *firstSymbol) const
+private:
+    Symbol *maybeDefinitionFor(Symbol *symbol)
+    {
+        if (Function *definition = m_finder.findMatchingDefinition(symbol, m_snapshot))
+            return definition;
+        return symbol;
+    }
+
+    BasicProposalItem *itemFromSymbol(Symbol *symbol) const
     {
         const QString text = m_overview.prettyName(LookupContext::fullyQualifiedName(symbol));
         const CPPEditorWidget::Link link = CPPEditorWidget::linkToSymbol(symbol);
@@ -159,19 +173,17 @@ public:
         BasicProposalItem *item = new VirtualFunctionProposalItem(link, m_openInNextSplit);
         item->setText(text);
         item->setIcon(m_icons.iconForSymbol(symbol));
-        if (symbol == firstSymbol)
-            item->setOrder(1000); // Ensure top position for function of static type
 
         return item;
     }
 
-private:
     Class *m_startClass;
     Function *m_function;
     Snapshot m_snapshot;
     bool m_openInNextSplit;
     Overview m_overview;
     Icons m_icons;
+    CppTools::SymbolFinder m_finder;
 };
 
 VirtualFunctionAssistProvider::VirtualFunctionAssistProvider()
diff --git a/src/plugins/cppeditor/followsymbol_switchmethoddecldef_test.cpp b/src/plugins/cppeditor/followsymbol_switchmethoddecldef_test.cpp
index 99be9591d5..f9dc85b528 100644
--- a/src/plugins/cppeditor/followsymbol_switchmethoddecldef_test.cpp
+++ b/src/plugins/cppeditor/followsymbol_switchmethoddecldef_test.cpp
@@ -1274,14 +1274,14 @@ void CppEditorPlugin::test_FollowSymbolUnderCursor_virtualFunctionCall_allOverri
             ;
 
     const OverrideItemList immediateResults = OverrideItemList()
-            << OverrideItem(QLatin1String("A::virt"), 1)
+            << OverrideItem(QLatin1String("A::virt"), 2)
             << OverrideItem(QLatin1String("...searching overrides"));
     const OverrideItemList finalResults = OverrideItemList()
-            << OverrideItem(QLatin1String("A::virt"), 1)
-            << OverrideItem(QLatin1String("B::virt"), 4)
-            << OverrideItem(QLatin1String("C::virt"), 7)
-            << OverrideItem(QLatin1String("CD1::virt"), 10)
-            << OverrideItem(QLatin1String("CD2::virt"), 13);
+            << OverrideItem(QLatin1String("A::virt"), 2)
+            << OverrideItem(QLatin1String("B::virt"), 5)
+            << OverrideItem(QLatin1String("C::virt"), 8)
+            << OverrideItem(QLatin1String("CD1::virt"), 11)
+            << OverrideItem(QLatin1String("CD2::virt"), 14);
 
     TestCase test(TestCase::FollowSymbolUnderCursorAction, source, immediateResults, finalResults);
     test.run();
@@ -1311,13 +1311,13 @@ void CppEditorPlugin::test_FollowSymbolUnderCursor_virtualFunctionCall_possibleO
             ;
 
     const OverrideItemList immediateResults = OverrideItemList()
-            << OverrideItem(QLatin1String("B::virt"), 4)
+            << OverrideItem(QLatin1String("B::virt"), 5)
             << OverrideItem(QLatin1String("...searching overrides"));
     const OverrideItemList finalResults = OverrideItemList()
-            << OverrideItem(QLatin1String("B::virt"), 4)
-            << OverrideItem(QLatin1String("C::virt"), 7)
-            << OverrideItem(QLatin1String("CD1::virt"), 10)
-            << OverrideItem(QLatin1String("CD2::virt"), 13);
+            << OverrideItem(QLatin1String("B::virt"), 5)
+            << OverrideItem(QLatin1String("C::virt"), 8)
+            << OverrideItem(QLatin1String("CD1::virt"), 11)
+            << OverrideItem(QLatin1String("CD2::virt"), 14);
 
     TestCase test(TestCase::FollowSymbolUnderCursorAction, source, immediateResults, finalResults);
     test.run();
@@ -1340,11 +1340,59 @@ void CppEditorPlugin::test_FollowSymbolUnderCursor_virtualFunctionCall_possibleO
             ;
 
     const OverrideItemList immediateResults = OverrideItemList()
-            << OverrideItem(QLatin1String("B::virt"), 4)
+            << OverrideItem(QLatin1String("B::virt"), 5)
+            << OverrideItem(QLatin1String("...searching overrides"));
+    const OverrideItemList finalResults = OverrideItemList()
+            << OverrideItem(QLatin1String("B::virt"), 5)
+            << OverrideItem(QLatin1String("D::virt"), 10);
+
+    TestCase test(TestCase::FollowSymbolUnderCursorAction, source, immediateResults, finalResults);
+    test.run();
+}
+
+/// Check: If no definition is found, fallback to the declaration.
+void CppEditorPlugin::test_FollowSymbolUnderCursor_virtualFunctionCall_fallbackToDeclaration()
+{
+    const QByteArray source =
+            "struct A { virtual void virt(); };\n"
+            "\n"
+            "int f(A *o) { o->$@virt(); }\n"
+            ;
+
+    const OverrideItemList immediateResults = OverrideItemList()
+            << OverrideItem(QLatin1String("A::virt"), 1)
             << OverrideItem(QLatin1String("...searching overrides"));
     const OverrideItemList finalResults = OverrideItemList()
-            << OverrideItem(QLatin1String("B::virt"), 4)
-            << OverrideItem(QLatin1String("D::virt"), 9);
+            << OverrideItem(QLatin1String("A::virt"), 1);
+
+    TestCase test(TestCase::FollowSymbolUnderCursorAction, source, immediateResults, finalResults);
+    test.run();
+}
+
+/// Check: Ensure that the first entry in the final results is the same as the first in the
+///        immediate results.
+void CppEditorPlugin::test_FollowSymbolUnderCursor_virtualFunctionCall_itemOrder()
+{
+    const QByteArray source =
+            "struct C { virtual void virt() = 0; };\n"
+            "void C::virt() {}\n"
+            "\n"
+            "struct B : C { void virt(); };\n"
+            "void B::virt() {}\n"
+            "\n"
+            "struct A : B { void virt(); };\n"
+            "void A::virt() {}\n"
+            "\n"
+            "int f(C *o) { o->$@virt(); }\n"
+            ;
+
+    const OverrideItemList immediateResults = OverrideItemList()
+            << OverrideItem(QLatin1String("C::virt"), 2)
+            << OverrideItem(QLatin1String("...searching overrides"));
+    const OverrideItemList finalResults = OverrideItemList()
+            << OverrideItem(QLatin1String("C::virt"), 2)
+            << OverrideItem(QLatin1String("A::virt"), 8)
+            << OverrideItem(QLatin1String("B::virt"), 5);
 
     TestCase test(TestCase::FollowSymbolUnderCursorAction, source, immediateResults, finalResults);
     test.run();
@@ -1361,10 +1409,10 @@ void CppEditorPlugin::test_FollowSymbolUnderCursor_virtualFunctionCall_onDotMemb
             ;
 
     const OverrideItemList immediateResults = OverrideItemList()
-            << OverrideItem(QLatin1String("A::virt"), 1)
+            << OverrideItem(QLatin1String("A::virt"), 2)
             << OverrideItem(QLatin1String("...searching overrides"));
     const OverrideItemList finalResults = OverrideItemList()
-            << OverrideItem(QLatin1String("A::virt"), 1);
+            << OverrideItem(QLatin1String("A::virt"), 2);
 
     TestCase test(TestCase::FollowSymbolUnderCursorAction, source, immediateResults, finalResults);
     test.run();