13

I use Sonarqube 5.1 and experiment with the “Sonar way” Java quality profile. The job is simple: I want to define a global String constant for a missing media type:

public interface Utf8MediaType {
    String APPLICATION_JSON = "application/json;charset=UTF-8";
}

However, Sonarqube tells me this is bad practice in rule squid:S1214 – Constants should not be defined in interfaces. The long text talks about implementing this interface, which I didn’t intend to, but I give in and create a class instead:

public class Utf8MediaType {
    public static final String APPLICATION_JSON = "application/json;charset=UTF-8";
}

However, this is considered to be a major design issue in rule squid:S1118 – Utility classes should not have public constructors. So it’s urging me to add a private constructor. Of course, this constructor has then to come first not to violate conventions in rule squid:S1213 – The members of an interface declaration or class should appear in a pre-defined order. I guess after that I might even get common-java:InsufficientBranchCoverage because the private constructor is not covered in tests.

These are the default rules and I feel they are a bit silly in combination. I have more examples where the defaults just don’t work for us (TestNG support is lacking). What can I do about it? What do you recommend?

  • Give in. Make it a class, add a private constructor, use introspection in the unit test. Makes the code ten times as big. For a String constant.
  • Create a list of exceptions. But doing this for each project may lead to long lists and invites people to add exceptions even for important stuff.
  • Deactivate rules. Now I would prefer not to tamper with the default profiles, because that may mean a lot of work on Sonarqube upgrades.
  • Create a profile that inherits from the default and overwrites things. It turns out that when you inherit from a profile you cannot deactivate rules. You can only add additional rules and change the configuration of rules (to lower their severity).
Michael Piefel
  • 18,660
  • 9
  • 81
  • 112
  • 1
    "you cannot deactivate rules" <-- are you sure about that? – fge May 29 '15 at 07:19
  • Well, @fge, if I knew how to do it, I’d do it. That would be my personal preference. – Michael Piefel May 29 '15 at 07:35
  • Log in as admin, click on the Rules tab, select the profile that you use, find the rule and click the "deactivate" button... – Jack Jun 16 '15 at 14:49
  • There is no ‘deactivate’ button in my version of Sonarqube. Be aware that I referred to profiles _inheriting_ from another profile. When I have a profile ‘My way’ inheriting from ‘Sonar way’, then that button is only on the ‘Sonar way’ line for a rule. – Michael Piefel Jun 17 '15 at 12:51
  • What about using an ENUM for all your media types? as your media types grows/changes, you can easily adjust your class for it? – blurfus Jul 10 '15 at 20:22
  • 1
    The media types are used as values for certain annotations. Therefore they have to be strings, and they have to be compile-time constants. – Michael Piefel Jul 13 '15 at 07:32
  • As a fan of brevity, I prefer `interface` as it makes `public static final` obsolete on all the lines. I don't see the risk of somebody wanting to implement your `Utf8MediaType` interface for example. Sonarlint certainly has not my support on that rule :D – Giszmo Oct 03 '19 at 09:55

2 Answers2

4

Give in. Make it a class, add a private constructor, use introspection in the unit test. Makes the code ten times as big. For a String constant.

This is the correct approach in general. You really do not want to create a "Constants" interface. The private constructor is needed to ensure that users do not inadvertently extend or instantiate an object that should not be instantiated.

How to add test coverage to a private constructor?

Create a list of exceptions. But doing this for each project may lead to long lists and invites people to add exceptions even for important stuff.

Too much work.

Deactivate rules. Now I would prefer not to tamper with the default profiles, because that may mean a lot of work on Sonarqube upgrades.

As you said... bad idea.

Create a profile that inherits from the default and overwrites things. It turns out that when you inherit from a profile you cannot deactivate rules. You can only add additional rules and change the configuration of rules (to lower their severity).

If you set the severity to "info" it will remove it from the Technical Debt calculation. I had to do that with squid:S1213 which is raised when I sort using the default order specified by Eclipse.

Community
  • 1
  • 1
Archimedes Trajano
  • 35,625
  • 19
  • 175
  • 265
0

It's been a few years since this was asked but sonarlint is still complaining about

public interface Utf8MediaType {
    String APPLICATION_JSON = "application/json;charset=UTF-8";
}

and now sonarlint proposes

public final class Utf8MediaType {
    public static final String APPLICATION_JSON = "application/json;charset=UTF-8";
}

but I still dislike these "extra" public static final that I don't need when using an interface. In our project we transition to Kotlin which can be done on a file by file basis and there the way to go is:

object Utf8MediaType {
    const val APPLICATION_JSON = "application/json;charset=UTF-8"
}

which is even shorter than a Java interface.

Giszmo
  • 1,944
  • 2
  • 20
  • 47
  • Though that doesn’t solve my Java problem at all, you get a big _YES_ from me for using Kotlin. I just have to wait a little bit longer until my employer officially allows it as a Java alternative. – Michael Piefel Oct 04 '19 at 16:30
  • I must say as an Android developer I'm getting massively deceived by Google. Java11 is much more like Kotlin than Java8 what we are stuck with on Android. If you are on Android though, there is no reason not to move to Kotlin. It's the official way to go according to Google. – Giszmo Oct 05 '19 at 02:06