Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Package name not recognized when opening standalone java files #1766

Closed
wants to merge 1 commit into from
Closed

Package name not recognized when opening standalone java files #1766

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion org.eclipse.jdt.ls.core/src/org/eclipse/jdt/ls/core/internal/ProjectUtils.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -460,7 +460,7 @@ private static boolean isBinary(Path file) {
public static IPath resolveGlobPath(IPath base, String glob) {
IPath pattern = new org.eclipse.core.runtime.Path(glob);
if (!pattern.isAbsolute()) { // Append cwd to relative path
pattern = base.append(pattern);
pattern = base != null ? base.append(pattern) : pattern.makeAbsolute();
}
if (pattern.getDevice() != null) { // VS Code only matches lower-case device
pattern = pattern.setDevice(pattern.getDevice().toLowerCase());
Expand Down
27 changes: 13 additions & 14 deletions org.eclipse.jdt.ls.core/src/org/eclipse/jdt/ls/core/internal/handlers/BaseInitHandler.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -108,6 +108,17 @@ public InitializeResult initialize(InitializeParams param) {
logInfo("No workspace folders or root uri was defined. Falling back on " + workspaceLocation);
rootPaths.add(workspaceLocation);
}
Collection<IPath> triggerPaths = new ArrayList<>();
Collection<String> triggerFiles = getInitializationOption(initializationOptions, "triggerFiles", Collection.class);
if (triggerFiles != null) {
for (String uri : triggerFiles) {
IPath filePath = ResourceUtils.canonicalFilePathFromURI(uri);
if (filePath != null) {
triggerPaths.add(filePath);
}
}
}
PreferenceManager.setTriggerFiles(triggerPaths);
if (initializationOptions.get(SETTINGS_KEY) instanceof Map) {
Object settings = initializationOptions.get(SETTINGS_KEY);
@SuppressWarnings("unchecked")
Expand All @@ -126,18 +137,6 @@ public InitializeResult initialize(InitializeParams param) {
} else {
preferenceManager.getPreferences().setRootPaths(rootPaths);
}

Collection<IPath> triggerPaths = new ArrayList<>();
Collection<String> triggerFiles = getInitializationOption(initializationOptions, "triggerFiles", Collection.class);
if (triggerFiles != null) {
for (String uri : triggerFiles) {
IPath filePath = ResourceUtils.canonicalFilePathFromURI(uri);
if (filePath != null) {
triggerPaths.add(filePath);
}
}
}
preferenceManager.getPreferences().setTriggerFiles(triggerPaths);
Integer processId = param.getProcessId();
if (processId != null) {
JavaLanguageServerPlugin.getLanguageServer().setParentProcessId(processId.longValue());
Expand All @@ -146,7 +145,7 @@ public InitializeResult initialize(InitializeParams param) {
return initializationOptions;
}

private void registerWorkspaceInitialized() {
public static void registerWorkspaceInitialized() {
IEclipsePreferences prefs = InstanceScope.INSTANCE.getNode(IConstants.PLUGIN_ID);
prefs.putBoolean(IConstants.WORKSPACE_INITIALIZED, true);
try {
Expand All @@ -156,7 +155,7 @@ private void registerWorkspaceInitialized() {
}
}

private boolean isWorkspaceInitialized() {
public static boolean isWorkspaceInitialized() {
IEclipsePreferences prefs = InstanceScope.INSTANCE.getNode(IConstants.PLUGIN_ID);
return prefs.getBoolean(IConstants.WORKSPACE_INITIALIZED, false);
}
Expand Down
14 changes: 14 additions & 0 deletions ...ipse.jdt.ls.core/src/org/eclipse/jdt/ls/core/internal/handlers/BuildWorkspaceHandler.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,11 +28,13 @@
import org.eclipse.core.runtime.IProgressMonitor;
import org.eclipse.core.runtime.OperationCanceledException;
import org.eclipse.core.runtime.SubMonitor;
import org.eclipse.core.runtime.jobs.Job;
import org.eclipse.jdt.core.IJavaModelMarker;
import org.eclipse.jdt.ls.core.internal.BuildWorkspaceStatus;
import org.eclipse.jdt.ls.core.internal.ProjectUtils;
import org.eclipse.jdt.ls.core.internal.ResourceUtils;
import org.eclipse.jdt.ls.core.internal.managers.ProjectsManager;
import org.eclipse.jdt.ls.core.internal.syntaxserver.SyntaxInitHandler;

/**
* @author xuzho
Expand All @@ -50,6 +52,18 @@ public BuildWorkspaceStatus buildWorkspace(boolean forceReBuild, IProgressMonito
if (monitor.isCanceled()) {
return BuildWorkspaceStatus.CANCELLED;
}
while (!BaseInitHandler.isWorkspaceInitialized()) {
try {
Thread.sleep(100);
} catch (InterruptedException e) {
// ignore and keep waiting
}
}
try {
Job.getJobManager().join(SyntaxInitHandler.JAVA_LS_INITIALIZATION_JOBS, null);
} catch (OperationCanceledException | InterruptedException e) {
logException(e.getMessage(), e);
}
Comment on lines +55 to +66
Copy link
Contributor

@rgrunber rgrunber May 12, 2021 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this support the usage of triggerFiles or is this some separate issue discovered ? I think we should avoid having requests from the client waiting on each other.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sometimes VS Code calls BuildWorkspaceHandler.buildWorkspace(boolean, IProgressMonitor) before initializing a workspace.
@rgrunber you have try the following:

  • clean the Java LS workspace
  • create a vscode-java vsix with and without this code
  • install the vsix
  • unzip wrong-packagename
cd wrong-packagename
code .

https://github.com/snjeza/vscode-test/raw/master/java-0.79.3.vsix includes this PR.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@rgrunber @jdneo See redhat-developer/vscode-java#1941 (comment)

projectsManager.cleanupResources(projectsManager.getDefaultProject());
if (forceReBuild) {
SubMonitor subMonitor = SubMonitor.convert(monitor, 100);
Expand Down
2 changes: 1 addition & 1 deletion ...e.jdt.ls.core/src/org/eclipse/jdt/ls/core/internal/handlers/DocumentLifeCycleHandler.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,7 @@ public ICompilationUnit resolveCompilationUnit(String uri) {
if (belongedRootPath.isPresent()) {
IPath rootPath = belongedRootPath.get();
try {
invisibleProjectEnabled = InvisibleProjectImporter.loadInvisibleProject(filePath, rootPath, false, new NullProgressMonitor());
invisibleProjectEnabled = InvisibleProjectImporter.loadInvisibleProject(filePath, rootPath, false, false, new NullProgressMonitor());
} catch (CoreException e) {
JavaLanguageServerPlugin.logException("Failed to load invisible project", e);
}
Expand Down
28 changes: 24 additions & 4 deletions ...e.jdt.ls.core/src/org/eclipse/jdt/ls/core/internal/managers/InvisibleProjectImporter.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@
import org.eclipse.core.runtime.IPath;
import org.eclipse.core.runtime.IProgressMonitor;
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.Status;
Expand Down Expand Up @@ -80,7 +81,7 @@ public void importToWorkspace(IProgressMonitor monitor) throws OperationCanceled
return;
}

Collection<IPath> triggerFiles = preferencesManager.getPreferences().getTriggerFiles();
Collection<IPath> triggerFiles = PreferenceManager.getTriggerFiles();
if (triggerFiles == null || triggerFiles.isEmpty()) {
return;
}
Expand All @@ -91,21 +92,40 @@ public void importToWorkspace(IProgressMonitor monitor) throws OperationCanceled
return;
}

loadInvisibleProject(triggerJavaFile.get(), rootPath, true, monitor);
loadInvisibleProject(triggerJavaFile.get(), rootPath, true, false, monitor);
}

@Override
public void reset() {
// do nothing
}

public static void updateSourcePaths(IJavaProject javaProject) throws CoreException {
IProject project = javaProject.getProject();
if (ProjectUtils.isVisibleProject(project) || project.equals(ProjectsManager.getDefaultProject())) {
return;
}
IFolder workspaceLinkFolder = javaProject.getProject().getFolder(ProjectUtils.WORKSPACE_LINK);
IPath rootPath = workspaceLinkFolder.getLocation();
if (rootPath == null) {
return;
}
Collection<IPath> triggerFiles = PreferenceManager.getTriggerFiles();
if (triggerFiles != null && !triggerFiles.isEmpty()) {
Optional<IPath> triggerJavaFile = triggerFiles.stream().filter(triggerFile -> rootPath.isPrefixOf(triggerFile)).findFirst();
if (triggerJavaFile.isPresent()) {
loadInvisibleProject(triggerJavaFile.get(), rootPath, true, true, new NullProgressMonitor());
}
}
}

/**
* Based on the trigger file, check whether to load the invisible project to
* manage it. Return true if an invisible project is enabled.
*
* @throws CoreException
*/
public static boolean loadInvisibleProject(IPath javaFile, IPath rootPath, boolean forceUpdateLibPath, IProgressMonitor monitor) throws CoreException {
public static boolean loadInvisibleProject(IPath javaFile, IPath rootPath, boolean forceUpdateLibPath, boolean isUpdate, IProgressMonitor monitor) throws CoreException {
if (!ProjectUtils.getVisibleProjects(rootPath).isEmpty()) {
return false;
}
Expand Down Expand Up @@ -151,7 +171,7 @@ public static boolean loadInvisibleProject(IPath javaFile, IPath rootPath, boole

List<IPath> excludingPaths = getExcludingPath(javaProject, rootPath, workspaceLinkFolder);

IPath outputPath = getOutputPath(javaProject, preferencesManager.getPreferences().getInvisibleProjectOutputPath(), false /*isUpdate*/);
IPath outputPath = getOutputPath(javaProject, preferencesManager.getPreferences().getInvisibleProjectOutputPath(), isUpdate);

IClasspathEntry[] classpathEntries = resolveClassPathEntries(javaProject, sourcePaths, excludingPaths, outputPath);
javaProject.setRawClasspath(classpathEntries, outputPath, monitor);
Expand Down
26 changes: 2 additions & 24 deletions ...c/org/eclipse/jdt/ls/core/internal/managers/InvisibleProjectPreferenceChangeListener.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,15 +13,9 @@

package org.eclipse.jdt.ls.core.internal.managers;

import java.util.List;
import java.util.Objects;

import org.eclipse.core.resources.IFolder;
import org.eclipse.core.resources.IProject;
import org.eclipse.core.runtime.CoreException;
import org.eclipse.core.runtime.IPath;
import org.eclipse.core.runtime.NullProgressMonitor;
import org.eclipse.jdt.core.IClasspathEntry;
import org.eclipse.jdt.core.IJavaProject;
import org.eclipse.jdt.ls.core.internal.JavaLanguageServerPlugin;
import org.eclipse.jdt.ls.core.internal.ProjectUtils;
Expand All @@ -37,29 +31,13 @@ public void preferencesChange(Preferences oldPreferences, Preferences newPrefere
if (!Objects.equals(oldPreferences.getInvisibleProjectOutputPath(), newPreferences.getInvisibleProjectOutputPath()) ||
!Objects.equals(oldPreferences.getInvisibleProjectSourcePaths(), newPreferences.getInvisibleProjectSourcePaths())) {
for (IJavaProject javaProject : ProjectUtils.getJavaProjects()) {
IProject project = javaProject.getProject();
if (ProjectUtils.isVisibleProject(project)) {
continue;
}
if (project.equals(ProjectsManager.getDefaultProject())) {
continue;
}

try {
IFolder workspaceLinkFolder = javaProject.getProject().getFolder(ProjectUtils.WORKSPACE_LINK);
IPath rootPath = ProjectUtils.findBelongedWorkspaceRoot(workspaceLinkFolder.getLocation());
if (rootPath == null) {
continue;
}
List<IPath> sourcePaths = InvisibleProjectImporter.getSourcePaths(newPreferences.getInvisibleProjectSourcePaths(), workspaceLinkFolder);
List<IPath> excludingPaths = InvisibleProjectImporter.getExcludingPath(javaProject, rootPath, workspaceLinkFolder);
IPath outputPath = InvisibleProjectImporter.getOutputPath(javaProject, newPreferences.getInvisibleProjectOutputPath(), true /*isUpdate*/);
IClasspathEntry[] classpathEntries = InvisibleProjectImporter.resolveClassPathEntries(javaProject, sourcePaths, excludingPaths, outputPath);
javaProject.setRawClasspath(classpathEntries, outputPath, new NullProgressMonitor());
InvisibleProjectImporter.updateSourcePaths(javaProject);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This will always use the trigger files to update source paths, regardless of the sourcePaths preference. It's a breaking change.

Only if sourcePaths preference is explicitly changed, then need to update source paths. Otherwise, just update project output path directly via javaProject.setOutputLocation.

See @jdneo's PR https://github.com/eclipse/eclipse.jdt.ls/pull/1767/files

} catch (CoreException e) {
JavaLanguageServerPlugin.getProjectsManager().getConnection().showMessage(new MessageParams(MessageType.Error, e.getMessage()));
}
}
}
}

}
11 changes: 11 additions & 0 deletions ...lipse.jdt.ls.core/src/org/eclipse/jdt/ls/core/internal/preferences/PreferenceManager.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
import java.io.IOException;
import java.io.StringWriter;
import java.io.Writer;
import java.util.Collection;
import java.util.Hashtable;
import java.util.LinkedHashMap;
import java.util.List;
Expand All @@ -25,6 +26,7 @@
import org.apache.commons.lang3.StringUtils;
import org.eclipse.core.resources.IResource;
import org.eclipse.core.runtime.CoreException;
import org.eclipse.core.runtime.IPath;
import org.eclipse.core.runtime.ISafeRunnable;
import org.eclipse.core.runtime.ListenerList;
import org.eclipse.core.runtime.SafeRunner;
Expand Down Expand Up @@ -68,6 +70,7 @@ public class PreferenceManager {
private ListenerList<IPreferencesChangeListener> preferencesChangeListeners;
private IEclipsePreferences eclipsePrefs;
private static Map<String, Template> templates = new LinkedHashMap<>();
private static Collection<IPath> triggerFiles;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should not persist the trigger files. They should be used only once. And only use them to calculate a default source path when initializing a new invisible project.

If the user modifies the source paths in a subsequent action, such as changing the workspace setting java.project.sourcePaths, we should always follow the user's action to update the source paths.

This means that if the user sets sourcePaths to null, we should just clean up all the source paths. There is no need to fall back and use trigger files to calculate the source paths again. Since the user knows the settings to change the source paths, he/she can change them back if he/she wants.


public PreferenceManager() {
preferences = new Preferences();
Expand Down Expand Up @@ -315,4 +318,12 @@ public boolean isClientSupportsCompletionDocumentationMarkDown() {
return getClientPreferences() != null && getClientPreferences().isSupportsCompletionDocumentationMarkdown();
}

public static Collection<IPath> getTriggerFiles() {
return triggerFiles;
}

public static void setTriggerFiles(Collection<IPath> triggerPaths) {
triggerFiles = triggerPaths;
}

}
10 changes: 0 additions & 10 deletions org.eclipse.jdt.ls.core/src/org/eclipse/jdt/ls/core/internal/preferences/Preferences.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -516,7 +516,6 @@ public class Preferences {
private String settingsUrl;
private String formatterProfileName;
private Collection<IPath> rootPaths;
private Collection<IPath> triggerFiles;
private int parallelBuildsCount;
private int maxCompletionResults;
private int importOnDemandThreshold;
Expand Down Expand Up @@ -1553,15 +1552,6 @@ public Collection<IPath> getRootPaths() {
return rootPaths;
}

public Preferences setTriggerFiles(Collection<IPath> triggerFiles) {
this.triggerFiles = triggerFiles;
return this;
}

public Collection<IPath> getTriggerFiles() {
return triggerFiles;
}

public boolean isJavaFormatOnTypeEnabled() {
return javaFormatOnTypeEnabled;
}
Expand Down
5 changes: 5 additions & 0 deletions org.eclipse.jdt.ls.tests/projects/singlefile/wrong-packagename/src/mypackage/Bar.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
package mypackage;

public class Bar {

}
8 changes: 8 additions & 0 deletions org.eclipse.jdt.ls.tests/projects/singlefile/wrong-packagename/src/mypackage/Foo.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
package mypackage;

public class Foo extends Bar {

public static void main(String[] args) {
System.err.println("Foo");
}
}
2 changes: 2 additions & 0 deletions ...lipse.jdt.ls.tests/src/org/eclipse/jdt/ls/core/internal/JavaLanguageServerTestPlugin.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
import org.eclipse.core.resources.ResourcesPlugin;
import org.eclipse.core.runtime.NullProgressMonitor;
import org.eclipse.jdt.core.JavaCore;
import org.eclipse.jdt.ls.core.internal.handlers.BaseInitHandler;
import org.osgi.framework.BundleActivator;
import org.osgi.framework.BundleContext;

Expand All @@ -39,6 +40,7 @@ public void start(BundleContext context) throws Exception {
IWorkspaceDescription description = workspace.getDescription();
description.setAutoBuilding(true);
workspace.setDescription(description);
BaseInitHandler.registerWorkspaceInitialized();
}

/* (non-Javadoc)
Expand Down
3 changes: 1 addition & 2 deletions ...tests/src/org/eclipse/jdt/ls/core/internal/managers/AbstractProjectsManagerBasedTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -423,8 +423,7 @@ protected IProject importRootFolder(File projectFolder, String triggerFile) thro
protected IProject importRootFolder(IPath rootPath, String triggerFile) throws Exception {
if (StringUtils.isNotBlank(triggerFile)) {
IPath triggerFilePath = rootPath.append(triggerFile);
Preferences preferences = preferenceManager.getPreferences();
preferences.setTriggerFiles(Arrays.asList(triggerFilePath));
PreferenceManager.setTriggerFiles(Arrays.asList(triggerFilePath));
}
final List<IPath> roots = Arrays.asList(rootPath);
IWorkspaceRunnable runnable = new IWorkspaceRunnable() {
Expand Down
Loading