spring-projects / sts4

The next generation of tooling for Spring Boot, including support for Cloud Foundry manifest files, Concourse CI pipeline definitions, BOSH deployment manifests, and more... - Available for Eclipse, Visual Studio Code, and Theia
https://spring.io/tools
Eclipse Public License 1.0
881 stars 205 forks source link

[Question] What are the features that are supported by idea-extensions/idea-boot-java #63

Closed gayanper closed 4 years ago

gayanper commented 6 years ago

What are features supported by idea-extensions/idea-boot-java ? And any planned development on this extension for future ?

gayanper commented 5 years ago

@BoykoAlex yes sorry i have accidentally ignored it. Just add in the latest PR

gayanper commented 5 years ago

@martinlippert Yes it seems that it works fine with non-springboot projects in vscode and eclipse. Can this be due to not passing any of the configurations into the language server in IDEA plugin ?

martinlippert commented 5 years ago

@gayanper The default value for the indexing of the Spring XML config files is "off", so you have to enable that via the configuration params to get that working. The indexing of the Spring annotations in non-boot projects should work out-of-the-box, there is no configuration option for that as far as I remember.

gayanper commented 5 years ago

@BoykoAlex will you be able to check the latest code with the changes from last PR and see why the symbols are not analyzed for none spring boot app. I'm kind if lost here why the symbols are not shown.

gayanper commented 5 years ago

@BoykoAlex and @martinlippert never mind its working, seems like its a IDEA LSP Client issue how the classpath is resolved in a multi module.

gayanper commented 5 years ago

@martinlippert seems like the IDEA plugin doesn't send sts.java.addClasspathListener, seems like the language server some how picks the projects pom.xml as a fallback, is that how it is working ?

kdvolder commented 5 years ago

seems like the IDEA plugin doesn't send sts.java.addClasspathListener

That would make sense. This is something that has been implemented specifically for Eclipse and Vscode environments. In Eclipse it directly ties into Eclipse JDT. In vscode it works by contributing an extension to the redhat Java language server (which is also essentially Eclipse JDT).

It might be possible to implement something similar for IntelliJ... but I haven't got good ideas on how to do this. I would guess that there's nothing resembling the JDT language server around in IntelliJ.

language server some how picks the projects pom.xml as a fallback, is that how it is working ?

Correct. We do want to get away from this at some point. But for the time being probably its the only realistic option in IntelliJ to rely on this.

BoykoAlex commented 5 years ago

Yes, that is correct. It shouldn't send that message because there is no JDT LS running along side to provide the classpath nor there is support on the IntelliJ client to send this message. The message is an extension to LSP. Therefore, the fallback mechanism is used which lives within spring boot LS. It runs maven to determine the classpath. See https://github.com/spring-projects/sts4/blob/master/headless-services/commons/commons-maven/src/main/java/org/springframework/ide/vscode/commons/maven/java/MavenProjectClasspath.java#L83

If you see highlights and code lenses I'd say classpath calculations is working... If it's about the non-boot spring projects XML not scanned then you need to send a config message to the server from the client... Examples: https://github.com/spring-projects/sts4/blob/master/eclipse-language-servers/org.springframework.tooling.boot.ls/src/org/springframework/tooling/boot/ls/DelegatingStreamConnectionProvider.java#L167 https://github.com/spring-projects/sts4/blob/master/atom-extensions/atom-spring-boot/lib/spring-boot-language-client.ts#L21

gayanper commented 5 years ago

Every thing is working fine, there was a problem with the pom.xml pickup on intellij multi module project, i will investigate that, seems something related to how workspace to LS is defined in intellij plugin.

At the same time a ver \y promising LS client implementation is going on called LSP4IntelliJ. https://github.com/NipunaRanasinghe/lsp4intellij

This is going to be backed by a well known OSS company WSO2. I'm gonna try to port this plugin to this inside my fork and see how well it work. Atleast we can get fixed approved in the lsp4intellij than the current plugin. :crossed_fingers:

thekalinga commented 5 years ago

@gayanper Its much better if the plugin is created using lsp4intellij as all developers will be able to contribute this plugin as its written completely in Java unlike the other library which is written in scala

gayanper commented 5 years ago

@thekalinga yes i'm in the process of migration of this plugin to lsp4intellij. I will commit some working state after i have the symbol support from lsp4intellij on my private fork.

gayanper commented 5 years ago

What are these methods defined in org.springframework.ide.vscode.commons.protocol.STS4LanguageClient. I mean the methods which take in JavaDataParams as parameters. Do they really need to implemented for the LS to work in non JDT environment ? what are their purpose ? @martinlippert @BoykoAlex

BoykoAlex commented 5 years ago

@gayanper As you noticed Spring Boot LS requires quite a bit of "Java knowledge" such as search for type given fully qualified name, access getters/setters on a java type, javadoc for various Java artifacts etc. At first we have implemented support for "Java knowledge" ourselves and use Jandex to index java classes:

The projects above is the result of that work.

Obviously, having support for Java indexing, search etc in the Spring Boot LS isn't ideal since this is the duplication of a Java LS extension features. Therefore, it'd be nice to get all java data either from JDT LS or simply JDT in the Eclipse case.

Later, we've discovered a way to communicate with JDT LS to fetch exactly the data we needed using JST LS extension plugins. The communication with JDT LS is done via the client. The messages sent to the client to get various java data are LSP extension messages and they are all in STS4LanguageClient interface (non-Java messages are: highlights, move cursor and progress. The rest are Java related)

There are 2 implementations for obtaining the Java data at the moment:

  1. JDT/JDT LS implementation with LSP extension messages

  2. Our own Indexing and search based on Jandex library.

  3. is used if sts/addClasspathListener request succeeds. Otherwise 2. is used as a fallback.

I believe in your case of IntelliJ extension the fallback implementation 2. is being used. It should work as most of our unit tests use the fallback implementation 2. Please double-check that you hit a breakpoint here https://github.com/spring-projects/sts4/blob/master/headless-services/spring-boot-language-server/src/main/java/org/springframework/ide/vscode/boot/app/BootLanguageServerParams.java#L120

However, it's desirable to switch to 1. for IntelliJ Boot extension. Therefore, ideally, we'd want to add support for all Java related messages from STS4LanguageClient in the IntelliJ extension's StsLanguageClientImpl class. It should be possible because all the Java data we'd like to have in Spring Boot LS is available in IntelliJ itself (its Java IDE plugin) If you're to start working on switching IntelliJ to 1. let me know I'll post instructions and sequence what's best to do first and what later.

gayanper commented 5 years ago

Great, i have converted the idea-plugin to java by integrating lsp4intellij LSP client library. I will send my work for you to review on my branch once lsp4intellij take all my PRs in and do a release. They we can start working on from their onward.

gayanper commented 5 years ago

@BoykoAlex I'm planning to start working on option 1 for Intellij. Do we have the documentation about it ? If we do this we can support for multiple modules in single project in idea as well like in eclipse right ?

BoykoAlex commented 5 years ago

I'd have to write the documentation. We didn't document our LSP extensions unfortunately. It is time to do so however. The documentation will appear on the wiki under STS LSP Extensions. I'll post here when it is completed. Not sure about the second question, but I think it is reasonable to expect it to be on the same level as in Eclipse.

BoykoAlex commented 5 years ago

@gayanper I have added documentation for STS4 LSP Java message extensions: https://github.com/spring-projects/sts4/wiki/Java-Messages There is still a part missing for Communication with JDT LS but it's irrelevant to IntelliJ's Spring Boot extension as you should be able to find all Java data with IntelliJ Java plugin. Let me know if anything is missing or needs clarification.

gayanper commented 5 years ago

@BoykoAlex i started implementing the classpath listeners, i have few questions

  1. Do we need to pass the output folders for each source folder ? because in case of generated sources the output folder will be same as the main source folder. In IDEA its really hard to find a corresponding source to output folder because they just entries of VirtualFile objects.

  2. Do we need to pass the source and documentation paths for dependent libraries, how to handle multiple jars in a single IDEA library ?

kdvolder commented 5 years ago

Trying to answer question 1.

The output folder is where the compiled classes end up. So it would never be a folder that contains source code, even if the source code is technically generated and, from some point of view, might be thought of as 'output'. For a generated source folder, you should figure out where it's compiled classes end up (since I assume the classes will get compiled), and that would be the corresponding output folder of that source folder.

In Eclipse not all source folders have explicit output folders. A project may also have a 'default' output folder, and this is considered implicitly as the output folder for any source folders that don't have an explicit output folder.

I don't quite understand question 2, so can't answer it. Maybe Alex can.

BoykoAlex commented 5 years ago

@gayanper I'll try to answer the question 2. Source - yes, Javadoc - not really as long as you implement the javadoc messages from STS4 protocol extension. Is IDEA library correspond to Eclipse "container"?... For example your IDEA library is a JRE/JDK lib with a bunch of JRE jars (for java 1.8 at least)? If yes, then each JAR is a classpath entry. For JRE/JDK JAR you'd need to set system flag to true

gayanper commented 5 years ago

@kdvolder and @BoykoAlex do i need the source and javadoc uris for libraries to start with ?

BoykoAlex commented 5 years ago

I think to get started on this you not necessarily need the source and javadoc. You can add source later and leave javadoc out even.

gayanper commented 5 years ago

@BoykoAlex if implement add and remove classpath messages, do i also need to implement all other java messages as well ?

BoykoAlex commented 5 years ago

@gayanper Not right away. It'll work overall but some features won't work. Like javadoc for example that is used in boot properties hovers. For that to work you'd either need to add javadoc location for claspath or implement the javadoc message handling. The hyperlinks to java resources from hovers won't work, go to definiton for boot properties etc.

gayanper commented 5 years ago

@BoykoAlex what should i set for TypeData.flags, is there any mapping to access modifiers to int values that STS LS understand ? How to represent public abstract ? Do i need to perform a BitWise operations like a binary bitwise OR to store multiple flags ?

gayanper commented 5 years ago

@BoykoAlex is the

FieldData declaringType: string; // Declaring type (in case of the current type being a nested type) binding key

is correct ?

gayanper commented 5 years ago

@BoykoAlex Do we have any utility classes to convert FQN to JVMNames (typebindings) and vise versa. Because there seems to be no utilities to JVM name of Fields and Methods in IntelliJ :(

BoykoAlex commented 5 years ago

@gayanper

  1. TypeData.flags. The flags should be standard for all IDEs i thought (since they seem to come from ASM). STS4 commons-java should be able to interpret that field: https://github.com/spring-projects/sts4/blob/master/headless-services/commons/commons-java/src/main/java/org/springframework/ide/vscode/commons/java/Flags.java
  2. FieldData.declaringType - copy/paste error. Should simply be the type in which the field is declared. Self-explanatory.
  3. FQName <-> Type Bindings. Yes:
gayanper commented 5 years ago

@BoykoAlex perfect, thanks i will try to use them.

gayanper commented 5 years ago

@BoykoAlex what do you think about implementing this plugin with kotlin ? Since idea recommend to use kotlin 🙂

BoykoAlex commented 5 years ago

Great idea! Initially it was a mix of Scala and Java because the language client lib API was Scala and I'm familiar with Java but Kotlin is the right choice i think.

gayanper commented 5 years ago

@BoykoAlex is the declaringType for

BoykoAlex commented 5 years ago

@gayanper JVMName (binding key)

gayanper commented 5 years ago

@BoykoAlex @martinlippert It seems my IDEA implementation just take symboles from all LS instances i start to Properties, YAML and Java. So i get duplicate entries. How you have solved this in vscode implementation ?

kdvolder commented 5 years ago

How you have solved this in vscode implementation ?

I don't think we ever had that problem. The vscode client generally doesn't start more than one instance. So maybe that is why. Is the IDEA LSP client starting one instance for every file you open or something?

gayanper commented 5 years ago

@kdvolder I start a Lang server for each language id which are JAVA, PROPERTIES and YAML, can i start a single server with all those lang ids ? So when i open all file types, that is a java file, a properties file and a yaml file there are 3 lang servers running.

kdvolder commented 5 years ago

can i start a single server with all those lang ids ?

That's how its meant to work. And that's how it is in vscode and also in Eclipse. I don't know how the setup is in IDEA (i.e the mechanics of how language server is started there, when you open a file that it cares about) so I don't know if that's possible there. If it isn't... then that's a problem and something that should be adressed somehow.

gayanper commented 5 years ago

@kdvolder Thanks for help, i fixed it :). Had to send a PR to the lsp4intellij to change how language id is handled which was done by myself few months back :).

gayanper commented 5 years ago

@BoykoAlex does sts4 language server provide diagnostic messages for yaml or property files ? If so in which cases does it provide ?

BoykoAlex commented 5 years ago

@gayanper yes, it does. If you enter server.port=blah the value blah should be underlined as an error because the number value is expected not a string. Unknown property keys would be underlined as warnings with quick fix available to add new property key to additional metadata file.

gayanper commented 5 years ago

@BoykoAlex what are the correct JSON configurations i should pass into language server to get xml support like content assist on spring configuration xml ? Does this support on fallback mode on the language server as well where i don't provide classpath entries ?

BoykoAlex commented 5 years ago

XML support is enabled-disabled via workspace/didChangeConfiguration messages. The contents of these messages have the JSON object with various settings. Usually client has settings UI where LS settings are contributed. Once settings on the client change the config changed message is sent. The config message is also sent once LS and client are initialized.

See org.springframework.ide.vscode.boot.app.BootJavaConfig for the details on the format of the configuration JSON.

Typical configuration message looks like:

{
  "jsonrpc": "2.0",
  "method": "workspace/didChangeConfiguration",
  "params": {
    "settings": {
      "boot-java": {
        "boot-hints": {
          "on": true
        },
        "support-spring-xml-config": {
          "content-assist": "true",
          "scan-folders-globs": "**/src/main/**",
          "hyperlinks": "true",
          "on": true // <--- XML support on/off
        },
        "remote-apps": [],
        "scan-java-test-sources": {
          "on": false
        },
        "change-detection": {
          "on": false
        }
      }
    }
  }
}
gayanper commented 5 years ago

@BoykoAlex thanks, i tried it and seems that it requires the sts4 specific java messages to be implemented to support content assistance on xml

thekalinga commented 5 years ago

@gayanper Can you plz share what is pending w.r.t this new extension. May be others might be able help

Thanks!

gayanper commented 5 years ago

@thekalinga You can see the changes here, this is kind of messed up branch which i do hackings, since i wanted to share the changes i just pushed the stuff. let me know if you are planning to contribute i can create the more cleaner branch to work with. https://github.com/gayanper/sts4/tree/lsp4intellij/idea-extensions/idea-boot-java

gayanper commented 5 years ago

I still believe the new idea LSP plugin which i use here is not live enough for this. Recently the activity of the plugin is really low. I was planning to create my own LSP plugin with the required changes to STS4 to start with. I'm open for volunteers who would like to work on it and keep it rolling without delays.

gayanper commented 5 years ago

@BoykoAlex i have implemented java message in https://github.com/gayanper/sts4/tree/java-messages. But i still get

WARNING: Unsupported request method: sts/addClasspathListener
12:45:28.501 [pool-3-thread-1] ERROR o.s.i.v.b.jdt.ls.JdtLsProjectCache - Unexpected error registering classpath listener with JDT. Fallback classpath provider will be used instead.
org.eclipse.lsp4j.jsonrpc.ResponseErrorException: Unsupported request method: sts/addClasspathListener
    at org.eclipse.lsp4j.jsonrpc.RemoteEndpoint.handleResponse(RemoteEndpoint.java:209)
    at org.eclipse.lsp4j.jsonrpc.RemoteEndpoint.consume(RemoteEndpoint.java:193)
    at org.springframework.ide.vscode.commons.languageserver.LanguageServerRunner.lambda$null$2(LanguageServerRunner.java:224)
    at org.eclipse.lsp4j.jsonrpc.json.StreamMessageProducer.handleMessage(StreamMessageProducer.java:194)
    at org.eclipse.lsp4j.jsonrpc.json.StreamMessageProducer.listen(StreamMessageProducer.java:94)
    at org.eclipse.lsp4j.jsonrpc.json.ConcurrentMessageProcessor.run(ConcurrentMessageProcessor.java:113)
    at java.util.concurrent.Executors$RunnableAdapter.call(Executors.java:511)
    at java.util.concurrent.FutureTask.run(FutureTask.java:266)
    at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1142)
    at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:617)
    at java.lang.Thread.run(Thread.java:745)
12:45:28.502 [pool-3-thread-1] ERROR o.s.i.v.c.l.u.SimpleLanguageServer - 
org.eclipse.lsp4j.jsonrpc.ResponseErrorException: Unsupported request method: sts/addClasspathListener
    at org.eclipse.lsp4j.jsonrpc.RemoteEndpoint.handleResponse(RemoteEndpoint.java:209)
    at org.eclipse.lsp4j.jsonrpc.RemoteEndpoint.consume(RemoteEndpoint.java:193)
    at org.springframework.ide.vscode.commons.languageserver.LanguageServerRunner.lambda$null$2(LanguageServerRunner.java:224)
    at org.eclipse.lsp4j.jsonrpc.json.StreamMessageProducer.handleMessage(StreamMessageProducer.java:194)
    at org.eclipse.lsp4j.jsonrpc.json.StreamMessageProducer.listen(StreamMessageProducer.java:94)
    at org.eclipse.lsp4j.jsonrpc.json.ConcurrentMessageProcessor.run(ConcurrentMessageProcessor.java:113)
    at java.util.concurrent.Executors$RunnableAdapter.call(Executors.java:511)
    at java.util.concurrent.FutureTask.run(FutureTask.java:266)
    at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1142)
    at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:617)
    at java.lang.Thread.run(Thread.java:745)
BoykoAlex commented 5 years ago

The Spring Boot IntelliJ extension should support sts/addClasspathListener and sts/removeClasspathListener requests. See how this is done for VSCode: https://github.com/spring-projects/sts4/blob/master/vscode-extensions/commons-vscode/src/classpath.ts The example closer to IntelliJ would be Eclipse's handling of these requests: https://github.com/spring-projects/sts4/blob/master/eclipse-language-servers/org.springframework.tooling.ls.eclipse.commons/src/org/springframework/tooling/ls/eclipse/commons/STS4LanguageClientImpl.java#L396 Follow the code starting from the line above.

gayanper commented 5 years ago

@BoykoAlex i added a debug point at my sts/addClasspathListener method implementation and i don't get a request into that. any reason for that ?

BoykoAlex commented 5 years ago

Where is the method implementation for sts/addClasspathListener? I'd expect it to be on the IntelliJ side not the LS side... As far as I understand Spring Boot LS sends sts/addClasspathListener request message successfully to the client. However the client on the IntelliJ side (extension of LSP client for IntelliJ) does not have a handler for the sts/addClasspathListener request and hence sends no response for the request. Therefore lsp4j blows up with org.eclipse.lsp4j.jsonrpc.ResponseErrorException If I understand correctly you should investigate a bit how to extend IntelliJ LSP client to add handlers for custom request messages such as sts/addClasspathListener... should be doable... think I've added some handlers for highlight notification messages to the plugin in the past...