Skip to content

Commit 73a6fa6

Browse files
angelozerrangelozerr
committed
fix: Pull diagnostics overwriting publish diagnostics
Fixes #968 Signed-off-by: azerr <azerr@redhat.com>
1 parent 4e34b99 commit 73a6fa6

17 files changed

Lines changed: 518 additions & 92 deletions

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

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,8 @@ public class ClosedDocument extends LSPDocumentBase {
3030
private List<Diagnostic> diagnostics;
3131

3232
@Override
33-
public boolean updateDiagnostics(@NotNull List<Diagnostic> diagnostics) {
33+
public boolean updateDiagnostics(@NotNull String identifier,
34+
@NotNull List<Diagnostic> diagnostics) {
3435
boolean changed = isDiagnosticsChanged(this.diagnostics != null ? this.diagnostics : Collections.emptyList(), diagnostics);
3536
this.diagnostics = diagnostics;
3637
return changed;

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

Lines changed: 46 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,8 @@
1919
import com.intellij.openapi.vfs.VirtualFile;
2020
import com.intellij.psi.PsiDocumentManager;
2121
import com.intellij.util.Alarm;
22+
import com.redhat.devtools.lsp4ij.client.features.FileUriSupport;
23+
import com.redhat.devtools.lsp4ij.client.features.LSPClientFeatures;
2224
import org.eclipse.lsp4j.*;
2325
import org.eclipse.lsp4j.jsonrpc.messages.Either;
2426
import org.eclipse.lsp4j.services.LanguageServer;
@@ -28,6 +30,7 @@
2830
import java.util.ArrayList;
2931
import java.util.Collections;
3032
import java.util.List;
33+
import java.util.Map;
3134
import java.util.concurrent.CompletableFuture;
3235
import java.util.concurrent.TimeUnit;
3336

@@ -282,7 +285,7 @@ public void refreshPullDiagnostic(@NotNull RefreshPullDiagnosticOrigin origin) {
282285
// called by 'textDocument/diagnostic' by dynamic registerCapability
283286
// we need to consume the 'textDocument/diagnostic' if when didOpen has occurred, the pull diagnostic
284287
// capability was not enabled.
285-
if (!diagnosticNotPulledOnDidOpen) {
288+
if (!isDiagnosticNotPulledOnDidOpen()) {
286289
// the didOpen have already consumed the 'textDocument/diagnostic', do nothing
287290
return;
288291
}
@@ -300,8 +303,9 @@ public void refreshPullDiagnostic(@NotNull RefreshPullDiagnosticOrigin origin) {
300303
* @param didFuture the didOpen, didChange future.
301304
* @param version the current document version.
302305
*/
303-
private void processPullDiagnosticIfNeeded(@Nullable CompletableFuture<LanguageServer> didFuture, int version) {
304-
boolean debounceValidation = didFuture != null;
306+
private void processPullDiagnosticIfNeeded(@Nullable CompletableFuture<LanguageServer> didFuture,
307+
int version) {
308+
boolean debounceValidation = didFuture != null && !ApplicationManager.getApplication().isUnitTestMode();
305309
if (!isPullDiagnosticsSupported()) {
306310
// The language server doesn't support pull diagnostics.
307311
diagnosticNotPulledOnDidOpen = didFuture != null && didFuture == didOpenFuture;
@@ -313,12 +317,15 @@ private void processPullDiagnosticIfNeeded(@Nullable CompletableFuture<LanguageS
313317
// The document has changed, do nothing
314318
return;
315319
}
320+
if (didOpenFuture == null) {
321+
return;
322+
}
316323
var ls = didOpenFuture.getNow(null);
317324
if (ls == null) {
318325
// The didOpen is not finished, ignore the pull diagnostic
319-
return;
326+
didOpenFuture.
327+
thenAccept(readyLs -> refreshPullDiagnostic(version, readyLs));
320328
}
321-
refreshPullDiagnostic(version, ls);
322329
return;
323330
}
324331
// Refresh pull diagnostic with debounce (after a didOpen , didChange).
@@ -342,32 +349,57 @@ private void processPullDiagnosticIfNeeded(@Nullable CompletableFuture<LanguageS
342349
});
343350
}
344351

345-
private void refreshPullDiagnostic(int version, LanguageServer ls) {
352+
private void refreshPullDiagnostic(int version, @NotNull LanguageServer ls) {
346353
// Consume 'textDocument/diagnostic'
347354
DocumentDiagnosticParams params = new DocumentDiagnosticParams();
348355
params.setTextDocument(new TextDocumentIdentifier(fileUri));
349356
ls.getTextDocumentService()
350357
.diagnostic(params)
351358
.thenAcceptAsync(diagnosticReport -> {
352-
if (version != -1 && version != this.version) {
359+
if (diagnosticReport == null || (version != -1 && version != this.version)) {
353360
// The document has changed, do nothing
354361
return;
355362
}
363+
var clientFeatures = languageServerWrapper.getClientFeatures();
356364
// Update the diagnostics cache from the opened file and refresh UI to process LSPDiagnosticAnnotator.
357365
if (diagnosticReport.isLeft()) {
358-
RelatedFullDocumentDiagnosticReport fullDocumentDiagnosticReport = diagnosticReport.getLeft();
359-
var items = fullDocumentDiagnosticReport.getItems();
366+
RelatedFullDocumentDiagnosticReport fileReport = diagnosticReport.getLeft();
360367
// Update the diagnostics cache from the opened file
361-
var openedDocument = languageServerWrapper.getOpenedDocument(LSPIJUtils.toUri(file));
362-
openedDocument.updateDiagnostics(items != null ? items : Collections.emptyList());
368+
updatePullDiagnostics(file, fileReport, clientFeatures);
363369
} else if (diagnosticReport.isRight()) {
364-
// TODO ...
365-
RelatedUnchangedDocumentDiagnosticReport relatedUnchangedDocumentDiagnosticReport = diagnosticReport.getRight();
370+
RelatedUnchangedDocumentDiagnosticReport allFilesReport = diagnosticReport.getRight();
371+
Map<String, Either<FullDocumentDiagnosticReport, UnchangedDocumentDiagnosticReport>> relatedDocuments = allFilesReport.getRelatedDocuments();
372+
if (relatedDocuments != null) {
373+
for (var relatedDocument : relatedDocuments.entrySet()) {
374+
String documentUri = relatedDocument.getKey();
375+
Either<FullDocumentDiagnosticReport, UnchangedDocumentDiagnosticReport> fileReport = relatedDocument.getValue();
376+
if (fileReport != null) {
377+
if (fileReport.isLeft()) {
378+
VirtualFile file = FileUriSupport.findFileByUri(documentUri, clientFeatures);
379+
if (file != null) {
380+
updatePullDiagnostics(file, fileReport.getLeft(), clientFeatures);
381+
}
382+
} else if (fileReport.isRight()) {
383+
// TODO: implement UnchangedDocumentDiagnosticReport
384+
}
385+
}
386+
}
387+
}
366388
}
367389

368390
});
369391
}
370392

393+
private void updatePullDiagnostics(@NotNull VirtualFile file,
394+
@NotNull FullDocumentDiagnosticReport diagnosticReport,
395+
@NotNull LSPClientFeatures clientFeatures) {
396+
// Update diagnostics for the opened/closed document
397+
var fileUri = FileUriSupport.getFileUri(file, clientFeatures);
398+
List<Diagnostic> diagnostics = diagnosticReport.getItems() != null ? diagnosticReport.getItems() : Collections.emptyList();
399+
String identifier = clientFeatures.getDiagnosticFeature().getDiagnosticIdentifier();
400+
languageServerWrapper.updateDiagnostics(fileUri, identifier, diagnostics);
401+
}
402+
371403
public boolean isPullDiagnosticsSupported() {
372404
return languageServerWrapper.getClientFeatures().getDiagnosticFeature().isDiagnosticSupported(file);
373405
}
@@ -383,7 +415,7 @@ private Alarm getDebouncePullDiagnosticsAlarm() {
383415
return debouncePullDiagnosticsAlarm;
384416
}
385417

386-
public boolean isDiagnosticNotPulledOnDidOpen() {
418+
private boolean isDiagnosticNotPulledOnDidOpen() {
387419
return diagnosticNotPulledOnDidOpen;
388420
}
389421
}

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

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -24,12 +24,16 @@
2424
*/
2525
public abstract class LSPDocumentBase {
2626

27+
public static final String PUBLISH_DIAGNOSTIC_IDENTIFIER = "lsp4ij.publish";
28+
public static final String PULL_DIAGNOSTIC_IDENTIFIER = "lsp4ij.pull";
29+
2730
/**
2831
* Update the diagnostics
29-
*
32+
* @param identifier the diagnostic identifier used to cache diagnostics.
3033
* @param diagnostics the new diagnostics.
3134
*/
32-
public abstract boolean updateDiagnostics(@NotNull List<Diagnostic> diagnostics);
35+
public abstract boolean updateDiagnostics(@NotNull String identifier,
36+
@NotNull List<Diagnostic> diagnostics);
3337

3438
/**
3539
* Returns the current diagnostics for the file reported by the language server.

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -479,7 +479,7 @@ public void restartDaemonCodeAnalyzerWithDebounce(@NotNull CancelChecker cancelC
479479
action.inSmartMode(project);
480480
}
481481
action.submit(AppExecutorUtil.getAppExecutorService());
482-
}, 1000);
482+
}, 2000);
483483
}
484484

485485
private Alarm getRefreshPsiFileAlarm() {

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

Lines changed: 30 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -70,7 +70,9 @@ public class LanguageServerWrapper implements Disposable {
7070

7171
private static final int MAX_NUMBER_OF_RESTART_ATTEMPTS = 20; // TODO move this max value in settings
7272

73-
record LSPFileConnectionInfo(@Nullable Document document, @Nullable String documentText, @Nullable String languageId, boolean waitForDidOpen) {}
73+
record LSPFileConnectionInfo(@Nullable Document document, @Nullable String documentText,
74+
@Nullable String languageId, boolean waitForDidOpen) {
75+
}
7476

7577
private MessageBusConnection messageBusConnection;
7678

@@ -381,7 +383,7 @@ public synchronized void start() throws LanguageServerException {
381383
}).thenRun(() -> this.languageServer.initialized(new InitializedParams())).thenRun(() -> {
382384
initializeFuture.thenRunAsync(() -> {
383385
for (VirtualFile fileToReconnect : filesToReconnect) {
384-
connect(fileToReconnect, new LSPFileConnectionInfo(null, null, null,true));
386+
connect(fileToReconnect, new LSPFileConnectionInfo(null, null, null, true));
385387
}
386388
});
387389

@@ -702,9 +704,9 @@ public boolean canOperate(Project project) {
702704
* the method return a CompletableFuture which returns null.
703705
* </p>
704706
*
705-
* @param file the file to connect to the language server
707+
* @param file the file to connect to the language server
706708
* @param fileConnectionInfo the document of the file and null otherwise. In the null case, the document will be retrieved from the file
707-
* by using a blocking read action.
709+
* by using a blocking read action.
708710
* @return the completable future with the language server instance or null.
709711
*/
710712
CompletableFuture<@Nullable LanguageServer> connect(@NotNull VirtualFile file,
@@ -1269,6 +1271,30 @@ public boolean isDidRenameFilesSupported(@NotNull PsiFile file) {
12691271
return fileOperationsManager.canDidRenameFiles(uri, file.isDirectory());
12701272
}
12711273

1274+
/**
1275+
* Update diagnostics for the given file URi.
1276+
* @param fileUri the file uri.
1277+
* @param identifier the diagnostic identifier (lsp4ij.publish, lsp4ij.push, custom identifier).
1278+
* @param diagnostics the diagnostics to update.
1279+
*/
1280+
public void updateDiagnostics(@NotNull URI fileUri,
1281+
@NotNull String identifier,
1282+
@NotNull List<Diagnostic> diagnostics) {
1283+
var openedDocument = getOpenedDocument(fileUri);
1284+
if (openedDocument != null) {
1285+
// Update diagnostics for opened file
1286+
synchronized (openedDocument) {
1287+
openedDocument.updateDiagnostics(identifier, diagnostics);
1288+
}
1289+
} else {
1290+
// Update diagnostics for closed file
1291+
var closedDocument = getClosedDocument(fileUri, true);
1292+
synchronized (closedDocument) {
1293+
closedDocument.updateDiagnostics(identifier, diagnostics);
1294+
}
1295+
}
1296+
}
1297+
12721298
@NotNull
12731299
public LSPClientFeatures getClientFeatures() {
12741300
if (clientFeatures == null) {

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

Lines changed: 9 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,6 @@
1515

1616
import com.intellij.openapi.vfs.VirtualFile;
1717
import com.intellij.psi.PsiFile;
18-
import com.intellij.util.Alarm;
1918
import com.redhat.devtools.lsp4ij.features.diagnostics.LSPDiagnosticsForServer;
2019
import org.eclipse.lsp4j.Diagnostic;
2120
import org.jetbrains.annotations.NotNull;
@@ -31,8 +30,6 @@
3130
*/
3231
public class OpenedDocument extends LSPDocumentBase {
3332

34-
private volatile Alarm refreshDiagnosticsAlarm = null;
35-
3633
private final VirtualFile file;
3734

3835
private final LSPDiagnosticsForServer diagnosticsForServer;
@@ -47,6 +44,7 @@ public OpenedDocument(@NotNull LanguageServerItem languageServer,
4744
this.file = file;
4845
this.synchronizer = synchronizer;
4946
this.diagnosticsForServer = new LSPDiagnosticsForServer(languageServer,file);
47+
this.displayingDiagnosticsTime = -1;
5048
}
5149

5250
/**
@@ -67,9 +65,10 @@ public LSPDiagnosticsForServer getDiagnosticsForServer() {
6765
}
6866

6967
@Override
70-
public boolean updateDiagnostics(@NotNull List<Diagnostic> diagnostics) {
68+
public boolean updateDiagnostics(@NotNull String identifier,
69+
@NotNull List<Diagnostic> diagnostics) {
7170
updatedDiagnosticsTime = System.currentTimeMillis();
72-
if (diagnosticsForServer.update(diagnostics)) {
71+
if (diagnosticsForServer.update(identifier, diagnostics)) {
7372
// LSP diagnostics has changed
7473
final PsiFile psiFile = LSPIJUtils.getPsiFile(file, diagnosticsForServer.getClientFeatures().getProject());
7574
if (psiFile != null) {
@@ -99,8 +98,11 @@ public void markAsDisplayingDiagnostics() {
9998

10099
boolean isDiagnosticsMustBeRefreshed(long displayingDiagnosticsTime) {
101100
long lastDisplayingDiagnosticsTime = getDisplayingDiagnosticsTime();
102-
return displayingDiagnosticsTime != lastDisplayingDiagnosticsTime// is diagnostics are already displayed with LSPDiagnosticAnnotator ?
103-
|| updatedDiagnosticsTime < lastDisplayingDiagnosticsTime; // is update of diagnostics has been done after the last display of diagnostics
101+
if (lastDisplayingDiagnosticsTime == -1) {
102+
return false;
103+
}
104+
return displayingDiagnosticsTime != lastDisplayingDiagnosticsTime// are diagnostics already displayed with LSPDiagnosticAnnotator?
105+
|| updatedDiagnosticsTime > lastDisplayingDiagnosticsTime; // has the diagnostics update occurred after the last display?
104106
}
105107

106108
long getDisplayingDiagnosticsTime() {

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -51,7 +51,7 @@ public interface FileUriSupport {
5151
@Nullable
5252
VirtualFile findFileByUri(@NotNull String fileUri);
5353

54-
@Nullable
54+
@NotNull
5555
public static URI getFileUri(@NotNull VirtualFile file,
5656
@Nullable FileUriSupport fileUriSupport) {
5757
URI fileUri = fileUriSupport != null ? fileUriSupport.getFileUri(file) : null;

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

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -337,4 +337,13 @@ public void setServerCapabilities(@Nullable ServerCapabilities serverCapabilitie
337337
diagnosticCapabilityRegistry.setServerCapabilities(serverCapabilities);
338338
}
339339
}
340+
341+
/**
342+
* Returns the diagnostic identifier to use to cache "pull" diagnostics.
343+
*
344+
* @return the diagnostic identifier to use to cache "pull" diagnostics.
345+
*/
346+
public @NotNull String getDiagnosticIdentifier() {
347+
return getDiagnosticCapabilityRegistry().getDiagnosticIdentifier();
348+
}
340349
}

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

Lines changed: 2 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -15,10 +15,7 @@
1515

1616
import com.intellij.openapi.project.Project;
1717
import com.intellij.openapi.vfs.VirtualFile;
18-
import com.redhat.devtools.lsp4ij.ClosedDocument;
19-
import com.redhat.devtools.lsp4ij.LSPIJUtils;
20-
import com.redhat.devtools.lsp4ij.LanguageServerWrapper;
21-
import com.redhat.devtools.lsp4ij.OpenedDocument;
18+
import com.redhat.devtools.lsp4ij.*;
2219
import com.redhat.devtools.lsp4ij.client.features.FileUriSupport;
2320
import org.eclipse.lsp4j.PublishDiagnosticsParams;
2421
import org.jetbrains.annotations.NotNull;
@@ -60,17 +57,7 @@ private void updateDiagnostics(@NotNull PublishDiagnosticsParams params, @NotNul
6057

6158
// Update LSP diagnostic reported by the language server id
6259
URI fileURI = LSPIJUtils.toUri(file);
63-
OpenedDocument openedDocument = languageServerWrapper.getOpenedDocument(fileURI);
64-
if (openedDocument != null) {
65-
// Update diagnostics for opened file
66-
synchronized (openedDocument) {
67-
openedDocument.updateDiagnostics(params.getDiagnostics());
68-
}
69-
} else {
70-
// Update diagnostics for closed file
71-
ClosedDocument closedDocument = languageServerWrapper.getClosedDocument(fileURI, true);
72-
closedDocument.updateDiagnostics(params.getDiagnostics());
73-
}
60+
languageServerWrapper.updateDiagnostics(fileURI, LSPDocumentBase.PUBLISH_DIAGNOSTIC_IDENTIFIER, params.getDiagnostics());
7461
}
7562

7663
}

0 commit comments

Comments
 (0)