Merge pull request #41756 from nipunayf/fix-cyclic-ls

Generate notifications for cyclic dependencies to the LS client

language-server/modules/langserver-core/src/main/java/org/ballerinalang/langserver/diagnostic/DiagnosticsHelper.java

@@ -20,15 +20,18 @@
 import io.ballerina.projects.ProjectKind;
 import io.ballerina.tools.text.LineRange;
 import org.ballerinalang.langserver.LSContextOperation;
+import org.ballerinalang.langserver.command.CommandUtil;
 import org.ballerinalang.langserver.common.utils.PathUtil;
 import org.ballerinalang.langserver.commons.DocumentServiceContext;
 import org.ballerinalang.langserver.commons.LanguageServerContext;
 import org.ballerinalang.langserver.commons.WorkspaceServiceContext;
 import org.ballerinalang.langserver.commons.client.ExtendedLanguageClient;
 import org.ballerinalang.langserver.commons.workspace.WorkspaceManager;
 import org.ballerinalang.langserver.workspace.BallerinaWorkspaceManager;
+import org.ballerinalang.util.diagnostic.DiagnosticErrorCode;
 import org.eclipse.lsp4j.Diagnostic;
 import org.eclipse.lsp4j.DiagnosticSeverity;
+import org.eclipse.lsp4j.MessageType;
 import org.eclipse.lsp4j.Position;
 import org.eclipse.lsp4j.PublishDiagnosticsParams;
 import org.eclipse.lsp4j.Range;
@@ -40,6 +43,7 @@
 import java.util.List;
 import java.util.Map;
 import java.util.Optional;
+import java.util.Stack;
 import java.util.concurrent.CompletableFuture;
 import java.util.concurrent.Executor;
 import java.util.concurrent.TimeUnit;
@@ -59,6 +63,7 @@ public class DiagnosticsHelper {
      */
     private final Map<Path, Map<String, List<Diagnostic>>> lastDiagnosticMap;
     private CompletableFuture<Boolean> latestScheduled = null;
+    private final Stack<String> cyclicDependencyErrors;
 
     public static DiagnosticsHelper getInstance(LanguageServerContext serverContext) {
         DiagnosticsHelper diagnosticsHelper = serverContext.get(DIAGNOSTICS_HELPER_KEY);
@@ -72,6 +77,7 @@ public static DiagnosticsHelper getInstance(LanguageServerContext serverContext)
     private DiagnosticsHelper(LanguageServerContext serverContext) {
         serverContext.put(DIAGNOSTICS_HELPER_KEY, this);
         this.lastDiagnosticMap = new HashMap<>();
+        this.cyclicDependencyErrors = new Stack<>();
     }
 
     /**
@@ -123,26 +129,7 @@ public synchronized void compileAndSendDiagnostics(ExtendedLanguageClient client
             return;
         }
         Map<String, List<Diagnostic>> latestDiagnostics = getLatestDiagnostics(context);
-
-        // If the client is null, returns
-        if (client == null) {
-            return;
-        }
-        Map<String, List<Diagnostic>> lastProjectDiagnostics =
-                lastDiagnosticMap.getOrDefault(project.get().sourceRoot(), new HashMap<>());
-
-        // Clear old diagnostic entries of the project with an empty list
-        lastProjectDiagnostics.forEach((key, value) -> {
-            if (!latestDiagnostics.containsKey(key)) {
-                client.publishDiagnostics(new PublishDiagnosticsParams(key, emptyDiagnosticList));
-            }
-        });
-
-        // Publish diagnostics for the project
-        latestDiagnostics.forEach((key, value) -> client.publishDiagnostics(new PublishDiagnosticsParams(key, value)));
-
-        // Replace old diagnostic map associated with the project
-        lastDiagnosticMap.put(project.get().sourceRoot(), latestDiagnostics);
+        sendDiagnostics(client, latestDiagnostics, project.get().sourceRoot());
     }
 
     /**
@@ -157,6 +144,11 @@ private synchronized void compileAndSendDiagnostics(ExtendedLanguageClient clien
                                                         WorkspaceManager workspaceManager) {
         Map<String, List<Diagnostic>> diagnosticMap =
                 toDiagnosticsMap(compilation.diagnosticResult().diagnostics(false), projectRoot, workspaceManager);
+        sendDiagnostics(client, diagnosticMap, projectRoot);
+    }
+
+    private synchronized void sendDiagnostics(ExtendedLanguageClient client,
+                                              Map<String, List<Diagnostic>> diagnosticMap, Path projectRoot) {
         // If the client is null, returns
         if (client == null) {
             return;
@@ -174,6 +166,11 @@ private synchronized void compileAndSendDiagnostics(ExtendedLanguageClient clien
         // Publish diagnostics for the project
         diagnosticMap.forEach((key, value) -> client.publishDiagnostics(new PublishDiagnosticsParams(key, value)));
 
+        // Show cyclic dependency error message if exists
+        while (!this.cyclicDependencyErrors.isEmpty()) {
+            CommandUtil.notifyClient(client, MessageType.Error, this.cyclicDependencyErrors.pop());
+        }
+
         // Replace old diagnostic map associated with the project
         lastDiagnosticMap.put(projectRoot, diagnosticMap);
     }
@@ -203,6 +200,10 @@ private Map<String, List<Diagnostic>> toDiagnosticsMap(Collection<io.ballerina.t
                                                            Path projectRoot, WorkspaceManager workspaceManager) {
         Map<String, List<Diagnostic>> diagnosticsMap = new HashMap<>();
         for (io.ballerina.tools.diagnostics.Diagnostic diag : diags) {
+            if (diag.diagnosticInfo().code()
+                    .equals(DiagnosticErrorCode.CYCLIC_MODULE_IMPORTS_DETECTED.diagnosticId())) {
+                this.cyclicDependencyErrors.push(diag.message());
+            }
             LineRange lineRange = diag.location().lineRange();
 
             int startLine = lineRange.startLine().line();

language-server/modules/langserver-core/src/main/java/org/ballerinalang/langserver/workspace/BallerinaWorkspaceManager.java

@@ -1483,6 +1483,7 @@ public static class ProjectContext {
         private ProjectContext(Project project, Lock lock) {
             this.project = project;
             this.lock = lock;
+            this.compilationCrashed = false;
         }
 
         public static ProjectContext from(Project project) {
@@ -1536,7 +1537,7 @@ public void setProject(Project project) {
          * @return whether the compilation is in a crashed state
          */
         public boolean compilationCrashed() {
-            return Boolean.TRUE.equals(this.compilationCrashed);
+            return this.compilationCrashed;
         }
 
         /**

language-server/modules/langserver-core/src/test/java/org/ballerinalang/langserver/diagnostics/CyclicDependenciesTest.java

@@ -0,0 +1,90 @@
+/*
+ * Copyright (c) 2023, WSO2 LLC. (https://www.wso2.com) All Rights Reserved.
+ *
+ * WSO2 LLC. licenses this file to you under the Apache License,
+ * Version 2.0 (the "License"); you may not use this file except
+ * in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.ballerinalang.langserver.diagnostics;
+
+import org.ballerinalang.langserver.LSContextOperation;
+import org.ballerinalang.langserver.commons.DocumentServiceContext;
+import org.ballerinalang.langserver.commons.LanguageServerContext;
+import org.ballerinalang.langserver.commons.client.ExtendedLanguageClient;
+import org.ballerinalang.langserver.commons.eventsync.exceptions.EventSyncException;
+import org.ballerinalang.langserver.commons.workspace.WorkspaceDocumentException;
+import org.ballerinalang.langserver.contexts.ContextBuilder;
+import org.ballerinalang.langserver.contexts.LanguageServerContextImpl;
+import org.ballerinalang.langserver.diagnostic.DiagnosticsHelper;
+import org.ballerinalang.langserver.util.FileUtils;
+import org.ballerinalang.langserver.workspace.BallerinaWorkspaceManager;
+import org.eclipse.lsp4j.MessageParams;
+import org.eclipse.lsp4j.MessageType;
+import org.mockito.Mockito;
+import org.testng.annotations.DataProvider;
+import org.testng.annotations.Test;
+
+import java.nio.file.Path;
+import java.util.List;
+
+/**
+ * Diagnostic tests related to cyclic dependencies.
+ *
+ * @since 2201.9.0
+ */
+public class CyclicDependenciesTest {
+
+    private final Path projectRoot = FileUtils.RES_DIR.resolve("diagnostics").resolve("sources");
+    private final LanguageServerContext serverContext = new LanguageServerContextImpl();
+    private final BallerinaWorkspaceManager workspaceManager = new BallerinaWorkspaceManager(serverContext);
+
+    @Test(dataProvider = "cyclic-package-provider")
+    public void testCyclicDependenciesOnOpen(String packageName, List<String> expectedMessages)
+            throws WorkspaceDocumentException, EventSyncException,
+            InterruptedException {
+        Path projectPath = projectRoot.resolve(packageName);
+        ExtendedLanguageClient mockClient = Mockito.mock(ExtendedLanguageClient.class);
+
+        DocumentServiceContext serviceContext =
+                ContextBuilder.buildDocumentServiceContext(projectPath.toUri().toString(),
+                        this.workspaceManager,
+                        LSContextOperation.TXT_DID_OPEN,
+                        this.serverContext);
+
+        this.workspaceManager.loadProject(projectPath);
+        DiagnosticsHelper diagnosticsHelper = DiagnosticsHelper.getInstance(this.serverContext);
+        diagnosticsHelper.schedulePublishDiagnostics(mockClient, serviceContext);
+        Thread.sleep(2000);
+
+        Mockito.verify(mockClient, Mockito.times(expectedMessages.size())).showMessage(Mockito.any());
+        expectedMessages.forEach(expectedMessage -> Mockito.verify(mockClient)
+                .showMessage(new MessageParams(MessageType.Error, expectedMessage)));
+    }
+
+    @DataProvider(name = "cyclic-package-provider")
+    public Object[][] cyclicDataProvider() {
+        return new Object[][]{
+                {"cyclic_simple",
+                        List.of("cyclic module imports detected 'lsorg/cyclic.mod1:0.1.0 -> lsorg/cyclic.mod2:0.1.0" +
+                                " -> lsorg/cyclic.mod1:0.1.0'")},
+                {"cyclic_multi",
+                        List.of("cyclic module imports detected 'lsorg/cyclic_multi.abc:0.1.0 -> lsorg/cyclic_multi" +
+                                        ".def:0.1.0 -> lsorg/cyclic_multi.ghi:0.1.0 -> lsorg/cyclic_multi.jkl:0.1.0 " +
+                                        "-> lsorg/cyclic_multi.abc:0.1.0'",
+                                "cyclic module imports detected 'lsorg/cyclic_multi.def:0.1.0 -> lsorg/cyclic_multi" +
+                                        ".ghi:0.1.0 -> lsorg/cyclic_multi.def:0.1.0'",
+                                "cyclic module imports detected 'lsorg/cyclic_multi.abc:0.1.0 -> lsorg/cyclic_multi" +
+                                        ".def:0.1.0 -> lsorg/cyclic_multi.ghi:0.1.0 -> lsorg/cyclic_multi.abc:0.1.0'")}
+        };
+    }
+}

language-server/modules/langserver-core/src/test/resources/diagnostics/sources/cyclic_multi/Ballerina.toml

@@ -0,0 +1,4 @@
+[package]
+org = "lsorg"
+name = "cyclic_multi"
+version = "0.1.0"

language-server/modules/langserver-core/src/test/resources/diagnostics/sources/cyclic_multi/modules/abc/main.bal

@@ -0,0 +1,7 @@
+import cyclic_multi.def as _;
+import cyclic_multi.mno as _;
+import cyclic_multi.pqr as _;
+
+function test() {
+
+}

language-server/modules/langserver-core/src/test/resources/diagnostics/sources/cyclic_multi/modules/def/main.bal

@@ -0,0 +1,5 @@
+import cyclic_multi.ghi as _;
+
+function test() {
+
+}

language-server/modules/langserver-core/src/test/resources/diagnostics/sources/cyclic_multi/modules/ghi/main.bal

@@ -0,0 +1,7 @@
+import cyclic_multi.def as _;
+import cyclic_multi.abc as _;
+import cyclic_multi.jkl as _;
+
+function test() {
+
+}

language-server/modules/langserver-core/src/test/resources/diagnostics/sources/cyclic_multi/modules/jkl/main.bal

@@ -0,0 +1,5 @@
+import cyclic_multi.abc as _;
+
+function test() {
+
+}

language-server/modules/langserver-core/src/test/resources/diagnostics/sources/cyclic_multi/modules/mno/main.bal

@@ -0,0 +1,5 @@
+import cyclic_multi.pqr as _;
+
+function test() {
+
+}

language-server/modules/langserver-core/src/test/resources/diagnostics/sources/cyclic_multi/modules/pqr/main.bal

@@ -0,0 +1,3 @@
+function test() {
+
+}

language-server/modules/langserver-core/src/test/resources/diagnostics/sources/cyclic_simple/Ballerina.toml

@@ -0,0 +1,4 @@
+[package]
+org = "lsorg"
+name = "cyclic"
+version = "0.1.0"

language-server/modules/langserver-core/src/test/resources/diagnostics/sources/cyclic_simple/modules/mod1/mod1.bal

@@ -0,0 +1,9 @@
+import cyclic.mod2;
+
+public function hello(string name) returns string {
+    _ = mod2:hello("A");
+    if name !is "" {
+        return "Hello, " + name;
+    }
+    return "Hello, World!";
+}

language-server/modules/langserver-core/src/test/resources/diagnostics/sources/cyclic_simple/modules/mod2/mod2.bal

@@ -0,0 +1,9 @@
+import cyclic.mod1;
+
+public function hello(string name) returns string {
+    _ = mod1:hello("A");
+    if name !is "" {
+        return "Hello, " + name;
+    }
+    return "Hello, World!";
+}