Disable per default or reduce severity of FindPublicAttributes, PA_PUBLIC_MUTABLE_OBJECT_ATTRIBUTE

[trancexpress]

After updating tospotbugs 4.8.0 (up from 4.7.3) we have dozens of issues found with the detector FindPublicAttributes, with medium severity.

Mutable public fields are common enough, having a medium issue for them will only result in what we did - disable the detector.

Probably the detector should be either disabled per default, or its severity should be low.

[iloveeclipse]

I would propose to lower the severity.

Simeon, can you check if you can propose a patch?
Original change introduced the detector is 64e8c9c / #1500.

@baloghadamsoftware, FYI.

[JuditKnoll]

@trancexpress Can you please share a bit more info about the hits? The detector is based on heuristics. Maybe it could be tweaked to cause less FPs.

[trancexpress]

@trancexpress Can you please share a bit more info about the hits? The detector is based on heuristics. Maybe it could be tweaked to cause less FPs.

I was under the impression that the detector reports public non-final fields? This is what we observe in our code base.

[iloveeclipse]

@JuditKnoll : this detector doesn't detect actual bugs, it is more about style or better design.

There are reasons to have classes with no getters and public fields, and reporting that as a "medium" prio warning reduces people confidence in spotbugs analysis.

The severity should be reduced by default, so it doesn't spam logs in environments (like ours) where any new high/medium prio bug warning is considered as a test fail.

[JuditKnoll]

@iloveeclipse : Thank you for explaining this. You are right, I agree with reducing the severity. However, even if it has low severity, if we can improve the detector and lower the annoying hits, that's an improvement, but these should be in separate PRs.

@trancexpress It reports the public non-final fields, which is written inside the defining class, but if it's only written in the constructor or some similar functions, then it's not reported. Also, in case of PA_PUBLIC_MUTABLE_OBJECT_ATTRIBUTE writing means that some kind of functions is called with the field as a parameter, which suggests that it writes the object.
See the details from the message for PA_PUBLIC_MUTABLE_OBJECT_ATTRIBUTE bugtype:

Please note that constructors, initializers and finalizers are exceptions, if only they write the field inside the class, the field is not considered as written by the class itself. In case of object-type fields "writing" means calling methods whose name suggest modification.

So if there are some other exceptions, or other way, the detector could be specified more precisely, or some obvious FPs, please let me know.

[iloveeclipse]

I agree with reducing the severity.

Could you please provide a PR, if you have time?