Skip to content

Commit

Permalink
[regression] Maven diagnostics show up and disappear on save
Browse files Browse the repository at this point in the history 
Signed-off-by: Snjezana Peco <[email protected]>
  • Loading branch information
@snjeza
snjeza committed Oct 15, 2018
1 parent 746e337 commit 8376285
Show file tree
Hide file tree
Showing 6 changed files with 88 additions and 96 deletions.
16 changes: 12 additions & 4 deletions ...ipse.jdt.ls.core/src/org/eclipse/jdt/ls/core/internal/handlers/BuildWorkspaceHandler.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
import static org.eclipse.jdt.ls.core.internal.JavaLanguageServerPlugin.logError;
import static org.eclipse.jdt.ls.core.internal.JavaLanguageServerPlugin.logException;

import java.util.ArrayList;
import java.util.Collections;
import java.util.List;
import java.util.stream.Collectors;
Expand Down Expand Up @@ -40,12 +41,10 @@
public class BuildWorkspaceHandler {
private JavaClientConnection connection;
private final ProjectsManager projectsManager;
private final WorkspaceDiagnosticsHandler workspaceDiagnosticsHandler;

public BuildWorkspaceHandler(JavaClientConnection connection, ProjectsManager projectsManager, WorkspaceDiagnosticsHandler workspaceDiagnosticHandler) {
public BuildWorkspaceHandler(JavaClientConnection connection, ProjectsManager projectsManager) {
this.connection = connection;
this.projectsManager = projectsManager;
this.workspaceDiagnosticsHandler = workspaceDiagnosticHandler;
}

public BuildWorkspaceStatus buildWorkspace(boolean forceReBuild, IProgressMonitor monitor) {
Expand All @@ -61,8 +60,17 @@ public BuildWorkspaceStatus buildWorkspace(boolean forceReBuild, IProgressMonito
connection.publishDiagnostics(new PublishDiagnosticsParams(ResourceUtils.toClientUri(uri), Collections.emptyList()));
}
}
ResourcesPlugin.getWorkspace().build(forceReBuild ? IncrementalProjectBuilder.CLEAN_BUILD : IncrementalProjectBuilder.INCREMENTAL_BUILD, monitor);
ResourcesPlugin.getWorkspace().build(forceReBuild ? IncrementalProjectBuilder.FULL_BUILD : IncrementalProjectBuilder.INCREMENTAL_BUILD, monitor);
List<IMarker> problemMarkers = workspaceDiagnosticsHandler.publishDiagnostics(monitor);
List<IMarker> problemMarkers = new ArrayList<>();
for (IProject project : projects) {
if (!project.equals(projectsManager.getDefaultProject())) {
List<IMarker> markers = ResourceUtils.getErrorMarkers(project);
if (markers != null) {
problemMarkers.addAll(markers);
}
}
}
List<String> errors = problemMarkers.stream().filter(m -> m.getAttribute(IMarker.SEVERITY, 0) == IMarker.SEVERITY_ERROR).map(e -> convertMarker(e)).collect(Collectors.toList());
if (errors.isEmpty()) {
return BuildWorkspaceStatus.SUCCEED;
Expand Down
11 changes: 1 addition & 10 deletions org.eclipse.jdt.ls.core/src/org/eclipse/jdt/ls/core/internal/handlers/JDTLanguageServer.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -215,15 +215,6 @@ public void initialized(InitializedParams params) {

workspaceDiagnosticsHandler = new WorkspaceDiagnosticsHandler(this.client, pm);
workspaceDiagnosticsHandler.addResourceChangeListener();

computeAsync((monitor) -> {
try {
workspaceDiagnosticsHandler.publishDiagnostics(monitor);
} catch (CoreException e) {
logException(e.getMessage(), e);
}
return new Object();
});
}

/**
Expand Down Expand Up @@ -710,7 +701,7 @@ public void projectConfigurationUpdate(TextDocumentIdentifier param) {
@Override
public CompletableFuture<BuildWorkspaceStatus> buildWorkspace(boolean forceReBuild) {
logInfo(">> java/buildWorkspace (" + (forceReBuild ? "full)" : "incremental)"));
BuildWorkspaceHandler handler = new BuildWorkspaceHandler(client, pm, workspaceDiagnosticsHandler);
BuildWorkspaceHandler handler = new BuildWorkspaceHandler(client, pm);
return computeAsync((monitor) -> handler.buildWorkspace(forceReBuild, monitor));
}

Expand Down
72 changes: 2 additions & 70 deletions ...dt.ls.core/src/org/eclipse/jdt/ls/core/internal/handlers/WorkspaceDiagnosticsHandler.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@
import java.util.Arrays;
import java.util.Collection;
import java.util.List;
import java.util.Map;
import java.util.stream.Collectors;
import java.util.stream.Stream;

Expand All @@ -29,12 +28,9 @@
import org.eclipse.core.resources.IResourceDeltaVisitor;
import org.eclipse.core.resources.ResourcesPlugin;
import org.eclipse.core.runtime.CoreException;
import org.eclipse.core.runtime.IProgressMonitor;
import org.eclipse.core.runtime.OperationCanceledException;
import org.eclipse.jdt.core.ICompilationUnit;
import org.eclipse.jdt.core.IJavaModelMarker;
import org.eclipse.jdt.core.JavaCore;
import org.eclipse.jdt.core.JavaModelException;
import org.eclipse.jdt.ls.core.internal.JDTUtils;
import org.eclipse.jdt.ls.core.internal.JavaClientConnection;
import org.eclipse.jdt.ls.core.internal.JavaLanguageServerPlugin;
Expand Down Expand Up @@ -121,10 +117,6 @@ public boolean visit(IResourceDelta delta) throws CoreException {
String uri = JDTUtils.getFileURI(project);
IFile pom = project.getFile("pom.xml");
List<IMarker> pomMarkers = new ArrayList<>();
if (pom.exists()) {
pomMarkers.addAll(Arrays.asList(pom.findMarkers(null, true, 1)));
}

for (IMarker marker : markers) {
if (!marker.exists() || CheckMissingNaturesListener.MARKER_TYPE.equals(marker.getType())) {
continue;
Expand All @@ -135,11 +127,11 @@ public boolean visit(IResourceDelta delta) throws CoreException {
projectMarkers.add(marker);
}
}

List<Diagnostic> diagnostics = toDiagnosticArray(range, projectMarkers);
String clientUri = ResourceUtils.toClientUri(uri);
connection.publishDiagnostics(new PublishDiagnosticsParams(clientUri, diagnostics));
if (pom.exists()) {
pomMarkers.addAll(Arrays.asList(pom.findMarkers(null, true, 1)));
diagnostics = toDiagnosticArray(range, pomMarkers);
connection.publishDiagnostics(new PublishDiagnosticsParams(ResourceUtils.toClientUri(clientUri + "/pom.xml"), diagnostics));
}
Expand All @@ -150,7 +142,6 @@ public boolean visit(IResourceDelta delta) throws CoreException {
return false;
}
IFile file = (IFile) resource;
String uri = JDTUtils.getFileURI(resource);
IDocument document = null;
IMarker[] markers = null;
// Check if it is a Java ...
Expand All @@ -168,71 +159,12 @@ else if (projectsManager.isBuildFile(file)) {
document = JsonRpcHelpers.toDocument(file);
}
if (document != null) {
String uri = JDTUtils.getFileURI(resource);
this.connection.publishDiagnostics(new PublishDiagnosticsParams(ResourceUtils.toClientUri(uri), toDiagnosticsArray(document, markers)));
}
return false;
}

public List<IMarker> publishDiagnostics(IProgressMonitor monitor) throws CoreException {
List<IMarker> problemMarkers = getProblemMarkers(monitor);
publishDiagnostics(problemMarkers);
return problemMarkers;
}

private List<IMarker> getProblemMarkers(IProgressMonitor monitor) throws CoreException {
IProject[] projects = ResourcesPlugin.getWorkspace().getRoot().getProjects();
List<IMarker> markers = new ArrayList<>();
for (IProject project : projects) {
if (monitor != null && monitor.isCanceled()) {
throw new OperationCanceledException();
}
markers.addAll(Arrays.asList(project.findMarkers(IJavaModelMarker.JAVA_MODEL_PROBLEM_MARKER, true, IResource.DEPTH_INFINITE)));
markers.addAll(Arrays.asList(project.findMarkers(IJavaModelMarker.TASK_MARKER, true, IResource.DEPTH_INFINITE)));
}
return markers;
}

private void publishDiagnostics(List<IMarker> markers) {
Map<IResource, List<IMarker>> map = markers.stream().collect(Collectors.groupingBy(IMarker::getResource));
for (Map.Entry<IResource, List<IMarker>> entry : map.entrySet()) {
IResource resource = entry.getKey();
// ignore problems caused by standalone files
if (JavaLanguageServerPlugin.getProjectsManager().getDefaultProject().equals(resource.getProject())) {
continue;
}
if (resource instanceof IProject) {
String uri = JDTUtils.getFileURI(resource);
Range range = new Range(new Position(0, 0), new Position(0, 0));
List<Diagnostic> diagnostics = WorkspaceDiagnosticsHandler.toDiagnosticArray(range, entry.getValue());
connection.publishDiagnostics(new PublishDiagnosticsParams(ResourceUtils.toClientUri(uri), diagnostics));
continue;
}
IFile file = resource.getAdapter(IFile.class);
if (file == null) {
continue;
}
IDocument document = null;
String uri = JDTUtils.getFileURI(resource);
if (JavaCore.isJavaLikeFileName(file.getName())) {
ICompilationUnit cu = JDTUtils.resolveCompilationUnit(uri);
try {
document = JsonRpcHelpers.toDocument(cu.getBuffer());
} catch (JavaModelException e) {
JavaLanguageServerPlugin.logException("Failed to publish diagnostics.", e);
}
} else if (projectsManager.isBuildFile(file)) {
document = JsonRpcHelpers.toDocument(file);
}

if (document != null) {
List<Diagnostic> diagnostics = WorkspaceDiagnosticsHandler.toDiagnosticsArray(document, entry.getValue().toArray(new IMarker[0]));
connection.publishDiagnostics(new PublishDiagnosticsParams(ResourceUtils.toClientUri(uri), diagnostics));
}
}
}



/**
* Transforms {@link IMarker}s into a list of {@link Diagnostic}s
*
Expand Down
18 changes: 10 additions & 8 deletions ...jdt.ls.tests/src/org/eclipse/jdt/ls/core/internal/handlers/BuildWorkspaceHandlerTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,8 @@
import java.nio.charset.StandardCharsets;

import org.eclipse.core.resources.IFile;
import org.eclipse.core.resources.IProject;
import org.eclipse.core.resources.ResourcesPlugin;
import org.eclipse.jdt.ls.core.internal.BuildWorkspaceStatus;
import org.eclipse.jdt.ls.core.internal.JavaClientConnection;
import org.eclipse.jdt.ls.core.internal.JavaClientConnection.JavaLanguageClient;
Expand All @@ -31,34 +33,35 @@ public class BuildWorkspaceHandlerTest extends AbstractProjectsManagerBasedTest
private JavaClientConnection javaClient = new JavaClientConnection(client);

private BuildWorkspaceHandler handler;
private IFile file;
private IProject project;

@Before
public void setUp() throws Exception {
handler = new BuildWorkspaceHandler(javaClient, projectsManager, new WorkspaceDiagnosticsHandler(javaClient, projectsManager));
handler = new BuildWorkspaceHandler(javaClient, projectsManager);
importProjects("maven/salut2");
file = linkFilesToDefaultProject("singlefile/Single.java");
project = ResourcesPlugin.getWorkspace().getRoot().getProject("salut2");
}

@Test
public void testSucceedCase() throws Exception {
BuildWorkspaceStatus result = handler.buildWorkspace(false, monitor);
waitForBackgroundJobs();
assertTrue(String.format("BuildWorkspaceStatus is: %s.", result.toString()), result == BuildWorkspaceStatus.SUCCEED);
}

@Test
public void testFailedCase() throws Exception {
String codeWithError = "public class Single2 {\n" +
//@formatter:off
String codeWithError = "package foo;\n" +
" public class Single2 {\n" +
" public static void main(String[] args){\n"+
" int ss = 1;;\n"+
" }\n"+
"}";
//@formatter:on
IFile file = project.getFile("src/main/java/foo/Bar.java");
try (InputStream stream = new ByteArrayInputStream(codeWithError.getBytes(StandardCharsets.UTF_8))) {
file.setContents(stream, true, false, monitor);
waitForBackgroundJobs();
BuildWorkspaceStatus result = handler.buildWorkspace(false, monitor);
waitForBackgroundJobs();
assertEquals(result, BuildWorkspaceStatus.WITH_ERROR);
}
}
Expand All @@ -67,7 +70,6 @@ public void testFailedCase() throws Exception {
public void testCanceledCase() throws Exception {
monitor.setCanceled(true);
BuildWorkspaceStatus result = handler.buildWorkspace(false, monitor);
waitForBackgroundJobs();
assertEquals(result, BuildWorkspaceStatus.CANCELLED);
}
}
64 changes: 62 additions & 2 deletions ....tests/src/org/eclipse/jdt/ls/core/internal/handlers/WorkspaceDiagnosticsHandlerTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
import static org.mockito.Mockito.verify;
import static org.mockito.Mockito.when;

import java.util.ArrayList;
import java.util.Collections;
import java.util.Comparator;
import java.util.List;
Expand All @@ -30,6 +31,7 @@
import org.eclipse.core.resources.IProject;
import org.eclipse.core.resources.IncrementalProjectBuilder;
import org.eclipse.core.resources.ResourcesPlugin;
import org.eclipse.core.runtime.NullProgressMonitor;
import org.eclipse.jdt.ls.core.internal.JavaClientConnection;
import org.eclipse.jdt.ls.core.internal.ResourceUtils;
import org.eclipse.jdt.ls.core.internal.managers.AbstractProjectsManagerBasedTest;
Expand Down Expand Up @@ -82,7 +84,7 @@ public void testToDiagnosticsArray() throws Exception {
when(d.getLineOffset(9)).thenReturn(1000);
when(d.getLineOffset(99)).thenReturn(10000);

List<Diagnostic> diags = handler.toDiagnosticsArray(d, new IMarker[]{m1, m2, m3});
List<Diagnostic> diags = WorkspaceDiagnosticsHandler.toDiagnosticsArray(d, new IMarker[]{m1, m2, m3});
assertEquals(3, diags.size());

Range r;
Expand Down Expand Up @@ -166,7 +168,7 @@ public void testMavenMarkers() throws Exception {
IDocument d = mock(IDocument.class);
when(d.getLineOffset(1)).thenReturn(90);

List<Diagnostic> diags = handler.toDiagnosticsArray(d, new IMarker[]{m1, null});
List<Diagnostic> diags = WorkspaceDiagnosticsHandler.toDiagnosticsArray(d, new IMarker[]{m1, null});
assertEquals(1, diags.size());

Range r;
Expand Down Expand Up @@ -291,6 +293,64 @@ public void testProjectConfigurationIsNotUpToDate() throws Exception {
assertEquals(diag.getSeverity(), DiagnosticSeverity.Warning);
}

@Test
public void testMissingDependencies() throws Exception {
importProjects("maven/salut");
IProject project = ResourcesPlugin.getWorkspace().getRoot().getProject("salut");
IFile pom = project.getFile("/pom.xml");
assertTrue(pom.exists());
ResourcesPlugin.getWorkspace().build(IncrementalProjectBuilder.CLEAN_BUILD, monitor);
assertNoErrors(project);
// edit pom.xml
ResourceUtils.setContent(pom, ResourceUtils.getContent(pom).replaceAll("<version>3.5</version>", "<version>3.5xx</version>"));
waitForBackgroundJobs();
ArgumentCaptor<PublishDiagnosticsParams> captor = ArgumentCaptor.forClass(PublishDiagnosticsParams.class);
verify(connection, atLeastOnce()).publishDiagnostics(captor.capture());
List<PublishDiagnosticsParams> allCalls = captor.getAllValues();
Collections.reverse(allCalls);
projectsManager.setConnection(client);
testDiagnostic(allCalls);
// update project
reset(connection);
captor = ArgumentCaptor.forClass(PublishDiagnosticsParams.class);
projectsManager.updateProject(project, true);
waitForBackgroundJobs();
verify(connection, atLeastOnce()).publishDiagnostics(captor.capture());
allCalls = captor.getAllValues();
Collections.reverse(allCalls);
testDiagnostic(allCalls);
// build workspace
reset(connection);
captor = ArgumentCaptor.forClass(PublishDiagnosticsParams.class);
BuildWorkspaceHandler bwh = new BuildWorkspaceHandler(connection, projectsManager);
bwh.buildWorkspace(true, new NullProgressMonitor());
verify(connection, atLeastOnce()).publishDiagnostics(captor.capture());
allCalls = captor.getAllValues();
Collections.reverse(allCalls);
testDiagnostic(allCalls);
}

private void testDiagnostic(List<PublishDiagnosticsParams> allCalls) {
List<Diagnostic> projectDiags = new ArrayList<>();
List<Diagnostic> pomDiags = new ArrayList<>();
for (PublishDiagnosticsParams diag : allCalls) {
if (diag.getUri().endsWith("maven/salut")) {
projectDiags.addAll(diag.getDiagnostics());
} else if (diag.getUri().endsWith("pom.xml")) {
pomDiags.addAll(diag.getDiagnostics());
}
}
assertTrue("No maven/salut errors were found", projectDiags.size() > 0);
Optional<Diagnostic> projectDiag = projectDiags.stream().filter(p -> p.getMessage().startsWith("Missing artifact")).findFirst();
assertTrue("No missing artifact diagnostic", projectDiag.isPresent());
assertEquals(projectDiag.get().getSeverity(), DiagnosticSeverity.Error);
assertTrue("No pom.xml errors were found", pomDiags.size() > 0);
Optional<Diagnostic> pomDiag = pomDiags.stream().filter(p -> p.getMessage().startsWith("Missing artifact")).findFirst();
assertTrue("No missing artifact diagnostic", pomDiag.isPresent());
assertTrue(pomDiag.get().getMessage().startsWith("Missing artifact"));
assertEquals(pomDiag.get().getSeverity(), DiagnosticSeverity.Error);
}

@Test
public void testResetPomDiagnostics() throws Exception {
//import project
Expand Down
3 changes: 1 addition & 2 deletions ...lipse.jdt.ls.tests/src/org/eclipse/jdt/ls/core/internal/managers/ProjectsManagerTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,6 @@
import org.eclipse.jdt.ls.core.internal.JavaClientConnection.JavaLanguageClient;
import org.eclipse.jdt.ls.core.internal.WorkspaceHelper;
import org.eclipse.jdt.ls.core.internal.handlers.BuildWorkspaceHandler;
import org.eclipse.jdt.ls.core.internal.handlers.WorkspaceDiagnosticsHandler;
import org.junit.Test;

/**
Expand Down Expand Up @@ -58,7 +57,7 @@ public void testCleanupDefaultProject() throws Exception {
java.io.File physicalFile = new java.io.File(file.getLocationURI());
physicalFile.delete();

BuildWorkspaceHandler handler = new BuildWorkspaceHandler(javaClient, projectsManager, new WorkspaceDiagnosticsHandler(javaClient, projectsManager));
BuildWorkspaceHandler handler = new BuildWorkspaceHandler(javaClient, projectsManager);
BuildWorkspaceStatus result = handler.buildWorkspace(true, monitor);

waitForBackgroundJobs();
Expand Down

0 comments on commit 8376285

Please sign in to comment.