fix: Pull diagnostics overwriting publish diagnostics

src/main/java/com/redhat/devtools/lsp4ij/ClosedDocument.java

@@ -30,7 +30,8 @@ public class ClosedDocument extends LSPDocumentBase {
     private List<Diagnostic> diagnostics;
 
     @Override
-    public boolean updateDiagnostics(@NotNull List<Diagnostic> diagnostics) {
+    public boolean updateDiagnostics(@NotNull String identifier,
+                                     @NotNull List<Diagnostic> diagnostics) {
         boolean changed = isDiagnosticsChanged(this.diagnostics != null ? this.diagnostics : Collections.emptyList(), diagnostics);
         this.diagnostics = diagnostics;
         return changed;

src/main/java/com/redhat/devtools/lsp4ij/DocumentContentSynchronizer.java

@@ -19,6 +19,8 @@
 import com.intellij.openapi.vfs.VirtualFile;
 import com.intellij.psi.PsiDocumentManager;
 import com.intellij.util.Alarm;
+import com.redhat.devtools.lsp4ij.client.features.FileUriSupport;
+import com.redhat.devtools.lsp4ij.client.features.LSPClientFeatures;
 import org.eclipse.lsp4j.*;
 import org.eclipse.lsp4j.jsonrpc.messages.Either;
 import org.eclipse.lsp4j.services.LanguageServer;
@@ -28,6 +30,7 @@
 import java.util.ArrayList;
 import java.util.Collections;
 import java.util.List;
+import java.util.Map;
 import java.util.concurrent.CompletableFuture;
 import java.util.concurrent.TimeUnit;
 
@@ -282,7 +285,7 @@ public void refreshPullDiagnostic(@NotNull RefreshPullDiagnosticOrigin origin) {
             // called by 'textDocument/diagnostic' by dynamic registerCapability
             // we need to consume the 'textDocument/diagnostic' if when didOpen has occurred, the pull diagnostic
             // capability was not enabled.
-            if (!diagnosticNotPulledOnDidOpen) {
+            if (!isDiagnosticNotPulledOnDidOpen()) {
                 // the didOpen have already consumed the 'textDocument/diagnostic', do nothing
                 return;
             }
@@ -300,8 +303,9 @@ public void refreshPullDiagnostic(@NotNull RefreshPullDiagnosticOrigin origin) {
      * @param didFuture the didOpen, didChange future.
      * @param version   the current document version.
      */
-    private void processPullDiagnosticIfNeeded(@Nullable CompletableFuture<LanguageServer> didFuture, int version) {
-        boolean debounceValidation = didFuture != null;
+    private void processPullDiagnosticIfNeeded(@Nullable CompletableFuture<LanguageServer> didFuture,
+                                               int version) {
+        boolean debounceValidation = didFuture != null && !ApplicationManager.getApplication().isUnitTestMode();
         if (!isPullDiagnosticsSupported()) {
             // The language server doesn't support pull diagnostics.
             diagnosticNotPulledOnDidOpen = didFuture != null && didFuture == didOpenFuture;
@@ -313,12 +317,15 @@ private void processPullDiagnosticIfNeeded(@Nullable CompletableFuture<LanguageS
                 // The document has changed, do nothing
                 return;
             }
+            if (didOpenFuture == null) {
+                return;
+            }
             var ls = didOpenFuture.getNow(null);
             if (ls == null) {
                 // The didOpen is not finished, ignore the pull diagnostic
-                return;
+                didOpenFuture.
+                        thenAccept(readyLs -> refreshPullDiagnostic(version, readyLs));
             }
-            refreshPullDiagnostic(version, ls);
             return;
         }
         // Refresh pull diagnostic with debounce (after a didOpen , didChange).
@@ -342,32 +349,57 @@ private void processPullDiagnosticIfNeeded(@Nullable CompletableFuture<LanguageS
                 });
     }
 
-    private void refreshPullDiagnostic(int version, LanguageServer ls) {
+    private void refreshPullDiagnostic(int version, @NotNull LanguageServer ls) {
         // Consume 'textDocument/diagnostic'
         DocumentDiagnosticParams params = new DocumentDiagnosticParams();
         params.setTextDocument(new TextDocumentIdentifier(fileUri));
         ls.getTextDocumentService()
                 .diagnostic(params)
                 .thenAcceptAsync(diagnosticReport -> {
-                    if (version != -1 && version != this.version) {
+                    if (diagnosticReport == null || (version != -1 && version != this.version)) {
                         // The document has changed, do nothing
                         return;
                     }
+                    var clientFeatures = languageServerWrapper.getClientFeatures();
                     // Update the diagnostics cache from the opened file and refresh UI to process LSPDiagnosticAnnotator.
                     if (diagnosticReport.isLeft()) {
-                        RelatedFullDocumentDiagnosticReport fullDocumentDiagnosticReport = diagnosticReport.getLeft();
-                        var items = fullDocumentDiagnosticReport.getItems();
+                        RelatedFullDocumentDiagnosticReport fileReport = diagnosticReport.getLeft();
                         // Update the diagnostics cache from the opened file
-                        var openedDocument = languageServerWrapper.getOpenedDocument(LSPIJUtils.toUri(file));
-                        openedDocument.updateDiagnostics(items != null ? items : Collections.emptyList());
+                        updatePullDiagnostics(file, fileReport, clientFeatures);
                     } else if (diagnosticReport.isRight()) {
-                        // TODO ...
-                        RelatedUnchangedDocumentDiagnosticReport relatedUnchangedDocumentDiagnosticReport = diagnosticReport.getRight();
+                        RelatedUnchangedDocumentDiagnosticReport allFilesReport = diagnosticReport.getRight();
+                        Map<String, Either<FullDocumentDiagnosticReport, UnchangedDocumentDiagnosticReport>> relatedDocuments = allFilesReport.getRelatedDocuments();
+                        if (relatedDocuments != null) {
+                            for (var relatedDocument : relatedDocuments.entrySet()) {
+                                String documentUri = relatedDocument.getKey();
+                                Either<FullDocumentDiagnosticReport, UnchangedDocumentDiagnosticReport> fileReport = relatedDocument.getValue();
+                                if (fileReport != null) {
+                                    if (fileReport.isLeft()) {
+                                        VirtualFile file = FileUriSupport.findFileByUri(documentUri, clientFeatures);
+                                        if (file != null) {
+                                            updatePullDiagnostics(file, fileReport.getLeft(), clientFeatures);
+                                        }
+                                    } else if (fileReport.isRight()) {
+                                        // TODO: implement UnchangedDocumentDiagnosticReport
+                                    }
+                                }
+                            }
+                        }
                     }
 
                 });
     }
 
+    private void updatePullDiagnostics(@NotNull VirtualFile file,
+                                       @NotNull FullDocumentDiagnosticReport diagnosticReport,
+                                       @NotNull LSPClientFeatures clientFeatures) {
+        // Update diagnostics for the opened/closed document
+        var fileUri = FileUriSupport.getFileUri(file, clientFeatures);
+        List<Diagnostic> diagnostics = diagnosticReport.getItems() != null ? diagnosticReport.getItems() : Collections.emptyList();
+        String identifier = clientFeatures.getDiagnosticFeature().getDiagnosticIdentifier();
+        languageServerWrapper.updateDiagnostics(fileUri, identifier, diagnostics);
+    }
+
     public boolean isPullDiagnosticsSupported() {
         return languageServerWrapper.getClientFeatures().getDiagnosticFeature().isDiagnosticSupported(file);
     }
@@ -383,7 +415,7 @@ private Alarm getDebouncePullDiagnosticsAlarm() {
         return debouncePullDiagnosticsAlarm;
     }
 
-    public boolean isDiagnosticNotPulledOnDidOpen() {
+    private boolean isDiagnosticNotPulledOnDidOpen() {
         return diagnosticNotPulledOnDidOpen;
     }
 }

src/main/java/com/redhat/devtools/lsp4ij/LSPDocumentBase.java

@@ -24,12 +24,16 @@
  */
 public abstract class LSPDocumentBase {
 
+    public static final String PUBLISH_DIAGNOSTIC_IDENTIFIER = "lsp4ij.publish";
+    public static final String PULL_DIAGNOSTIC_IDENTIFIER = "lsp4ij.pull";
+
     /**
      * Update the diagnostics
-     *
+     * @param identifier the diagnostic identifier used to cache diagnostics.
      * @param diagnostics the new diagnostics.
      */
-    public abstract boolean updateDiagnostics(@NotNull List<Diagnostic> diagnostics);
+    public abstract boolean updateDiagnostics(@NotNull String identifier,
+                                              @NotNull List<Diagnostic> diagnostics);
 
     /**
      * Returns the current diagnostics for the file reported by the language server.

src/main/java/com/redhat/devtools/lsp4ij/LSPFileSupport.java

@@ -479,7 +479,7 @@ public void restartDaemonCodeAnalyzerWithDebounce(@NotNull CancelChecker cancelC
                 action.inSmartMode(project);
             }
             action.submit(AppExecutorUtil.getAppExecutorService());
-        }, 1000);
+        }, 2000);
     }
 
     private Alarm getRefreshPsiFileAlarm() {

src/main/java/com/redhat/devtools/lsp4ij/LanguageServerWrapper.java

@@ -70,7 +70,9 @@ public class LanguageServerWrapper implements Disposable {
 
     private static final int MAX_NUMBER_OF_RESTART_ATTEMPTS = 20; // TODO move this max value in settings
 
-    record LSPFileConnectionInfo(@Nullable Document document, @Nullable String documentText, @Nullable String languageId, boolean waitForDidOpen) {}
+    record LSPFileConnectionInfo(@Nullable Document document, @Nullable String documentText,
+                                 @Nullable String languageId, boolean waitForDidOpen) {
+    }
 
     private MessageBusConnection messageBusConnection;
 
@@ -381,7 +383,7 @@ public synchronized void start() throws LanguageServerException {
                     }).thenRun(() -> this.languageServer.initialized(new InitializedParams())).thenRun(() -> {
                         initializeFuture.thenRunAsync(() -> {
                             for (VirtualFile fileToReconnect : filesToReconnect) {
-                                connect(fileToReconnect, new LSPFileConnectionInfo(null, null, null,true));
+                                connect(fileToReconnect, new LSPFileConnectionInfo(null, null, null, true));
                             }
                         });
 
@@ -702,9 +704,9 @@ public boolean canOperate(Project project) {
      *     the method return a CompletableFuture which returns null.
      * </p>
      *
-     * @param file             the file to connect to the language server
+     * @param file               the file to connect to the language server
      * @param fileConnectionInfo the document of the file and null otherwise. In the null case, the document will be retrieved from the file
-     *                         by using a blocking read action.
+     *                           by using a blocking read action.
      * @return the completable future with the language server instance or null.
      */
     CompletableFuture<@Nullable LanguageServer> connect(@NotNull VirtualFile file,
@@ -1269,6 +1271,30 @@ public boolean isDidRenameFilesSupported(@NotNull PsiFile file) {
         return fileOperationsManager.canDidRenameFiles(uri, file.isDirectory());
     }
 
+    /**
+     * Update diagnostics for the given file URi.
+     * @param fileUri the file uri.
+     * @param identifier the diagnostic identifier (lsp4ij.publish, lsp4ij.push, custom identifier).
+     * @param diagnostics the diagnostics to update.
+     */
+    public void updateDiagnostics(@NotNull URI fileUri,
+                                  @NotNull String identifier,
+                                  @NotNull List<Diagnostic> diagnostics) {
+        var openedDocument = getOpenedDocument(fileUri);
+        if (openedDocument != null) {
+            // Update diagnostics for opened file
+            synchronized (openedDocument) {
+                openedDocument.updateDiagnostics(identifier, diagnostics);
+            }
+        } else {
+            // Update diagnostics for closed file
+            var closedDocument = getClosedDocument(fileUri, true);
+            synchronized (closedDocument) {
+                closedDocument.updateDiagnostics(identifier, diagnostics);
+            }
+        }
+    }
+
     @NotNull
     public LSPClientFeatures getClientFeatures() {
         if (clientFeatures == null) {

src/main/java/com/redhat/devtools/lsp4ij/OpenedDocument.java

@@ -15,7 +15,6 @@
 
 import com.intellij.openapi.vfs.VirtualFile;
 import com.intellij.psi.PsiFile;
-import com.intellij.util.Alarm;
 import com.redhat.devtools.lsp4ij.features.diagnostics.LSPDiagnosticsForServer;
 import org.eclipse.lsp4j.Diagnostic;
 import org.jetbrains.annotations.NotNull;
@@ -31,8 +30,6 @@
  */
 public class OpenedDocument extends LSPDocumentBase {
 
-    private volatile Alarm refreshDiagnosticsAlarm = null;
-
     private final VirtualFile file;
 
     private final LSPDiagnosticsForServer diagnosticsForServer;
@@ -47,6 +44,7 @@ public OpenedDocument(@NotNull LanguageServerItem languageServer,
         this.file = file;
         this.synchronizer = synchronizer;
         this.diagnosticsForServer = new LSPDiagnosticsForServer(languageServer,file);
+        this.displayingDiagnosticsTime = -1;
     }
 
     /**
@@ -67,9 +65,10 @@ public LSPDiagnosticsForServer getDiagnosticsForServer() {
     }
 
     @Override
-    public boolean updateDiagnostics(@NotNull List<Diagnostic> diagnostics) {
+    public boolean updateDiagnostics(@NotNull String identifier,
+                                     @NotNull List<Diagnostic> diagnostics) {
         updatedDiagnosticsTime = System.currentTimeMillis();
-        if (diagnosticsForServer.update(diagnostics)) {
+        if (diagnosticsForServer.update(identifier, diagnostics)) {
             // LSP diagnostics has changed
             final PsiFile psiFile = LSPIJUtils.getPsiFile(file, diagnosticsForServer.getClientFeatures().getProject());
             if (psiFile != null) {
@@ -99,8 +98,11 @@ public void markAsDisplayingDiagnostics() {
 
     boolean isDiagnosticsMustBeRefreshed(long displayingDiagnosticsTime) {
         long lastDisplayingDiagnosticsTime = getDisplayingDiagnosticsTime();
-        return displayingDiagnosticsTime !=  lastDisplayingDiagnosticsTime// is diagnostics are already displayed with LSPDiagnosticAnnotator ?
-                || updatedDiagnosticsTime < lastDisplayingDiagnosticsTime; // is update of diagnostics has been done after the last display of diagnostics
+        if (lastDisplayingDiagnosticsTime == -1) {
+            return false;
+        }
+        return displayingDiagnosticsTime != lastDisplayingDiagnosticsTime// are diagnostics already displayed with LSPDiagnosticAnnotator?
+                || updatedDiagnosticsTime > lastDisplayingDiagnosticsTime; // has the diagnostics update occurred after the last display?
     }
 
     long getDisplayingDiagnosticsTime() {

src/main/java/com/redhat/devtools/lsp4ij/client/features/FileUriSupport.java

@@ -51,7 +51,7 @@ public interface FileUriSupport {
     @Nullable
     VirtualFile findFileByUri(@NotNull String fileUri);
 
-    @Nullable
+    @NotNull
     public static URI getFileUri(@NotNull VirtualFile file,
                                  @Nullable FileUriSupport fileUriSupport) {
         URI fileUri = fileUriSupport != null ? fileUriSupport.getFileUri(file) : null;

src/main/java/com/redhat/devtools/lsp4ij/client/features/LSPDiagnosticFeature.java

@@ -337,4 +337,13 @@ public void setServerCapabilities(@Nullable ServerCapabilities serverCapabilitie
             diagnosticCapabilityRegistry.setServerCapabilities(serverCapabilities);
         }
     }
+
+    /**
+     * Returns the diagnostic identifier to use to cache "pull" diagnostics.
+     *
+     * @return the diagnostic identifier to use to cache "pull" diagnostics.
+     */
+    public @NotNull String getDiagnosticIdentifier() {
+        return getDiagnosticCapabilityRegistry().getDiagnosticIdentifier();
+    }
 }

src/main/java/com/redhat/devtools/lsp4ij/features/diagnostics/LSPDiagnosticHandler.java

@@ -15,10 +15,7 @@
 
 import com.intellij.openapi.project.Project;
 import com.intellij.openapi.vfs.VirtualFile;
-import com.redhat.devtools.lsp4ij.ClosedDocument;
-import com.redhat.devtools.lsp4ij.LSPIJUtils;
-import com.redhat.devtools.lsp4ij.LanguageServerWrapper;
-import com.redhat.devtools.lsp4ij.OpenedDocument;
+import com.redhat.devtools.lsp4ij.*;
 import com.redhat.devtools.lsp4ij.client.features.FileUriSupport;
 import org.eclipse.lsp4j.PublishDiagnosticsParams;
 import org.jetbrains.annotations.NotNull;
@@ -60,17 +57,7 @@ private void updateDiagnostics(@NotNull PublishDiagnosticsParams params, @NotNul
 
         // Update LSP diagnostic reported by the language server id
         URI fileURI = LSPIJUtils.toUri(file);
-        OpenedDocument openedDocument = languageServerWrapper.getOpenedDocument(fileURI);
-        if (openedDocument != null) {
-            // Update diagnostics for opened file
-            synchronized (openedDocument) {
-                openedDocument.updateDiagnostics(params.getDiagnostics());
-            }
-        } else {
-            // Update diagnostics for closed file
-            ClosedDocument closedDocument = languageServerWrapper.getClosedDocument(fileURI, true);
-            closedDocument.updateDiagnostics(params.getDiagnostics());
-        }
+        languageServerWrapper.updateDiagnostics(fileURI, LSPDocumentBase.PUBLISH_DIAGNOSTIC_IDENTIFIER, params.getDiagnostics());
     }
 
 }