From 86be67134a76177f0d1b8e9aabd345f39de06cf3 Mon Sep 17 00:00:00 2001 From: Fred Bricon Date: Tue, 16 Apr 2019 17:10:48 -0400 Subject: [PATCH] Fix jar detection on windows - don't create filewatcher with devices (prefix with ** instead of device) - fixed filewatcher on .settings/ - avoid generating overlapping filewatchers Signed-off-by: Fred Bricon --- .../jdt/ls/core/internal/ResourceUtils.java | 26 +++++++++++++ .../internal/managers/ProjectsManager.java | 38 +++++++++++-------- .../ls/core/internal/ResourceUtilsTest.java | 30 +++++++++++++++ .../internal/handlers/InitHandlerTest.java | 16 ++++---- .../AbstractInvisibleProjectBasedTest.java | 12 +++++- .../InvisibleProjectImporterTest.java | 27 ++++++++++++- 6 files changed, 124 insertions(+), 25 deletions(-) create mode 100644 org.eclipse.jdt.ls.tests/src/org/eclipse/jdt/ls/core/internal/ResourceUtilsTest.java diff --git a/org.eclipse.jdt.ls.core/src/org/eclipse/jdt/ls/core/internal/ResourceUtils.java b/org.eclipse.jdt.ls.core/src/org/eclipse/jdt/ls/core/internal/ResourceUtils.java index fb3e654ca8..eebdeeeaee 100644 --- a/org.eclipse.jdt.ls.core/src/org/eclipse/jdt/ls/core/internal/ResourceUtils.java +++ b/org.eclipse.jdt.ls.core/src/org/eclipse/jdt/ls/core/internal/ResourceUtils.java @@ -213,6 +213,9 @@ public static IPath canonicalFilePathFromURI(String uriStr) { } public static boolean isContainedIn(IPath location, Collection paths) { + if (location == null || paths == null || paths.isEmpty()) { + return false; + } for (IPath path : paths) { if (path.isPrefixOf(location)) { return true; @@ -234,4 +237,27 @@ public static String expandPath(String path) { return path; } + /** + * Convert an {@link IPath} to a glob pattern (i.e. ending with /**) + * + * @param path + * the path to convert + * @return a glob pattern prefixed with the path + */ + public static String toGlobPattern(IPath path) { + if (path == null) { + return null; + } + String globPattern = path.toPortableString(); + if (path.getDevice() != null) { + //This seems pretty hack-ish: need to remove device as it seems to break + // file detection, at least on vscode + globPattern = globPattern.replace(path.getDevice(), "**"); + } + if (!globPattern.endsWith("/")) { + globPattern += "/"; + } + return globPattern + "**"; + } + } diff --git a/org.eclipse.jdt.ls.core/src/org/eclipse/jdt/ls/core/internal/managers/ProjectsManager.java b/org.eclipse.jdt.ls.core/src/org/eclipse/jdt/ls/core/internal/managers/ProjectsManager.java index 9a284329d5..99084681f5 100644 --- a/org.eclipse.jdt.ls.core/src/org/eclipse/jdt/ls/core/internal/managers/ProjectsManager.java +++ b/org.eclipse.jdt.ls.core/src/org/eclipse/jdt/ls/core/internal/managers/ProjectsManager.java @@ -14,6 +14,7 @@ import static java.util.Arrays.asList; import static org.eclipse.jdt.ls.core.internal.JVMConfigurator.configureJVMSettings; import static org.eclipse.jdt.ls.core.internal.JavaLanguageServerPlugin.logInfo; +import static org.eclipse.jdt.ls.core.internal.ResourceUtils.isContainedIn; import java.io.File; import java.net.MalformedURLException; @@ -25,6 +26,7 @@ import java.util.Collection; import java.util.Collections; import java.util.HashSet; +import java.util.LinkedHashSet; import java.util.List; import java.util.Map; import java.util.Optional; @@ -53,6 +55,7 @@ import org.eclipse.core.runtime.IStatus; import org.eclipse.core.runtime.NullProgressMonitor; import org.eclipse.core.runtime.OperationCanceledException; +import org.eclipse.core.runtime.Path; import org.eclipse.core.runtime.Platform; import org.eclipse.core.runtime.Status; import org.eclipse.core.runtime.SubMonitor; @@ -89,20 +92,20 @@ public class ProjectsManager implements ISaveParticipant { public static final String DEFAULT_PROJECT_NAME = "jdt.ls-java-project"; - private static final Set watchers = new HashSet<>(); + private static final Set watchers = new LinkedHashSet<>(); private PreferenceManager preferenceManager; private JavaLanguageClient client; //@formatter:off - private static final String[] basicWatchers = new String[] { + private static final List basicWatchers = Arrays.asList( "**/*.java", "**/pom.xml", "**/*.gradle", "**/gradle.properties", "**/.project", "**/.classpath", - "**/settings/*.prefs", + "**/.settings/*.prefs", "**/src/**" - }; + ); //@formatter:on public enum CHANGE_TYPE { @@ -562,7 +565,7 @@ public boolean setAutoBuilding(boolean enable) throws CoreException { public List registerWatchers() { logInfo(">> registerFeature 'workspace/didChangeWatchedFiles'"); if (preferenceManager.getClientPreferences().isWorkspaceChangeWatchedFilesDynamicRegistered()) { - Set sources = new HashSet<>(); + Set sources = new HashSet<>(); try { IProject[] projects = ResourcesPlugin.getWorkspace().getRoot().getProjects(); for (IProject project : projects) { @@ -579,8 +582,8 @@ public List registerWatchers() { IFolder folder = ResourcesPlugin.getWorkspace().getRoot().getFolder(path); if (folder.exists() && !folder.isDerived()) { IPath location = folder.getLocation(); - if (location != null) { - sources.add(location.toString() + "/**"); + if (location != null && !isContainedIn(location, sources)) { + sources.add(location); } } @@ -592,7 +595,9 @@ public List registerWatchers() { IPath realFolderPath = project.getFolder(ProjectUtils.WORKSPACE_LINK).getLocation(); if (realFolderPath != null) { IPath libFolderPath = realFolderPath.append(InvisibleProjectBuildSupport.LIB_FOLDER); - sources.add(libFolderPath.toString() + "/**"); + if (!isContainedIn(libFolderPath, sources)) { + sources.add(libFolderPath); + } } } } @@ -608,24 +613,27 @@ public List registerWatchers() { file = findFile(formatterUrl); } if (file != null && file.isFile()) { - sources.add(file.getAbsolutePath()); + IPath formatterPath = new Path(file.getAbsolutePath()); + if (!isContainedIn(formatterPath, sources)) { + sources.add(formatterPath); + } } } - for (String pattern : basicWatchers) { - sources.add(pattern); - } - for (String pattern : sources) { + Set patterns = new LinkedHashSet<>(basicWatchers); + patterns.addAll(sources.stream().map(ResourceUtils::toGlobPattern).collect(Collectors.toList())); + + for (String pattern : patterns) { FileSystemWatcher watcher = new FileSystemWatcher(pattern); fileWatchers.add(watcher); } - if (!sources.equals(watchers)) { + if (!patterns.equals(watchers)) { logInfo(">> registerFeature 'workspace/didChangeWatchedFiles'"); DidChangeWatchedFilesRegistrationOptions didChangeWatchedFilesRegistrationOptions = new DidChangeWatchedFilesRegistrationOptions(fileWatchers); JavaLanguageServerPlugin.getInstance().unregisterCapability(Preferences.WORKSPACE_WATCHED_FILES_ID, Preferences.WORKSPACE_WATCHED_FILES); JavaLanguageServerPlugin.getInstance().registerCapability(Preferences.WORKSPACE_WATCHED_FILES_ID, Preferences.WORKSPACE_WATCHED_FILES, didChangeWatchedFilesRegistrationOptions); watchers.clear(); - watchers.addAll(sources); + watchers.addAll(patterns); } return fileWatchers; } diff --git a/org.eclipse.jdt.ls.tests/src/org/eclipse/jdt/ls/core/internal/ResourceUtilsTest.java b/org.eclipse.jdt.ls.tests/src/org/eclipse/jdt/ls/core/internal/ResourceUtilsTest.java new file mode 100644 index 0000000000..f00cf4cd1b --- /dev/null +++ b/org.eclipse.jdt.ls.tests/src/org/eclipse/jdt/ls/core/internal/ResourceUtilsTest.java @@ -0,0 +1,30 @@ +/******************************************************************************* + * Copyright (c) 2019 Red Hat Inc. and others. + * All rights reserved. This program and the accompanying materials + * are made available under the terms of the Eclipse Public License v1.0 + * which accompanies this distribution, and is available at + * http://www.eclipse.org/legal/epl-v10.html + * + * Contributors: + * Red Hat Inc. - initial API and implementation + *******************************************************************************/ +package org.eclipse.jdt.ls.core.internal; + +import static org.eclipse.jdt.ls.core.internal.ResourceUtils.toGlobPattern; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertNull; + +import org.eclipse.core.runtime.Path; +import org.junit.Test; + +public class ResourceUtilsTest { + + @Test + public void testToGlobPattern() { + assertNull(toGlobPattern(null)); + assertEquals("/foo/bar/**", toGlobPattern(Path.forPosix("/foo/bar"))); + assertEquals("/foo/bar/**", toGlobPattern(Path.forPosix("/foo/bar/"))); + assertEquals("**/foo/bar/**", toGlobPattern(Path.forWindows("c:/foo/bar/"))); + assertEquals("**/foo/bar/**", toGlobPattern(Path.forWindows("c:\\foo\\bar"))); + } +} diff --git a/org.eclipse.jdt.ls.tests/src/org/eclipse/jdt/ls/core/internal/handlers/InitHandlerTest.java b/org.eclipse.jdt.ls.tests/src/org/eclipse/jdt/ls/core/internal/handlers/InitHandlerTest.java index 0498191d2d..9c7c95d320 100644 --- a/org.eclipse.jdt.ls.tests/src/org/eclipse/jdt/ls/core/internal/handlers/InitHandlerTest.java +++ b/org.eclipse.jdt.ls.tests/src/org/eclipse/jdt/ls/core/internal/handlers/InitHandlerTest.java @@ -202,14 +202,14 @@ public int compare(FileSystemWatcher o1, FileSystemWatcher o2) { } }); assertEquals("Unexpected watchers:\n" + toString(watchers), 8, watchers.size()); - assertEquals(watchers.get(0).getGlobPattern(), "**/*.gradle"); - assertEquals(watchers.get(1).getGlobPattern(), "**/*.java"); - assertEquals(watchers.get(2).getGlobPattern(), "**/.classpath"); - assertEquals(watchers.get(3).getGlobPattern(), "**/.project"); - assertEquals(watchers.get(4).getGlobPattern(), "**/gradle.properties"); - assertEquals(watchers.get(5).getGlobPattern(), "**/pom.xml"); - assertEquals(watchers.get(6).getGlobPattern(), "**/settings/*.prefs"); - assertEquals(watchers.get(7).getGlobPattern(), "**/src/**"); + assertEquals("**/*.gradle", watchers.get(0).getGlobPattern()); + assertEquals("**/*.java", watchers.get(1).getGlobPattern()); + assertEquals("**/.classpath", watchers.get(2).getGlobPattern()); + assertEquals("**/.project", watchers.get(3).getGlobPattern()); + assertEquals("**/.settings/*.prefs", watchers.get(4).getGlobPattern()); + assertEquals("**/gradle.properties", watchers.get(5).getGlobPattern()); + assertEquals("**/pom.xml", watchers.get(6).getGlobPattern()); + assertEquals("**/src/**", watchers.get(7).getGlobPattern()); IProject project = ResourcesPlugin.getWorkspace().getRoot().getProject("salut"); String location = project.getLocation().toString(); IJavaProject javaProject = JavaCore.create(project); diff --git a/org.eclipse.jdt.ls.tests/src/org/eclipse/jdt/ls/core/internal/managers/AbstractInvisibleProjectBasedTest.java b/org.eclipse.jdt.ls.tests/src/org/eclipse/jdt/ls/core/internal/managers/AbstractInvisibleProjectBasedTest.java index 6cbfb0d2f3..d155558e67 100644 --- a/org.eclipse.jdt.ls.tests/src/org/eclipse/jdt/ls/core/internal/managers/AbstractInvisibleProjectBasedTest.java +++ b/org.eclipse.jdt.ls.tests/src/org/eclipse/jdt/ls/core/internal/managers/AbstractInvisibleProjectBasedTest.java @@ -58,9 +58,19 @@ protected File createSourceFolderWithMissingLibs(String name) throws Exception { } protected File createSourceFolderWithLibs(String name, boolean addLibs) throws Exception { + return createSourceFolderWithLibs(name, null, addLibs); + } + + protected File createSourceFolderWithLibs(String name, String srcDir, boolean addLibs) throws Exception { java.nio.file.Path projectPath = Files.createTempDirectory(name); File projectFolder = projectPath.toFile(); - FileUtils.copyDirectory(new File(getSourceProjectDirectory(), "eclipse/source-attachment/src"), projectFolder); + File sourceFolder; + if (org.apache.commons.lang3.StringUtils.isBlank(srcDir)) { + sourceFolder = projectFolder; + } else { + sourceFolder = new File(projectFolder, srcDir); + } + FileUtils.copyDirectory(new File(getSourceProjectDirectory(), "eclipse/source-attachment/src"), sourceFolder); if (addLibs) { addLibs(projectPath); } diff --git a/org.eclipse.jdt.ls.tests/src/org/eclipse/jdt/ls/core/internal/managers/InvisibleProjectImporterTest.java b/org.eclipse.jdt.ls.tests/src/org/eclipse/jdt/ls/core/internal/managers/InvisibleProjectImporterTest.java index 6fc1ade6b0..9f8b08a4de 100644 --- a/org.eclipse.jdt.ls.tests/src/org/eclipse/jdt/ls/core/internal/managers/InvisibleProjectImporterTest.java +++ b/org.eclipse.jdt.ls.tests/src/org/eclipse/jdt/ls/core/internal/managers/InvisibleProjectImporterTest.java @@ -56,12 +56,37 @@ public void importCompleteFolderWithoutTriggerFile() throws Exception { } @Test + public void automaticJarDetectionLibUnderSource() throws Exception { + ClientPreferences mockCapabilies = mock(ClientPreferences.class); + when(mockCapabilies.isWorkspaceChangeWatchedFilesDynamicRegistered()).thenReturn(Boolean.TRUE); + when(preferenceManager.getClientPreferences()).thenReturn(mockCapabilies); + + File projectFolder = createSourceFolderWithLibs("automaticJarDetectionLibUnderSource"); + + IProject invisibleProject = importRootFolder(projectFolder, "Test.java"); + assertNoErrors(invisibleProject); + + IJavaProject javaProject = JavaCore.create(invisibleProject); + IClasspathEntry[] classpath = javaProject.getRawClasspath(); + assertEquals("Unexpected classpath:\n" + JavaProjectHelper.toString(classpath), 3, classpath.length); + assertEquals("foo.jar", classpath[2].getPath().lastSegment()); + assertEquals("foo-sources.jar", classpath[2].getSourceAttachmentPath().lastSegment()); + + List watchers = projectsManager.registerWatchers(); + watchers.sort((a, b) -> a.getGlobPattern().compareTo(b.getGlobPattern())); + assertEquals(9, watchers.size()); + String srcGlobPattern = watchers.get(7).getGlobPattern(); + assertTrue("Unexpected source glob pattern: " + srcGlobPattern, srcGlobPattern.equals("**/src/**")); + String libGlobPattern = watchers.get(8).getGlobPattern(); + assertTrue("Unexpected project glob pattern: " + libGlobPattern, libGlobPattern.endsWith(projectFolder.getName() + "/**")); + } + public void automaticJarDetection() throws Exception { ClientPreferences mockCapabilies = mock(ClientPreferences.class); when(mockCapabilies.isWorkspaceChangeWatchedFilesDynamicRegistered()).thenReturn(Boolean.TRUE); when(preferenceManager.getClientPreferences()).thenReturn(mockCapabilies); - File projectFolder = createSourceFolderWithLibs("automaticJarDetection"); + File projectFolder = createSourceFolderWithLibs("automaticJarDetection", "src", true); IProject invisibleProject = importRootFolder(projectFolder, "Test.java"); assertNoErrors(invisibleProject);