spotbugs / spotbugs-maven-plugin

Maven Mojo Plug-In to generate reports based on the SpotBugs Analyzer
https://spotbugs.github.io/spotbugs-maven-plugin/
Apache License 2.0
69 stars 51 forks source link

Extend via dependencies rather than custom configuration #576

Open delanym opened 1 year ago

delanym commented 1 year ago

The usual way to extend plugin functionality is to add a dependency https://maven.apache.org/guides/mini/guide-configuring-plugins.html#Using_the_.3Cdependencies.3E_Tag

Is it too late for that?

hazendaz commented 1 year ago

Not sure I follow. Are you trying to solve something here?

Sent from my Verizon, Samsung Galaxy smartphone Get Outlook for Androidhttps://aka.ms/AAb9ysg


From: Delany @.> Sent: Friday, April 14, 2023 9:38:29 AM To: spotbugs/spotbugs-maven-plugin @.> Cc: Subscribed @.***> Subject: [spotbugs/spotbugs-maven-plugin] Extend via dependencies rather than custom configuration (Issue #576)

The usual way to extend plugin functionality is to add a dependency https://maven.apache.org/guides/mini/guide-configuring-plugins.html#Using_the_.3Cdependencies.3E_Tag

Is it too late for that?

— Reply to this email directly, view it on GitHubhttps://github.com/spotbugs/spotbugs-maven-plugin/issues/576, or unsubscribehttps://github.com/notifications/unsubscribe-auth/AAHODI2UVMA2476BEUOSWCDXBFHNLANCNFSM6AAAAAAW6OBWCQ. You are receiving this because you are subscribed to this thread.Message ID: @.***>

delanym commented 1 year ago

Well for one thing the plugin-plugins can't be declared under dependencyManagement and all the instrumentation that goes with that like updating via versions-maven-plugin or including in a bom. The dependency:resolve-plugins goal also comes up dry, and I imagine security checkers will not find these plugins.

Im also getting this annoying issue: https://github.com/spotbugs/spotbugs-maven-plugin/issues/296

See also https://github.com/spotbugs/spotbugs-maven-plugin/issues/23

hazendaz commented 1 year ago

@delanym Thank you, no its not too late to figure out a better way to write this. The plugins are passed to spotbugs though which is probably why the original design was done. But I'd have to look at this more to understand the original reason and why it cannot use standard practice. If you have an idea already how to rewrite this to do just that, feel free to try doing the work and raising PR for same. We have extensive integration tests on this so in theory it should be possible. Not to mention, it should be rather trivial if spotbugs requires special pass to it to switch to doing just that in standard way. In fact, I'm pretty sure the current logic even from maven standpoint is deprecated here and needs work as it is for maven 3.9 and ultimately 4.0.

I don't personally have a lot of time to look at this right now but do understand what the need is. Security checkers do see this. For example, see https://github.com/hazendaz/base-parent/issues/437 from Renovate, it knows of the plugins and will send updates when they are available to which it does.

delanym commented 1 year ago

Looks complicated. Not sure I've got the chops for it.

hazendaz commented 1 year ago

Thought more on this, here is part of the reason its done the way that it is. The plugins end up passed to spotbugs. But also there could be some unlimited # of plugins to add that we don't even know about. Anyone could write an extension. So that is likely why its this way. Yes having it known here in maven would allow it to work quite a bit differently but we would need to still know what to translate over to plugins in the end when calling spotbugs as its spotbugs that deals with the extensions themselves. Even if just on classpath, they would not really know its an extension to use as far as I know.

IMHO, the versions plugin is effectively redundant these days. It might also very well know about this as usage of spotbugs maven plugin is extremely high. Keeping this open though as interesting idea in general but would need to be balanced with keeping it working as-is as well.

hazendaz commented 1 year ago

So one thing that potentially could be done is to add them as dependencies, then have a pluginList that would know which ones to pluck from the classpath to pass onto spotbugs. That way its in no way limited but in some regard that duplicates setup and not sure of the value other than maybe to versions plugin if it doesn't already understand spotbugs. I say that only because at one point in time this plugin was actually under codehaus with the other mojos at least in various forks. And if not mistaken some long timers on maven worked on this at points in time long back.

delanym commented 1 year ago

IMHO, the versions plugin is effectively redundant these days.

Ok, but whatever replaces it would follow the same approach?

hazendaz commented 1 year ago

correct, it would have to. spotbugs requirements not ours in the plugin.