Why deprecate Java S2039 (Member variable visibility should be specified)?

I recently noticed that rule S2039 was deprecated, meaning it is subject to removal. I see this was a while back (ref: [SONARJAVA-4018] - Jira) so I probably should have noticed it sooner, but hey… sometimes you can go a while without a developer running into a specific rule, right? :slight_smile:

Just to recap for those not familiar with this rule.

  • This is currently a “code smell”
  • It’s not enabled by default in the Sonar Way profile
  • Current write-up: “Failing to explicitly declare the visibility of a member variable could result it in having a visibility you don’t expect, and potentially leave it open to unexpected modification by other classes.”
  • Deprecation notes say this is a valid language feature, and “even if the use of this visibility is rather uncommon, we end up raising an issue for every legitimate usage.”

I will start with an acknowledgement that I understand why some would not want to enable it, and why it may not be included as a default in “Sonar Way”; it is a language feature and there are reasons to use it.

That being said… I think this is a good rule.

I would start by saying that >99% of the time this comes up, the reason is because a developer just quickly added a field and didn’t bother to put the visibility to it. Junior developers or developers coming from other languages can make this error easily. This is why I only now encountered this deprecation, after all!

More importantly… I’ve been in Java a long time – literally 20 years – and the number of times that I have seen a legitimate use of default visible member fields is less than once a year. Every one of those has been a very cautious design choice and has required explicit documentation. It’s breaking encapsulation. This is the exact same risk as for public fields. As per s1104 “Member values are subject to change from anywhere in the code and may not meet the programmer’s assumptions.” I’ve seen code that is hard to fix/figure out because the field is being accessed directly, and it’s one of the reasons I have this (S2039) currently enabled on our rule set!

So… bottom line… my request is to please maintain this rule. It’s really not hurting anything sitting there as an optional rule, and I do think it is a good one even if it’s not “good enough” for the core sonar way profile.