CppEditor: Handle classes that do not override

...when searching the overrides for virtual functions.

In case there is no override for the static type of a function call
expression, make sure to:

1) include the last provided override (look up bases)
2) and all overrides whose classes are derived from that static type

Task-number: QTCREATORBUG-10470

Change-Id: I2c01bfdc6cb35c5a01a000ebd81a2b322ce2b795
Reviewed-by: Orgad Shaneh <orgads@gmail.com>
Reviewed-by: Erik Verbruggen <erik.verbruggen@digia.com>

src/plugins/cppeditor/cppeditorplugin.h

@@ -141,6 +141,11 @@ private slots:
     void test_FollowSymbolUnderCursor_virtualFunctionCall_fallbackToDeclaration();
     void test_FollowSymbolUnderCursor_virtualFunctionCall_itemOrder();
     void test_FollowSymbolUnderCursor_virtualFunctionCall_instantiatedSymbols();
+    void test_FollowSymbolUnderCursor_virtualFunctionCall_QSharedPointer();
+    void test_FollowSymbolUnderCursor_virtualFunctionCall_multipeDocuments();
+    void test_FollowSymbolUnderCursor_virtualFunctionCall_noSiblings_references();
+    void test_FollowSymbolUnderCursor_virtualFunctionCall_noSiblings_pointers();
+    void test_FollowSymbolUnderCursor_virtualFunctionCall_noSiblings_noBaseExpression();
     void test_FollowSymbolUnderCursor_virtualFunctionCall_onDotMemberAccessOfReferenceTypes();
     void test_FollowSymbolUnderCursor_virtualFunctionCall_notOnDotMemberAccessOfNonReferenceType();
     void test_FollowSymbolUnderCursor_virtualFunctionCall_notOnQualified();

src/plugins/cppeditor/cppfollowsymbolundercursor.cpp

@@ -35,6 +35,7 @@
 #include <cplusplus/ASTPath.h>
 #include <cplusplus/BackwardsScanner.h>
 #include <cplusplus/ExpressionUnderCursor.h>
+#include <cplusplus/ResolveExpression.h>
 #include <cplusplus/SimpleLexer.h>
 #include <cplusplus/TypeOfExpression.h>
 #include <cpptools/cppmodelmanagerinterface.h>
@@ -60,75 +61,85 @@ public:
     VirtualFunctionHelper(const TypeOfExpression &typeOfExpression,
                           Scope *scope,
                           const Document::Ptr &document,
-                          const Snapshot &snapshot);
+                          const Snapshot &snapshot,
+                          SymbolFinder *symbolFinder);
 
-    bool canLookupVirtualFunctionOverrides(const Function *function) const;
+    bool canLookupVirtualFunctionOverrides(Function *function);
+
+    /// Returns != 0 if canLookupVirtualFunctionOverrides() succeeded.
+    Class *staticClassOfFunctionCallExpression() const
+    { return m_staticClassOfFunctionCallExpression; }
 
 private:
     VirtualFunctionHelper();
     Q_DISABLE_COPY(VirtualFunctionHelper)
 
-    ExpressionAST *getBaseExpressionAST() const
+    Class *staticClassOfFunctionCallExpression_internal() const;
-    {
-        if (!m_expressionAST)
-            return 0;
-        CallAST *callAST = m_expressionAST->asCall();
-        if (!callAST)
-            return 0;
-        if (ExpressionAST *baseExpressionAST = callAST->base_expression)
-            return baseExpressionAST;
-        return 0;
-    }
 
 private:
+    // Provided
     const QSharedPointer<TypeOfExpression> m_typeOfExpression;
-    ExpressionAST *m_expressionAST;
     const Document::Ptr m_expressionDocument;
     Scope *m_scope;
     const Document::Ptr &m_document;
     const Snapshot &m_snapshot;
+    SymbolFinder *m_finder;
+
+    // Determined
+    ExpressionAST *m_baseExpressionAST;
+    Function *m_function;
+    int m_accessTokenKind;
+    Class *m_staticClassOfFunctionCallExpression; // Output
 };
 
 VirtualFunctionHelper::VirtualFunctionHelper(const TypeOfExpression &typeOfExpression,
                                              Scope *scope,
                                              const Document::Ptr &document,
-                                             const Snapshot &snapshot)
+                                             const Snapshot &snapshot,
-    : m_expressionAST(typeOfExpression.expressionAST())
+                                             SymbolFinder *finder)
-    , m_expressionDocument(typeOfExpression.context().expressionDocument())
+    : m_expressionDocument(typeOfExpression.context().expressionDocument())
     , m_scope(scope)
     , m_document(document)
     , m_snapshot(snapshot)
+    , m_finder(finder)
+    , m_baseExpressionAST(0)
+    , m_function(0)
+    , m_accessTokenKind(0)
+    , m_staticClassOfFunctionCallExpression(0)
 {
+    if (ExpressionAST *expressionAST = typeOfExpression.expressionAST()) {
+        if (CallAST *callAST = expressionAST->asCall()) {
+            if (ExpressionAST *baseExpressionAST = callAST->base_expression)
+                m_baseExpressionAST = baseExpressionAST;
+        }
+    }
 }
 
-bool VirtualFunctionHelper::canLookupVirtualFunctionOverrides(const Function *function) const
+bool VirtualFunctionHelper::canLookupVirtualFunctionOverrides(Function *function)
 {
-    if (!m_expressionDocument || !m_document || !function || !m_scope || m_scope->isClass()
+    m_function = function;
-            || m_snapshot.isEmpty()) {
+    if (!m_function || !m_baseExpressionAST || !m_expressionDocument || !m_document || !m_scope
+            || m_scope->isClass() || m_snapshot.isEmpty()) {
         return false;
     }
 
-    ExpressionAST *baseExpressionAST = getBaseExpressionAST();
-    if (!baseExpressionAST)
-        return false;
-
     bool result = false;
 
-    if (IdExpressionAST *idExpressionAST = baseExpressionAST->asIdExpression()) {
+    if (IdExpressionAST *idExpressionAST = m_baseExpressionAST->asIdExpression()) {
         NameAST *name = idExpressionAST->name;
         const bool nameIsQualified = name && name->asQualifiedName();
         result = !nameIsQualified && FunctionHelper::isVirtualFunction(function, m_snapshot);
-    } else if (MemberAccessAST *memberAccessAST = baseExpressionAST->asMemberAccess()) {
+    } else if (MemberAccessAST *memberAccessAST = m_baseExpressionAST->asMemberAccess()) {
         NameAST *name = memberAccessAST->member_name;
         const bool nameIsQualified = name && name->asQualifiedName();
         if (!nameIsQualified && FunctionHelper::isVirtualFunction(function, m_snapshot)) {
             TranslationUnit *unit = m_expressionDocument->translationUnit();
             QTC_ASSERT(unit, return false);
-            const int accessTokenKind = unit->tokenKind(memberAccessAST->access_token);
+            m_accessTokenKind = unit->tokenKind(memberAccessAST->access_token);
 
-            if (accessTokenKind == T_ARROW) {
+            if (m_accessTokenKind == T_ARROW) {
                 result = true;
-            } else if (accessTokenKind == T_DOT) {
+            } else if (m_accessTokenKind == T_DOT) {
                 TypeOfExpression typeOfExpression;
                 typeOfExpression.init(m_document, m_snapshot);
                 typeOfExpression.setExpandTemplates(true);
@@ -143,6 +154,50 @@ bool VirtualFunctionHelper::canLookupVirtualFunctionOverrides(const Function *fu
         }
     }
 
+    if (!result)
+        return false;
+    return (m_staticClassOfFunctionCallExpression = staticClassOfFunctionCallExpression_internal());
+}
+
+/// For "f()" in "class C { void g() { f(); };" return class C.
+/// For "c->f()" in "{ C *c; c->f(); }" return class C.
+Class *VirtualFunctionHelper::staticClassOfFunctionCallExpression_internal() const
+{
+    if (!m_finder)
+        return 0;
+
+    Class *result = 0;
+
+    if (m_baseExpressionAST->asIdExpression()) {
+        for (Scope *s = m_scope; s ; s = s->enclosingScope()) {
+            if (Function *function = s->asFunction()) {
+                result = m_finder->findMatchingClassDeclaration(function, m_snapshot);
+                break;
+            }
+        }
+    } else if (MemberAccessAST *memberAccessAST = m_baseExpressionAST->asMemberAccess()) {
+        QTC_ASSERT(m_accessTokenKind == T_ARROW || m_accessTokenKind == T_DOT, return result);
+        TypeOfExpression typeOfExpression;
+        typeOfExpression.init(m_document, m_snapshot);
+        typeOfExpression.setExpandTemplates(true);
+        const QList<LookupItem> items = typeOfExpression(memberAccessAST->base_expression,
+                                                         m_document, m_scope);
+        ResolveExpression resolveExpression(typeOfExpression.context());
+        ClassOrNamespace *binding = resolveExpression.baseExpression(items, m_accessTokenKind);
+        if (binding) {
+            if (Class *klass = binding->rootClass()) {
+                result = klass;
+            } else {
+                const QList<Symbol *> symbols = binding->symbols();
+                if (!symbols.isEmpty()) {
+                    Symbol * const first = symbols.first();
+                    if (first->isForwardClassDeclaration())
+                        result = m_finder->findMatchingClassDeclaration(first, m_snapshot);
+                }
+            }
+        }
+    }
+
     return result;
 }
 
@@ -590,11 +645,12 @@ BaseTextEditorWidget::Link FollowSymbolUnderCursor::findLink(const QTextCursor &
 
             // Consider to show a pop-up displaying overrides for the function
             Function *function = symbol->type()->asFunctionType();
-            VirtualFunctionHelper helper(*typeOfExpression, scope, doc, snapshot);
+            VirtualFunctionHelper helper(*typeOfExpression, scope, doc, snapshot, symbolFinder);
 
             if (helper.canLookupVirtualFunctionOverrides(function)) {
                 VirtualFunctionAssistProvider::Parameters params;
                 params.function = function;
+                params.staticClass = helper.staticClassOfFunctionCallExpression();
                 params.typeOfExpression = typeOfExpression;
                 params.snapshot = snapshot;
                 params.cursorPosition = cursor.position();

src/plugins/cppeditor/cppvirtualfunctionassistprovider.cpp

@@ -133,6 +133,7 @@ public:
     IAssistProposal *perform(const IAssistInterface *)
     {
         QTC_ASSERT(m_params.function, return 0);
+        QTC_ASSERT(m_params.staticClass, return 0);
         QTC_ASSERT(!m_params.snapshot.isEmpty(), return 0);
 
         Class *functionsClass = m_finder.findMatchingClassDeclaration(m_params.function,
@@ -140,8 +141,8 @@ public:
         if (!functionsClass)
             return 0;
 
-        const QList<Symbol *> overrides
+        const QList<Symbol *> overrides = FunctionHelper::overrides(
-            = FunctionHelper::overrides(m_params.function, functionsClass, m_params.snapshot);
+            m_params.function, functionsClass, m_params.staticClass, m_params.snapshot);
         if (overrides.isEmpty())
             return 0;
 
@@ -254,11 +255,40 @@ bool FunctionHelper::isPureVirtualFunction(const Function *function, const Snaps
     return isVirtualFunction_helper(function, snapshot, PureVirtual);
 }
 
-QList<Symbol *> FunctionHelper::overrides(Function *function, Class *functionsClass,
+static bool isDerivedOf(Class *derivedClassCandidate, Class *baseClass,
                         const Snapshot &snapshot)
+{
+    QTC_ASSERT(derivedClassCandidate && baseClass, return false);
+
+    QList<CppClass> l = QList<CppClass>() << CppClass(derivedClassCandidate);
+
+    while (!l.isEmpty()) {
+        CppClass clazz = l.takeFirst();
+        QTC_ASSERT(clazz.declaration, continue);
+
+        const QString fileName = QString::fromUtf8(clazz.declaration->fileName());
+        const Document::Ptr document = snapshot.document(fileName);
+        if (!document)
+            continue;
+        const LookupContext context(document, snapshot);
+        clazz.lookupBases(clazz.declaration, context);
+
+        foreach (const CppClass &base, clazz.bases) {
+            if (base.declaration == baseClass)
+                return true;
+            if (!l.contains(base))
+                l << base;
+        }
+    }
+
+    return false;
+}
+
+QList<Symbol *> FunctionHelper::overrides(Function *function, Class *functionsClass,
+                                          Class *staticClass, const Snapshot &snapshot)
 {
     QList<Symbol *> result;
-    QTC_ASSERT(function && functionsClass, return result);
+    QTC_ASSERT(function && functionsClass && staticClass, return result);
 
     FullySpecifiedType referenceType = function->type();
     const Name *referenceName = function->name();
@@ -274,15 +304,22 @@ QList<Symbol *> FunctionHelper::overrides(Function *function, Class *functionsCl
     while (!l.isEmpty()) {
         // Add derived
         CppClass clazz = l.takeFirst();
+
+        QTC_ASSERT(clazz.declaration, continue);
+        Class *c = clazz.declaration->asClass();
+        QTC_ASSERT(c, continue);
+
+        if (c != functionsClass && c != staticClass) {
+            if (!isDerivedOf(c, staticClass, snapshot))
+                continue;
+        }
+
         foreach (const CppClass &d, clazz.derived) {
             if (!l.contains(d))
                 l << d;
         }
 
         // Check member functions
-        QTC_ASSERT(clazz.declaration, continue);
-        Class *c = clazz.declaration->asClass();
-        QTC_ASSERT(c, continue);
         for (int i = 0, total = c->memberCount(); i < total; ++i) {
             Symbol *candidate = c->memberAt(i);
             const Name *candidateName = candidate->name();

src/plugins/cppeditor/cppvirtualfunctionassistprovider.h

@@ -48,9 +48,10 @@ public:
     VirtualFunctionAssistProvider();
 
     struct Parameters {
-        Parameters() : function(0), cursorPosition(-1), openInNextSplit(false) {}
+        Parameters() : function(0), staticClass(0), cursorPosition(-1), openInNextSplit(false) {}
 
         CPlusPlus::Function *function;
+        CPlusPlus::Class *staticClass;
         QSharedPointer<CPlusPlus::TypeOfExpression> typeOfExpression; // Keeps instantiated symbols.
         CPlusPlus::Snapshot snapshot;
         int cursorPosition;
@@ -80,6 +81,7 @@ public:
 
     static QList<CPlusPlus::Symbol *> overrides(CPlusPlus::Function *function,
                                                 CPlusPlus::Class *functionsClass,
+                                                CPlusPlus::Class *staticClass,
                                                 const CPlusPlus::Snapshot &snapshot);
 };
 

src/plugins/cppeditor/followsymbol_switchmethoddecldef_test.cpp

@@ -176,7 +176,9 @@ public:
         , editor(0)
         , editorWidget(0)
     {
+        if (initialCursorPosition != -1 || targetCursorPosition != -1)
             QVERIFY(initialCursorPosition != targetCursorPosition);
+
         if (initialCursorPosition > targetCursorPosition) {
             source.remove(initialCursorPosition, 1);
             if (targetCursorPosition != -1) {
@@ -1420,6 +1422,141 @@ void CppEditorPlugin::test_FollowSymbolUnderCursor_virtualFunctionCall_instantia
     test.run();
 }
 
+/// Check: Static type is nicely resolved, especially for QSharedPointers.
+void CppEditorPlugin::test_FollowSymbolUnderCursor_virtualFunctionCall_QSharedPointer()
+{
+    const QByteArray source =
+            "template <class T>\n"
+            "class Basic\n"
+            "{\n"
+            "public:\n"
+            "    inline T &operator*() const;\n"
+            "    inline T *operator->() const;\n"
+            "};\n"
+            "\n"
+            "template <class T> class ExternalRefCount: public Basic<T> {};\n"
+            "template <class T> class QSharedPointer: public ExternalRefCount<T> {};\n"
+            "\n"
+            "struct A { virtual void virt() {} };\n"
+            "struct B : public A { void virt() {} };\n"
+            "\n"
+            "int f()\n"
+            "{\n"
+            "    QSharedPointer<A> p(new A);\n"
+            "    p->$@virt();\n"
+            "}\n"
+            ;
+
+    const OverrideItemList immediateResults = OverrideItemList()
+            << OverrideItem(QLatin1String("A::virt"), 12)
+            << OverrideItem(QLatin1String("...searching overrides"));
+    const OverrideItemList finalResults = OverrideItemList()
+            << OverrideItem(QLatin1String("A::virt"), 12)
+            << OverrideItem(QLatin1String("B::virt"), 13);
+
+    TestCase test(TestCase::FollowSymbolUnderCursorAction, source, immediateResults, finalResults);
+    test.run();
+}
+
+/// Check: Base classes can be found although these might be defined in distinct documents.
+void CppEditorPlugin::test_FollowSymbolUnderCursor_virtualFunctionCall_multipeDocuments()
+{
+    QList<TestDocumentPtr> testFiles = QList<TestDocumentPtr>()
+            << TestDocument::create("struct A { virtual void virt(int) = 0; };\n",
+                                    QLatin1String("a.h"))
+            << TestDocument::create("#include \"a.h\"\n"
+                                    "struct B : A { void virt(int) {} };\n",
+                                    QLatin1String("b.h"))
+            << TestDocument::create("#include \"a.h\"\n"
+                                    "void f(A *o) { o->$@virt(42); }\n",
+                                    QLatin1String("u.cpp"))
+            ;
+
+    const OverrideItemList immediateResults = OverrideItemList()
+            << OverrideItem(QLatin1String("A::virt"), 1)
+            << OverrideItem(QLatin1String("...searching overrides"));
+    const OverrideItemList finalResults = OverrideItemList()
+            << OverrideItem(QLatin1String("A::virt"), 1)
+            << OverrideItem(QLatin1String("B::virt"), 2);
+
+    TestCase test(TestCase::FollowSymbolUnderCursorAction, testFiles, immediateResults,
+                  finalResults);
+    test.run();
+}
+
+/// Check: In case there is no override for the static type of a function call expression,
+///        make sure to:
+///         1) include the last provided override (look up bases)
+///         2) and all overrides whose classes are derived from that static type
+void CppEditorPlugin::test_FollowSymbolUnderCursor_virtualFunctionCall_noSiblings_references()
+{
+    const QByteArray source =
+            "struct A { virtual void virt(); };\n"
+            "struct B : A { void virt() {} };\n"
+            "struct C1 : B { void virt() {} };\n"
+            "struct C2 : B { };\n"
+            "struct D : C2 { void virt() {} };\n"
+            "\n"
+            "void f(C2 &o) { o.$@virt(); }\n"
+            ;
+
+    const OverrideItemList immediateResults = OverrideItemList()
+            << OverrideItem(QLatin1String("B::virt"), 2)
+            << OverrideItem(QLatin1String("...searching overrides"));
+    const OverrideItemList finalResults = OverrideItemList()
+            << OverrideItem(QLatin1String("B::virt"), 2)
+            << OverrideItem(QLatin1String("D::virt"), 5);
+
+    TestCase test(TestCase::FollowSymbolUnderCursorAction, source, immediateResults, finalResults);
+    test.run();
+}
+
+/// Variation of test_FollowSymbolUnderCursor_virtualFunctionCall_noSiblings_references
+void CppEditorPlugin::test_FollowSymbolUnderCursor_virtualFunctionCall_noSiblings_pointers()
+{
+    const QByteArray source =
+            "struct A { virtual void virt(); };\n"
+            "struct B : A { void virt() {} };\n"
+            "struct C1 : B { void virt() {} };\n"
+            "struct C2 : B { };\n"
+            "struct D : C2 { void virt() {} };\n"
+            "\n"
+            "void f(C2 *o) { o->$@virt(); }\n"
+            ;
+
+    const OverrideItemList immediateResults = OverrideItemList()
+            << OverrideItem(QLatin1String("B::virt"), 2)
+            << OverrideItem(QLatin1String("...searching overrides"));
+    const OverrideItemList finalResults = OverrideItemList()
+            << OverrideItem(QLatin1String("B::virt"), 2)
+            << OverrideItem(QLatin1String("D::virt"), 5);
+
+    TestCase test(TestCase::FollowSymbolUnderCursorAction, source, immediateResults, finalResults);
+    test.run();
+}
+
+/// Variation of test_FollowSymbolUnderCursor_virtualFunctionCall_noSiblings_references
+void CppEditorPlugin::test_FollowSymbolUnderCursor_virtualFunctionCall_noSiblings_noBaseExpression()
+{
+    const QByteArray source =
+            "struct A { virtual void virt() {} };\n"
+            "struct B : A { void virt() {} };\n"
+            "struct C1 : B { void virt() {} };\n"
+            "struct C2 : B { void g() { $@virt(); } };\n"
+            "struct D : C2 { void virt() {} };\n"
+            ;
+
+    const OverrideItemList immediateResults = OverrideItemList()
+            << OverrideItem(QLatin1String("B::virt"), 2)
+            << OverrideItem(QLatin1String("...searching overrides"));
+    const OverrideItemList finalResults = OverrideItemList()
+            << OverrideItem(QLatin1String("B::virt"), 2)
+            << OverrideItem(QLatin1String("D::virt"), 5);
+
+    TestCase test(TestCase::FollowSymbolUnderCursorAction, source, immediateResults, finalResults);
+    test.run();
+}
+
 /// Check: Trigger on a.virt() if a is of type &A.
 void CppEditorPlugin::test_FollowSymbolUnderCursor_virtualFunctionCall_onDotMemberAccessOfReferenceTypes()
 {