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.
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.
You can find plugin.key in this pom, it’s now “guavamigration”.
I’ve also confirmed that MANIFEST.MF contains expected plugin key.
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.
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.
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.
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.