Use java-checks-testkit to detect rule violations

[fermadeiral]

This PR fixes #75.

We have two groups of processors: sonar-based and spoon-based. The difference between them is that in sonar-based processors the detection of rule violations is performed with Sonar web API, and in spoon-based processors the detection is purely done with Spoon. However, per discussion (see #75), we would like to have the help of SonarJava in the detection of rule violations, without the usage of the web API. This PR addresses this, and make all processors compatible.

On SonarJava side: It's not possible to directly retrieve the issues found by SonarJava. To do so, then, I followed what had started here, and my changes are here. With those changes we are able to access the issues found by SonarJava. Because I changed the SonarJava source code, we use java-checks-testkit through a jar added in Sonarqube-repair. Additionally, there was a bug in SonarJava when multiple files are analyzed, which is in a module other than java-checks-testkit, and it was also fixed, this is why we have a second jar (i.e., java-frontend-5.14.0-SNAPSHOT.jar) in our project.

On Sonarqube-repair side: In summary, in the method isToBeProcessed of each processor now, let's say the processor for rule X, there is a check to see if, according to SonarJava, there is a violation of the rule X in the position (considering file and line number) of the element that entered in the isToBeProcessed. I already realized that only this check is not enough to just return true in every processor we have. Because of this, I kept the other checks done with Spoon for now (we can clean unnecessary checks later, if any).

Some random points:

• The logic of CompareToReturnValueProcessor (see rule description), recently implemented by @henry-lp, was completely changed; the thing is that Henry was filtering CtMethod (because we wanted compareTo methods), but SonarJava actually creates an issue in the return line (which is in fact the noncompliant line), and then the line number of what Henry was filtering didn't match the line number of what SonarJava was returning. I fixed it.
• Because some processors used the Sonar web API, we had an argument named projectKey, which now was completely removed from the project.
• Not only all tests are passing, but I also compared the output files and they are exactly the same as before those changes.

I would like that at least 2 collaborators review this PR before merging, since it's a major change.

[monperrus]

This PR addresses this, and make all processors compatible.

Congrats!

Overall, it's good.

We don't want to push custom jars file in the repo. Instead, we add in the .travis.yml the lines to build it from your fork, something like

git clone --branch modified-version https://github.com/fermadeiral/sonar-java/
mvn install

[monperrus]

Plus do you plan to make a PR upstream on sonar-java?

[fermadeiral]

We don't want to push custom jars file in the repo. Instead, we add in the .travis.yml the lines to build it from your fork, something like

Done in the last commit, please check.

Plus do you plan to make a PR upstream on sonar-java?

No.

About the change regarding retrieving the issues found by SonarJava: I believe that their API is like that because the expected usage of the tool is not how we use it. I can open a PR about it, but I don't think it will be accepted.

About the change regarding fixing the bug around multiple files being analyzed: I don't know if this bug still exists in SonarJava. Let me explain. We are using the version 5.14.0, which is from August 2019. There are several versions after that one. We don't use a more recent one because all versions after 5.14.0 have a conflict with Spoon: apparently both of them use different versions of jdt. I don't know how to fix this issue, and to be honest I didn't try to search too much because my priority is to make all processors compatible and the version we are using is not that old (for now). However, I'm going to create an issue in our repo about this after this PR is merged, because we want to be able to upgrade SonarJava here one day. Finally, I don't know if the bug still exists in SonarJava because we are not using latest versions and maybe the bug was already fixed in them.

[monperrus]

Thanks for the explanation, I agree with the prioritization (follow-up is #89).

LGTM.

@henry-lp would you have a look?

[henry-lp]

@monperrus yes :) .

@fermadeiral , we need to install your modded sonar-java as a local dependency to run ?

This might be a bit problematic later when we run this in the CI like Jenkins (Not every Jenkins server has Maven). We can either :

• Make double checking with sonar-java as an optional step specified through JSAP
• Deploy the modded version to maven repo and import that for now (Until we maybe can merge these to the main sonar-java repo sometimes in the future)

WDYT ? :)

[fermadeiral]

we need to install your modded sonar-java as a local dependency to run ?

Yes, see the changes in the README.md and .travis.yml files.

This might be a bit problematic later when we run this in the CI like Jenkins (Not every Jenkins server has Maven).

How are we going to run sonarqube-repair in the first place, then? My point is: if we are going to run sonarqube-repair with a jar, the dependency of the changed sonar-java would be already packed with our sonarqube-repair jar.

Make double checking with sonar-java as an optional step specified through JSAP

I don't follow you. What does that mean?

Deploy the modded version to maven repo and import that for now

I believe we can do that, but I'm not sure what that changes, maybe I don't follow the root cause of the problem you are predicting.

[henry-lp]

@fermadeiral , I see. If the deployed version of Sonarqube repair contains your modded dependency then there's no problem. :)

[fermadeiral]

@henry-lp are you sure? Because we can find alternatives if how the things are now will complicate our life later.

[henry-lp]

I am sure about the integration part with Repairnator , since I import this as a dependency and as long as that contains the correct packages then I have no issues :) .

[fermadeiral]

Ok, then. Thank you for your feedback, @henry-lp!

[fermadeiral]

I performed two more commits, but only with minor updates according to the major changes this PR contains.