DocumentLocatorFilter: Refactor internals

Avoid use of LocatorFilterEntry::internalData inside
LspCurrentDocumentFilter.

Change-Id: I5eb4831919281ab11a630bf7810890a72a1423bb
Reviewed-by: David Schulz <david.schulz@qt.io>
Reviewed-by: <github-actions-qt-creator@cristianadam.eu>
Reviewed-by: Qt CI Bot <qt_ci_bot@qt-project.org>

src/plugins/clangcodemodel/clangdlocatorfilters.cpp

@@ -209,46 +209,54 @@ private:
         m_content = TextEditor::TextDocument::currentTextDocument()->plainText();
     }
 
-    LocatorFilterEntry generateLocatorEntry(const DocumentSymbol &info,
-                                            const LocatorFilterEntry &parent) override
-    {
-        LocatorFilterEntry entry;
-        entry.filter = this;
-        entry.displayName = ClangdClient::displayNameFromDocumentSymbol(
-                    static_cast<SymbolKind>(info.kind()), info.name(),
-                    info.detail().value_or(QString()));
-        entry.internalData = QVariant::fromValue(info);
-        const Position pos = info.range().start();
-        entry.linkForEditor = {m_currentFilePath, pos.line() + 1, pos.character()};
-        entry.extraInfo = parent.extraInfo;
-        if (!entry.extraInfo.isEmpty())
-            entry.extraInfo.append("::");
-        entry.extraInfo.append(parent.displayName);
-
-        // TODO: Can we extend clangd to send visibility information?
-        entry.displayIcon = LanguageClient::symbolIcon(info.kind());
-
-        return entry;
-    }
-
     // Filter out declarations for which a definition is also present.
     QList<LocatorFilterEntry> matchesFor(QFutureInterface<LocatorFilterEntry> &future,
                                          const QString &entry) override
     {
-        QList<LocatorFilterEntry> allMatches
+        struct Entry
-            = DocumentLocatorFilter::matchesFor(future, entry);
+        {
-        QHash<QString, QList<LocatorFilterEntry>> possibleDuplicates;
+            LocatorFilterEntry entry;
-        for (const LocatorFilterEntry &e : std::as_const(allMatches))
+            DocumentSymbol symbol;
-            possibleDuplicates[e.displayName + e.extraInfo] << e;
+        };
+        QList<Entry> docEntries;
+
+        const auto docSymbolGenerator = [&](const DocumentSymbol &info,
+                                            const LocatorFilterEntry &parent) {
+            LocatorFilterEntry entry;
+            entry.filter = this;
+            entry.displayName = ClangdClient::displayNameFromDocumentSymbol(
+                static_cast<SymbolKind>(info.kind()), info.name(),
+                info.detail().value_or(QString()));
+            entry.internalData = QVariant::fromValue(info);
+            entry.linkForEditor = linkForDocSymbol(info);
+            entry.extraInfo = parent.extraInfo;
+            if (!entry.extraInfo.isEmpty())
+                entry.extraInfo.append("::");
+            entry.extraInfo.append(parent.displayName);
+
+            // TODO: Can we extend clangd to send visibility information?
+            entry.displayIcon = LanguageClient::symbolIcon(info.kind());
+            docEntries.append({entry, info});
+            return entry;
+        };
+
+        QList<LocatorFilterEntry> allMatches = matchesForImpl(future, entry, docSymbolGenerator);
+        if (docEntries.isEmpty())
+            return allMatches; // SymbolInformation case
+
+        QTC_CHECK(docEntries.size() == allMatches.size());
+        QHash<QString, QList<Entry>> possibleDuplicates;
+        for (const Entry &e : std::as_const(docEntries))
+            possibleDuplicates[e.entry.displayName + e.entry.extraInfo] << e;
         const QTextDocument doc(m_content);
         for (auto it = possibleDuplicates.cbegin(); it != possibleDuplicates.cend(); ++it) {
-            const QList<LocatorFilterEntry> &duplicates = it.value();
+            const QList<Entry> &duplicates = it.value();
             if (duplicates.size() == 1)
                 continue;
-            QList<LocatorFilterEntry> declarations;
+            QList<Entry> declarations;
-            QList<LocatorFilterEntry> definitions;
+            QList<Entry> definitions;
-            for (const LocatorFilterEntry &candidate : duplicates) {
+            for (const Entry &candidate : duplicates) {
-                const auto symbol = qvariant_cast<DocumentSymbol>(candidate.internalData);
+                const DocumentSymbol symbol = candidate.symbol;
                 const SymbolKind kind = static_cast<SymbolKind>(symbol.kind());
                 if (kind != SymbolKind::Class && kind != SymbolKind::Function)
                     break;
@@ -273,14 +281,14 @@ private:
             }
             if (definitions.size() == 1
                 && declarations.size() + definitions.size() == duplicates.size()) {
-                for (const LocatorFilterEntry &decl : std::as_const(declarations)) {
+                for (const Entry &decl : std::as_const(declarations)) {
-                    Utils::erase(allMatches, [&decl](const LocatorFilterEntry &e) {
+                    Utils::erase(docEntries, [&decl](const Entry &e) {
-                        return e.internalData == decl.internalData;
+                        return e.symbol == decl.symbol;
                     });
                 }
             }
         }
-        return allMatches;
+        return Utils::transform(docEntries, [](const Entry &entry) { return entry.entry; });
     }
 
     QString m_content;

src/plugins/languageclient/locatorfilter.cpp

@@ -98,69 +98,31 @@ static LocatorFilterEntry generateLocatorEntry(const SymbolInformation &info,
     return entry;
 }
 
-LocatorFilterEntry DocumentLocatorFilter::generateLocatorEntry(const SymbolInformation &info)
+QList<LocatorFilterEntry> DocumentLocatorFilter::entriesForSymbolsInfo(
+    const QList<SymbolInformation> &infoList, const QRegularExpression &regexp)
 {
     QTC_ASSERT(m_pathMapper, return {});
-    return LanguageClient::generateLocatorEntry(info, this, m_pathMapper);
-}
-
-QList<LocatorFilterEntry> DocumentLocatorFilter::generateLocatorEntries(
-        const SymbolInformation &info, const QRegularExpression &regexp,
-        const LocatorFilterEntry &parent)
-{
-    Q_UNUSED(parent)
-    if (regexp.match(info.name()).hasMatch())
-        return {generateLocatorEntry(info)};
-    return {};
-}
-
-LocatorFilterEntry DocumentLocatorFilter::generateLocatorEntry(const DocumentSymbol &info,
-                                                               const LocatorFilterEntry &parent)
-{
-    Q_UNUSED(parent)
-    LocatorFilterEntry entry;
-    entry.filter = this;
-    entry.displayName = info.name();
-    if (std::optional<QString> detail = info.detail())
-        entry.extraInfo = detail.value_or(QString());
-    entry.displayIcon = symbolIcon(info.kind());
-    const Position &pos = info.range().start();
-    entry.linkForEditor = {m_currentFilePath, pos.line() + 1, pos.character()};
-    return entry;
-}
-
-QList<LocatorFilterEntry> DocumentLocatorFilter::generateLocatorEntries(
-        const DocumentSymbol &info, const QRegularExpression &regexp,
-        const LocatorFilterEntry &parent)
-{
     QList<LocatorFilterEntry> entries;
-    const QList<DocumentSymbol> children = info.children().value_or(QList<DocumentSymbol>());
+    for (const SymbolInformation &info : infoList) {
-    const bool hasMatch = regexp.match(info.name()).hasMatch();
+        if (regexp.match(info.name()).hasMatch())
-    LocatorFilterEntry entry;
+            entries << LanguageClient::generateLocatorEntry(info, this, m_pathMapper);
-    if (hasMatch || !children.isEmpty())
+    }
-        entry = generateLocatorEntry(info, parent);
-    if (hasMatch)
-        entries << entry;
-    for (const DocumentSymbol &child : children)
-        entries << generateLocatorEntries(child, regexp, entry);
     return entries;
 }
 
-template<class T>
+QList<LocatorFilterEntry> DocumentLocatorFilter::entriesForDocSymbols(
-QList<LocatorFilterEntry> DocumentLocatorFilter::generateEntries(const QList<T> &list,
+    const QList<DocumentSymbol> &infoList, const QRegularExpression &regexp,
-                                                                 const QString &filter)
+    const DocSymbolGenerator &docSymbolGenerator, const LocatorFilterEntry &parent)
 {
     QList<LocatorFilterEntry> entries;
-    FuzzyMatcher::CaseSensitivity caseSensitivity
+    for (const DocumentSymbol &info : infoList) {
-        = ILocatorFilter::caseSensitivity(filter) == Qt::CaseSensitive
+        const QList<DocumentSymbol> children = info.children().value_or(QList<DocumentSymbol>());
-              ? FuzzyMatcher::CaseSensitivity::CaseSensitive
+        const bool hasMatch = regexp.match(info.name()).hasMatch();
-              : FuzzyMatcher::CaseSensitivity::CaseInsensitive;
+        const LocatorFilterEntry entry = hasMatch ? docSymbolGenerator(info, parent) : parent;
-    const QRegularExpression regexp = FuzzyMatcher::createRegExp(filter, caseSensitivity);
+        if (hasMatch)
-    if (!regexp.isValid())
+            entries << entry;
-        return entries;
+        entries << entriesForDocSymbols(children, regexp, docSymbolGenerator, entry);
-
+    }
-    for (const T &item : list)
-        entries << generateLocatorEntries(item, regexp, {});
     return entries;
 }
 
@@ -177,6 +139,39 @@ void DocumentLocatorFilter::prepareSearch(const QString &/*entry*/)
 QList<LocatorFilterEntry> DocumentLocatorFilter::matchesFor(
     QFutureInterface<LocatorFilterEntry> &future, const QString &entry)
 {
+    const auto docSymbolGenerator = [this](const DocumentSymbol &info,
+                                           const LocatorFilterEntry &parent) {
+        Q_UNUSED(parent)
+        LocatorFilterEntry entry;
+        entry.filter = this;
+        entry.displayName = info.name();
+        if (std::optional<QString> detail = info.detail())
+            entry.extraInfo = detail.value_or(QString());
+        entry.displayIcon = symbolIcon(info.kind());
+        entry.linkForEditor = linkForDocSymbol(info);
+        return entry;
+    };
+    return matchesForImpl(future, entry, docSymbolGenerator);
+}
+
+Utils::Link DocumentLocatorFilter::linkForDocSymbol(const DocumentSymbol &info) const
+{
+    const Position &pos = info.range().start();
+    return {m_currentFilePath, pos.line() + 1, pos.character()};
+}
+
+QList<LocatorFilterEntry> DocumentLocatorFilter::matchesForImpl(
+    QFutureInterface<LocatorFilterEntry> &future, const QString &entry,
+    const DocSymbolGenerator &docSymbolGenerator)
+{
+    const FuzzyMatcher::CaseSensitivity caseSensitivity
+        = ILocatorFilter::caseSensitivity(entry) == Qt::CaseSensitive
+              ? FuzzyMatcher::CaseSensitivity::CaseSensitive
+              : FuzzyMatcher::CaseSensitivity::CaseInsensitive;
+    const QRegularExpression regExp = FuzzyMatcher::createRegExp(entry, caseSensitivity);
+    if (!regExp.isValid())
+        return {};
+
     QMutexLocker locker(&m_mutex);
     if (!m_symbolCache)
         return {};
@@ -195,9 +190,9 @@ QList<LocatorFilterEntry> DocumentLocatorFilter::matchesFor(
     QTC_ASSERT(m_currentSymbols.has_value(), return {});
 
     if (auto list = std::get_if<QList<DocumentSymbol>>(&*m_currentSymbols))
-        return generateEntries(*list, entry);
+        return entriesForDocSymbols(*list, regExp, docSymbolGenerator);
     else if (auto list = std::get_if<QList<SymbolInformation>>(&*m_currentSymbols))
-        return generateEntries(*list, entry);
+        return entriesForSymbolsInfo(*list, regExp);
 
     return {};
 }

src/plugins/languageclient/locatorfilter.h

@@ -36,31 +36,29 @@ signals:
 protected:
     void forceUse() { m_forced = true; }
 
-    QPointer<DocumentSymbolCache> m_symbolCache;
-    LanguageServerProtocol::DocumentUri m_currentUri;
     Utils::FilePath m_currentFilePath;
 
+    using DocSymbolGenerator = std::function<Core::LocatorFilterEntry(
+        const LanguageServerProtocol::DocumentSymbol &, const Core::LocatorFilterEntry &)>;
+
+    Utils::Link linkForDocSymbol(const LanguageServerProtocol::DocumentSymbol &info) const;
+    QList<Core::LocatorFilterEntry> matchesForImpl(
+        QFutureInterface<Core::LocatorFilterEntry> &future, const QString &entry,
+        const DocSymbolGenerator &docSymbolGenerator);
+
 private:
     void updateCurrentClient();
     void updateSymbols(const LanguageServerProtocol::DocumentUri &uri,
                        const LanguageServerProtocol::DocumentSymbolsResult &symbols);
     void resetSymbols();
 
-    template<class T>
+    QList<Core::LocatorFilterEntry> entriesForSymbolsInfo(
-    QList<Core::LocatorFilterEntry> generateEntries(const QList<T> &list, const QString &filter);
+        const QList<LanguageServerProtocol::SymbolInformation> &infoList,
-    QList<Core::LocatorFilterEntry> generateLocatorEntries(
+        const QRegularExpression &regexp);
-            const LanguageServerProtocol::SymbolInformation &info,
+    QList<Core::LocatorFilterEntry> entriesForDocSymbols(
-            const QRegularExpression &regexp,
+        const QList<LanguageServerProtocol::DocumentSymbol> &infoList,
-            const Core::LocatorFilterEntry &parent);
+        const QRegularExpression &regexp, const DocSymbolGenerator &docSymbolGenerator,
-    QList<Core::LocatorFilterEntry> generateLocatorEntries(
+        const Core::LocatorFilterEntry &parent = {});
-            const LanguageServerProtocol::DocumentSymbol &info,
-            const QRegularExpression &regexp,
-            const Core::LocatorFilterEntry &parent);
-    virtual Core::LocatorFilterEntry generateLocatorEntry(
-            const LanguageServerProtocol::DocumentSymbol &info,
-            const Core::LocatorFilterEntry &parent);
-    virtual Core::LocatorFilterEntry generateLocatorEntry(
-            const LanguageServerProtocol::SymbolInformation &info);
 
     QMutex m_mutex;
     QMetaObject::Connection m_updateSymbolsConnection;
@@ -68,6 +66,8 @@ private:
     std::optional<LanguageServerProtocol::DocumentSymbolsResult> m_currentSymbols;
     LanguageServerProtocol::DocumentUri::PathMapper m_pathMapper;
     bool m_forced = false;
+    QPointer<DocumentSymbolCache> m_symbolCache;
+    LanguageServerProtocol::DocumentUri m_currentUri;
 };
 
 class LANGUAGECLIENT_EXPORT WorkspaceLocatorFilter : public Core::ILocatorFilter