CppTools: Also look up definitions for variables

... not just functions.
This includes global variables and static members.

Fixes: QTCREATORBUG-18828
Change-Id: Iee9f83a4f955a859c6fc4038c61997b30afdaec8
Reviewed-by: Christian Stenger <christian.stenger@qt.io>

src/plugins/cppeditor/cppeditorwidget.cpp

@@ -696,6 +696,7 @@ void CppEditorWidget::switchDeclarationDefinition(bool inNextSplit)
     // Find function declaration or definition under cursor
     Function *functionDefinitionSymbol = nullptr;
     Symbol *functionDeclarationSymbol = nullptr;
+    Symbol *declarationSymbol = nullptr;
 
     ASTPath astPathFinder(d->m_lastSemanticInfo.doc);
     const QList<AST *> astPath = astPathFinder(textCursor());
@@ -707,9 +708,12 @@ void CppEditorWidget::switchDeclarationDefinition(bool inNextSplit)
         } else if (SimpleDeclarationAST *simpleDeclaration = ast->asSimpleDeclaration()) {
             if (List<Symbol *> *symbols = simpleDeclaration->symbols) {
                 if (Symbol *symbol = symbols->value) {
-                    if (symbol->isDeclaration() && symbol->type()->isFunctionType()) {
+                    if (symbol->isDeclaration()) {
-                        functionDeclarationSymbol = symbol;
+                        declarationSymbol = symbol;
-                        break; // Function declaration found!
+                        if (symbol->type()->isFunctionType()) {
+                            functionDeclarationSymbol = symbol;
+                            break; // Function declaration found!
+                        }
                     }
                 }
             }
@@ -723,6 +727,11 @@ void CppEditorWidget::switchDeclarationDefinition(bool inNextSplit)
                 ->findMatchingDefinition(functionDeclarationSymbol, d->m_modelManager->snapshot());
         if (symbol)
             symbolLink = symbol->toLink();
+    } else if (declarationSymbol) {
+        Symbol *symbol = d->m_modelManager->symbolFinder()
+                ->findMatchingVarDefinition(declarationSymbol, d->m_modelManager->snapshot());
+        if (symbol)
+            symbolLink = symbol->toLink();
     } else if (functionDefinitionSymbol) {
         const Snapshot snapshot = d->m_modelManager->snapshot();
         LookupContext context(d->m_lastSemanticInfo.doc, snapshot);

src/plugins/cppeditor/followsymbol_switchmethoddecldef_test.cpp

@@ -554,6 +554,31 @@ void CppEditorPlugin::test_SwitchMethodDeclarationDefinition_data()
         "class Foo { void $" TEST_UNICODE_IDENTIFIER "(); };\n"
         "void Foo::@" TEST_UNICODE_IDENTIFIER "() {}\n"
     ) << _();
+
+    QTest::newRow("globalVar")
+            << _("namespace NS { extern int @globalVar; }\n")
+            << _("int globalVar;\n"
+                 "namespace NS {\n"
+                 "extern int globalVar;\n"
+                 "int $globalVar;\n"
+                 "}\n");
+
+    QTest::newRow("staticMemberVar")
+            << _("namespace NS {\n"
+                 "class Test {\n"
+                 "    static int @var;\n"
+                 "};\n"
+                 "class OtherClass { static int var; };\n"
+                 "}\n"
+                 "class OtherClass { static int var; };\n")
+            << _("#include \"file.h\"\n"
+                 "int var;\n"
+                 "int OtherClass::var;\n"
+                 "namespace NS {\n"
+                 "int OtherClass::var;\n"
+                 "float Test::var;\n"
+                 "int Test::$var;\n"
+                 "}\n");
 }
 
 void CppEditorPlugin::test_SwitchMethodDeclarationDefinition()
@@ -1145,6 +1170,34 @@ void CppEditorPlugin::test_FollowSymbolUnderCursor_multipleDocuments_data()
                                 "bool *$fun(C *) const {}\n",
                                 "foo.cpp")
     );
+
+    QTest::newRow("globalVar") << QList<TestDocumentPtr>{
+            TestDocument::create("namespace NS { extern int @globalVar; }\n", "file.h"),
+            TestDocument::create(
+                "int globalVar;\n"
+                "namespace NS {\n"
+                "extern int globalVar;\n"
+                "int $globalVar;\n"
+                "}\n", "file.cpp")};
+
+    QTest::newRow("staticMemberVar") << QList<TestDocumentPtr>{
+        TestDocument::create(
+            "namespace NS {\n"
+            "class Test {\n"
+            "    static int @var;\n"
+            "};\n"
+            "class OtherClass { static int var; };\n"
+            "}\n"
+            "class OtherClass { static int var; };\n", "file.h"),
+        TestDocument::create(
+            "#include \"file.h\"\n"
+            "int var;\n"
+            "int OtherClass::var;\n"
+            "namespace NS {\n"
+            "int OtherClass::var;\n"
+            "float Test::var;\n"
+            "int Test::$var;\n"
+            "}\n", "file.cpp")};
 }
 
 void CppEditorPlugin::test_FollowSymbolUnderCursor_multipleDocuments()

src/plugins/cpptools/cppfollowsymbolundercursor.cpp

@@ -292,9 +292,9 @@ inline LookupItem skipForwardDeclarations(const QList<LookupItem> &resolvedSymbo
     return result;
 }
 
-Link attemptFuncDeclDef(const QTextCursor &cursor, Snapshot snapshot,
+Link attemptDeclDef(const QTextCursor &cursor, Snapshot snapshot,
-                        const Document::Ptr &document,
+                    const Document::Ptr &document,
-                        SymbolFinder *symbolFinder)
+                    SymbolFinder *symbolFinder)
 {
     Link result;
     QTC_ASSERT(document, return result);
@@ -333,11 +333,6 @@ Link attemptFuncDeclDef(const QTextCursor &cursor, Snapshot snapshot,
     }
     if (!decl || !declParent)
         return result;
-    if (!decl->postfix_declarator_list || !decl->postfix_declarator_list->value)
-        return result;
-    FunctionDeclaratorAST *funcDecl = decl->postfix_declarator_list->value->asFunctionDeclarator();
-    if (!funcDecl)
-        return result;
 
     Symbol *target = nullptr;
     if (FunctionDefinitionAST *funDef = declParent->asFunctionDefinition()) {
@@ -346,8 +341,14 @@ Link attemptFuncDeclDef(const QTextCursor &cursor, Snapshot snapshot,
                                                         funDef->symbol);
         if (!candidates.isEmpty()) // TODO: improve disambiguation
             target = candidates.first();
-    } else if (declParent->asSimpleDeclaration()) {
+    } else if (const SimpleDeclarationAST * const simpleDecl = declParent->asSimpleDeclaration()) {
-        target = symbolFinder->findMatchingDefinition(funcDecl->symbol, snapshot);
+        FunctionDeclaratorAST *funcDecl = nullptr;
+        if (decl->postfix_declarator_list && decl->postfix_declarator_list->value)
+            funcDecl = decl->postfix_declarator_list->value->asFunctionDeclarator();
+        if (funcDecl)
+            target = symbolFinder->findMatchingDefinition(funcDecl->symbol, snapshot);
+        else
+            target = symbolFinder->findMatchingVarDefinition(simpleDecl->symbols->value, snapshot);
     }
 
     if (target) {
@@ -514,9 +515,9 @@ void FollowSymbolUnderCursor::findLink(
         int pos = tc.position();
         while (document->characterAt(pos).isSpace())
             ++pos;
-        if (document->characterAt(pos) == QLatin1Char('(')) {
+        const QChar ch = document->characterAt(pos);
-            link = attemptFuncDeclDef(cursor, snapshot, documentFromSemanticInfo,
+        if (ch == '(' || ch == ';') {
-                                      symbolFinder);
+            link = attemptDeclDef(cursor, snapshot, documentFromSemanticInfo, symbolFinder);
             if (link.hasValidLinkText())
                 return processLinkCallback(link);
         }
@@ -585,16 +586,15 @@ void FollowSymbolUnderCursor::findLink(
             if (positionInBlock >= tk.utf16charsBegin() && positionInBlock <= tk.utf16charsEnd()) {
                 cursorRegionReached = true;
                 if (tk.is(T_OPERATOR)) {
-                    link = attemptFuncDeclDef(cursor, theSnapshot,
+                    link = attemptDeclDef(cursor, theSnapshot,
-                                              documentFromSemanticInfo, symbolFinder);
+                                          documentFromSemanticInfo, symbolFinder);
                     if (link.hasValidLinkText())
                         return processLinkCallback(link);
                 } else if (tk.isPunctuationOrOperator() && i > 0 && tokens.at(i - 1).is(T_OPERATOR)) {
                     QTextCursor c = cursor;
                     c.movePosition(QTextCursor::Left, QTextCursor::MoveAnchor,
                                    positionInBlock - tokens.at(i - 1).utf16charsBegin());
-                    link = attemptFuncDeclDef(c, theSnapshot, documentFromSemanticInfo,
+                    link = attemptDeclDef(c, theSnapshot, documentFromSemanticInfo, symbolFinder);
-                                              symbolFinder);
                     if (link.hasValidLinkText())
                         return processLinkCallback(link);
                 }

src/plugins/cpptools/symbolfinder.cpp

@@ -36,6 +36,7 @@
 #include <utils/qtcassert.h>
 
 #include <QDebug>
+#include <QPair>
 
 #include <algorithm>
 #include <utility>
@@ -84,6 +85,46 @@ public:
     }
 };
 
+class FindMatchingVarDefinition: public SymbolVisitor
+{
+    Symbol *_declaration = nullptr;
+    QList<Declaration *> _result;
+    const Identifier *_className = nullptr;
+
+public:
+    explicit FindMatchingVarDefinition(Symbol *declaration)
+        : _declaration(declaration)
+    {
+        if (declaration->isStatic() && declaration->enclosingScope()->asClass()
+                && declaration->enclosingClass()->asClass()->name()) {
+            _className = declaration->enclosingScope()->name()->identifier();
+        }
+    }
+
+    const QList<Declaration *> result() const { return _result; }
+
+    using SymbolVisitor::visit;
+
+    bool visit(Declaration *decl) override
+    {
+        if (!decl->type()->match(_declaration->type().type()))
+            return false;
+        if (!_declaration->identifier()->equalTo(decl->identifier()))
+            return false;
+        if (_className) {
+            const QualifiedNameId * const qualName = decl->name()->asQualifiedNameId();
+            if (!qualName)
+                return false;
+            if (!qualName->base() || !qualName->base()->identifier()->equalTo(_className))
+                return false;
+        }
+        _result.append(decl);
+        return false;
+    }
+
+    bool visit(Block *) override { return false; }
+};
+
 } // end of anonymous namespace
 
 static const int kMaxCacheSize = 10;
@@ -207,6 +248,87 @@ Function *SymbolFinder::findMatchingDefinition(Symbol *declaration,
     return nullptr;
 }
 
+Symbol *SymbolFinder::findMatchingVarDefinition(Symbol *declaration, const Snapshot &snapshot)
+{
+    if (!declaration)
+        return nullptr;
+    for (const Scope *s = declaration->enclosingScope(); s; s = s->enclosingScope()) {
+        if (s->asBlock())
+            return nullptr;
+    }
+
+    QString declFile = QString::fromUtf8(declaration->fileName(), declaration->fileNameLength());
+    const Document::Ptr thisDocument = snapshot.document(declFile);
+    if (!thisDocument) {
+        qWarning() << "undefined document:" << declaration->fileName();
+        return nullptr;
+    }
+
+    using SymbolWithPriority = QPair<Symbol *, bool>;
+    QList<SymbolWithPriority> candidates;
+    QList<SymbolWithPriority> fallbacks;
+    foreach (const QString &fileName, fileIterationOrder(declFile, snapshot)) {
+        Document::Ptr doc = snapshot.document(fileName);
+        if (!doc) {
+            clearCache(declFile, fileName);
+            continue;
+        }
+
+        const Identifier *id = declaration->identifier();
+        if (id && !doc->control()->findIdentifier(id->chars(), id->size()))
+            continue;
+
+        FindMatchingVarDefinition finder(declaration);
+        finder.accept(doc->globalNamespace());
+        if (finder.result().isEmpty())
+            continue;
+
+        LookupContext context(doc, snapshot);
+        ClassOrNamespace * const enclosingType = context.lookupType(declaration);
+        for (Symbol * const symbol : finder.result()) {
+            const QList<LookupItem> items = context.lookup(symbol->name(),
+                                                           symbol->enclosingScope());
+            bool addFallback = true;
+            for (const LookupItem &item : items) {
+                if (item.declaration() == symbol)
+                    addFallback = false;
+                candidates << qMakePair(item.declaration(),
+                                        context.lookupType(item.declaration()) == enclosingType);
+            }
+            // TODO: This is a workaround for static member definitions not being found by
+            //       the lookup() function.
+            if (addFallback)
+                fallbacks << qMakePair(symbol, context.lookupType(symbol) == enclosingType);
+        }
+    }
+
+    candidates << fallbacks;
+    SymbolWithPriority best;
+    for (const auto &candidate : qAsConst(candidates)) {
+        if (candidate.first == declaration)
+            continue;
+        if (QLatin1String(candidate.first->fileName()) == declFile
+                && candidate.first->sourceLocation() == declaration->sourceLocation())
+            continue;
+        if (!candidate.first->asDeclaration())
+            continue;
+        if (declaration->isExtern() && candidate.first->isStatic())
+            continue;
+        if (!best.first) {
+            best = candidate;
+            continue;
+        }
+        if (!best.second && candidate.second) {
+            best = candidate;
+            continue;
+        }
+        if (best.first->isExtern() && !candidate.first->isExtern())
+            best = candidate;
+    }
+
+    return best.first;
+}
+
 Class *SymbolFinder::findMatchingClassDeclaration(Symbol *declaration, const Snapshot &snapshot)
 {
     if (!declaration->identifier())

src/plugins/cpptools/symbolfinder.h

@@ -54,6 +54,9 @@ public:
                                                 const CPlusPlus::Snapshot &snapshot,
                                                 bool strict = false);
 
+    CPlusPlus::Symbol *findMatchingVarDefinition(CPlusPlus::Symbol *declaration,
+                                                 const CPlusPlus::Snapshot &snapshot);
+
     CPlusPlus::Class *findMatchingClassDeclaration(CPlusPlus::Symbol *declaration,
                                                    const CPlusPlus::Snapshot &snapshot);