Skip to content

Commit

Permalink
Merge pull request #43735 from NipunaRanasinghe/fast-run-improvements…
Browse files Browse the repository at this point in the history
…-master

Fix LS fast-run to show compiler diagnostics and fail-fast on compilation errors
  • Loading branch information
NipunaRanasinghe authored Jan 9, 2025
2 parents 6dffd69 + cbab642 commit 35de750
Show file tree
Hide file tree
Showing 9 changed files with 158 additions and 68 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
/*
* Copyright (c) 2025, WSO2 LLC. (http://www.wso2.org).
*
* 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.commons.workspace;

import io.ballerina.tools.diagnostics.Diagnostic;

import java.util.Collection;

/**
* Represents the result of {@link WorkspaceManager#run(RunContext)} operation.
*
* @param process {@link Process} instance representing the run operation
* @param diagnostics diagnostics generated during the compilation
* @since 2201.12.0
*/
public record RunResult(Process process, Collection<Diagnostic> diagnostics) {

}
Original file line number Diff line number Diff line change
Expand Up @@ -219,7 +219,7 @@ public interface WorkspaceManager {
* @throws WorkspaceDocumentException when project or document not found
*/
void didChangeWatched(Path filePath, FileEvent fileEvent) throws WorkspaceDocumentException;

/**
* The file change notification is sent from the client to the server to signal changes to watched files.
*
Expand All @@ -245,7 +245,7 @@ public interface WorkspaceManager {
* @throws IOException If failed to start the process.
* @since 2201.6.0
*/
Optional<Process> run(RunContext runContext) throws IOException;
RunResult run(RunContext runContext) throws IOException;

/**
* Stop a running process started with {@link #run}.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,13 +19,17 @@
import com.google.gson.JsonElement;
import com.google.gson.JsonObject;
import com.google.gson.JsonPrimitive;
import io.ballerina.tools.diagnostics.Diagnostic;
import io.ballerina.tools.diagnostics.DiagnosticSeverity;
import org.ballerinalang.annotation.JavaSPIService;
import org.ballerinalang.compiler.BLangCompilerException;
import org.ballerinalang.langserver.commons.ExecuteCommandContext;
import org.ballerinalang.langserver.commons.client.ExtendedLanguageClient;
import org.ballerinalang.langserver.commons.command.CommandArgument;
import org.ballerinalang.langserver.commons.command.LSCommandExecutorException;
import org.ballerinalang.langserver.commons.command.spi.LSCommandExecutor;
import org.ballerinalang.langserver.commons.workspace.RunContext;
import org.ballerinalang.langserver.commons.workspace.RunResult;
import org.eclipse.lsp4j.LogTraceParams;

import java.io.IOException;
Expand All @@ -34,10 +38,12 @@
import java.nio.file.Files;
import java.nio.file.InvalidPathException;
import java.nio.file.Path;
import java.util.Collection;
import java.util.Collections;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
import java.util.Objects;
import java.util.Optional;
import java.util.function.Supplier;
import java.util.stream.StreamSupport;
Expand Down Expand Up @@ -67,25 +73,40 @@ public class RunExecutor implements LSCommandExecutor {
public Boolean execute(ExecuteCommandContext context) throws LSCommandExecutorException {
try {
RunContext workspaceRunContext = getWorkspaceRunContext(context);
Optional<Process> processOpt = context.workspace().run(workspaceRunContext);
if (processOpt.isEmpty()) {
RunResult runResult = context.workspace().run(workspaceRunContext);

Collection<Diagnostic> diagnostics = runResult.diagnostics();
for (Diagnostic diagnostic : diagnostics) {
LogTraceParams diagnosticMessage = new LogTraceParams(diagnostic.toString(), ERROR_CHANNEL);
context.getLanguageClient().logTrace(diagnosticMessage);
}
if (diagnostics.stream().anyMatch(d -> d.diagnosticInfo().severity() == DiagnosticSeverity.ERROR)) {
LogTraceParams error = new LogTraceParams("error: compilation contains errors", ERROR_CHANNEL);
context.getLanguageClient().logTrace(error);
return false;
}
Process process = processOpt.get();

Process process = runResult.process();
if (Objects.isNull(process)) {
return false;
}

listenOutputAsync(context.getLanguageClient(), process::getInputStream, OUT_CHANNEL);
listenOutputAsync(context.getLanguageClient(), process::getErrorStream, ERROR_CHANNEL);
return true;
} catch (BLangCompilerException e) {
LogTraceParams error = new LogTraceParams(e.getMessage(), ERROR_CHANNEL);
context.getLanguageClient().logTrace(error);
} catch (IOException e) {
LogTraceParams error = new LogTraceParams("Error while running the program in fast-run mode: " +
e.getMessage(), ERROR_CHANNEL);
context.getLanguageClient().logTrace(error);
throw new LSCommandExecutorException(e);
} catch (Exception e) {
LogTraceParams error = new LogTraceParams("Unexpected error while executing the fast-run: " +
e.getMessage(), ERROR_CHANNEL);
context.getLanguageClient().logTrace(error);
throw new LSCommandExecutorException(e);
}
return false;
}

private RunContext getWorkspaceRunContext(ExecuteCommandContext context) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,7 @@
import org.ballerinalang.langserver.commons.eventsync.EventKind;
import org.ballerinalang.langserver.commons.eventsync.exceptions.EventSyncException;
import org.ballerinalang.langserver.commons.workspace.RunContext;
import org.ballerinalang.langserver.commons.workspace.RunResult;
import org.ballerinalang.langserver.commons.workspace.WorkspaceDocumentException;
import org.ballerinalang.langserver.commons.workspace.WorkspaceDocumentManager;
import org.ballerinalang.langserver.commons.workspace.WorkspaceManager;
Expand Down Expand Up @@ -92,6 +93,7 @@
import java.util.Collections;
import java.util.HashMap;
import java.util.HashSet;
import java.util.LinkedList;
import java.util.List;
import java.util.Locale;
import java.util.Map;
Expand Down Expand Up @@ -595,18 +597,39 @@ public String uriScheme() {
}

@Override
public Optional<Process> run(RunContext executionContext) throws IOException {
public RunResult run(RunContext executionContext) throws IOException {
Path projectRoot = projectRoot(executionContext.balSourcePath());
Optional<ProjectContext> projectContext = validateProjectContext(projectRoot);
if (projectContext.isEmpty()) {
return Optional.empty();
return new RunResult(null, Collections.emptyList());
}

if (!prepareProjectForExecution(projectContext.get())) {
return Optional.empty();
if (!stopProject(projectContext.get())) {
logError("Run command execution aborted because couldn't stop the previous run");
return new RunResult(null, Collections.emptyList());
}

Project project = projectContext.get().project();
Optional<PackageCompilation> packageCompilation = waitAndGetPackageCompilation(project.sourceRoot(), true);
if (packageCompilation.isEmpty()) {
logError("Run command execution aborted because package compilation failed");
return new RunResult(null, Collections.emptyList());
}

return executeProject(projectContext.get(), executionContext);
JBallerinaBackend jBallerinaBackend = execBackend(projectContext.get(), packageCompilation.get());
Collection<Diagnostic> diagnostics = new LinkedList<>();
// check for compilation errors
diagnostics.addAll(jBallerinaBackend.diagnosticResult().diagnostics(false));
// Add tool resolution diagnostics to diagnostics
diagnostics.addAll(project.currentPackage().getBuildToolResolution().getDiagnosticList());

if (diagnostics.stream().anyMatch(d -> d.diagnosticInfo().severity() == DiagnosticSeverity.ERROR)) {
return new RunResult(null, diagnostics);
}

Optional<Process> process = executeProject(projectContext.get(), executionContext);
return process.map(value -> new RunResult(value, diagnostics))
.orElseGet(() -> new RunResult(null, diagnostics));
}

private Optional<ProjectContext> validateProjectContext(Path projectRoot) {
Expand All @@ -619,31 +642,6 @@ private Optional<ProjectContext> validateProjectContext(Path projectRoot) {
return projectContextOpt;
}

private boolean prepareProjectForExecution(ProjectContext projectContext) {
// stop previous project run
if (!stopProject(projectContext)) {
logError("Run command execution aborted because couldn't stop the previous run");
return false;
}

Project project = projectContext.project();
Optional<PackageCompilation> packageCompilation = waitAndGetPackageCompilation(project.sourceRoot(), true);
if (packageCompilation.isEmpty()) {
logError("Run command execution aborted because package compilation failed");
return false;
}

// check for compilation errors
JBallerinaBackend jBallerinaBackend = execBackend(projectContext, packageCompilation.get());
Collection<Diagnostic> diagnostics = jBallerinaBackend.diagnosticResult().diagnostics(false);
if (diagnostics.stream().anyMatch(BallerinaWorkspaceManager::isError)) {
logError("Run command execution aborted due to compilation errors: " + diagnostics);
return false;
}

return true;
}

private Optional<Process> executeProject(ProjectContext projectContext, RunContext context) throws IOException {
Project project = projectContext.project();
Package pkg = project.currentPackage();
Expand Down Expand Up @@ -718,7 +716,7 @@ private void logError(String message) {

@Override
public boolean stop(Path filePath) {
Optional<ProjectContext> projectPairOpt = projectContext(projectRoot(filePath));
Optional<ProjectContext> projectPairOpt = projectContext(projectRoot(filePath).toAbsolutePath());
if (projectPairOpt.isEmpty()) {
clientLogger.logWarning("Failed to stop process: Project not found");
return false;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,7 @@
* @since 2.0.0
*/
public class TestWorkspaceManager {

private static final Path RESOURCE_DIRECTORY = Path.of("src/test/resources/project");
private final String dummyContent = "function foo() {" + CommonUtil.LINE_SEPARATOR + "}";
private final String dummyDidChangeContent = "function foo1() {" + CommonUtil.LINE_SEPARATOR + "}";
Expand Down Expand Up @@ -558,16 +559,32 @@ public void testWSRunStopProject()
throws WorkspaceDocumentException, EventSyncException, LSCommandExecutorException {
Path projectPath = RESOURCE_DIRECTORY.resolve("long_running");
Path filePath = projectPath.resolve("main.bal");
ExecuteCommandContext execContext = runViaLs(filePath);
stopViaLs(execContext, projectPath);
RunResult runResult = executeRunCommand(filePath);
Assert.assertTrue(runResult.success());
Assert.assertEquals(runResult.programOutput[0].trim(), "Hello, World!");
executeStopCommand(projectPath);
}

@Test
public void testWSRunProjectWithCompilationErrors()
throws WorkspaceDocumentException, EventSyncException, LSCommandExecutorException {
Path projectPath = RESOURCE_DIRECTORY.resolve("pkg_with_compilation_errors");
Path filePath = projectPath.resolve("main.bal");
RunResult runResult = executeRunCommand(filePath);
Assert.assertFalse(runResult.success());
Assert.assertTrue(runResult.errorOutput().length > 0);
Assert.assertEquals(runResult.errorOutput()[0], "ERROR [main.bal:(5:1,5:1)] missing semicolon token");
Assert.assertEquals(runResult.errorOutput()[1], "error: compilation contains errors");
executeStopCommand(projectPath);
}

@Test
public void testSemanticApiAfterWSRun()
throws WorkspaceDocumentException, EventSyncException, LSCommandExecutorException {
Path projectPath = RESOURCE_DIRECTORY.resolve("hello_service");
Path filePath = projectPath.resolve("main.bal");
ExecuteCommandContext execContext = runViaLs(filePath);
RunResult runResult = executeRunCommand(filePath);
Assert.assertTrue(runResult.success());

// Test syntax tree api
JsonElement syntaxTreeJSON = DiagramUtil.getSyntaxTreeJSON(workspaceManager.document(filePath).orElseThrow(),
Expand All @@ -581,7 +598,7 @@ public void testSemanticApiAfterWSRun()
Assert.assertEquals(execPositions.getAsJsonArray().get(0).getAsJsonObject().get("name").getAsString(),
"hello");

stopViaLs(execContext, projectPath);
executeStopCommand(projectPath);
}

@Test
Expand All @@ -602,7 +619,8 @@ public void testSemanticApiAfterWSRunMultiMod()
workspaceManager.document(filePath).orElseThrow(),
semanticModelPreExec);

ExecuteCommandContext execContext = runViaLs(filePath);
RunResult runResult = executeRunCommand(filePath);
Assert.assertTrue(runResult.success());

SemanticModel semanticModelPostExec = workspaceManager.semanticModel(filePath).orElseThrow();
JsonElement syntaxTreeJSONPostExec = DiagramUtil.getSyntaxTreeJSON(
Expand All @@ -612,48 +630,53 @@ public void testSemanticApiAfterWSRunMultiMod()
Gson gson = new GsonBuilder().setPrettyPrinting().create();
Assert.assertEquals(gson.toJson(syntaxTreeJSONPreExec), gson.toJson(syntaxTreeJSONPostExec));

stopViaLs(execContext, projectPath);
executeStopCommand(projectPath);
}


private ExecuteCommandContext runViaLs(Path filePath)
private RunResult executeRunCommand(Path filePath)
throws WorkspaceDocumentException, EventSyncException, LSCommandExecutorException {
System.setProperty("java.command", guessJavaPath());
System.setProperty(BALLERINA_HOME, "./build");
workspaceManager.loadProject(filePath);
RunExecutor runExecutor = new RunExecutor();
MockSettings mockSettings = Mockito.withSettings().stubOnly();
ExecuteCommandContext execContext = Mockito.mock(ExecuteCommandContext.class, mockSettings);
CommandArgument arg = CommandArgument.from("path", new JsonPrimitive(filePath.toString()));
Mockito.when(execContext.getArguments()).thenReturn(Collections.singletonList(arg));
Mockito.when(execContext.workspace()).thenReturn(workspaceManager);

ExecuteCommandContext execContext = createExecutionContextMock(filePath);
ExtendedLanguageClient languageClient = Mockito.mock(ExtendedLanguageClient.class, mockSettings);
ArgumentCaptor<LogTraceParams> logCaptor = ArgumentCaptor.forClass(LogTraceParams.class);
Mockito.doNothing().when(languageClient).logTrace(logCaptor.capture());
Mockito.when(execContext.getLanguageClient()).thenReturn(languageClient);
Boolean didRan = runExecutor.execute(execContext);
Assert.assertTrue(didRan);
Assert.assertEquals(reduceToOutString(logCaptor), "Hello, World!" + System.lineSeparator());
return execContext;

return new RunResult(didRan, extractLogs(logCaptor, "out"), extractLogs(logCaptor, "err"));
}

private static void stopViaLs(ExecuteCommandContext execContext, Path projectPath) {
private void executeStopCommand(Path projectPath) {
StopExecutor stopExecutor = new StopExecutor();
ExecuteCommandContext execContext = createExecutionContextMock(projectPath);
Boolean didStop = stopExecutor.execute(execContext);
Assert.assertTrue(didStop);

Path target = projectPath.resolve("target");
FileUtils.deleteQuietly(target.toFile());
}

private static String reduceToOutString(ArgumentCaptor<LogTraceParams> logCaptor) {
private ExecuteCommandContext createExecutionContextMock(Path filePath) {
MockSettings mockSettings = Mockito.withSettings().stubOnly();
ExecuteCommandContext execContext = Mockito.mock(ExecuteCommandContext.class, mockSettings);

CommandArgument arg = CommandArgument.from("path", new JsonPrimitive(filePath.toString()));
Mockito.when(execContext.getArguments()).thenReturn(Collections.singletonList(arg));
Mockito.when(execContext.workspace()).thenReturn(workspaceManager);
return execContext;
}

private static String[] extractLogs(ArgumentCaptor<LogTraceParams> logCaptor, String channel) {
List<LogTraceParams> params = waitGetAllValues(logCaptor);
StringBuilder sb = new StringBuilder();
for (LogTraceParams param : params) {
sb.append(param.getMessage());
Assert.assertEquals(param.getVerbose(), "out"); // not "err"
}
return sb.toString();
return params.stream()
.filter(param -> param.getVerbose().equals(channel))
.map(LogTraceParams::getMessage)
.toArray(String[]::new);
}

private static List<LogTraceParams> waitGetAllValues(ArgumentCaptor<LogTraceParams> logCaptor) {
Expand Down Expand Up @@ -742,4 +765,8 @@ public Object[] workspaceEventsTestDataProvider() {
RESOURCE_DIRECTORY.resolve("myproject").resolve("main.bal").toAbsolutePath()
};
}

private record RunResult(boolean success, String[] programOutput, String[] errorOutput) {

}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
[package]
org = "baltest"
name = "pkg_with_compilation_errors"
version = "0.1.0"
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
import ballerina/lang.runtime;

public function main() {
runtime:sleep(20)
}
Loading

0 comments on commit 35de750

Please sign in to comment.