-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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.
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
Switch Java infrastructure to JDT LS #6157
Comments
/cc @fbricon |
Depends on: #6575 |
Are |
@fbricon yes, they're different. |
@tsmaeder Created eclipse-lsp4j/lsp4j#163 for LSP4J that brings the proposed LSP changes for file level operations. |
Following the proof of concept work in #5730, we have come up with a list of gaps that we need to fill in order to keep the current level of Java support in Che with a jdt.ls-based back end.
Endgame
Go criteria
Currently (with a fixed 1Gig memory for jdt.ls), The Che in Che scenario takes a very long time and fails at import in the end. There are two approaches to fix this problem:
There is a suite of selenium tests that covers Java IDE stuff quite well. We need to update it to work with jdt.ls make it pass. Product management to decide whether any differences from "classic" behaviour we find is ok.
We need to decide what kind of testing we need to do to make us confident about jdt.ls in Che.
Current top concerns
PoC cleanup
Normally, the Che LSP implementation starts one language server instance per project (vs. one instance for the whole workspace). This probably does not scale in the case of jdt.ls. We could manage a single LS instance for the whole workspace and have the per project instances act as proxies for the single LS.
jdt.ls user preferences can be set for jdt.ls through the LSP. Che currently has not support for this.
Set up jdt.ls extension project
Necessary bugfixes
References/Definitions
The current "usages" view has has two main differences to the LSP references and defintions views.
An excerpt of the source code around the usage is shown for each occurrence.
We will address this gap in two steps:
Implementations
There is no support for this in the LSP. Move the current code into jdt.ls che extension + make available as custom command.
Refactoring/move
There is no support for this in the LSP. Take the code from Che and put it into the jdt.ls extension. Move to jdt.ls when convenient.
see eclipse-jdtls/eclipse.jdt.ls#363
Refactoring/rename
jdt.ls supports this LSP call. However, the resulting workspace edit command does not support renaming or moving files, unlike the current rename support (see microsoft/language-server-protocol#272). Our preferred solution would be to get the LSP bug fixed. @gorkem is talking to MS on this topic. Otherwise we'd have to have a protocol extension (custom command) from jdt.ls that allows "full" rename refactorings.
Organize Imports
There is no support for this in LSP. This is implemented in jdt.ls as a code action, but we'd need a protocol extension to call it explicitly.
Show external libraries
The current Java support shows "external libraries" like the jre libraries in the explorer. LSP has no support for this, so we'd have to extend jdt.ls with a suitable protocol. This would be a good opportunity to show maven dependencies and such in the libraries.
There is also Expose Java Classpath eclipse-jdtls/eclipse.jdt.ls#48, but it's probably easier to get this up an running in our own extension.
Configure classpath
Che has UI to manipulate the classpath for "simple" Java projects. It seems that the classpath is managed in a file in /.che/classpath that looks a lot like a JDT classpath file.
We will implement a custom command that talks to the jdt IProject and adds libraries/source folders.
Migration of current "Simple Java" project could be done via jdt.ls extension point or we could detect the case in Che and offer conversion.
Navigate File Structure
Che Java support shows a structured view of the syntactic structure of Java files ("Navigate file structure").
![innerclasses](https://user-images.githubusercontent.com/13163770/30634456-748e68e6-9def-11e7-97f2-4950e7e60998.png)
LSP allows to fetch the document symbols as a flat list. While symbol information can contain the name of the container, in a case like the above, a unique tree structure cannot be deduced from that information for the two inner classes named "bork" (because their parents are named the same).
So in order to correctly implement this, we must rely on the containment of source ranges on the symbols.
Also, "Navigate Structure" allows to show all inherited members, as well. There is no protocol for this in LSP, so we'd need to add an extension to jdt.ls.
The idea is to implement two commands in jdt.ls, one with inherited members and one without.
Maven Plugin
While removing jdt stuff from Che, I found out that maven project type has things which rely on a maven server+jdt running in the maven server plugin:
Report progress on maven
The maven plugin reports progress on updating and resolving projects. Right now, the Java LS does not report any meaningful progress (It has a non-standard protocol extension for reporting status, though). Note that while inconvenient, lack of this feature would not be a loss of functionality.
@fbricon not quite sure how to report progress for multiple tasks.
Get effective pom
This should be available in jdt.ls and can be exposed via a protocol extension. Make custom command in jdt.ls Che extension & call m2e API to get the effective pom.xml.
Error reporting for pom.xml
According to @fbricon, jdt.ls sends diagnostics for pom.xml. We can remove this support from the maven plugin.
Reimport Maven Projects
This sounds a lot like "update project" in M2E. Is this necessary?
Reimport project deletes the project dependencies from the local .m2 repo, as well. Need to port to a custom command and do the correct thing.
Java Debugger Plugin
Put code into a jdt.ls extension custom command:
Testing plugins
plugin-testing-classpath
Again, move the code into jdt.ls Che extension & provide custom commands.
Simple Java Projects
Support for adding source folders and jars from the workspace. We would need to add support for this in jdt.ls. As far as I know, jdt.ls alread DOES handle preexisting Eclipse/Java projects, so the addition would only involve the creation/manipulation of the project classpath/setup
Find out whether we can tell jdt.ls to use the classpath file in ".che/classpath". If we can, we can reuse that location. Otherwise, need to implement some migration code (use a importer extension in jdt.ls). Need to move the code that initializes the java project (AbstractJavaInitHandler subclass) to the jdt.ls extension
The text was updated successfully, but these errors were encountered: