This is an automated email from the ASF dual-hosted git repository.
jlahoda pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/netbeans.git
The following commit(s) were added to refs/heads/master by this push:
new 02c6c65 Improving reparsing for errors - not sending updates when the
code has already changed.
02c6c65 is described below
commit 02c6c658888cb118f083d85ac3d91627303db1fd
Author: Jan Lahoda <[email protected]>
AuthorDate: Mon Jun 7 06:10:03 2021 +0200
Improving reparsing for errors - not sending updates when the code has
already changed.
---
.../server/protocol/TextDocumentServiceImpl.java | 156 +++++++++++++++------
.../java/lsp/server/protocol/ServerTest.java | 54 ++++++-
2 files changed, 167 insertions(+), 43 deletions(-)
diff --git
a/java/java.lsp.server/src/org/netbeans/modules/java/lsp/server/protocol/TextDocumentServiceImpl.java
b/java/java.lsp.server/src/org/netbeans/modules/java/lsp/server/protocol/TextDocumentServiceImpl.java
index 6c62e25..d425b46 100644
---
a/java/java.lsp.server/src/org/netbeans/modules/java/lsp/server/protocol/TextDocumentServiceImpl.java
+++
b/java/java.lsp.server/src/org/netbeans/modules/java/lsp/server/protocol/TextDocumentServiceImpl.java
@@ -46,14 +46,17 @@ import java.util.EnumMap;
import java.util.EnumSet;
import java.util.HashMap;
import java.util.HashSet;
+import java.util.Iterator;
import java.util.List;
import java.util.Map;
+import java.util.Map.Entry;
import java.util.Set;
import java.util.concurrent.CompletableFuture;
import java.util.concurrent.ConcurrentHashMap;
import java.util.concurrent.atomic.AtomicBoolean;
import java.util.concurrent.atomic.AtomicInteger;
import java.util.function.BiConsumer;
+import java.util.function.Consumer;
import java.util.function.IntFunction;
import java.util.logging.Level;
import java.util.logging.Logger;
@@ -66,6 +69,8 @@ import javax.lang.model.element.TypeElement;
import javax.lang.model.type.DeclaredType;
import javax.lang.model.type.TypeKind;
import javax.lang.model.type.TypeMirror;
+import javax.swing.event.DocumentEvent;
+import javax.swing.event.DocumentListener;
import javax.swing.text.BadLocationException;
import javax.swing.text.Document;
import javax.swing.text.StyledDocument;
@@ -131,6 +136,9 @@ import org.eclipse.lsp4j.services.LanguageClient;
import org.eclipse.lsp4j.services.LanguageClientAware;
import org.eclipse.lsp4j.services.TextDocumentService;
import org.netbeans.api.annotations.common.CheckForNull;
+import org.netbeans.api.editor.EditorUtilities;
+import org.netbeans.api.editor.document.LineDocument;
+import org.netbeans.api.editor.document.LineDocumentUtils;
import org.netbeans.api.java.lexer.JavaTokenId;
import org.netbeans.api.java.project.JavaProjectConstants;
import org.netbeans.api.java.source.CompilationController;
@@ -153,6 +161,7 @@ import org.netbeans.api.project.Project;
import org.netbeans.api.project.ProjectUtils;
import org.netbeans.api.project.SourceGroup;
import org.netbeans.api.project.Sources;
+import org.netbeans.editor.DocumentUtilities;
import org.netbeans.modules.editor.java.GoToSupport;
import org.netbeans.modules.editor.java.GoToSupport.GoToTarget;
import
org.netbeans.modules.gsf.testrunner.ui.api.TestMethodController.TestMethod;
@@ -247,7 +256,7 @@ public class TextDocumentServiceImpl implements
TextDocumentService, LanguageCli
Set<String> documents = new HashSet<>(openedDocuments.keySet());
for (String doc : documents) {
- runDiagnoticTasks(doc);
+ runDiagnosticTasks(doc);
}
}
@@ -1356,7 +1365,7 @@ public class TextDocumentServiceImpl implements
TextDocumentService, LanguageCli
// attempt to open the directly owning project, delay diagnostics
after project open:
server.asyncOpenFileOwner(file).thenRun(() ->
- runDiagnoticTasks(params.getTextDocument().getUri())
+ runDiagnosticTasks(params.getTextDocument().getUri())
);
} catch (IOException ex) {
throw new IllegalStateException(ex);
@@ -1384,7 +1393,7 @@ public class TextDocumentServiceImpl implements
TextDocumentService, LanguageCli
}
});
}
- runDiagnoticTasks(params.getTextDocument().getUri());
+ runDiagnosticTasks(params.getTextDocument().getUri());
reportNotificationDone("didChange", params);
}
@@ -1511,41 +1520,101 @@ public class TextDocumentServiceImpl implements
TextDocumentService, LanguageCli
return CompletableFuture.completedFuture(location);
}
- private void runDiagnoticTasks(String uri) {
+ private void runDiagnosticTasks(String uri) {
if (server.openedProjects().getNow(null) == null) {
return;
}
- //XXX: cancelling/deferring the tasks!
diagnosticTasks.computeIfAbsent(uri, u -> {
return BACKGROUND_TASKS.create(() -> {
- computeDiags(u, (info, doc) -> {
+ Document originalDoc = openedDocuments.get(uri);
+ long originalVersion = documentVersion(originalDoc);
+ List<Diagnostic> errorDiags = computeDiags(u, (info, doc) -> {
ErrorHintsProvider ehp = new ErrorHintsProvider();
- return ehp.computeErrors(info, doc, "text/x-java");
//TODO: mimetype?
- }, "errors", false);
- BACKGROUND_TASKS.create(() -> {
- computeDiags(u, (info, doc) -> {
- Set<Severity> disabled =
org.netbeans.modules.java.hints.spiimpl.Utilities.disableErrors(info.getFileObject());
- if (disabled.size() == Severity.values().length) {
- return Collections.emptyList();
+ class CancelListener implements DocumentListener {
+ @Override
+ public void insertUpdate(DocumentEvent e) {
+ checkCancel();
+ }
+ @Override
+ public void removeUpdate(DocumentEvent e) {
+ checkCancel();
}
- return new
HintsInvoker(HintsSettings.getGlobalSettings(), new
AtomicBoolean()).computeHints(info)
-
.stream()
-
.filter(ed -> !disabled.contains(ed.getSeverity()))
-
.collect(Collectors.toList());
- }, "hints", true);
- }).schedule(DELAY);
+ private void checkCancel() {
+ if (documentVersion(doc) != originalVersion) {
+ ehp.cancel();
+ }
+ }
+ @Override
+ public void changedUpdate(DocumentEvent e) {}
+ }
+ CancelListener l = new CancelListener();
+ try {
+ doc.addDocumentListener(l);
+ l.checkCancel();
+ return ehp.computeErrors(info, doc, "text/x-java");
//TODO: mimetype?
+ } finally {
+ doc.removeDocumentListener(l);
+ }
+ }, "errors");
+ if (documentVersion(originalDoc) == originalVersion) {
+ publishDiagnostics(uri, errorDiags);
+ BACKGROUND_TASKS.create(() -> {
+ List<Diagnostic> hintDiags = computeDiags(u, (info,
doc) -> {
+ Set<Severity> disabled =
org.netbeans.modules.java.hints.spiimpl.Utilities.disableErrors(info.getFileObject());
+ if (disabled.size() == Severity.values().length) {
+ return Collections.emptyList();
+ }
+ AtomicBoolean cancel = new AtomicBoolean();
+ HintsInvoker invoker = new
HintsInvoker(HintsSettings.getGlobalSettings(), cancel);
+ class CancelListener implements DocumentListener {
+ @Override
+ public void insertUpdate(DocumentEvent e) {
+ checkCancel();
+ }
+ @Override
+ public void removeUpdate(DocumentEvent e) {
+ checkCancel();
+ }
+ private void checkCancel() {
+ if (documentVersion(doc) !=
originalVersion) {
+ cancel.set(true);
+ }
+ }
+ @Override
+ public void changedUpdate(DocumentEvent e) {}
+ }
+ CancelListener l = new CancelListener();
+ try {
+ doc.addDocumentListener(l);
+ l.checkCancel();
+ return invoker.computeHints(info)
+ .stream()
+ .filter(ed ->
!disabled.contains(ed.getSeverity()))
+ .collect(Collectors.toList());
+ } finally {
+ doc.removeDocumentListener(l);
+ }
+ }, "hints");
+ Document doc = openedDocuments.get(uri);
+ if (documentVersion(doc) == originalVersion) {
+ publishDiagnostics(uri, hintDiags);
+ }
+ }).schedule(DELAY);
+ }
});
}).schedule(DELAY);
}
private static final int DELAY = 500;
+ static Consumer<String> computeDiagsCallback; //for tests
- private void computeDiags(String uri, ProduceErrors produceErrors, String
keyPrefix, boolean update) {
+ private List<Diagnostic> computeDiags(String uri, ProduceErrors
produceErrors, String keyPrefix) {
+ List<Diagnostic> result = new ArrayList<>();
try {
FileObject file = fromURI(uri);
if (file == null) {
// the file does not exist.
- return;
+ return result;
}
EditorCookie ec = file.getLookup().lookup(EditorCookie.class);
Document doc = ec.openDocument();
@@ -1555,52 +1624,50 @@ public class TextDocumentServiceImpl implements
TextDocumentService, LanguageCli
CompilationController cc =
CompilationController.get(it.getParserResult());
if (cc != null) {
cc.toPhase(JavaSource.Phase.RESOLVED);
+ if (computeDiagsCallback != null) {
+ computeDiagsCallback.accept(keyPrefix);
+ }
Map<String, ErrorDescription> id2Errors = new
HashMap<>();
- List<Diagnostic> diags = new ArrayList<>();
- int idx = 0;
List<ErrorDescription> errors =
produceErrors.computeErrors(cc, doc);
if (errors == null) {
errors = Collections.emptyList();
}
+ int idx = 0;
for (ErrorDescription err : errors) {
- Diagnostic diag = new Diagnostic(new
Range(Utils.createPosition(cc.getCompilationUnit(),
err.getRange().getBegin().getOffset()),
-
Utils.createPosition(cc.getCompilationUnit(),
err.getRange().getEnd().getOffset())),
-
err.getDescription());
- switch (err.getSeverity()) {
- case ERROR:
diag.setSeverity(DiagnosticSeverity.Error); break;
- case VERIFIER:
- case WARNING:
diag.setSeverity(DiagnosticSeverity.Warning); break;
- case HINT:
diag.setSeverity(DiagnosticSeverity.Hint); break;
- default:
diag.setSeverity(DiagnosticSeverity.Information); break;
- }
String id = keyPrefix + ":" + idx++ + "-" +
err.getId();
- diag.setCode(id);
id2Errors.put(id, err);
- diags.add(diag);
}
doc.putProperty("lsp-errors-" + keyPrefix, id2Errors);
- doc.putProperty("lsp-errors-diags-" + keyPrefix,
diags);
Map<String, ErrorDescription> mergedId2Errors = new
HashMap<>();
- List<Diagnostic> mergedDiags = new ArrayList<>();
for (String k : ERROR_KEYS) {
Map<String, ErrorDescription> prevErrors =
(Map<String, ErrorDescription>) doc.getProperty("lsp-errors-" + k);
if (prevErrors != null) {
mergedId2Errors.putAll(prevErrors);
}
- List<Diagnostic> prevDiags = (List<Diagnostic>)
doc.getProperty("lsp-errors-diags-" + k);
- if (prevDiags != null) {
- mergedDiags.addAll(prevDiags);
+ }
+ for (Entry<String, ErrorDescription> id2Error :
mergedId2Errors.entrySet()) {
+ ErrorDescription err = id2Error.getValue();
+ Diagnostic diag = new Diagnostic(new
Range(Utils.createPosition(cc.getFileObject(),
err.getRange().getBegin().getOffset()),
+
Utils.createPosition(cc.getFileObject(), err.getRange().getEnd().getOffset())),
+
err.getDescription());
+ switch (err.getSeverity()) {
+ case ERROR:
diag.setSeverity(DiagnosticSeverity.Error); break;
+ case VERIFIER:
+ case WARNING:
diag.setSeverity(DiagnosticSeverity.Warning); break;
+ case HINT:
diag.setSeverity(DiagnosticSeverity.Hint); break;
+ default:
diag.setSeverity(DiagnosticSeverity.Information); break;
}
+ diag.setCode(id2Error.getKey());
+ result.add(diag);
}
doc.putProperty("lsp-errors", mergedId2Errors);
- doc.putProperty("lsp-errors-diags", mergedDiags);
- publishDiagnostics(uri, mergedDiags);
}
}
});
} catch (IOException | ParseException ex) {
throw new IllegalStateException(ex);
}
+ return result;
}
private FileObject fromURI(String uri) {
@@ -1743,6 +1810,11 @@ public class TextDocumentServiceImpl implements
TextDocumentService, LanguageCli
return qualifiedName != null ? qualifiedName.replace('.', '/') +
".class" : null;
}
+ private static long documentVersion(Document doc) {
+ Object ver = doc != null ? doc.getProperty("version") : null;
+ return ver instanceof Number ? ((Number) ver).longValue() : -1;
+ }
+
private static void reportNotificationDone(String s, Object parameter) {
if (HOOK_NOTIFICATION != null) {
HOOK_NOTIFICATION.accept(s, parameter);
diff --git
a/java/java.lsp.server/test/unit/src/org/netbeans/modules/java/lsp/server/protocol/ServerTest.java
b/java/java.lsp.server/test/unit/src/org/netbeans/modules/java/lsp/server/protocol/ServerTest.java
index 51a4bcd..9a16022 100644
---
a/java/java.lsp.server/test/unit/src/org/netbeans/modules/java/lsp/server/protocol/ServerTest.java
+++
b/java/java.lsp.server/test/unit/src/org/netbeans/modules/java/lsp/server/protocol/ServerTest.java
@@ -215,7 +215,7 @@ public class ServerTest extends NbTestCase {
OpenProjects.getDefault().close(OpenProjects.getDefault().getOpenProjects());
}
- List<Diagnostic>[] diags = new List[1];
+ final List<Diagnostic>[] diags = new List[1];
Set<String> diagnosticURIs = Collections.synchronizedSet(new HashSet<>());
void clearDiagnostics() {
@@ -4297,6 +4297,58 @@ public class ServerTest extends NbTestCase {
assertTrue(lc.perCent < 100);
}
+ public void testFileModificationDiags() throws Exception {
+ File src = new File(getWorkDir(), "Test.java");
+ src.getParentFile().mkdirs();
+ String code = "public class Test {\n" +
+ " public void run(String str) {\n" +
+ " System.err.println(1);\n" +
+ " String s = str.substring(0);\n" +
+ " }\n" +
+ "}\n";
+ try (Writer w = new FileWriter(src)) {
+ w.write(code);
+ }
+ Launcher<LanguageServer> serverLauncher =
LSPLauncher.createClientLauncher(new LspClient(), client.getInputStream(),
client.getOutputStream());
+ serverLauncher.startListening();
+ LanguageServer server = serverLauncher.getRemoteProxy();
+ InitializeResult result = server.initialize(new
InitializeParams()).get();
+ server.getTextDocumentService().didOpen(new
DidOpenTextDocumentParams(new TextDocumentItem(toURI(src), "java", 0, code)));
+ assertDiags(diags);//errors
+ assertDiags(diags, "Warning:3:15-3:16");//hints
+ VersionedTextDocumentIdentifier id = new
VersionedTextDocumentIdentifier(toURI(src), 0);
+ CountDownLatch waitForErrorLatch = new CountDownLatch(1);
+ TextDocumentServiceImpl.computeDiagsCallback = key -> {
+ if ("errors".equals(key)) {
+ waitForErrorLatch.countDown();
+ server.getTextDocumentService().didChange(new
DidChangeTextDocumentParams(id, Arrays.asList(new
TextDocumentContentChangeEvent(new Range(new Position(2, 27), new Position(2,
28)), 1, "1"))));
+ TextDocumentServiceImpl.computeDiagsCallback = null;
+ }
+ };
+ server.getTextDocumentService().didChange(new
DidChangeTextDocumentParams(id, Arrays.asList(new
TextDocumentContentChangeEvent(new Range(new Position(2, 27), new Position(2,
27)), 0, "d"))));
+ assertDiags(diags, "Warning:3:15-3:16");//errors
+ assertDiags(diags, "Warning:3:15-3:16");//hints
+ //verify no more diags coming:
+ synchronized (diags) {
+ long timeout = 1000;
+ long start = System.currentTimeMillis();
+ while (diags[0] == null && (System.currentTimeMillis() - start) <
timeout) {
+ try {
+ diags.wait(timeout / 10);
+ } catch (InterruptedException ex) {
+ //ignore
+ }
+ }
+ assertNull(diags[0]);
+ }
+ server.getTextDocumentService().didChange(new
DidChangeTextDocumentParams(id, Arrays.asList(new
TextDocumentContentChangeEvent(new Range(new Position(2, 1), new Position(2,
1)), 0, " \n "))));
+ assertDiags(diags, "Warning:4:15-4:16");//errors
+ assertDiags(diags, "Warning:4:15-4:16");//hints
+ server.getTextDocumentService().didChange(new
DidChangeTextDocumentParams(id, Arrays.asList(new
TextDocumentContentChangeEvent(new Range(new Position(4, 1), new Position(4,
1)), 0, " "))));
+ assertDiags(diags, "Warning:4:16-4:17");//errors
+ assertDiags(diags, "Warning:4:16-4:17");//hints
+ }
+
static {
System.setProperty("SourcePath.no.source.filter", "true");
}
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists