Adds Java support #28
Conversation
|
Is there any plans on incorporating this? |
|
@morganfogg Until they accept (or not) your merge request, how can I install it locally ? (I'm on Windows) |
|
@eurakilon You'll need NodeJS and the Gulp CLI installed ( After that, a file called 'sonarlint-vscode-1.6.0-SNAPSHOT.vsix' should appear in the sonarlint-vscode directory. In VS Code, open the command palette and run "Install from VSIX". Select the file. After a few seconds, you'll get a notification saying the extension was installed. (My development machine runs Linux so I haven't actually had a chance to test it on Windows, but I assume it should work the same) |
|
SonarJava works only when sonar.java.libraries and sonar.java.binaries are provided. So to me this change is not enough. We would have to integrate or replicate what Java language servers (like redhat one) do. I'm closing the PR for now, having support of Java in VSCode is currently not our priority. |
|
@henryju What's actually needed for you to support Java in this extension? We (Java team from Microsoft) would like to work with you together to resolve this. Will it be enough if we resolve this issue microsoft/vscode-java-dependency#158? Or do you need anything else? |
|
Hi @hexiaokai I'm very pleased to hear that you are interested in working together to solve this issue. Having the classes and libraries is already a good first step. The other information we would need to have all rules behavior correct would be the project source level. This is to set the property |
|
@hexiaokai would having maven or Gradle dependencies mapped into the build help drive this? I think that most teams wanting this capability would be able/willing to support that. |
|
Hi @henryju, Maybe a stupid question here: Just curious does that mean that some sules of |
|
@jdneo in fact the output/library paths are mandatory for all rules, not only Bug/Vulnerability. In the past we did the distinction of rules needing classpath vs "simple" rules, but it was confusing for users to have only partial results at the end of the analysis. Now SonarJava requires a proper classpath configuration to build the typed AST we are using to write rules. |
|
Thank you @henryju. So the SonarJava will only partially work if the output/library paths are not provided, is that true? Cuz I tried to directly enable the |
|
My bad, in the SonarLint context SonarJava will not fail the analysis but only log a warning and then work "at best". That's why you can see some issues. Still this is not satisfying for us, and not the user experience we want to offer to users. |
|
I see. Thank you @henryju for sharing. 😄 |
|
Hi @henryju, I have another question, for example, if there is an API like this: /**
* Get the classpath information
* @param uri Uri of the file/project needs to be queried
* @param classpathQueryOptions Query options
*/
export function getClasspath(uri: Uri, classpathQueryOptions: ClasspathQueryOptions): Promise<string[]>
export interface ClasspathQueryOptions {
/**
* determine the result should contain with test or not.
*/
includingTest: boolean;
}it just returns the classpaths of the project instead of separating the output and libs, will it work? Or will sonarlint require that binary output path and the lib path must be separated? |
|
@jdneo SonarJava separates libraries and binaries for historical reasons/backward compatibility with the SonarQube Findbugs plugin. For SonarLint that's not a big deal since we don't run third party analyzers, and your example of API would be great. |
|
Thank you @henryju. I will try to make out a private build for you recently. |
|
It's a pleasure to read that someone is working on the porting of Sonar Java into VCS. Regards, |
|
Hi @henryju, I've created related PRs to expose APIs to get project settings/classpaths, the source code changes can be found here:
API document is here: https://github.com/redhat-developer/vscode-java/blob/ce8c4797cb0222ccb0f705457942094711d64797/src/extension.api.ts#L9-L38 You can also use this private build to verify the APIs: https://drive.google.com/open?id=1ocOrSaFGs9qGjLCUtKgSPkuxbTe_pbmB Below is a simple usage example: const extension: Extension<any> | undefined = extensions.getExtension('redhat.java');
try {
const extensionApi: any = await extension!.activate();
if (extensionApi) {
// Suppose current active editor is a java source file.
const uri: string = window.activeTextEditor!.document.uri.toString();
console.log(await extensionApi.getProjectSettings(uri, ['org.eclipse.jdt.core.compiler.compliance']));
console.log(await extensionApi.getClasspaths(uri, { excludingTests: false }));
}
} catch (error) {
// Swallow the error
}If you have any concern, please just feel free to comment in the PRs. Thanks, |
|
Hi @jdneo and happy new year. I will have a look and let you know. |
|
Works fine so far, thanks. Do you know how expensive it is to call those methods? Especially In my test I'm querying classpath for every analysis. If performance is a concern, I should probably implement some caching mechanism (querying only once when file get opened). But then I would need a way to be notified when the configuration/classpath change on Java LS side (for example user adding a dependency to the pom.xml), to evict the cache. |
|
Hi @henryju, Thank you for your feedback. Some update:
Thanks. |
|
Another question: do you know if it would be possible to expose an API so that we could query if a given Java file is in a test container or not (to decide which classpath we should use). Currently in VSCode, we are deciding based on a file name pattern (**/Test.) but this is a bit weak, and since JDT has the information, it would be safer for us to get it, and not rely on the file name. |
|
Yes it's possible to expose the API to query the test scope, I can do that. |
|
Thanks! |
|
I did some updates to the API. The private build is here: https://drive.google.com/open?id=1RTng3i12kYu7C-v9MECTJPnJPNtk9lmA Please check here about the API changes and let me know if it meets the requirements. The changes are:
|
|
I have updated the two PRs on our side to take benefit of your latest additions. This is quick and dirty code, but feel free to comment if you see obvious mistakes (especially in the way to register the listener for the @ALL watching this thread, feel free to give a try using this artifact: @jdneo let me know if you need more tests from me. We will wait for the changes to be released in vscode-java and then do a proper sprint to add officially the support of Java in SonarLint VSCode! |
|
Hi @henryju, Thank you for the update! I tried the artifact but not able to launch. Seems that the Also I have some maybe stupid questions here 😄
|
My bad, fixed in https://repox.jfrog.io/repox/sonarsource/org/sonarsource/sonarlint/vscode/sonarlint-vscode/1.14.0-build.14248/sonarlint-vscode-1.14.0-build.14248.vsix
Yep, very likely. I will give a try with a modular project.
Good catch. I haven't tried a multi-module project, and I was assuming each module would be mapped to a workspace root folder (similar to the Eclipse behavior with modules mapped to Eclipse projects). I will try and let you know. |
|
Additional request/question: I see that JDK libraries are not included into the classpath. I am not sure this is a bid deal today, because of this issue on SonarJava side: https://jira.sonarsource.com/browse/SONARJAVA-3056 But just in case, do you think we could get access to the JDK libraries used to compile the project. For reference this is what we do on SonarLint Eclipse: |
In the APIs, there is a field called |
I think so, thanks. |
|
Hi @henryju I tried the new bits on my Mac, and found that sometime it will keep resolving the quickfix but without any response. Do you know why? |
|
That's strange, because we are only returning "static" commands as quick fixes (show rule description / disable rule) so it should not involve any Java specific code. |
|
Hi @henryju, I was trying diagnostic the problem, I tried to toggle the rule but then I got this error: The logs in output: BTW, if there is any other API request, please let me know. Thanks, |
|
@jdneo this is a busy week for me and my team, so I will investigate the issue on MacOS only next week.
This is all good IMO, we are waiting for the merge in vscode-java and then we'll do a cleaner implementation on our side shortly after. |
|
@henryju Ok. I will add some UTs for the APIs and make sure they could be merged ASAP. Thanks! |
|
Update: The PR has been merged, here is a latest build which you can play with: https://download.jboss.org/jbosstools/jdt.ls/staging/java-0.56.0-2015.vsix |
|
Hourra (and thanks)! We are planning a sprint very soon. Do you have any insight of when the new release of vscode-java with those changes will be published on the marketplace? |
|
We are in the QA process now. If everything works fine, hopefully it could be released in the next week. Stay tuned, I'll leave comments here if there is any updates. Thanks. |
|
Just FYI, the new version(0.56.0) with the APIs has been released. |
|
Perfect, on our side the sprint is planned to start next week. |
|
@jdneo The latest version of sonarlint-vscode is currently 1.14.0 if I'm not mistaken. Is the new version 0.56.0 about something else? |
|
@nazarimilad 0.56.0 is the version of vscode-java extension that SonarLint will require to enable analysis of Java code. New API exposed by vscode-java extension is a pre-requisite before we could have a good execution of our Java analyzer. |
|
I see, thanks for the clarification |
|
Hi @henryju, May I ask will SonarLint start the work to support Java in your next sprint(week)? |
Definitely, this will be the sprint purpose :) |
|
Great! Can't wait to see SonarLint supports Java in VS Code. 😃 |
|
Version 1.15.0 just released! Thanks again @jdneo for the support, and to all participants of this topic for the patience :) |
|
Celebration time! 🎉 @jdneo and @hexiaokai : this was a nice cross-team effort. Any chance you guys can echo the announcement to your VS Code community as well? |
Sure. We plan to mention this in various channels including our monthly blog and next release of Java extension pack. Thank you for the great work! |
9 participants
Adds support for the Java programming language. Tested on several projects, found no issues so far. Screenshot below.