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

Reply via email to