From 52770b746e3672d726d9b29b8efb3468af3ff690 Mon Sep 17 00:00:00 2001 From: David Schulz Date: Mon, 16 May 2022 13:31:10 +0200 Subject: [PATCH] LSP: fix hover request result According to the protocol a hover request can also return a Null as a result. Reflect this in the protocol implementation and adapt usages. Change-Id: I14ce71639c64b6de00e9c1198617083c1a3de9eb Reviewed-by: Christian Kandeler --- .../languagefeatures.cpp | 15 +++++ .../languageserverprotocol/languagefeatures.h | 11 +++- src/plugins/clangcodemodel/clangdclient.cpp | 61 +++++++++++-------- .../languageclienthoverhandler.cpp | 20 +++--- 4 files changed, 71 insertions(+), 36 deletions(-) diff --git a/src/libs/languageserverprotocol/languagefeatures.cpp b/src/libs/languageserverprotocol/languagefeatures.cpp index ceea23e569d..367f22799d8 100644 --- a/src/libs/languageserverprotocol/languagefeatures.cpp +++ b/src/libs/languageserverprotocol/languagefeatures.cpp @@ -403,4 +403,19 @@ Utils::optional CodeLens::data() const return contains(dataKey) ? Utils::make_optional(value(dataKey)) : Utils::nullopt; } +HoverResult::HoverResult(const QJsonValue &value) +{ + if (value.isObject()) + emplace(Hover(value.toObject())); + else + emplace(nullptr); +} + +bool HoverResult::isValid() const +{ + if (auto hover = Utils::get_if(this)) + return hover->isValid(); + return true; +} + } // namespace LanguageServerProtocol diff --git a/src/libs/languageserverprotocol/languagefeatures.h b/src/libs/languageserverprotocol/languagefeatures.h index b82fccdaf10..36979692ab9 100644 --- a/src/libs/languageserverprotocol/languagefeatures.h +++ b/src/libs/languageserverprotocol/languagefeatures.h @@ -101,8 +101,17 @@ public: bool isValid() const override { return contains(contentsKey); } }; +class LANGUAGESERVERPROTOCOL_EXPORT HoverResult : public Utils::variant +{ +public: + HoverResult() : variant(nullptr) {} + explicit HoverResult(const Hover &hover) : variant(hover) {} + explicit HoverResult(const QJsonValue &value); + bool isValid() const; +}; + class LANGUAGESERVERPROTOCOL_EXPORT HoverRequest - : public Request + : public Request { public: explicit HoverRequest(const TextDocumentPositionParams ¶ms); diff --git a/src/plugins/clangcodemodel/clangdclient.cpp b/src/plugins/clangcodemodel/clangdclient.cpp index 40d9cf52c5e..4c4245fe511 100644 --- a/src/plugins/clangcodemodel/clangdclient.cpp +++ b/src/plugins/clangcodemodel/clangdclient.cpp @@ -2344,35 +2344,38 @@ void ClangdClient::findLocalUsages(TextDocument *document, const QTextCursor &cu void ClangdClient::gatherHelpItemForTooltip(const HoverRequest::Response &hoverResponse, const DocumentUri &uri) { - if (const Utils::optional result = hoverResponse.result()) { - const HoverContent content = result->content(); - const MarkupContent * const markup = Utils::get_if(&content); - if (markup) { - const QString markupString = markup->content(); + if (const Utils::optional result = hoverResponse.result()) { + if (auto hover = Utils::get_if(&(*result))) { + const HoverContent content = hover->content(); + const MarkupContent *const markup = Utils::get_if(&content); + if (markup) { + const QString markupString = markup->content(); - // Macros aren't locatable via the AST, so parse the formatted string. - static const QString magicMacroPrefix = "### macro `"; - if (markupString.startsWith(magicMacroPrefix)) { - const int nameStart = magicMacroPrefix.length(); - const int closingQuoteIndex = markupString.indexOf('`', nameStart); - if (closingQuoteIndex != -1) { - const QString macroName = markupString.mid(nameStart, - closingQuoteIndex - nameStart); - d->setHelpItemForTooltip(hoverResponse.id(), macroName, HelpItem::Macro); - return; + // Macros aren't locatable via the AST, so parse the formatted string. + static const QString magicMacroPrefix = "### macro `"; + if (markupString.startsWith(magicMacroPrefix)) { + const int nameStart = magicMacroPrefix.length(); + const int closingQuoteIndex = markupString.indexOf('`', nameStart); + if (closingQuoteIndex != -1) { + const QString macroName = markupString.mid(nameStart, + closingQuoteIndex - nameStart); + d->setHelpItemForTooltip(hoverResponse.id(), macroName, HelpItem::Macro); + return; + } } - } - // Is it the file path for an include directive? - QString cleanString = markupString; - cleanString.remove('`'); - const QStringList lines = cleanString.trimmed().split('\n'); - if (!lines.isEmpty()) { - const auto filePath = Utils::FilePath::fromUserInput(lines.last().simplified()); - if (filePath.exists()) { - d->setHelpItemForTooltip(hoverResponse.id(), filePath.fileName(), - HelpItem::Brief); - return; + // Is it the file path for an include directive? + QString cleanString = markupString; + cleanString.remove('`'); + const QStringList lines = cleanString.trimmed().split('\n'); + if (!lines.isEmpty()) { + const auto filePath = Utils::FilePath::fromUserInput(lines.last().simplified()); + if (filePath.exists()) { + d->setHelpItemForTooltip(hoverResponse.id(), + filePath.fileName(), + HelpItem::Brief); + return; + } } } } @@ -2382,7 +2385,11 @@ void ClangdClient::gatherHelpItemForTooltip(const HoverRequest::Response &hoverR QTC_ASSERT(doc, return); const auto astHandler = [this, uri, hoverResponse](const AstNode &ast, const MessageId &) { const MessageId id = hoverResponse.id(); - const Range range = hoverResponse.result()->range().value_or(Range()); + Range range; + if (const Utils::optional result = hoverResponse.result()) { + if (auto hover = Utils::get_if(&(*result))) + range = hover->range().value_or(Range()); + } const QList path = getAstPath(ast, range); if (path.isEmpty()) { d->setHelpItemForTooltip(id); diff --git a/src/plugins/languageclient/languageclienthoverhandler.cpp b/src/plugins/languageclient/languageclienthoverhandler.cpp index 6cb306fe435..2eca331daaa 100644 --- a/src/plugins/languageclient/languageclienthoverhandler.cpp +++ b/src/plugins/languageclient/languageclienthoverhandler.cpp @@ -58,8 +58,10 @@ void HoverHandler::setHelpItem(const LanguageServerProtocol::MessageId &msgId, const Core::HelpItem &help) { if (msgId == m_response.id()) { - if (Utils::optional result = m_response.result()) - setContent(result->content()); + if (Utils::optional result = m_response.result()) { + if (auto hover = Utils::get_if(&(*result))) + setContent(hover->content()); + } m_response = {}; setLastHelpItemIdentified(help); m_report(priority()); @@ -129,13 +131,15 @@ void HoverHandler::handleResponse(const HoverRequest::Response &response) if (m_client) m_client->log(*error); } - if (Utils::optional result = response.result()) { - if (m_helpItemProvider) { - m_response = response; - m_helpItemProvider(response, m_uri); - return; + if (Utils::optional result = response.result()) { + if (auto hover = Utils::get_if(&(*result))) { + if (m_helpItemProvider) { + m_response = response; + m_helpItemProvider(response, m_uri); + return; + } + setContent(hover->content()); } - setContent(result->content()); } m_report(priority()); }