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.

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

feat: adjust component analysis for java-api #128

Merged
merged 5 commits into from
Sep 14, 2023

Conversation

xieshenzh
Copy link
Collaborator

Get rid of the lsp. Create UI for component analysis that leverages exhort-java-api.

@xieshenzh
Copy link
Collaborator Author

@zvigrinberg @ilan-pinto Please take a look. Thanks.

<vendor email="developers@redhat.com" url="https://www.redhat.com">Red-Hat</vendor>
<id>org.jboss.tools.intellij.analytics</id>
<name>Dependency Analytics</name>
<version>1.0</version>

Choose a reason for hiding this comment

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

should be version 0.7

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Copy link
Collaborator

@zvigrinberg zvigrinberg left a comment

Choose a reason for hiding this comment

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

@xieshenzh seems to be Very good work!.
In addition to @ilan-pinto comment, see also couple of minor comments of mine.
kindly update README.md as well , to indicate that currently only npm and maven package managers are supported in this version.

Did you had a chance to test it on couple of pom.xml and package.json manifests?
have you had a chance to test it on manifests with the exhortignore feature? ( checking the behavior that the plugin ignore them completely - the java-api shouldn't return vulnerabilities/recommendations of dependencies marked in manifests with exhortignore back in the report) - i think we can approve at this stage without this feature, but it would be nice to check whether it's working as expected.

public static boolean performAnalysis(String packageManager,
String fileName,
String filePath,
Collection<Dependency> dependencys) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Kindly rename argument collection name to dependencies

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Renamed, thanks.

}

public static Map<Dependency, DependencyReport> getReports(String filePath, Collection<Dependency> dependencys) {
return getCache(filePath).getAllPresent(dependencys);
Copy link
Collaborator

Choose a reason for hiding this comment

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

Kindly rename argument collection name to dependencies

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Renamed.

@xieshenzh
Copy link
Collaborator Author

Thanks @zvigrinberg

README.md is updated in this PR: #129

I tested it mainly with these two projects: https://github.com/snyk-labs/java-goof and https://github.com/snyk-labs/nodejs-goof

Scanning npm dependencies works when I changed the java-api to run the npm ls command in the project directory that contains the package.json file. If the package.json file is copied to a temp folder, the npm ls command could not generate the correct versions of the dependencies.
And exhortignore works for npm projects.

For maven projects, there is a scenario that the java-api doesn't work: the mvn help:effective-pom command fails for this pom.xml, when it is copied to a temp folder.
And exhortignore doesn't work for maven projects, I think it is because of this line of code. Also I am not sure if exhortignore works for maven dependencies with implicit versions (e.g. use a property as version or version is inherited).

I also need to check exhortignore on the client side. But I haven't implemented that yet.

@zvigrinberg
Copy link
Collaborator

Thanks @zvigrinberg

README.md is updated in this PR: #129

I tested it mainly with these two projects: https://github.com/snyk-labs/java-goof and https://github.com/snyk-labs/nodejs-goof

Scanning npm dependencies works when I changed the java-api to run the npm ls command in the project directory that contains the package.json file. If the package.json file is copied to a temp folder, the npm ls command could not generate the correct versions of the dependencies. And exhortignore works for npm projects.

For maven projects, there is a scenario that the java-api doesn't work: the mvn help:effective-pom command fails for this pom.xml, when it is copied to a temp folder. And exhortignore doesn't work for maven projects, I think it is because of this line of code. Also I am not sure if exhortignore works for maven dependencies with implicit versions (e.g. use a property as version or version is inherited).

I also need to check exhortignore on the client side. But I haven't implemented that yet.

Thanks @xieshenzh

For what concern the error you've experienced with mvn help:effective-pom, Yes you're right, when starting the API Clients, we probably didn't take into account pom.xml with modules inside it, as it was designed only to pass content of the pom.xml and that's it( for component analysis), so in component analysis, we'll need to consider if we want to enhance it and to expose another overloaded method that will let you pass in a path/string of a directory containing pom.xml with directory modules - i'll track this activity by opening a new ticket.

for exhortignore with maven, both cases you mentioned, i'll check it out.

@sonarcloud
Copy link

sonarcloud bot commented Sep 14, 2023

SonarCloud Quality Gate failed.    Quality Gate failed

Bug C 6 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

idea Catch issues before they fail your Quality Gate with our IDE extension sonarlint SonarLint

@zvigrinberg zvigrinberg merged commit dc3e21d into redhat-developer:main Sep 14, 2023
2 of 3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants