LSP: move text marks from the manager to the individual clients

Diagnostics are “owned” by the server so it is the server’s
responsibility to clear them if necessary. So do not delete the
corresponding text mark after a file was closed, because the server does
not have to resend the diagnostics when the file is reopened. Only
delete text marks when they are replaced or when the client is deleted
or reset.

Change-Id: Ief821c7ec401f4c52ee30d99f8dec47dcd6f1c98
Reviewed-by: Christian Stenger <christian.stenger@qt.io>

src/plugins/languageclient/client.cpp

@@ -39,11 +39,13 @@
 #include <texteditor/semantichighlighter.h>
 #include <texteditor/textdocument.h>
 #include <texteditor/texteditor.h>
+#include <texteditor/textmark.h>
 #include <projectexplorer/project.h>
 #include <projectexplorer/session.h>
 #include <utils/mimetypes/mimedatabase.h>
 #include <utils/qtcprocess.h>
 #include <utils/synchronousprocess.h>
+#include <utils/utilsicons.h>
 
 #include <QDebug>
 #include <QLoggingCategory>
@@ -62,6 +64,25 @@ namespace LanguageClient {
 
 static Q_LOGGING_CATEGORY(LOGLSPCLIENT, "qtc.languageclient.client", QtWarningMsg);
 
+class TextMark : public TextEditor::TextMark
+{
+public:
+    TextMark(const Utils::FileName &fileName, const Diagnostic &diag)
+        : TextEditor::TextMark(fileName, diag.range().start().line() + 1, "lspmark")
+    {
+        using namespace Utils;
+        setLineAnnotation(diag.message());
+        setToolTip(diag.message());
+        const bool isError
+            = diag.severity().value_or(DiagnosticSeverity::Hint) == DiagnosticSeverity::Error;
+        setColor(isError ? Theme::CodeModel_Error_TextMarkColor
+                         : Theme::CodeModel_Warning_TextMarkColor);
+
+        setIcon(isError ? Icons::CODEMODEL_ERROR.icon()
+                        : Icons::CODEMODEL_WARNING.icon());
+    }
+};
+
 Client::Client(BaseClientInterface *clientInterface)
     : m_id(Core::Id::fromString(QUuid::createUuid().toString()))
     , m_completionProvider(this)
@@ -88,6 +109,8 @@ Client::~Client()
             widget->setRefactorMarkers(RefactorMarker::filterOutType(widget->refactorMarkers(), id()));
         }
     }
+    for (const DocumentUri &uri : m_diagnostics.keys())
+        removeDiagnostics(uri);
 }
 
 void Client::initialize()
@@ -155,10 +178,12 @@ void Client::openDocument(Core::IDocument *document)
                 return;
         }
     }
+    auto uri = DocumentUri::fromFileName(filePath);
+    showDiagnostics(uri);
     auto textDocument = qobject_cast<TextDocument *>(document);
     TextDocumentItem item;
     item.setLanguageId(TextDocumentItem::mimeTypeToLanguageId(document->mimeType()));
-    item.setUri(DocumentUri::fromFileName(filePath));
+    item.setUri(uri);
     item.setText(QString::fromUtf8(document->contents()));
     item.setVersion(textDocument ? textDocument->document()->revision() : 0);
 
@@ -672,6 +697,8 @@ bool Client::reset()
     m_openedDocument.clear();
     m_serverCapabilities = ServerCapabilities();
     m_dynamicCapabilities.reset();
+    for (const DocumentUri &uri : m_diagnostics.keys())
+        removeDiagnostics(uri);
     return true;
 }
 
@@ -748,6 +775,25 @@ void Client::showMessageBox(const ShowMessageRequestParams &message, const Messa
     box->show();
 }
 
+void Client::showDiagnostics(const DocumentUri &uri)
+{
+    if (TextEditor::TextDocument *doc = textDocumentForFileName(uri.toFileName())) {
+        for (TextMark *mark : m_diagnostics.value(uri))
+            doc->addMark(mark);
+    }
+}
+
+void Client::removeDiagnostics(const DocumentUri &uri)
+{
+    TextEditor::TextDocument *doc = textDocumentForFileName(uri.toFileName());
+
+    for (TextMark *mark : m_diagnostics.take(uri)) {
+        if (doc)
+            doc->removeMark(mark);
+        delete mark;
+    }
+}
+
 void Client::handleResponse(const MessageId &id, const QByteArray &content, QTextCodec *codec)
 {
     if (auto handler = m_responseHandlers[id])
@@ -762,7 +808,7 @@ void Client::handleMethod(const QString &method, MessageId id, const IContent *c
         auto params = dynamic_cast<const PublishDiagnosticsNotification *>(content)->params().value_or(PublishDiagnosticsParams());
         paramsValid = params.isValid(&error);
         if (paramsValid)
-            LanguageClientManager::publishDiagnostics(m_id, params, this);
+            handleDiagnostics(params);
     } else if (method == LogMessageNotification::methodName) {
         auto params = dynamic_cast<const LogMessageNotification *>(content)->params().value_or(LogMessageParams());
         paramsValid = params.isValid(&error);
@@ -822,6 +868,21 @@ void Client::handleMethod(const QString &method, MessageId id, const IContent *c
     delete content;
 }
 
+void Client::handleDiagnostics(const PublishDiagnosticsParams &params)
+{
+    const DocumentUri &uri = params.uri();
+
+    removeDiagnostics(uri);
+    const QList<Diagnostic> &diagnostics = params.diagnostics();
+    m_diagnostics[uri] =
+        Utils::transform(diagnostics, [fileName = uri.toFileName()](const Diagnostic &diagnostic) {
+            return new TextMark(fileName, diagnostic);
+    });
+    showDiagnostics(uri);
+
+    requestCodeActions(uri, diagnostics);
+}
+
 void Client::intializeCallback(const InitializeRequest::Response &initResponse)
 {
     QTC_ASSERT(m_state == InitializeRequested, return);

src/plugins/languageclient/client.h

@@ -33,12 +33,13 @@
 #include <coreplugin/messagemanager.h>
 #include <utils/link.h>
 
+#include <languageserverprotocol/client.h>
+#include <languageserverprotocol/diagnostics.h>
 #include <languageserverprotocol/initializemessages.h>
+#include <languageserverprotocol/languagefeatures.h>
+#include <languageserverprotocol/messages.h>
 #include <languageserverprotocol/shutdownmessages.h>
 #include <languageserverprotocol/textsynchronization.h>
-#include <languageserverprotocol/messages.h>
-#include <languageserverprotocol/client.h>
-#include <languageserverprotocol/languagefeatures.h>
 
 #include <QBuffer>
 #include <QHash>
@@ -50,13 +51,15 @@ namespace Core { class IDocument; }
 namespace ProjectExplorer { class Project; }
 namespace TextEditor
 {
-    class TextDocument;
-    class TextEditorWidget;
+class TextDocument;
+class TextEditorWidget;
+class TextMark;
 }
 
 namespace LanguageClient {
 
 class BaseClientInterface;
+class TextMark;
 
 class Client : public QObject
 {
@@ -153,6 +156,8 @@ private:
     void handleMethod(const QString &method, LanguageServerProtocol::MessageId id,
                       const LanguageServerProtocol::IContent *content);
 
+    void handleDiagnostics(const LanguageServerProtocol::PublishDiagnosticsParams &params);
+
     void intializeCallback(const LanguageServerProtocol::InitializeRequest::Response &initResponse);
     void shutDownCallback(const LanguageServerProtocol::ShutdownRequest::Response &shutdownResponse);
     bool sendWorkspceFolderChanges() const;
@@ -162,6 +167,9 @@ private:
     void showMessageBox(const LanguageServerProtocol::ShowMessageRequestParams &message,
                         const LanguageServerProtocol::MessageId &id);
 
+    void showDiagnostics(const LanguageServerProtocol::DocumentUri &uri);
+    void removeDiagnostics(const LanguageServerProtocol::DocumentUri &uri);
+
     using ContentHandler = std::function<void(const QByteArray &, QTextCodec *, QString &,
                                               LanguageServerProtocol::ResponseHandlers,
                                               LanguageServerProtocol::MethodHandler)>;
@@ -180,6 +188,7 @@ private:
     QHash<LanguageServerProtocol::DocumentUri, LanguageServerProtocol::MessageId> m_highlightRequests;
     int m_restartsLeft = 5;
     QScopedPointer<BaseClientInterface> m_clientInterface;
+    QMap<LanguageServerProtocol::DocumentUri, QList<TextMark *>> m_diagnostics;
 };
 
 } // namespace LanguageClient

src/plugins/languageclient/languageclientmanager.cpp

@@ -49,30 +49,6 @@ namespace LanguageClient {
 
 static LanguageClientManager *managerInstance = nullptr;
 
-class LanguageClientMark : public TextEditor::TextMark
-{
-public:
-    LanguageClientMark(const Utils::FileName &fileName, const Diagnostic &diag)
-        : TextEditor::TextMark(fileName, diag.range().start().line() + 1, "lspmark")
-    {
-        using namespace Utils;
-        setLineAnnotation(diag.message());
-        setToolTip(diag.message());
-        const bool isError
-                = diag.severity().value_or(DiagnosticSeverity::Hint) == DiagnosticSeverity::Error;
-        setColor(isError ? Theme::CodeModel_Error_TextMarkColor
-                         : Theme::CodeModel_Warning_TextMarkColor);
-
-        setIcon(isError ? Icons::CODEMODEL_ERROR.icon()
-                        : Icons::CODEMODEL_WARNING.icon());
-    }
-
-    void removedFromEditor() override
-    {
-        LanguageClientManager::removeMark(this);
-    }
-};
-
 LanguageClientManager::LanguageClientManager()
 {
     JsonRpcMessageHandler::registerMessageProvider<PublishDiagnosticsNotification>();
@@ -107,68 +83,6 @@ void LanguageClientManager::init()
             managerInstance, &LanguageClientManager::projectRemoved);
 }
 
-void LanguageClientManager::publishDiagnostics(const Core::Id &id,
-                                               const PublishDiagnosticsParams &params,
-                                               Client *publishingClient)
-{
-    const Utils::FileName fileName = params.uri().toFileName();
-    TextEditor::TextDocument *doc = textDocumentForFileName(fileName);
-    if (!doc)
-        return;
-
-    removeMarks(fileName, id);
-    managerInstance->m_marks[fileName][id].reserve(params.diagnostics().size());
-    QList<Diagnostic> diagnostics = params.diagnostics();
-    for (const Diagnostic& diagnostic : diagnostics) {
-        auto mark = new LanguageClientMark(fileName, diagnostic);
-        managerInstance->m_marks[fileName][id].append(mark);
-        doc->addMark(mark);
-    }
-
-    publishingClient->requestCodeActions(params.uri(), diagnostics);
-}
-
-void LanguageClientManager::removeMark(LanguageClientMark *mark)
-{
-    for (auto &marks : managerInstance->m_marks[mark->fileName()])
-        marks.removeAll(mark);
-    delete mark;
-}
-
-void LanguageClientManager::removeMarks(const Utils::FileName &fileName)
-{
-    TextEditor::TextDocument *doc = textDocumentForFileName(fileName);
-    if (!doc)
-        return;
-
-    for (const auto &marks : qAsConst(managerInstance->m_marks[fileName])) {
-        for (TextEditor::TextMark *mark : marks) {
-            doc->removeMark(mark);
-            delete mark;
-        }
-    }
-    managerInstance->m_marks[fileName].clear();
-}
-
-void LanguageClientManager::removeMarks(const Utils::FileName &fileName, const Core::Id &id)
-{
-    TextEditor::TextDocument *doc = textDocumentForFileName(fileName);
-    if (!doc)
-        return;
-
-    for (TextEditor::TextMark *mark : managerInstance->m_marks[fileName][id]) {
-        doc->removeMark(mark);
-        delete mark;
-    }
-    managerInstance->m_marks[fileName][id].clear();
-}
-
-void LanguageClientManager::removeMarks(const Core::Id &id)
-{
-    for (const Utils::FileName &fileName : managerInstance->m_marks.keys())
-        removeMarks(fileName, id);
-}
-
 void LanguageClientManager::startClient(Client *client)
 {
     QTC_ASSERT(client, return);
@@ -210,7 +124,6 @@ void LanguageClientManager::deleteClient(Client *client)
 {
     QTC_ASSERT(client, return);
     client->disconnect();
-    managerInstance->removeMarks(client->id());
     managerInstance->m_clients.removeAll(client);
     client->deleteLater();
 }
@@ -269,7 +182,6 @@ void LanguageClientManager::clientFinished(Client *client)
     const bool unexpectedFinish = client->state() != Client::Shutdown
             && client->state() != Client::ShutdownRequested;
     if (unexpectedFinish && !m_shuttingDown && client->reset()) {
-        removeMarks(client->id());
         client->disconnect(this);
         client->log(tr("Unexpectedly finished. Restarting in %1 seconds.").arg(restartTimeoutS),
                     Core::MessageManager::Flash);
@@ -312,7 +224,6 @@ void LanguageClientManager::editorsClosed(const QList<Core::IEditor *> &editors)
 {
     for (auto iEditor : editors) {
         if (auto editor = qobject_cast<TextEditor::BaseTextEditor *>(iEditor)) {
-            removeMarks(editor->document()->filePath());
             const DidCloseTextDocumentParams params(TextDocumentIdentifier(
                     DocumentUri::fromFileName(editor->document()->filePath())));
             for (Client *interface : reachableClients())

src/plugins/languageclient/languageclientmanager.h

@@ -55,14 +55,6 @@ public:
 
     static void init();
 
-    static void publishDiagnostics(const Core::Id &id,
-                                   const LanguageServerProtocol::PublishDiagnosticsParams &params, Client *publishingClient);
-
-    static void removeMark(LanguageClientMark *mark);
-    static void removeMarks(const Utils::FileName &fileName);
-    static void removeMarks(const Utils::FileName &fileName, const Core::Id &id);
-    static void removeMarks(const Core::Id &id);
-
     static void startClient(Client *client);
     static QVector<Client *> clients();
 
@@ -101,7 +93,6 @@ private:
 
     bool m_shuttingDown = false;
     QVector<Client *> m_clients;
-    QHash<Utils::FileName, QHash<Core::Id, QVector<LanguageClientMark *>>> m_marks;
     QHash<LanguageServerProtocol::MessageId, QList<Client *>> m_exclusiveRequests;
 
     friend class LanguageClientPlugin;