Breaking backward compatibility for utility classes

Development
1

Hi devs,

Since this is about breaking backward compatibility I need to send a vote. Basically in order to fix SonarQube rule java:S1118 (utility classes should not be instantiable), I’ve introduced a private constructor. Doing so removes the implicit public constructor, hence the breakage.

Now, utility classes contain only static methods and are not meant to be instantiated so while it’s technically a breakage, and some code could have instantiated the classes, it’s a “small” breakage. Also, we should probably not make utility classes public in the first place IMO so we could also consider that these classes were not meant to be public.

            <revapi.differences>
              <justification>
                These utility classes (only static members) had their implicit public constructor made private to
                follow SonarQube rule java:S1118 (utility classes should not be instantiable). Instantiating them was
                never useful, but this is technically a real breakage since existing code calling the constructor will
                fail at runtime.
              </justification>
              <criticality>highlight</criticality>
              <differences>
                <item>
                  <ignore>true</ignore>
                  <code>java.method.visibilityReduced</code>
                  <old>method void com.xpn.xwiki.XWikiConstant::&lt;init&gt;()</old>
                  <new>method void com.xpn.xwiki.XWikiConstant::&lt;init&gt;()</new>
                  <oldVisibility>public</oldVisibility>
                  <newVisibility>private</newVisibility>
                </item>
                <item>
                  <ignore>true</ignore>
                  <code>java.method.visibilityReduced</code>
                  <old>method void com.xpn.xwiki.i18n.i18n::&lt;init&gt;()</old>
                  <new>method void com.xpn.xwiki.i18n.i18n::&lt;init&gt;()</new>
                  <oldVisibility>public</oldVisibility>
                  <newVisibility>private</newVisibility>
                </item>
                <item>
                  <ignore>true</ignore>
                  <code>java.method.visibilityReduced</code>
                  <old>method void com.xpn.xwiki.util.TOCGenerator::&lt;init&gt;()</old>
                  <new>method void com.xpn.xwiki.util.TOCGenerator::&lt;init&gt;()</new>
                  <oldVisibility>public</oldVisibility>
                  <newVisibility>private</newVisibility>
                </item>
                <item>
                  <ignore>true</ignore>
                  <code>java.method.visibilityReduced</code>
                  <old>method void com.xpn.xwiki.web.Utils::&lt;init&gt;()</old>
                  <new>method void com.xpn.xwiki.web.Utils::&lt;init&gt;()</new>
                  <oldVisibility>public</oldVisibility>
                  <newVisibility>private</newVisibility>
                </item>
              </differences>
            </revapi.differences>

Note that the VOTE is not just about these classes but about the idea of fixing java:S1118 issues by introducing private constructors for utility classes.

WDYT?

Here’s my +1

Thanks

2

Note that I’ve pushed [Misc] Fix various SonarCloud issues (unused private code and utility… · xwiki/xwiki-platform@213958d · GitHub for now to unfail the build but I’ll revert it (and the commit that lead to it ([Misc] Fix various SonarCloud issues (unused private code and utility… · xwiki/xwiki-platform@220d2e9 · GitHub) in case we don’t get an agreement.

Thx

3

+1, if the classes have only static methods and properties, preventing the instantiation is relatively safe.

4

+1

Thanks,
Marius