From a5b08a9687c796374bbe204b24dd59a014d77a2e Mon Sep 17 00:00:00 2001 From: Nipuna Fernando Date: Wed, 15 Nov 2023 10:37:14 +0530 Subject: [PATCH 1/2] Display cyclic dependencies error popup --- .../diagnostic/DiagnosticsHelper.java | 41 ++++++++++--------- .../workspace/BallerinaWorkspaceManager.java | 3 +- 2 files changed, 23 insertions(+), 21 deletions(-) diff --git a/language-server/modules/langserver-core/src/main/java/org/ballerinalang/langserver/diagnostic/DiagnosticsHelper.java b/language-server/modules/langserver-core/src/main/java/org/ballerinalang/langserver/diagnostic/DiagnosticsHelper.java index 890e269c3347..50e7f50e07fe 100644 --- a/language-server/modules/langserver-core/src/main/java/org/ballerinalang/langserver/diagnostic/DiagnosticsHelper.java +++ b/language-server/modules/langserver-core/src/main/java/org/ballerinalang/langserver/diagnostic/DiagnosticsHelper.java @@ -20,6 +20,7 @@ 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; @@ -27,8 +28,10 @@ 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>> lastDiagnosticMap; private CompletableFuture latestScheduled = null; + private final Stack 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> latestDiagnostics = getLatestDiagnostics(context); - - // If the client is null, returns - if (client == null) { - return; - } - Map> 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> diagnosticMap = toDiagnosticsMap(compilation.diagnosticResult().diagnostics(false), projectRoot, workspaceManager); + sendDiagnostics(client, diagnosticMap, projectRoot); + } + + private synchronized void sendDiagnostics(ExtendedLanguageClient client, + Map> 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> toDiagnosticsMap(Collection> 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(); diff --git a/language-server/modules/langserver-core/src/main/java/org/ballerinalang/langserver/workspace/BallerinaWorkspaceManager.java b/language-server/modules/langserver-core/src/main/java/org/ballerinalang/langserver/workspace/BallerinaWorkspaceManager.java index 556b2be176e8..c381da8f41cf 100644 --- a/language-server/modules/langserver-core/src/main/java/org/ballerinalang/langserver/workspace/BallerinaWorkspaceManager.java +++ b/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; } /** From 4d232fd4981a97be75a417f239a4c96e39d438be Mon Sep 17 00:00:00 2001 From: Nipuna Fernando Date: Wed, 22 Nov 2023 20:48:14 +0530 Subject: [PATCH 2/2] Add cyclic dependency test for LS --- .../diagnostics/CyclicDependenciesTest.java | 90 +++++++++++++++++++ .../sources/cyclic_multi/Ballerina.toml | 4 + .../sources/cyclic_multi/modules/abc/main.bal | 7 ++ .../sources/cyclic_multi/modules/def/main.bal | 5 ++ .../sources/cyclic_multi/modules/ghi/main.bal | 7 ++ .../sources/cyclic_multi/modules/jkl/main.bal | 5 ++ .../sources/cyclic_multi/modules/mno/main.bal | 5 ++ .../sources/cyclic_multi/modules/pqr/main.bal | 3 + .../sources/cyclic_simple/Ballerina.toml | 4 + .../cyclic_simple/modules/mod1/mod1.bal | 9 ++ .../cyclic_simple/modules/mod2/mod2.bal | 9 ++ 11 files changed, 148 insertions(+) create mode 100644 language-server/modules/langserver-core/src/test/java/org/ballerinalang/langserver/diagnostics/CyclicDependenciesTest.java create mode 100644 language-server/modules/langserver-core/src/test/resources/diagnostics/sources/cyclic_multi/Ballerina.toml create mode 100644 language-server/modules/langserver-core/src/test/resources/diagnostics/sources/cyclic_multi/modules/abc/main.bal create mode 100644 language-server/modules/langserver-core/src/test/resources/diagnostics/sources/cyclic_multi/modules/def/main.bal create mode 100644 language-server/modules/langserver-core/src/test/resources/diagnostics/sources/cyclic_multi/modules/ghi/main.bal create mode 100644 language-server/modules/langserver-core/src/test/resources/diagnostics/sources/cyclic_multi/modules/jkl/main.bal create mode 100644 language-server/modules/langserver-core/src/test/resources/diagnostics/sources/cyclic_multi/modules/mno/main.bal create mode 100644 language-server/modules/langserver-core/src/test/resources/diagnostics/sources/cyclic_multi/modules/pqr/main.bal create mode 100644 language-server/modules/langserver-core/src/test/resources/diagnostics/sources/cyclic_simple/Ballerina.toml create mode 100644 language-server/modules/langserver-core/src/test/resources/diagnostics/sources/cyclic_simple/modules/mod1/mod1.bal create mode 100644 language-server/modules/langserver-core/src/test/resources/diagnostics/sources/cyclic_simple/modules/mod2/mod2.bal diff --git a/language-server/modules/langserver-core/src/test/java/org/ballerinalang/langserver/diagnostics/CyclicDependenciesTest.java b/language-server/modules/langserver-core/src/test/java/org/ballerinalang/langserver/diagnostics/CyclicDependenciesTest.java new file mode 100644 index 000000000000..49b0989fb3fd --- /dev/null +++ b/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 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'")} + }; + } +} diff --git a/language-server/modules/langserver-core/src/test/resources/diagnostics/sources/cyclic_multi/Ballerina.toml b/language-server/modules/langserver-core/src/test/resources/diagnostics/sources/cyclic_multi/Ballerina.toml new file mode 100644 index 000000000000..5492423f404b --- /dev/null +++ b/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" diff --git a/language-server/modules/langserver-core/src/test/resources/diagnostics/sources/cyclic_multi/modules/abc/main.bal b/language-server/modules/langserver-core/src/test/resources/diagnostics/sources/cyclic_multi/modules/abc/main.bal new file mode 100644 index 000000000000..353515ad7a53 --- /dev/null +++ b/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() { + +} diff --git a/language-server/modules/langserver-core/src/test/resources/diagnostics/sources/cyclic_multi/modules/def/main.bal b/language-server/modules/langserver-core/src/test/resources/diagnostics/sources/cyclic_multi/modules/def/main.bal new file mode 100644 index 000000000000..865afc0e0e1d --- /dev/null +++ b/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() { + +} diff --git a/language-server/modules/langserver-core/src/test/resources/diagnostics/sources/cyclic_multi/modules/ghi/main.bal b/language-server/modules/langserver-core/src/test/resources/diagnostics/sources/cyclic_multi/modules/ghi/main.bal new file mode 100644 index 000000000000..7bd3656cecbd --- /dev/null +++ b/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() { + +} diff --git a/language-server/modules/langserver-core/src/test/resources/diagnostics/sources/cyclic_multi/modules/jkl/main.bal b/language-server/modules/langserver-core/src/test/resources/diagnostics/sources/cyclic_multi/modules/jkl/main.bal new file mode 100644 index 000000000000..34409d909012 --- /dev/null +++ b/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() { + +} diff --git a/language-server/modules/langserver-core/src/test/resources/diagnostics/sources/cyclic_multi/modules/mno/main.bal b/language-server/modules/langserver-core/src/test/resources/diagnostics/sources/cyclic_multi/modules/mno/main.bal new file mode 100644 index 000000000000..56ae9aa1ec5c --- /dev/null +++ b/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() { + +} diff --git a/language-server/modules/langserver-core/src/test/resources/diagnostics/sources/cyclic_multi/modules/pqr/main.bal b/language-server/modules/langserver-core/src/test/resources/diagnostics/sources/cyclic_multi/modules/pqr/main.bal new file mode 100644 index 000000000000..4afe2d08b877 --- /dev/null +++ b/language-server/modules/langserver-core/src/test/resources/diagnostics/sources/cyclic_multi/modules/pqr/main.bal @@ -0,0 +1,3 @@ +function test() { + +} diff --git a/language-server/modules/langserver-core/src/test/resources/diagnostics/sources/cyclic_simple/Ballerina.toml b/language-server/modules/langserver-core/src/test/resources/diagnostics/sources/cyclic_simple/Ballerina.toml new file mode 100644 index 000000000000..65c4c9d9b492 --- /dev/null +++ b/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" diff --git a/language-server/modules/langserver-core/src/test/resources/diagnostics/sources/cyclic_simple/modules/mod1/mod1.bal b/language-server/modules/langserver-core/src/test/resources/diagnostics/sources/cyclic_simple/modules/mod1/mod1.bal new file mode 100644 index 000000000000..c676f715ed42 --- /dev/null +++ b/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!"; +} diff --git a/language-server/modules/langserver-core/src/test/resources/diagnostics/sources/cyclic_simple/modules/mod2/mod2.bal b/language-server/modules/langserver-core/src/test/resources/diagnostics/sources/cyclic_simple/modules/mod2/mod2.bal new file mode 100644 index 000000000000..91c14a82d8ed --- /dev/null +++ b/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!"; +}