From e7396e08be20c682277512fd27ab104daf87e4c5 Mon Sep 17 00:00:00 2001 From: Fred Bricon Date: Tue, 2 Oct 2018 11:28:14 -0400 Subject: [PATCH] WIP - Ensure project and pom.xml diagnostics are properly reset Signed-off-by: Fred Bricon --- .../handlers/WorkspaceDiagnosticsHandler.java | 34 ++++---- .../multimodule/module1/childmodule/pom.xml | 17 ++-- .../maven/multimodule/module1/pom.xml | 27 ++++--- .../maven/multimodule/module2/pom.xml | 17 ++-- .../maven/multimodule/module3/pom.xml | 18 +++-- .../projects/maven/multimodule/pom.xml | 55 ++++++------- .../WorkspaceDiagnosticsHandlerTest.java | 79 ++++++++++++++++++- 7 files changed, 172 insertions(+), 75 deletions(-) diff --git a/org.eclipse.jdt.ls.core/src/org/eclipse/jdt/ls/core/internal/handlers/WorkspaceDiagnosticsHandler.java b/org.eclipse.jdt.ls.core/src/org/eclipse/jdt/ls/core/internal/handlers/WorkspaceDiagnosticsHandler.java index 6e2e688717..2e576c8e28 100644 --- a/org.eclipse.jdt.ls.core/src/org/eclipse/jdt/ls/core/internal/handlers/WorkspaceDiagnosticsHandler.java +++ b/org.eclipse.jdt.ls.core/src/org/eclipse/jdt/ls/core/internal/handlers/WorkspaceDiagnosticsHandler.java @@ -15,7 +15,6 @@ import java.util.Collection; import java.util.List; import java.util.Map; -import java.util.Map.Entry; import java.util.stream.Collectors; import java.util.stream.Stream; @@ -51,9 +50,6 @@ import org.eclipse.m2e.core.internal.IMavenConstants; import org.eclipse.m2e.core.internal.Messages; -import com.google.common.collect.HashMultimap; -import com.google.common.collect.Multimap; - /** * Listens to the resource change events and converts {@link IMarker}s to {@link Diagnostic}s. * @@ -115,29 +111,37 @@ public boolean visit(IResourceDelta delta) throws CoreException { if (JavaLanguageServerPlugin.getProjectsManager().getDefaultProject().equals(resource.getProject())) { return false; } + IProject project = (IProject) resource; // report problems for other projects - IMarker[] markers = resource.findMarkers(null, true, IResource.DEPTH_ONE); + IMarker[] markers = project.findMarkers(null, true, IResource.DEPTH_ONE); Range range = new Range(new Position(0, 0), new Position(0, 0)); - Multimap allMarkers = HashMultimap.create(1, markers.length); + List projectMarkers = new ArrayList<>(markers.length); + + String uri = JDTUtils.getFileURI(project); + IFile pom = project.getFile("pom.xml"); + List pomMarkers = new ArrayList<>(); + if (pom.exists()) { + pomMarkers.addAll(Arrays.asList(pom.findMarkers(null, true, 1))); + } - String uri = JDTUtils.getFileURI(resource); for (IMarker marker : markers) { if (!marker.exists() || CheckMissingNaturesListener.MARKER_TYPE.equals(marker.getType())) { continue; } if (IMavenConstants.MARKER_CONFIGURATION_ID.equals(marker.getType())) { - String message = marker.getAttribute(IMarker.MESSAGE, ""); - if (Messages.ProjectConfigurationUpdateRequired.equals(message)) { - allMarkers.put(uri + "/pom.xml", marker); - } + pomMarkers.add(marker); } else { - allMarkers.put(uri, marker); + projectMarkers.add(marker); } } - for (Entry> e : allMarkers.asMap().entrySet()) { - List diagnostics = toDiagnosticArray(range, e.getValue()); - connection.publishDiagnostics(new PublishDiagnosticsParams(ResourceUtils.toClientUri(e.getKey()), diagnostics)); + + List diagnostics = toDiagnosticArray(range, projectMarkers); + String clientUri = ResourceUtils.toClientUri(uri); + connection.publishDiagnostics(new PublishDiagnosticsParams(clientUri, diagnostics)); + if (pom.exists()) { + diagnostics = toDiagnosticArray(range, pomMarkers); + connection.publishDiagnostics(new PublishDiagnosticsParams(ResourceUtils.toClientUri(clientUri + "/pom.xml"), diagnostics)); } return true; } diff --git a/org.eclipse.jdt.ls.tests/projects/maven/multimodule/module1/childmodule/pom.xml b/org.eclipse.jdt.ls.tests/projects/maven/multimodule/module1/childmodule/pom.xml index 60bd89c05e..30283b4a81 100644 --- a/org.eclipse.jdt.ls.tests/projects/maven/multimodule/module1/childmodule/pom.xml +++ b/org.eclipse.jdt.ls.tests/projects/maven/multimodule/module1/childmodule/pom.xml @@ -1,8 +1,11 @@ - - 4.0.0 - foo.bar - childmodule - 0.0.1-SNAPSHOT - jar - + + 4.0.0 + + foo.bar + module1 + 0.0.1-SNAPSHOT + + childmodule \ No newline at end of file diff --git a/org.eclipse.jdt.ls.tests/projects/maven/multimodule/module1/pom.xml b/org.eclipse.jdt.ls.tests/projects/maven/multimodule/module1/pom.xml index 573fa1200e..0a4e419cd1 100644 --- a/org.eclipse.jdt.ls.tests/projects/maven/multimodule/module1/pom.xml +++ b/org.eclipse.jdt.ls.tests/projects/maven/multimodule/module1/pom.xml @@ -1,12 +1,17 @@ - - 4.0.0 - foo.bar - module1 - 0.0.1-SNAPSHOT - pom - - - childmodule - - + + 4.0.0 + + foo.bar + multimodule + 0.0.1-SNAPSHOT + + module1 + pom + + + childmodule + + \ No newline at end of file diff --git a/org.eclipse.jdt.ls.tests/projects/maven/multimodule/module2/pom.xml b/org.eclipse.jdt.ls.tests/projects/maven/multimodule/module2/pom.xml index 250938012d..467f0ec6e5 100644 --- a/org.eclipse.jdt.ls.tests/projects/maven/multimodule/module2/pom.xml +++ b/org.eclipse.jdt.ls.tests/projects/maven/multimodule/module2/pom.xml @@ -1,8 +1,11 @@ - - 4.0.0 - foo.bar - module2 - 0.0.1-SNAPSHOT - jar - + + 4.0.0 + + foo.bar + multimodule + 0.0.1-SNAPSHOT + + module2 \ No newline at end of file diff --git a/org.eclipse.jdt.ls.tests/projects/maven/multimodule/module3/pom.xml b/org.eclipse.jdt.ls.tests/projects/maven/multimodule/module3/pom.xml index 9c5faaf227..f55249e298 100644 --- a/org.eclipse.jdt.ls.tests/projects/maven/multimodule/module3/pom.xml +++ b/org.eclipse.jdt.ls.tests/projects/maven/multimodule/module3/pom.xml @@ -1,8 +1,12 @@ - - 4.0.0 - foo.bar - module3 - 0.0.1-SNAPSHOT - jar - + + 4.0.0 + + foo.bar + multimodule + 0.0.1-SNAPSHOT + + module3 + jar \ No newline at end of file diff --git a/org.eclipse.jdt.ls.tests/projects/maven/multimodule/pom.xml b/org.eclipse.jdt.ls.tests/projects/maven/multimodule/pom.xml index 6b00841d1e..7916666fb9 100644 --- a/org.eclipse.jdt.ls.tests/projects/maven/multimodule/pom.xml +++ b/org.eclipse.jdt.ls.tests/projects/maven/multimodule/pom.xml @@ -1,28 +1,29 @@ - - 4.0.0 - foo.bar - multimodule - 0.0.1-SNAPSHOT - pom - - module1 - - - - - test - - module3 - - - - default - - true - - - module2 - - - + + 4.0.0 + foo.bar + multimodule + 0.0.1-SNAPSHOT + pom + + module1 + + + + test + + module3 + + + + default + + true + + + module2 + + + \ No newline at end of file diff --git a/org.eclipse.jdt.ls.tests/src/org/eclipse/jdt/ls/core/internal/handlers/WorkspaceDiagnosticsHandlerTest.java b/org.eclipse.jdt.ls.tests/src/org/eclipse/jdt/ls/core/internal/handlers/WorkspaceDiagnosticsHandlerTest.java index 741025d8ba..0ffc235268 100644 --- a/org.eclipse.jdt.ls.tests/src/org/eclipse/jdt/ls/core/internal/handlers/WorkspaceDiagnosticsHandlerTest.java +++ b/org.eclipse.jdt.ls.tests/src/org/eclipse/jdt/ls/core/internal/handlers/WorkspaceDiagnosticsHandlerTest.java @@ -15,6 +15,7 @@ import static org.junit.Assert.assertTrue; import static org.mockito.Mockito.atLeastOnce; import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.reset; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; @@ -22,6 +23,7 @@ import java.util.Comparator; import java.util.List; import java.util.Optional; +import java.util.stream.Collectors; import org.eclipse.core.resources.IFile; import org.eclipse.core.resources.IMarker; @@ -251,7 +253,8 @@ public void testMissingNatures() throws Exception { Collections.reverse(allCalls); projectsManager.setConnection(client); Optional projectDiags = allCalls.stream().filter(p -> p.getUri().endsWith("eclipse/wtpproject")).findFirst(); - assertFalse(projectDiags.isPresent()); + assertTrue(projectDiags.isPresent()); + assertEquals("Unexpected diagnostics:\n" + projectDiags.get().getDiagnostics(), 0, projectDiags.get().getDiagnostics().size()); } @Test @@ -288,6 +291,80 @@ public void testProjectConfigurationIsNotUpToDate() throws Exception { assertEquals(diag.getSeverity(), DiagnosticSeverity.Warning); } + @Test + public void testResetPomDiagnostics() throws Exception { + //import project + importProjects("maven/multimodule"); + IProject project = ResourcesPlugin.getWorkspace().getRoot().getProject("multimodule"); + IFile pom = project.getFile("/pom.xml"); + assertTrue(pom.exists()); + //@formatter:off + String compilerPlugin = "\n\n" + + " \n" + + " \n" + + " \n" + + " maven-compiler-plugin\n" + + " 3.8.0\n" + + " \n" + + " 9\n" + + " \n" + + " \n" + + " \n" + + " \n" + + "\n"; + //@formatter:on + + ResourceUtils.setContent(pom, ResourceUtils.getContent(pom).replaceAll("", compilerPlugin + "\n")); + + ResourcesPlugin.getWorkspace().build(IncrementalProjectBuilder.INCREMENTAL_BUILD, monitor); + + ArgumentCaptor captor = ArgumentCaptor.forClass(PublishDiagnosticsParams.class); + verify(connection, atLeastOnce()).publishDiagnostics(captor.capture()); + List allCalls = captor.getAllValues(); + + List pomDiags = allCalls.stream().filter(p -> p.getUri().endsWith("pom.xml") && !p.getDiagnostics().isEmpty()).collect(Collectors.toList()); + assertEquals("No pom.xml errors were found", 3, pomDiags.size()); + assertTrue(pomDiags.get(0).getUri(), pomDiags.get(0).getUri().endsWith("childmodule/pom.xml")); + assertEquals(1, pomDiags.get(0).getDiagnostics().size()); + assertEquals(pomDiags.get(0).getDiagnostics().get(0).getMessage(), WorkspaceDiagnosticsHandler.PROJECT_CONFIGURATION_IS_NOT_UP_TO_DATE_WITH_POM_XML); + assertTrue(pomDiags.get(1).getUri().endsWith("module2/pom.xml")); + assertEquals(1, pomDiags.get(1).getDiagnostics().size()); + assertEquals(pomDiags.get(1).getDiagnostics().get(0).getMessage(), WorkspaceDiagnosticsHandler.PROJECT_CONFIGURATION_IS_NOT_UP_TO_DATE_WITH_POM_XML); + assertTrue(pomDiags.get(2).getUri().endsWith("module3/pom.xml")); + assertEquals(1, pomDiags.get(2).getDiagnostics().size()); + assertEquals(pomDiags.get(2).getDiagnostics().get(0).getMessage(), WorkspaceDiagnosticsHandler.PROJECT_CONFIGURATION_IS_NOT_UP_TO_DATE_WITH_POM_XML); + + reset(connection); + captor = ArgumentCaptor.forClass(PublishDiagnosticsParams.class); + projectsManager.updateProject(project, true); + waitForBackgroundJobs(); + + verify(connection, atLeastOnce()).publishDiagnostics(captor.capture()); + allCalls = captor.getAllValues(); + + pomDiags = allCalls.stream().filter(p -> p.getUri().endsWith("pom.xml")).collect(Collectors.toList()); + boolean reset1 = false; + boolean reset2 = false; + boolean reset3 = true; + for (PublishDiagnosticsParams diag : pomDiags) { + String uri = diag.getUri(); + if (uri.endsWith("childmodule/pom.xml")) { + assertEquals("Unexpected diagnostics:\n" + diag.getDiagnostics(), 0, diag.getDiagnostics().size()); + reset1 = true; + } else if (uri.endsWith("module2/pom.xml")) { + assertEquals("Unexpected diagnostics:\n" + diag.getDiagnostics(), 0, diag.getDiagnostics().size()); + reset2 = true; + } else if (uri.endsWith("module3/pom.xml")) {//not a active module so was not updated. But this is actually a dubious behavior. Need to change that + assertEquals("Unexpected diagnostics:\n" + diag.getDiagnostics(), 1, diag.getDiagnostics().size()); + reset3 = false; + } + } + assertTrue("childmodule/pom.xml diagnostics were not reset", reset1); + assertTrue("module2/pom.xml diagnostics were not reset", reset2); + assertFalse("module3/pom.xml diagnostics were reset", reset3); + + } + private IMarker createMarker(int severity, String msg, int line, int start, int end) { IMarker m = mock(IMarker.class); when(m.exists()).thenReturn(true);