CppEditor: Fix "Switch Method Decl/Def" for cursor on return type

If the cursor is on the return type, Document::lastVisibleSymbol()
returns the function defined before the current function definition,
which is unfavorable for "Switch Method Decl/Def".

Fixed by using ASTPath instead of Document::lastVisibleSymbol().

Task-Number: QTCREATORBUG-9206

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

3 files changed, 87 insertions, 44 deletions

diff --git a/src/plugins/cppeditor/cppeditor.cpp b/src/plugins/cppeditor/cppeditor.cpp
index 84c60425c1..dee2d1456e 100644
--- a/src/plugins/cppeditor/cppeditor.cpp
+++ b/src/plugins/cppeditor/cppeditor.cpp
@@ -1180,60 +1180,70 @@ void CPPEditorWidget::finishHighlightSymbolUsages()
 
 void CPPEditorWidget::switchDeclarationDefinition(bool inNextSplit)
 {
-    if (! m_modelManager)
+    if (!m_modelManager)
         return;
 
-    const Snapshot snapshot = m_modelManager->snapshot();
-
-    if (Document::Ptr thisDocument = snapshot.document(editorDocument()->fileName())) {
-        int line = 0, positionInBlock = 0;
-        convertPosition(position(), &line, &positionInBlock);
+    if (!m_lastSemanticInfo.doc)
+        return;
 
-        Symbol *lastVisibleSymbol = thisDocument->lastVisibleSymbolAt(line, positionInBlock + 1);
-        if (! lastVisibleSymbol)
-            return;
+    // Find function declaration or definition under cursor
+    Function *functionDefinitionSymbol = 0;
+    Symbol *functionDeclarationSymbol = 0;
+
+    ASTPath astPathFinder(m_lastSemanticInfo.doc);
+    const QList<AST *> astPath = astPathFinder(textCursor());
+
+    for (int i = 0, size = astPath.size(); i < size; ++i) {
+        AST *ast = astPath.at(i);
+        if (FunctionDefinitionAST *functionDefinitionAST = ast->asFunctionDefinition()) {
+            if ((functionDefinitionSymbol = functionDefinitionAST->symbol))
+                break; // Function definition found!
+        } else if (SimpleDeclarationAST *simpleDeclaration = ast->asSimpleDeclaration()) {
+            if (List<Symbol *> *symbols = simpleDeclaration->symbols) {
+                if (Symbol *symbol = symbols->value) {
+                    if (symbol->isDeclaration() && symbol->type()->isFunctionType()) {
+                        functionDeclarationSymbol = symbol;
+                        break; // Function declaration found!
+                    }
+                }
+            }
+        }
+    }
 
-        Function *function = lastVisibleSymbol->asFunction();
-        if (! function)
-            function = lastVisibleSymbol->enclosingFunction();
-
-        CPPEditorWidget::Link symbolLink;
-
-        if (function) {
-            LookupContext context(thisDocument, snapshot);
-
-            Function *functionDefinition = function->asFunction();
-            ClassOrNamespace *binding = context.lookupType(functionDefinition);
-
-            const QList<LookupItem> declarations = context.lookup(functionDefinition->name(),
-                functionDefinition->enclosingScope());
-            QList<Symbol *> best;
-            foreach (const LookupItem &r, declarations) {
-                if (Symbol *decl = r.declaration()) {
-                    if (Function *funTy = decl->type()->asFunctionType()) {
-                        if (funTy->isEqualTo(function)) {
-                            if (decl != function && binding == r.binding())
-                                best.prepend(decl);
-                            else
-                                best.append(decl);
-                        }
+    // Link to function definition/declaration
+    CPPEditorWidget::Link symbolLink;
+    if (functionDeclarationSymbol) {
+        symbolLink = linkToSymbol(symbolFinder()
+            ->findMatchingDefinition(functionDeclarationSymbol, modelManager()->snapshot()));
+    } else if (functionDefinitionSymbol) {
+        const Snapshot snapshot = m_modelManager->snapshot();
+        LookupContext context(m_lastSemanticInfo.doc, snapshot);
+        ClassOrNamespace *binding = context.lookupType(functionDefinitionSymbol);
+        const QList<LookupItem> declarations = context.lookup(functionDefinitionSymbol->name(),
+            functionDefinitionSymbol->enclosingScope());
+
+        QList<Symbol *> best;
+        foreach (const LookupItem &r, declarations) {
+            if (Symbol *decl = r.declaration()) {
+                if (Function *funTy = decl->type()->asFunctionType()) {
+                    if (funTy->isEqualTo(functionDefinitionSymbol)) {
+                        if (decl != functionDefinitionSymbol && binding == r.binding())
+                            best.prepend(decl);
+                        else
+                            best.append(decl);
                     }
                 }
             }
-            if (best.isEmpty())
-                return;
-
-            symbolLink = linkToSymbol(best.first());
-        } else if (lastVisibleSymbol
-                   && lastVisibleSymbol->isDeclaration()
-                   && lastVisibleSymbol->type()->isFunctionType()) {
-            symbolLink = linkToSymbol(
-                symbolFinder()->findMatchingDefinition(lastVisibleSymbol, snapshot));
         }
 
-        if (symbolLink.hasValidTarget())
-            openCppEditorAt(symbolLink, inNextSplit != alwaysOpenLinksInNextSplit());
+        if (best.isEmpty())
+            return;
+        symbolLink = linkToSymbol(best.first());
     }
+
+    // Open Editor at link position
+    if (symbolLink.hasValidTarget())
+        openCppEditorAt(symbolLink, inNextSplit != alwaysOpenLinksInNextSplit());
 }
 
 static inline LookupItem skipForwardDeclarations(const QList<LookupItem> &resolvedSymbols)
diff --git a/src/plugins/cppeditor/cppeditorplugin.h b/src/plugins/cppeditor/cppeditorplugin.h
index edf3a6eb2e..828ff24985 100644
--- a/src/plugins/cppeditor/cppeditorplugin.h
+++ b/src/plugins/cppeditor/cppeditorplugin.h
@@ -96,6 +96,7 @@ private slots:
     void test_SwitchMethodDeclarationDefinition_fromFunctionDeclarationSymbol();
     void test_SwitchMethodDeclarationDefinition_fromFunctionDefinitionSymbol();
     void test_SwitchMethodDeclarationDefinition_fromFunctionBody();
+    void test_SwitchMethodDeclarationDefinition_fromReturnType();
 
     void test_FollowSymbolUnderCursor_globalVarFromFunction();
     void test_FollowSymbolUnderCursor_funLocalVarHidesClassMember();
diff --git a/src/plugins/cppeditor/followsymbol_switchmethoddecldef_test.cpp b/src/plugins/cppeditor/followsymbol_switchmethoddecldef_test.cpp
index c2b3af4b1e..428b6e2079 100644
--- a/src/plugins/cppeditor/followsymbol_switchmethoddecldef_test.cpp
+++ b/src/plugins/cppeditor/followsymbol_switchmethoddecldef_test.cpp
@@ -420,6 +420,38 @@ void CppEditorPlugin::test_SwitchMethodDeclarationDefinition_fromFunctionBody()
     test.run();
 }
 
+void CppEditorPlugin::test_SwitchMethodDeclarationDefinition_fromReturnType()
+{
+    QList<TestDocumentPtr> testFiles;
+
+    const QByteArray headerContents =
+        "class C\n"
+        "{\n"
+        "public:\n"
+        "    C();\n"
+        "    int $function();\n"
+        "};\n"
+        ;
+    testFiles << TestDocument::create(headerContents, QLatin1String("file.h"));
+
+    const QByteArray sourceContents =
+        "#include \"file.h\"\n"
+        "\n"
+        "C::C()\n"
+        "{\n"
+        "}\n"                   // Line 5
+        "\n"
+        "@int C::function()\n"
+        "{\n"
+        "    return 1 + 1;\n"
+        "}\n"                   // Line 10
+        ;
+    testFiles << TestDocument::create(sourceContents, QLatin1String("file.cpp"));
+
+    TestCase test(TestCase::SwitchBetweenMethodDeclarationDefinition, testFiles);
+    test.run();
+}
+
 /// Check ...
 void CppEditorPlugin::test_FollowSymbolUnderCursor_globalVarFromFunction()
 {