[Misc] Fix 30 SonarCloud java:S5785 issues by using dedicated JUnit assertions

[claude]

Fixes 30 SonarCloud java:S5785
issues in xwiki-rendering — "use the dedicated JUnit assertion instead of a boolean one".

Change

• assertTrue(a.equals(b)) → assertEquals(a, b)
• assertFalse(a.equals(b)) → assertNotEquals(a, b)
• assertTrue(a.hashCode() == b.hashCode()) → assertEquals(a.hashCode(), b.hashCode())
• assertFalse(a == b) → assertNotSame(a, b)

Operand order is deliberately preserved (the original receiver stays first): JUnit's
assertEquals(expected, actual) calls expected.equals(actual), so keeping the receiver first
reproduces the original call exactly — which matters for the equals(null) /
equals("other class") contract assertions in these tests.

Static imports were adjusted accordingly (assertTrue/assertFalse removed only where the file no
longer uses them).

Notes

• Test code only, no production change.
• Verified with mvn clean install -Plegacy,quality (Checkstyle, Revapi, JaCoCo and the unit tests)
  on the 3 modified modules — the converted assertions are exactly the tests that run.

Related

Same sweep in the sibling repos:

• xwiki-platform: [Misc] Fix 48 SonarCloud issues in various modules xwiki-platform#5993
• xwiki-commons: [Misc] Fix 28 SonarCloud issues in various modules xwiki-commons#1852

---

Generated by Claude Code

[michitux]

This feels wrong, these tests explicitly test the equals methods and using dedicated JUnit assertions might not call them in the ways that the test wanted to call them, or at the very least it is not obvious anymore that they are called correctly. For example, when comparing to null, nobody assures us that null will actually be passed to equals and assertNotEquals doesn't just always return false when one argument is null and the other is non-null.

[claude]

Thanks for the review — you're right that this is the wrong change to make here, and I'm closing the PR.

One factual clarification for the record, because it's not obvious from the call site (which is
precisely your point): with the argument order used in this PR the equals call is still made, and
made on the original receiver. JUnit 5's assertEquals/assertNotEquals both go through
AssertionUtils.objectsAreEqual(a, b), which is

if (a == null) {
    return b == null;
}
return a.equals(b);

so assertNotEquals(reference1, null) really does evaluate reference1.equals(null) (verified
against the junit-jupiter-api bytecode). That is why the conversion kept the original receiver
first everywhere instead of using Sonar's suggested order.

But that guarantee lives in JUnit's internals rather than in the test, which is your actual
objection, and it is a fair one: a reader can no longer see which object's equals runs, and the
next person to "tidy" these lines would very likely swap the arguments — Sonar itself has a rule
(java:S3415, "swap these 2 arguments") that would tell them to. For tests whose entire purpose is
to pin down the equals/hashCode contract, assertTrue(a.equals(b)) states the intent
unambiguously and is worth keeping.

Since all 30 sites in this PR are in dedicated equals/hashCode contract tests, there is no
useful subset to keep, so closing rather than reworking. The corresponding SonarCloud java:S5785
issues are marked as accepted/won't-fix with this rationale, and I've recorded it so future
automated passes skip java:S5785 inside equality-contract tests.

The two sibling PRs from the same sweep are unrelated to this concern (comment cleanup,
.stream().forEach(), Boolean.valueOf → parseBoolean) and are still open:
xwiki/xwiki-platform#5993 and xwiki/xwiki-commons#1852.

---

Generated by Claude Code

[claude]

Superseded by #390, which is the better way to close this out.

Rather than leaving these issues merely "accepted" in SonarCloud — where the reasoning is invisible to
anyone reading the test — #390 records it in the code, per the
SonarQube section of the Java Code Style:
@SuppressWarnings("java:S5785") with an explanatory comment on each of the 6 equals()/hashCode()
contract test methods, and not a single assertion touched. That way the next developer sees the reason
next to the code and is not tempted to "fix" it again.

---

Generated by Claude Code