ClangCodeModel: Use clangd for local renaming

Change-Id: I1536265a8d46c9840e722bdfcb8638906d3f45cf
Reviewed-by: David Schulz <david.schulz@qt.io>

src/plugins/clangcodemodel/clangcodemodelplugin.cpp

@@ -207,6 +207,7 @@ QVector<QObject *> ClangCodeModelPlugin::createTestObjects() const
         new Tests::ClangCodeCompletionTest,
         new Tests::ClangdTestFindReferences,
         new Tests::ClangdTestFollowSymbol,
+        new Tests::ClangdTestLocalReferences,
     };
 }
 #endif

src/plugins/clangcodemodel/clangdclient.cpp

@@ -25,6 +25,7 @@
 
 #include "clangdclient.h"
 
+#include <clangsupport/sourcelocationscontainer.h>
 #include <coreplugin/editormanager/editormanager.h>
 #include <coreplugin/find/searchresultitem.h>
 #include <coreplugin/find/searchresultwindow.h>
@@ -71,10 +72,11 @@ class AstParams : public JsonObject
 {
 public:
     AstParams() {}
-    AstParams(const TextDocumentIdentifier &document, const Range &range)
+    AstParams(const TextDocumentIdentifier &document, const Range &range = {})
     {
         setTextDocument(document);
-        setRange(range);
+        if (range.isValid())
+            setRange(range);
     }
 
     using JsonObject::JsonObject;
@@ -575,6 +577,28 @@ public:
     Utils::optional<AstNode> ast;
 };
 
+class LocalRefsData {
+public:
+    LocalRefsData(quint64 id, TextEditor::TextDocument *doc, const QTextCursor &cursor,
+                      CppTools::RefactoringEngineInterface::RenameCallback &&callback)
+        : id(id), document(doc), cursor(cursor), callback(std::move(callback)),
+          uri(DocumentUri::fromFilePath(doc->filePath())), revision(doc->document()->revision())
+    {}
+
+    ~LocalRefsData()
+    {
+        if (callback)
+            callback({}, {}, revision);
+    }
+
+    const quint64 id;
+    const QPointer<TextEditor::TextDocument> document;
+    const QTextCursor cursor;
+    CppTools::RefactoringEngineInterface::RenameCallback callback;
+    const DocumentUri uri;
+    const int revision;
+};
+
 
 class ClangdClient::Private
 {
@@ -600,14 +624,18 @@ public:
 
     void handleDeclDefSwitchReplies();
 
+    QString searchTermFromCursor(const QTextCursor &cursor) const;
+
     ClangdClient * const q;
     QHash<quint64, ReferencesData> runningFindUsages;
     Utils::optional<FollowSymbolData> followSymbolData;
     Utils::optional<SwitchDeclDefData> switchDeclDefData;
+    Utils::optional<LocalRefsData> localRefsData;
     Utils::optional<QVersionNumber> versionNumber;
     quint64 nextFindUsagesKey = 0;
     quint64 nextFollowSymbolId = 0;
     quint64 nextSwitchDeclDefId = 0;
+    quint64 nextLocalRefsId = 0;
     bool isFullyIndexed = false;
     bool isTesting = false;
 };
@@ -694,9 +722,8 @@ void ClangdClient::closeExtraFile(const Utils::FilePath &filePath)
 void ClangdClient::findUsages(TextEditor::TextDocument *document, const QTextCursor &cursor,
                               const Utils::optional<QString> &replacement)
 {
-    QTextCursor termCursor(cursor);
+    // TODO: This will be wrong for e.g. operators. Use a Symbol info request to get the real symbol string.
-    termCursor.select(QTextCursor::WordUnderCursor);
+    const QString searchTerm = d->searchTermFromCursor(cursor);
-    const QString searchTerm = termCursor.selectedText(); // TODO: This will be wrong for e.g. operators. Use a Symbol info request to get the real symbol string.
     if (searchTerm.isEmpty())
         return;
 
@@ -1054,6 +1081,94 @@ void ClangdClient::switchDeclDef(TextEditor::TextDocument *document, const QText
 
 }
 
+void ClangdClient::findLocalUsages(TextEditor::TextDocument *document, const QTextCursor &cursor,
+        CppTools::RefactoringEngineInterface::RenameCallback &&callback)
+{
+    QTC_ASSERT(documentOpen(document), openDocument(document));
+
+    qCDebug(clangdLog) << "local references requested" << document->filePath()
+                       << (cursor.blockNumber() + 1) << (cursor.positionInBlock() + 1);
+
+    d->localRefsData.emplace(++d->nextLocalRefsId, document, cursor, std::move(callback));
+    const QString searchTerm = d->searchTermFromCursor(cursor);
+    if (searchTerm.isEmpty()) {
+        d->localRefsData.reset();
+        return;
+    }
+
+    // Step 1: Go to definition
+    const auto gotoDefCallback = [this, id = d->localRefsData->id](const Utils::Link &link) {
+        qCDebug(clangdLog) << "received go to definition response" << link.targetFilePath
+                           << link.targetLine << (link.targetColumn + 1);
+        if (!d->localRefsData || id != d->localRefsData->id)
+            return;
+        if (!link.hasValidTarget()) {
+            d->localRefsData.reset();
+            return;
+        }
+
+        // Step 2: Get AST and check whether it's a local variable.
+        AstRequest astRequest(AstParams(TextDocumentIdentifier(d->localRefsData->uri)));
+        astRequest.setResponseCallback([this, link, id](const AstRequest::Response &response) {
+            qCDebug(clangdLog) << "received ast response";
+            if (!d->localRefsData || id != d->localRefsData->id)
+                return;
+            const auto result = response.result();
+            if (!result || !d->localRefsData->document) {
+                d->localRefsData.reset();
+                return;
+            }
+
+            const Position linkPos(link.targetLine - 1, link.targetColumn);
+            const QList<AstNode> astPath = getAstPath(*result, Range(linkPos, linkPos));
+            bool isVar = false;
+            for (auto it = astPath.rbegin(); it != astPath.rend(); ++it) {
+                if (it->role() == "declaration" && it->kind() == "Function") {
+                    if (!isVar)
+                        break;
+
+                    // Step 3: Find references.
+                    qCDebug(clangdLog) << "finding references for local var";
+                    symbolSupport().findUsages(d->localRefsData->document,
+                                               d->localRefsData->cursor,
+                                               [this, id](const QList<Location> &locations) {
+                        qCDebug(clangdLog) << "found" << locations.size() << "local references";
+                        if (!d->localRefsData || id != d->localRefsData->id)
+                            return;
+                        ClangBackEnd::SourceLocationsContainer container;
+                        for (const Location &loc : locations) {
+                            container.insertSourceLocation({}, loc.range().start().line() + 1,
+                                                           loc.range().start().character() + 1);
+                        }
+
+                        // The callback only uses the symbol length, so we just create a dummy.
+                        // Note that the calculation will be wrong for identifiers with
+                        // embedded newlines, but we've never supported that.
+                        QString symbol;
+                        if (!locations.isEmpty()) {
+                            const Range r = locations.first().range();
+                            symbol = QString(r.end().character() - r.start().character(), 'x');
+                        }
+                        d->localRefsData->callback(symbol, container, d->localRefsData->revision);
+                        d->localRefsData->callback = {};
+                        d->localRefsData.reset();
+                    });
+                    return;
+                }
+                if (!isVar && it->role() == "declaration"
+                        && (it->kind() == "Var" || it->kind() == "ParmVar")) {
+                    isVar = true;
+                }
+            }
+            d->localRefsData.reset();
+        });
+        qCDebug(clangdLog) << "sending ast request for link";
+        sendContent(astRequest);
+
+    };
+    symbolSupport().findLinkAt(document, cursor, std::move(gotoDefCallback), true);
+}
+
 void ClangdClient::Private::handleGotoDefinitionResult()
 {
     QTC_ASSERT(followSymbolData->defLink.hasValidTarget(), return);
@@ -1285,6 +1400,13 @@ void ClangdClient::Private::handleDeclDefSwitchReplies()
     switchDeclDefData.reset();
 }
 
+QString ClangdClient::Private::searchTermFromCursor(const QTextCursor &cursor) const
+{
+    QTextCursor termCursor(cursor);
+    termCursor.select(QTextCursor::WordUnderCursor);
+    return termCursor.selectedText();
+}
+
 void ClangdClient::VirtualFunctionAssistProcessor::cancel()
 {
     resetData();

src/plugins/clangcodemodel/clangdclient.h

@@ -25,6 +25,7 @@
 
 #pragma once
 
+#include <cpptools/refactoringengineinterface.h>
 #include <languageclient/client.h>
 #include <utils/link.h>
 #include <utils/optional.h>
@@ -66,6 +67,9 @@ public:
             CppTools::CppEditorWidgetInterface *editorWidget,
             Utils::ProcessLinkCallback &&callback);
 
+    void findLocalUsages(TextEditor::TextDocument *document, const QTextCursor &cursor,
+                         CppTools::RefactoringEngineInterface::RenameCallback &&callback);
+
     void enableTesting();
 
 signals:

src/plugins/clangcodemodel/clangrefactoringengine.cpp

@@ -42,6 +42,14 @@ void RefactoringEngine::startLocalRenaming(const CppTools::CursorInEditor &data,
                                            CppTools::ProjectPart *,
                                            RenameCallback &&renameSymbolsCallback)
 {
+    ClangdClient * const client
+            = ClangModelManagerSupport::instance()->clientForFile(data.filePath());
+    if (client && client->reachable()) {
+        client->findLocalUsages(data.textDocument(), data.cursor(),
+                                std::move(renameSymbolsCallback));
+        return;
+    }
+
     ClangEditorDocumentProcessor *processor = ClangEditorDocumentProcessor::get(
         data.filePath().toString());
     const int startRevision = data.cursor().document()->revision();

src/plugins/clangcodemodel/test/clangdtests.cpp

@@ -30,6 +30,7 @@
 #include "../clangdclient.h"
 #include "../clangmodelmanagersupport.h"
 
+#include <clangsupport/sourcelocationscontainer.h>
 #include <cplusplus/FindUsages.h>
 #include <cpptools/cppcodemodelsettings.h>
 #include <cpptools/cpptoolsreuse.h>
@@ -47,6 +48,8 @@
 #include <QTimer>
 #include <QtTest>
 
+#include <tuple>
+
 using namespace CPlusPlus;
 using namespace Core;
 using namespace CppTools::Tests;
@@ -119,8 +122,10 @@ void ClangdTest::initTestCase()
         QSKIP("clangd is too old");
 
     // Wait for index to build.
-    if (!m_client->isFullyIndexed())
+    if (!m_client->isFullyIndexed()) {
-        QVERIFY(waitForSignalOrTimeout(m_client, &ClangdClient::indexingFinished, timeOutInMs()));
+        QVERIFY(waitForSignalOrTimeout(m_client, &ClangdClient::indexingFinished,
+                                       clangdIndexingTimeout()));
+    }
     QVERIFY(m_client->isFullyIndexed());
 
     // Open cpp documents.
@@ -359,6 +364,127 @@ void ClangdTestFollowSymbol::test()
     QCOMPARE(actualLink.targetColumn + 1, targetColumn);
 }
 
+
+ClangdTestLocalReferences::ClangdTestLocalReferences()
+{
+    setProjectFileName("local-references.pro");
+    setSourceFileNames({"references.cpp"});
+    setMinimumVersion(13);
+}
+
+using Range = std::tuple<int, int, int>;
+
+// We currently only support local variables, but if and when clangd implements
+// the linkedEditingRange request, we can change the expected values for
+// the file-scope test cases from empty ranges to the actual locations.
+void ClangdTestLocalReferences::test_data()
+{
+    QTest::addColumn<int>("sourceLine");
+    QTest::addColumn<int>("sourceColumn");
+    QTest::addColumn<QList<Range>>("expectedRanges");
+
+    QTest::newRow("cursor not on identifier") << 3 << 5 << QList<Range>();
+    QTest::newRow("local variable, one use") << 3 << 9 << QList<Range>{{3, 9, 3}};
+    QTest::newRow("local variable, two uses") << 10 << 9
+                                              << QList<Range>{{10, 9, 3}, {11, 12, 3}};
+    QTest::newRow("class name") << 16 << 7 << QList<Range>()
+            /* QList<Range>{{16, 7, 3}, {19, 5, 3}} */;
+    QTest::newRow("namespace") << 24 << 11 << QList<Range>()
+            /* QList<Range>{{24, 11, 1}, {25, 11, 1}, {26, 1, 1}} */;
+    QTest::newRow("class name via using") << 30 << 21 << QList<Range>()
+            /* QList<Range>{{30, 21, 3}, {31, 10, 3}} */;
+    QTest::newRow("forward-declared class") << 35 << 7 << QList<Range>()
+            /* QList<Range>{{35, 7, 3}, {36, 14, 3}} */;
+    QTest::newRow("class name and new expression") << 40 << 7 << QList<Range>()
+            /* QList<Range>{{40, 7, 3}, {43, 9, 3}} */;
+    QTest::newRow("instantiated template object") << 52 << 19
+                                                  << QList<Range>{{52, 19, 3}, {53, 5, 3}};
+    QTest::newRow("variable in template") << 62 << 13 << QList<Range>()
+            /* QList<Range>{{62, 13, 3}, {63, 11, 3}} */;
+    QTest::newRow("member in template") << 67 << 7 << QList<Range>()
+            /* QList<Range>{{64, 16, 3}, {67, 7, 3}} */;
+    QTest::newRow("template type") << 58 << 19 << QList<Range>()
+            /* QList<Range>{{58, 19, 1}, {60, 5, 1}, {67, 5, 1}} */;
+    QTest::newRow("template parameter member access") << 76 << 9 << QList<Range>();
+    QTest::newRow("constructor as type") << 82 << 5 << QList<Range>()
+            /* QList<Range>{{81, 8, 3}, {82, 5, 3}, {83, 6, 3}} */;
+    QTest::newRow("freestanding overloads") << 88 << 5 << QList<Range>()
+            /* QList<Range>{{88, 5, 3}, {89, 5, 3}} */;
+    QTest::newRow("member function overloads") << 94 << 9 << QList<Range>()
+            /* QList<Range>{{94, 9, 3}, {95, 9, 3}} */;
+    QTest::newRow("function and function template") << 100 << 26 << QList<Range>()
+            /* QList<Range>{{100, 26, 3}, {101, 5, 3}} */;
+    QTest::newRow("function and function template as member") << 106 << 30 << QList<Range>()
+            /* QList<Range>{{106, 30, 3}, {107, 9, 3}} */;
+    QTest::newRow("enum type") << 112 << 6 << QList<Range>()
+            /* QList<Range>{{112, 6, 2}, {113, 8, 2}} */;
+    QTest::newRow("captured lambda var") << 122 << 15
+                                         << QList<Range>{{122, 15, 3}, {122, 33, 3}};
+    QTest::newRow("lambda initializer") << 122 << 19
+                                         << QList<Range>{{121, 19, 3}, {122, 19, 3}};
+    QTest::newRow("template specialization") << 127 << 25 << QList<Range>()
+            /* QList<Range>{{127, 5, 3}, {128, 25, 3}, {129, 18, 3}} */;
+    QTest::newRow("dependent name") << 133 << 34 << QList<Range>()
+            /* QList<Range>{{133, 34, 3} */;
+    QTest::newRow("function call and definition") << 140 << 5 << QList<Range>()
+            /* QList<Range>{{140, 5, 3}, {142, 25, 3}} */;
+    QTest::newRow("object-like macro") << 147 << 9 << QList<Range>()
+            /* QList<Range>{{147, 9, 3}, {150, 12, 3}} */;
+    QTest::newRow("function-like macro") << 155 << 9 << QList<Range>()
+            /* QList<Range>{{155, 9, 3}, {158, 12, 3}} */;
+    QTest::newRow("argument to function-like macro") << 156 << 27
+            << QList<Range>{{156, 27, 3}, {158, 16, 3}};
+    QTest::newRow("overloaded bracket operator argument") << 172 << 7
+            << QList<Range>{{171, 7, 1}, {172, 7, 1}, {172, 12, 1},
+                     {173, 7, 1}, {173, 10, 1}};
+    QTest::newRow("overloaded call operator second argument") << 173 << 10
+            << QList<Range>{{171, 7, 1}, {172, 7, 1}, {172, 12, 1},
+                     {173, 7, 1}, {173, 10, 1}};
+    QTest::newRow("overloaded operators arguments from outside") << 171 << 7
+            << QList<Range>{{171, 7, 1}, {172, 7, 1}, {172, 12, 1},
+                     {173, 7, 1}, {173, 10, 1}};
+}
+
+void ClangdTestLocalReferences::test()
+{
+    QFETCH(int, sourceLine);
+    QFETCH(int, sourceColumn);
+    QFETCH(QList<Range>, expectedRanges);
+
+    TextEditor::TextDocument * const doc = document("references.cpp");
+    QVERIFY(doc);
+
+    QTimer timer;
+    timer.setSingleShot(true);
+    QEventLoop loop;
+    QObject::connect(&timer, &QTimer::timeout, &loop, &QEventLoop::quit);
+    QList<Range> actualRanges;
+    const auto handler = [&actualRanges, &loop](const QString &symbol,
+            const ClangBackEnd::SourceLocationsContainer &container, int) {
+        for (const ClangBackEnd::SourceLocationContainer &c
+             : container.m_sourceLocationContainers) {
+            actualRanges << Range(c.line, c.column, symbol.length());
+        }
+        loop.quit();
+    };
+
+    QTextCursor cursor(doc->document());
+    const int pos = Utils::Text::positionInText(doc->document(), sourceLine, sourceColumn);
+    cursor.setPosition(pos);
+    client()->findLocalUsages(doc, cursor, std::move(handler));
+    timer.start(10000);
+    loop.exec();
+    QEXPECT_FAIL("cursor not on identifier", "clangd bug: go to definition does not return", Abort);
+    QEXPECT_FAIL("template parameter member access",
+                 "clangd bug: go to definition does not return", Abort);
+    QVERIFY(timer.isActive());
+    timer.stop();
+
+    QCOMPARE(actualRanges, expectedRanges);
+}
+
 } // namespace Tests
 } // namespace Internal
 } // namespace ClangCodeModel
+
+Q_DECLARE_METATYPE(ClangCodeModel::Internal::Tests::Range)

src/plugins/clangcodemodel/test/clangdtests.h

@@ -104,6 +104,17 @@ private slots:
     void test();
 };
 
+class ClangdTestLocalReferences : public ClangdTest
+{
+    Q_OBJECT
+public:
+    ClangdTestLocalReferences();
+
+private slots:
+    void test_data();
+    void test();
+};
+
 } // namespace Tests
 } // namespace Internal
 } // namespace ClangCodeModel

src/plugins/clangcodemodel/test/data/clangtestdata.qrc

@@ -37,5 +37,7 @@
         <file>follow-symbol/follow-symbol.pro</file>
         <file>follow-symbol/header.h</file>
         <file>follow-symbol/main.cpp</file>
+        <file>local-references/local-references.pro</file>
+        <file>local-references/references.cpp</file>
     </qresource>
 </RCC>

src/plugins/clangcodemodel/test/data/local-references/local-references.pro

@@ -0,0 +1,5 @@
+TEMPLATE = app
+
+CONFIG -= qt
+
+SOURCES = references.cpp

src/plugins/clangcodemodel/test/data/local-references/references.cpp

@@ -0,0 +1,174 @@
+void variableSingleReference()
+{
+    int foo;
+}
+
+
+
+int variableMultipleReferences()
+{
+    int foo = 0;
+    return foo;
+}
+
+
+
+class Foo {};
+void bla()
+{
+    Foo foo;
+}
+
+
+
+namespace N { class Bar {}; }
+namespace N { class Baz {}; }
+N::Bar bar;
+
+
+
+namespace G { class App {}; }
+using G::App;
+
+
+
+class Hoo;
+void f(const Hoo &);
+
+
+
+class Moo {};
+void x()
+{
+    new Moo;
+}
+
+
+
+class Element {};
+template<typename T> struct Wrap { T member; };
+void g()
+{
+    Wrap<Element> con;
+    con.member;
+}
+
+
+
+template<typename T>
+struct Wrapper {
+    T f()
+    {
+        int foo;
+        ++foo;
+        return mem;
+    }
+
+    T mem;
+};
+
+
+
+template<typename T>
+void f()
+{
+    T mem;
+    mem.foo();
+}
+
+
+
+struct Woo {
+    Woo();
+    ~Woo();
+};
+
+
+
+int muu();
+int muu(int);
+
+
+
+struct Doo {
+    int muu();
+    int muu(int);
+};
+
+
+
+template<typename T> int tuu();
+int tuu(int);
+
+
+
+struct Xoo {
+    template<typename T> int tuu();
+    int tuu(int);
+};
+
+
+
+enum ET { E1 };
+bool e(ET e)
+{
+    return e == E1;
+}
+
+
+
+struct LData { int member; };
+void lambda(LData foo) {
+    auto l = [bar=foo] { return bar.member; };
+}
+
+
+
+template<class T> class Coo;
+template<class T> class Coo<T*>;
+template<> class Coo<int>;
+
+
+
+template<typename T> typename T::foo n()
+{
+    typename T::bla hello;
+}
+
+
+
+int rec(int n = 100)
+{
+    return n == 0 ? 0 : rec(--n);
+}
+
+
+
+#define FOO 3
+int objectLikeMacro()
+{
+    return FOO;
+}
+
+
+
+#define BAR(x) x
+int functionLikeMacro(int foo)
+{
+    return BAR(foo);
+}
+
+template<class T>
+class Container
+{
+public:
+    T &operator[](int); T &operator()(int, int);
+};
+
+int testOperator() {
+  Container<int> vec;
+
+  int n = 10;
+  vec[n] = n * 100;
+  vec(n, n) = 100;
+}

src/plugins/cppeditor/cppeditorwidget.cpp

@@ -691,7 +691,7 @@ void CppEditorWidget::renameSymbolUnderCursor()
     viewport()->setCursor(Qt::BusyCursor);
     d->m_modelManager->startLocalRenaming(CppTools::CursorInEditor{textCursor(),
                                                                    textDocument()->filePath(),
-                                                                   this},
+                                                                   this, textDocument()},
                                           projPart,
                                           std::move(renameSymbols));
 }