Do not enforce private constructors on utility classes

in #16923 I already disagreed with Utility classes should not have public constructors (squid:S1118), because it makes code unnecessarily verbose, but I didn’t make myself clear: Classes with only “static” methods should not be instantiated (java:S2440) is the better rule, and that’s the only one that should exist. Which is what I request from the SonarQube developers.

I simply don’t consider it best practice to protect ourselves so hard from issues that simply never happen if we use common sense: if a class looks like a utility function, and behaves like a utility function then it is a utility function. This is duck typing, and works well in certain languages like Python. Unlike Kotlin, Java never had a good solution for this problem, but that doesn’t mean we should increase verbosity just because there is a very slight chance that someone may do the wrong thing. Instead, the wrong thing should be detected and reported as an issue, that’s what java:S2440 is about. I explained the details in the linked forum topic.

Also, there is a potential side effect of the rule:

You say, we can use Lombok to solve the problem too, well, it’s not necessarily a good library to use. It does come with downsides and risks, see for example:

So instead of being overly defensive in our code, we should focus on readability first and what actually yields productivity.

=> Please remove squid:S1118.

Hi,

I think the answer is going to be “If you don’t like the rule, don’t include it in your profile. But don’t try to kill it for the ones that do like it.”

But I’ll flag this for the language devs nonetheless.

 
Ann

1 Like

Hi @rkrisztian,
Thanks for starting this discussion again.

As @ganncamp proposed, I think that if the rule stands in your way and you are confident that the measures you have taken prevent users of your code from instantiating the class, then I think you could drop the rule from your quality profile.

However, you are correct in pointing out that our suggestion for a fix to introduce a private constructor that throws an exception is not the right way to go since it negatively impacts coverage.

So I created 2 tickets:

  1. SONARJAVA-5927 to replace the misleading compliant example in the docs
  2. SONARJAVA-5928 to introduce a quickfix to avoid reproducing the old issue

Let us know what you think.

Cheers,

Dorian