I want to deploy a SonarQube plugin that depends on SonarQube 5.6.6LTS, SonarJava 4.0 and FindBugs 3.5. This plugin helps developer to migrate from legacy Guava classes (such as Optional and SettableFuture) to standard Java 8 classes (such as Optional and CompletableFuture). Here is necessary information:
I find this kind of rules very useful, but I think they should ultimately be added to SonarJava, so that they can appear in SonarLint.
I canāt really give you feedback on the rule implementation, since I donāt know the findbugs API, but I find the plugin key a bit too generic. Maybe something like āguavamigrationā would be more specific.
Sorry I cannot get your point. You mentioned two things, then what is TODO in my side?
If itās just replacing key with āguavamigrationā, itās OK, I will do in next release 1.0.4.
If you suggest me to add to SonarJava, I need to say that I have no motivation to do so.
I havenāt tried to test it yet, but off-hand it looks like there are issues with requirements 3 and 7.
Specifically, since you donāt declare a sonar.pluginKey property (at least, I didnāt see one) then your entire artifact id becomes the plugin key: guava-helper-for-java-8, and that breaks the rule to have only alpha-numeric (lower case) keys. I know youāre planning to re-key the plugin, so please just keep this point in mind.
On #7, you donāt provide a link to a SonarCloud analysis. Maybe itās there, but Iām too busy (lazy) to go looking right now.
Beyond those requirements, Iām a bit confused. You said initially your plugin depends on the SpotBugs plugin, but your README says it provides the pluginā¦? And if the latter, what happens if I install your plugin on an instance where SpotBugs is already installed?
And in either case, I have to wonder if the rules wouldnāt be better in the SpotBugs plugin. But like Julien, Iām just expressing a personal opinion.
Soā¦ once the things listed above are sorted out, Iāll do the testing required before I can add you to the Marketplace, and weāll go from there.
Just now I released v1.0.4 that contains fix for plugin key, please have a try.
requirement 3
You can find plugin.key in this pom, itās now āguavamigrationā.
Iāve also confirmed that MANIFEST.MF contains expected plugin key.
requirement 7
You can find SonarCloud analysis in this page. Sorry for my laziness.
Quality gate has no problem.
Beyond those requirements, Iām a bit confused. You said initially your plugin depends on the SpotBugs plugin, but your README says it provides the pluginā¦?
My tool provides both SpotBugs plugin (not the āSpotBugs pluginā for SonarQube, itās plugin for SpotBugs) and SonarQube plugin, this is the point making you confused. Please kindly ignore āproviding SpotBugs pluginā part.
In my initial testing my server (v7.2) fails to start, even after I install the most recent FindBugs plugin.
I failed to pick up on this earlier (sorry!) but your initial message said your plugin depends on
None of those versions is current. In fact, since the fall, 6.7 has been the SonarQube LTS, and 5.6 hit EOL a couple months ago. Beyond that, SonarJava 4.0 was released two years ago; the current version is 5.5, and the current version of the FindBugs plugin is 3.7.
Sorry, but I cannot add a new plugin that is only compatible with a deprecated version of SonarQube. If you want to bring this up to compatibility with 6.7 and ping me again, Iāll be happy to re-start my testing.
That said, Iād like to give you a couple of totally optional sidenotes:
By convention, release artifacts (the plugin jars) are typically attached to the releases in GitHub. Youāve provided Maven download links, and after sorting through the directory, I was able to find what I needed, but it would be easier for users if you gave a direct link to the jar and/or added the jars to the releases in GitHub
Your initial message indicates a dependency on the FindBugs plugin, but I didnāt see that in a quick skim of your docs/README.md (maybe itās there; I didnāt ready thoroughly). When I installed FindBugs via the Marketplace, it upgraded SonarJava to the required version for me, so it may be that this dependency would be handled automatically once you made it into the Marketplace (I donāt have a good way to test that) but it would be helpful/friendly if your README proactively alerted users to the fact that theyāll need both - whether or not the FindBugs install is handled automatically by the Marketplace. Thatās especially important in the case of FindBugs since its sensor seems to run whether or not youāve got FindBugs rules in your profile, and that can have an impact on analysis time.
I also confirmed that my plugin doesnāt work with the latest sonar-java, that uses Guava 19.0.
It seems that I should not package Guava to my plugin.
Iāll test with latest SonarQube LTS, and propose next release v1.0.5.
server startup is successful with the new version of your plugin \o/
analysis with your rule enabled completes successfully (Note that I was too lazy to come up with a project that would actually violate the rule.)
However, I think this must be addressed:
youāve added your rule to the findbugs repository, and then overridden the name of that repository to be āGuavaHelperā. I find this renaming to be deceptive. In fact, your plugin doesnāt add 459 rules; it adds 1. Its FindBugs dependency adds the other 458. Instead, you should create your own repository and add your single rule to it.
And finally, what I think should be addressed:
I understand that you wrote this as a FindBugs rule so that you could provide a dual-purpose artifact: SonarQube plugin and SpotBugs plugin. However, this is a bit heavy on the SonarQube side. As I said earlier, the FindBugs sensor runs whether or not you have FindBugs rules enabled, and this can have a strong impact on analysis times, i.e. a high price to pay on a (more than) daily basis for your single rule. I believe this rule could be written for SonarQube without the FindBugs dependency. But thatās up to you.
the rule description is rather terse. It would benefit from expansion with at least the first few paragraphs from your README.md.
Ann
P.S. Itās incumbent on me to point out that this same job could be accomplished with SonarJavaās āTrack uses of disallowed classesā rule template.
Thanks for your review! I will reflect them to the next patch version.
According to this Javadoc in sonar-plugin-api, it is allowed to add rule to existing repository. But I renamed the FindBugs repository wrongly, Iāve fixed my code in this commit.
OK I will consider to implement rules based on sonar-java in next minor version. Firstly let me confirm that it works with sonar-findbugs.
I think youāre making a mistake in terms of visibility by adding your rule to the FindBugs repo, but thatās up to you. Iāve added this to the update center.
sorry to interfere here, but I was bit surprised by this statement.
What kind of issue have you encountered? Usually best way to deliver plugins is to use maven shade plugin and create fat jar by relocating all your dependencies inside your package. That way it shouldnāt matter which version of Guava you use.
I hadnāt note stacktrace, but I remember that basePlugin has dependency on method in legacy Guava that was removed recently. I tried to use useChildFirstClassLoader but it couldnāt be solution.
