Skip to content

Commit

Permalink
Merge pull request #2 from rasika/ls-prj-api-migration
Browse files Browse the repository at this point in the history
Fix diagnostic issues
  • Loading branch information
nadeeshaan authored Nov 13, 2020
2 parents 5da315c + 5768f96 commit 827a6d9
Show file tree
Hide file tree
Showing 7 changed files with 153 additions and 132 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ public interface WorkspaceManager {
/**
* Returns module from the path provided.
*
* @param filePath ballerina project or standalone file path
* @param filePath file path of the document
* @return project of applicable type
*/
Optional<Module> module(Path filePath);
Expand All @@ -71,32 +71,43 @@ public interface WorkspaceManager {
/**
* Returns syntax tree from the path provided.
*
* @param filePath ballerina project or standalone file path
* @param filePath file path of the document
* @return {@link io.ballerina.compiler.syntax.tree.SyntaxTree}
*/
Optional<SyntaxTree> syntaxTree(Path filePath);

/**
* Returns semantic model from the path provided.
*
* @param filePath ballerina project or standalone file path
* @param filePath file path of the document
* @return project of applicable type
*/
Optional<SemanticModel> semanticModel(Path filePath);

/**
* The document open notification is sent from the client to the server to signal newly opened text documents.
*
* @param filePath {@link Path} of the document
* @param params {@link DidOpenTextDocumentParams}
*/
void didOpen(DidOpenTextDocumentParams params) throws WorkspaceDocumentException;
void didOpen(Path filePath, DidOpenTextDocumentParams params);

/**
* The document change notification is sent from the client to the server to signal changes to a text document.
*
* @param filePath {@link Path} of the document
* @param params {@link DidChangeTextDocumentParams}
* @throws WorkspaceDocumentException when project or document not found
*/
void didChange(DidChangeTextDocumentParams params) throws WorkspaceDocumentException;
void didChange(Path filePath, DidChangeTextDocumentParams params) throws WorkspaceDocumentException;

/**
* The document close notification is sent from the client to the server when the document got closed in the
* client.
*
* @param filePath {@link Path} of the document
* @param params {@link DidCloseTextDocumentParams}
* @throws WorkspaceDocumentException project not found
*/
void didClose(DidCloseTextDocumentParams params) throws WorkspaceDocumentException;
void didClose(Path filePath, DidCloseTextDocumentParams params) throws WorkspaceDocumentException;
}
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,6 @@
import org.ballerinalang.langserver.commons.NewLSContext;
import org.ballerinalang.langserver.commons.SignatureContext;
import org.ballerinalang.langserver.commons.capability.LSClientCapabilities;
import org.ballerinalang.langserver.commons.client.ExtendedLanguageClient;
import org.ballerinalang.langserver.commons.codeaction.CodeActionNodeType;
import org.ballerinalang.langserver.compiler.LSClientLogger;
import org.ballerinalang.langserver.compiler.config.ClientConfigListener;
Expand Down Expand Up @@ -380,7 +379,7 @@ public CompletableFuture<List<Either<Command, CodeAction>>> codeAction(CodeActio
params.getRange().getStart());
try {
// Compile and get Top level node
CodeActionNodeType nodeType = CodeActionUtil.topLevelNodeInLine(identifier, line, null);
CodeActionNodeType nodeType = CodeActionUtil.topLevelNodeInLine(identifier, line, context.workspace());
List<Diagnostic> rangeDiagnostics = params.getContext().getDiagnostics();

// Add code actions
Expand Down Expand Up @@ -516,36 +515,14 @@ public CompletableFuture<List<? extends TextEdit>> rangeFormatting(DocumentRange

@Override
public void didOpen(DidOpenTextDocumentParams params) {
String docUri = params.getTextDocument().getUri();
// TODO: check the untitled file path issue
String fileUri = params.getTextDocument().getUri();
try {
this.workspaceManager.didOpen(params);
ExtendedLanguageClient client = this.languageServer.getClient();
NewLSContext context = ContextBuilder.buildBaseContext(docUri,
this.workspaceManager,
LSContextOperation.TXT_DID_OPEN);
// String fileUri = context.get(DocumentServiceKeys.FILE_URI_KEY);

/*
In order to support definition within the standard libraries, we cache the standard library content
at this stage for the cached sources. We ignore this particular step at any other operation including
didChange.
*/
// if (CommonUtil.isCachedExternalSource(docUri)) {
// context.put(DocumentServiceKeys.IS_CACHE_SUPPORTED, true);
// context.put(DocumentServiceKeys.IS_CACHE_OUTDATED_SUPPORTED, true);
// LSModuleCompiler.getBLangPackages(context, docManager, false, true, true);
// // Populate the Standard Library Cache
// CommonUtil.updateStdLibCache(context);
// // Note: If the source is a cached stdlib source then return early and ignore sending diagnostics
// return;
// }

diagnosticsHelper.compileAndSendDiagnostics(client, context);
/*
For the non-cached sources we send the diagnostics and then update the standard lib cache
*/
// CommonUtil.updateStdLibCache(context);
NewLSContext context = ContextBuilder.buildBaseContext(fileUri, this.workspaceManager,
LSContextOperation.TXT_DID_OPEN);
this.workspaceManager.didOpen(context.filePath(), params);
LSClientLogger.logTrace("Operation '" + LSContextOperation.TXT_DID_OPEN.getName() +
"' {fileUri: '" + fileUri + "'} opened}");
diagnosticsHelper.compileAndSendDiagnostics(this.languageServer.getClient(), context);
} catch (Throwable e) {
String msg = "Operation 'text/didOpen' failed!";
TextDocumentIdentifier identifier = new TextDocumentIdentifier(params.getTextDocument().getUri());
Expand All @@ -556,22 +533,20 @@ public void didOpen(DidOpenTextDocumentParams params) {
@Override
public void didChange(DidChangeTextDocumentParams params) {
String fileUri = params.getTextDocument().getUri();
Optional<Path> changedPath = CommonUtil.getPathFromURI(fileUri);
// Note: If the source is a cached stdlib source or path does not exist, then return early and ignore
if (changedPath.isEmpty() || CommonUtil.isCachedExternalSource(fileUri)) {
return;
}
Path compilationPath = getUntitledFilePath(changedPath.toString()).orElse(changedPath.get());
try {
// Update content
workspaceManager.didChange(params);
LSClientLogger.logTrace("Operation '" + LSContextOperation.TXT_DID_CHANGE.getName() + "' {fileUri: '" +
compilationPath + "'} updated}");
ExtendedLanguageClient client = this.languageServer.getClient();
NewLSContext context = ContextBuilder.buildBaseContext(fileUri,
this.workspaceManager,
LSContextOperation.TXT_DID_CHANGE);
diagnosticsHelper.compileAndSendDiagnostics(client, context);
this.workspaceManager,
LSContextOperation.TXT_DID_CHANGE);
// Note: If the source is a cached stdlib source or path does not exist, then return early and ignore
if (CommonUtil.isCachedExternalSource(fileUri)) {
// TODO: Check whether still we need this check
return;
}
workspaceManager.didChange(context.filePath(), params);
LSClientLogger.logTrace("Operation '" + LSContextOperation.TXT_DID_CHANGE.getName() +
"' {fileUri: '" + fileUri + "'} updated}");
diagnosticsHelper.compileAndSendDiagnostics(this.languageServer.getClient(), context);
} catch (Throwable e) {
String msg = "Operation 'text/didChange' failed!";
logError(msg, e, params.getTextDocument(), (Position) null);
Expand All @@ -580,8 +555,14 @@ public void didChange(DidChangeTextDocumentParams params) {

@Override
public void didClose(DidCloseTextDocumentParams params) {
String fileUri = params.getTextDocument().getUri();
try {
workspaceManager.didClose(params);
NewLSContext context = ContextBuilder.buildBaseContext(fileUri,
this.workspaceManager,
LSContextOperation.TXT_DID_CLOSE);
workspaceManager.didClose(context.filePath(), params);
LSClientLogger.logTrace("Operation '" + LSContextOperation.TXT_DID_CLOSE.getName() +
"' {fileUri: '" + fileUri + "'} closed}");
} catch (Throwable e) {
String msg = "Operation 'text/didClose' failed!";
logError(msg, e, params.getTextDocument(), (Position) null);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@
public enum LSContextOperation implements LSOperation {
TXT_COMPLETION("text/completion"),
TXT_DID_CHANGE("text/didChange"),
TXT_DID_CLOSE("text/didClose"),
DIAGNOSTICS("debouncer/diagnostics"),
TXT_DID_OPEN("text/didOpen"),
TXT_HOVER("text/hover"),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,14 +23,14 @@
import io.ballerina.compiler.syntax.tree.ServiceBodyNode;
import io.ballerina.compiler.syntax.tree.ServiceDeclarationNode;
import io.ballerina.compiler.syntax.tree.SyntaxKind;
import io.ballerina.compiler.syntax.tree.SyntaxTree;
import io.ballerina.compiler.syntax.tree.TypeDefinitionNode;
import io.ballerina.tools.diagnostics.Diagnostic;
import io.ballerina.tools.diagnostics.Location;
import io.ballerina.tools.text.LineRange;
import org.ballerinalang.langserver.common.utils.CommonUtil;
import org.ballerinalang.langserver.commons.codeaction.CodeActionNodeType;
import org.ballerinalang.langserver.commons.workspace.WorkspaceDocumentException;
import org.ballerinalang.langserver.commons.workspace.WorkspaceDocumentManager;
import org.ballerinalang.langserver.commons.workspace.WorkspaceManager;
import org.eclipse.lsp4j.DiagnosticSeverity;
import org.eclipse.lsp4j.Position;
import org.eclipse.lsp4j.Range;
Expand All @@ -55,25 +55,24 @@ private CodeActionUtil() {
/**
* Get the top level node type at the cursor line.
*
* @param identifier Document Identifier
* @param cursorLine Cursor line
* @param docManager Workspace document manager
* @param identifier Document Identifier
* @param cursorLine Cursor line
* @param workspaceManager Workspace manager
* @return {@link String} Top level node type
*/
public static CodeActionNodeType topLevelNodeInLine(TextDocumentIdentifier identifier, int cursorLine,
WorkspaceDocumentManager docManager) {
WorkspaceManager workspaceManager) {
Optional<Path> filePath = CommonUtil.getPathFromURI(identifier.getUri());
if (filePath.isEmpty()) {
return null;
}

ModulePartNode modulePartNode;
try {
modulePartNode = docManager.getTree(filePath.get()).rootNode();
} catch (WorkspaceDocumentException e) {
Optional<SyntaxTree> syntaxTree = workspaceManager.syntaxTree(filePath.get());
if (syntaxTree.isEmpty()) {
return null;
}

ModulePartNode modulePartNode = syntaxTree.get().rootNode();
List<ModuleMemberDeclarationNode> members = modulePartNode.members().stream().collect(Collectors.toList());
for (ModuleMemberDeclarationNode member : members) {
boolean isSameLine = member.lineRange().startLine().line() == cursorLine;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@
import io.ballerina.projects.Module;
import io.ballerina.projects.Project;
import io.ballerina.projects.ProjectKind;
import io.ballerina.tools.diagnostics.Location;
import io.ballerina.tools.text.LineRange;
import org.ballerinalang.langserver.commons.NewLSContext;
import org.ballerinalang.langserver.commons.client.ExtendedLanguageClient;
Expand All @@ -30,6 +29,7 @@

import java.nio.file.Path;
import java.util.ArrayList;
import java.util.Collection;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
Expand Down Expand Up @@ -77,7 +77,9 @@ public synchronized void compileAndSendDiagnostics(ExtendedLanguageClient client
} else {
sourceRoot = projectRoot.resolve(module.moduleName().moduleNamePart());
}
populateDiagnostics(diagnosticMap, module, sourceRoot);
String moduleName = module.moduleName().moduleNamePart();
Path modulePath = (moduleName != null) ? sourceRoot.resolve(moduleName) : sourceRoot;
diagnosticMap.putAll(getDiagnostics(module.getCompilation().diagnostics().diagnostics(), modulePath));
}

// If the client is null, returns
Expand All @@ -95,16 +97,12 @@ public synchronized void compileAndSendDiagnostics(ExtendedLanguageClient client
lastDiagnosticMap = diagnosticMap;
}

private void populateDiagnostics(Map<String, List<Diagnostic>> diagnosticsMap, Module module, Path sourceRoot) {
for (io.ballerina.tools.diagnostics.Diagnostic diag : module.getCompilation().diagnostics().diagnostics()) {
private Map<String, List<Diagnostic>> getDiagnostics(Collection<io.ballerina.tools.diagnostics.Diagnostic> diags,
Path modulePath) {
Map<String, List<Diagnostic>> diagnosticsMap = new HashMap<>();
for (io.ballerina.tools.diagnostics.Diagnostic diag : diags) {
LineRange lineRange = diag.location().lineRange();

Location location = diag.location();
String filePath = location.lineRange().filePath();

String fileURI = sourceRoot.resolve(filePath).toUri().toString();
diagnosticsMap.putIfAbsent(fileURI, new ArrayList<>());

LineRange lineRange = location.lineRange();
int startLine = lineRange.startLine().line();
int startChar = lineRange.startLine().offset();
int endLine = lineRange.endLine().line();
Expand All @@ -116,16 +114,25 @@ private void populateDiagnostics(Map<String, List<Diagnostic>> diagnosticsMap, M
Range range = new Range(new Position(startLine, startChar), new Position(endLine, endChar));
Diagnostic diagnostic = new Diagnostic(range, diag.message());

io.ballerina.tools.diagnostics.DiagnosticSeverity severity = diag.diagnosticInfo().severity();
if (severity == io.ballerina.tools.diagnostics.DiagnosticSeverity.ERROR) {
// set diagnostic log kind
diagnostic.setSeverity(DiagnosticSeverity.Error);
} else if (severity == io.ballerina.tools.diagnostics.DiagnosticSeverity.WARNING) {
diagnostic.setSeverity(DiagnosticSeverity.Warning);
switch (diag.diagnosticInfo().severity()) {
case ERROR:
diagnostic.setSeverity(DiagnosticSeverity.Error);
break;
case WARNING:
diagnostic.setSeverity(DiagnosticSeverity.Warning);
break;
case HINT:
diagnostic.setSeverity(DiagnosticSeverity.Hint);
break;
case INFO:
diagnostic.setSeverity(DiagnosticSeverity.Information);
break;
}

List<Diagnostic> clientDiagnostics = diagnosticsMap.get(fileURI);
String fileURI = modulePath.resolve(lineRange.filePath()).toUri().toString();
List<Diagnostic> clientDiagnostics = diagnosticsMap.computeIfAbsent(fileURI, s -> new ArrayList<>());
clientDiagnostics.add(diagnostic);
}
return diagnosticsMap;
}
}
Loading

0 comments on commit 827a6d9

Please sign in to comment.