Skip to content

Commit 529e341

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

14 files changed

Lines changed: 374 additions & 50 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 & 11 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,7 +303,8 @@ 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) {
306+
private void processPullDiagnosticIfNeeded(@Nullable CompletableFuture<LanguageServer> didFuture,
307+
int version) {
304308
boolean debounceValidation = didFuture != null;
305309
if (!isPullDiagnosticsSupported()) {
306310
// The language server doesn't support pull diagnostics.
@@ -313,7 +317,7 @@ private void processPullDiagnosticIfNeeded(@Nullable CompletableFuture<LanguageS
313317
// The document has changed, do nothing
314318
return;
315319
}
316-
var ls = didOpenFuture.getNow(null);
320+
var ls = didOpenFuture != null ? didOpenFuture.getNow(null) : null;
317321
if (ls == null) {
318322
// The didOpen is not finished, ignore the pull diagnostic
319323
return;
@@ -342,7 +346,7 @@ private void processPullDiagnosticIfNeeded(@Nullable CompletableFuture<LanguageS
342346
});
343347
}
344348

345-
private void refreshPullDiagnostic(int version, LanguageServer ls) {
349+
private void refreshPullDiagnostic(int version, @NotNull LanguageServer ls) {
346350
// Consume 'textDocument/diagnostic'
347351
DocumentDiagnosticParams params = new DocumentDiagnosticParams();
348352
params.setTextDocument(new TextDocumentIdentifier(fileUri));
@@ -353,21 +357,52 @@ private void refreshPullDiagnostic(int version, LanguageServer ls) {
353357
// The document has changed, do nothing
354358
return;
355359
}
360+
var clientFeatures = languageServerWrapper.getClientFeatures();
356361
// Update the diagnostics cache from the opened file and refresh UI to process LSPDiagnosticAnnotator.
357362
if (diagnosticReport.isLeft()) {
358-
RelatedFullDocumentDiagnosticReport fullDocumentDiagnosticReport = diagnosticReport.getLeft();
359-
var items = fullDocumentDiagnosticReport.getItems();
363+
RelatedFullDocumentDiagnosticReport fileReport = diagnosticReport.getLeft();
360364
// Update the diagnostics cache from the opened file
361-
var openedDocument = languageServerWrapper.getOpenedDocument(LSPIJUtils.toUri(file));
362-
openedDocument.updateDiagnostics(items != null ? items : Collections.emptyList());
365+
updatePullDiagnostics(file, fileReport, clientFeatures);
363366
} else if (diagnosticReport.isRight()) {
364-
// TODO ...
365-
RelatedUnchangedDocumentDiagnosticReport relatedUnchangedDocumentDiagnosticReport = diagnosticReport.getRight();
367+
RelatedUnchangedDocumentDiagnosticReport allFilesReport = diagnosticReport.getRight();
368+
Map<String, Either<FullDocumentDiagnosticReport, UnchangedDocumentDiagnosticReport>> relatedDocuments = allFilesReport.getRelatedDocuments();
369+
if (relatedDocuments != null) {
370+
for (var relatedDocument : relatedDocuments.entrySet()) {
371+
String documentUri = relatedDocument.getKey();
372+
Either<FullDocumentDiagnosticReport, UnchangedDocumentDiagnosticReport> fileReport = relatedDocument.getValue();
373+
if (fileReport != null) {
374+
if (fileReport.isLeft()) {
375+
VirtualFile file = FileUriSupport.findFileByUri(documentUri, clientFeatures);
376+
if (file != null) {
377+
updatePullDiagnostics(file, fileReport.getLeft(), clientFeatures);
378+
}
379+
} else if (fileReport.isRight()) {
380+
// TODO: implement UnchangedDocumentDiagnosticReport
381+
}
382+
}
383+
}
384+
}
366385
}
367386

368387
});
369388
}
370389

390+
private void updatePullDiagnostics(@NotNull VirtualFile file,
391+
@NotNull FullDocumentDiagnosticReport diagnosticReport,
392+
@NotNull LSPClientFeatures clientFeatures) {
393+
// Update diagnostics for the opened/closed document
394+
var fileUri = FileUriSupport.getFileUri(file, clientFeatures);
395+
List<Diagnostic> diagnostics = diagnosticReport.getItems() != null ? diagnosticReport.getItems() : Collections.emptyList();
396+
String identifier = clientFeatures.getDiagnosticFeature().getDiagnosticIdentifier();
397+
var openedDocument = languageServerWrapper.getOpenedDocument(fileUri);
398+
if (openedDocument != null) {
399+
openedDocument.updateDiagnostics(identifier, diagnostics);
400+
} else {
401+
var closedDocument = languageServerWrapper.getClosedDocument(fileUri, true);
402+
closedDocument.updateDiagnostics(identifier, diagnostics);
403+
}
404+
}
405+
371406
public boolean isPullDiagnosticsSupported() {
372407
return languageServerWrapper.getClientFeatures().getDiagnosticFeature().isDiagnosticSupported(file);
373408
}
@@ -383,7 +418,7 @@ private Alarm getDebouncePullDiagnosticsAlarm() {
383418
return debouncePullDiagnosticsAlarm;
384419
}
385420

386-
public boolean isDiagnosticNotPulledOnDidOpen() {
421+
private boolean isDiagnosticNotPulledOnDidOpen() {
387422
return diagnosticNotPulledOnDidOpen;
388423
}
389424
}

‎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 = "publish";
28+
public static final String PULL_DIAGNOSTIC_IDENTIFIER = "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/OpenedDocument.java‎

Lines changed: 3 additions & 5 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;
@@ -67,9 +64,10 @@ public LSPDiagnosticsForServer getDiagnosticsForServer() {
6764
}
6865

6966
@Override
70-
public boolean updateDiagnostics(@NotNull List<Diagnostic> diagnostics) {
67+
public boolean updateDiagnostics(@NotNull String identifier,
68+
@NotNull List<Diagnostic> diagnostics) {
7169
updatedDiagnosticsTime = System.currentTimeMillis();
72-
if (diagnosticsForServer.update(diagnostics)) {
70+
if (diagnosticsForServer.update(identifier, diagnostics)) {
7371
// LSP diagnostics has changed
7472
final PsiFile psiFile = LSPIJUtils.getPsiFile(file, diagnosticsForServer.getClientFeatures().getProject());
7573
if (psiFile != null) {

‎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: 3 additions & 6 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;
@@ -64,12 +61,12 @@ private void updateDiagnostics(@NotNull PublishDiagnosticsParams params, @NotNul
6461
if (openedDocument != null) {
6562
// Update diagnostics for opened file
6663
synchronized (openedDocument) {
67-
openedDocument.updateDiagnostics(params.getDiagnostics());
64+
openedDocument.updateDiagnostics(LSPDocumentBase.PUBLISH_DIAGNOSTIC_IDENTIFIER, params.getDiagnostics());
6865
}
6966
} else {
7067
// Update diagnostics for closed file
7168
ClosedDocument closedDocument = languageServerWrapper.getClosedDocument(fileURI, true);
72-
closedDocument.updateDiagnostics(params.getDiagnostics());
69+
closedDocument.updateDiagnostics(LSPDocumentBase.PUBLISH_DIAGNOSTIC_IDENTIFIER, params.getDiagnostics());
7370
}
7471
}
7572

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

Lines changed: 57 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -49,7 +49,11 @@ private record DiagnosticData(Range range, List<Diagnostic> diagnostics) {};
4949
private final @Nullable VirtualFile file;
5050

5151
// Map which contains all current diagnostics (as key) and future which load associated quick fixes (as value)
52-
private Map<Diagnostic, LSPLazyCodeActions> diagnostics;
52+
private @NotNull Map<Diagnostic, LSPLazyCodeActions> diagnostics;
53+
54+
private @Nullable Map<String /* diagnostic identifier */, Collection<Diagnostic>> diagnosticsPerIdentifier;
55+
56+
private String firstIdentifier;
5357

5458
public LSPDiagnosticsForServer(@NotNull LanguageServerItem languageServer,
5559
@Nullable VirtualFile file) {
@@ -58,24 +62,57 @@ public LSPDiagnosticsForServer(@NotNull LanguageServerItem languageServer,
5862
this.diagnostics = Collections.emptyMap();
5963
}
6064

61-
public LSPClientFeatures getClientFeatures() {
62-
return languageServer.getClientFeatures();
63-
}
6465
/**
6566
* Update the new LSP published diagnostics.
6667
*
67-
* @param diagnostics the new LSP published diagnostics
68+
* @param identifier the diagnostic identifier
69+
* @param diagnostics the new LSP published/pulled diagnostics
6870
*/
69-
public boolean update(@NotNull List<Diagnostic> diagnostics) {
70-
Collection<Diagnostic> oldDiagnostic = this.diagnostics != null ? this.diagnostics.keySet() : Collections.emptySet();
71-
boolean changed = LSPDocumentBase.isDiagnosticsChanged(oldDiagnostic, diagnostics);
71+
public boolean update(@NotNull String identifier,
72+
@NotNull List<Diagnostic> diagnostics) {
73+
if (diagnosticsPerIdentifier == null) {
74+
if (firstIdentifier == null) {
75+
firstIdentifier = identifier;
76+
} else {
77+
if (!firstIdentifier.equals(identifier)) {
78+
// We need to have several diagnostic cache
79+
diagnosticsPerIdentifier = new HashMap<>();
80+
diagnosticsPerIdentifier.put(firstIdentifier, this.diagnostics.keySet());
81+
}
82+
}
83+
}
84+
85+
Collection<Diagnostic> oldDiagnostics = getOldDiagnostics();
86+
Collection<Diagnostic> newDiagnostics = getNewDiagnostics(identifier, diagnostics);
87+
boolean changed = LSPDocumentBase.isDiagnosticsChanged(oldDiagnostics, newDiagnostics);
7288
// initialize diagnostics map
73-
this.diagnostics = toMap(diagnostics, this.diagnostics);
89+
this.diagnostics = toMap(newDiagnostics, this.diagnostics);
90+
if (diagnosticsPerIdentifier != null) {
91+
diagnosticsPerIdentifier.put(identifier, diagnostics);
92+
}
7493
return changed;
7594
}
7695

77-
private Map<Diagnostic, LSPLazyCodeActions> toMap(@NotNull List<Diagnostic> diagnostics,
78-
Map<Diagnostic, LSPLazyCodeActions> existingDiagnostics) {
96+
private Collection<Diagnostic> getOldDiagnostics() {
97+
return this.diagnostics != null ? this.diagnostics.keySet() : Collections.emptySet();
98+
}
99+
100+
private Collection<Diagnostic> getNewDiagnostics(@NotNull String identifier,
101+
@NotNull List<Diagnostic> diagnostics) {
102+
if (diagnosticsPerIdentifier == null) {
103+
return diagnostics;
104+
}
105+
Set<Diagnostic> newDiagnostics = new HashSet<>(diagnostics);
106+
for(var entry : diagnosticsPerIdentifier.entrySet()) {
107+
if (!entry.getKey().equals(identifier)) {
108+
newDiagnostics.addAll(entry.getValue());
109+
}
110+
}
111+
return newDiagnostics;
112+
}
113+
114+
private Map<Diagnostic, LSPLazyCodeActions> toMap(@NotNull Collection<Diagnostic> diagnostics,
115+
@NotNull Map<Diagnostic, LSPLazyCodeActions> existingDiagnostics) {
79116
// Collect quick fixes from LSP code action
80117
Map<Diagnostic, LSPLazyCodeActions> map = new HashMap<>(diagnostics.size());
81118
// Sort diagnostics by range
@@ -169,4 +206,13 @@ private static boolean isCodeActionSupported(@NotNull LanguageServerWrapper lang
169206
return languageServerWrapper.getClientFeatures().getCodeActionFeature().isCodeActionSupported(file);
170207
}
171208

209+
/**
210+
* Returns the client features.
211+
*
212+
* @return the client features.
213+
*/
214+
public LSPClientFeatures getClientFeatures() {
215+
return languageServer.getClientFeatures();
216+
}
217+
172218
}

‎src/main/java/com/redhat/devtools/lsp4ij/server/capabilities/DiagnosticCapabilityRegistry.java‎

Lines changed: 21 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,9 @@
1515
import com.intellij.psi.PsiFile;
1616
import com.redhat.devtools.lsp4ij.DocumentContentSynchronizer;
1717
import com.redhat.devtools.lsp4ij.JSONUtils;
18+
import com.redhat.devtools.lsp4ij.LSPDocumentBase;
1819
import com.redhat.devtools.lsp4ij.client.features.LSPClientFeatures;
20+
import com.redhat.devtools.lsp4ij.internal.StringUtils;
1921
import org.eclipse.lsp4j.DiagnosticRegistrationOptions;
2022
import org.eclipse.lsp4j.ServerCapabilities;
2123
import org.jetbrains.annotations.NotNull;
@@ -77,13 +79,29 @@ public boolean isDiagnosticSupported(@NotNull VirtualFile file) {
7779

7880
@Override
7981
public @Nullable DiagnosticRegistrationOptions registerCapability(@NotNull JsonObject registerOptions) {
80-
var options = super.registerCapability(registerOptions);
82+
var options = super.registerCapability(registerOptions);
8183
// Force the pull diagnostic for all opened document
8284
// - didOpen have not processed pull diagnostic
8385
// - and if language server support it for now if textDocument/diagnostic has been dynamically registered.
84-
for (var openedDocument : getClientFeatures().getServerWrapper().getOpenedDocuments()) {
85-
openedDocument.getSynchronizer().refreshPullDiagnostic(DocumentContentSynchronizer.RefreshPullDiagnosticOrigin.ON_REGISTER_CAPABILITY);
86+
for (var openedDocument : getClientFeatures().getServerWrapper().getOpenedDocuments()) {
87+
openedDocument.getSynchronizer()
88+
.refreshPullDiagnostic(DocumentContentSynchronizer.RefreshPullDiagnosticOrigin.ON_REGISTER_CAPABILITY);
8689
}
8790
return options;
8891
}
92+
93+
/**
94+
* Returns the diagnostic identifier declared in the {@link DiagnosticRegistrationOptions} and "pull" otherwise.
95+
*
96+
* @return the diagnostic identifier declared in the {@link DiagnosticRegistrationOptions} and "pull" otherwise.
97+
*/
98+
public @NotNull String getDiagnosticIdentifier() {
99+
for (var option : getOptions()) {
100+
String identifier = option.getIdentifier();
101+
if (!StringUtils.isEmpty(identifier)) {
102+
return identifier;
103+
}
104+
}
105+
return LSPDocumentBase.PULL_DIAGNOSTIC_IDENTIFIER;
106+
}
89107
}

0 commit comments

Comments
 (0)