SonarCloud raises java:S1214 (“Move constants defined in this interface to another class or enum”) on old public constant interfaces, e.g. org.xwiki.rendering.wikimodel.IWemConstants.
The observation is correct, but the rule’s remediation is a public API break: the interface has to be deleted and the constants republished elsewhere. We can do that with our legacy strategy (constants holder in the main module, interface re-added in the -legacy module), but the cost/benefit is poor: an API break plus a deprecation cycle plus a permanently duplicated interface, in exchange for no runtime or readability gain for anyone. And for the constant interfaces that classes actually implements to inherit the constants (the usual shape of that anti-pattern), it additionally costs one Compatibility* interface and one declare parents aspect per implementing class.
So I propose:
When a Sonar rule’s only remediation on a published API would be backward-incompatible, and the API is old and stable, we don’t refactor. We record the decision in the code — @SuppressWarnings("java:Sxxxx") plus a one-line comment stating why — rather than marking the issue “Accepted” in SonarCloud, where the reasoning is invisible to the next developer who comes along to “fix” it.
This applies to published APIs only. In internal packages there’s no backward-compatibility constraint, so we fix java:S1214 (and its like) properly — no suppression, no exclusion.
Add java:S1214 to the legacy-wide Sonar exclusions in xwiki-commons-pom (next to the existing java:S1133 one), since -legacy modules are exactly where such interfaces are supposed to live and must never be refactored. We should arguably have done this already.
If we agree, I’ll document it on JavaCodeStyle#SonarQube in a new section — including the existing java:S1133/legacy exclusion — and in our LLM coding guidelines.
+1 for this specific rule, but I wouldn’t blindly apply this to all backwards-incompatible changes on public APIs. For me, it depends more on the specific case - how likely is a breakage to actually affect somebody, how complex is the legacy move, how much does it improve our code in terms of cleanliness and consistency, …
Okay, great, then we agree, it’s just that after reading the following, I wasn’t so sure that this proposal wasn’t generic for all rules whose fix would be backwards-incompatible: