Jira URL

None — this is a [Misc] SonarQube cleanup commit.

Changes

Description

Records, in the code, why the transient modifiers that SonarQube reports as
java:S2065
("Fields in non-serializable classes should not be transient") are deliberate.

The 41 issues in SonarCloud

41 issues over 13 files. The diff is annotations and comments only — and
@SuppressWarnings has SOURCE retention, so the compiled bytecode is byte-for-byte identical.
No behaviour can change.

Why the rule is wrong here

The rule's premise is that transient is meaningless unless the class implements
java.io.Serializable, because java.io serialization is the only thing that reads the modifier.
In XWiki it is not: job statuses, job requests and the extension data reachable from them are
persisted by the job status store, which serializes them with XStream, and XStream honours
transient whatever the class implements.

That is verifiable in xwiki-commons rather than taken on trust:

• org.xwiki.job.internal.JobStatusSerializer#write serializes the job status with
  org.xwiki.xstream.internal.SafeXStream.
• SafeXStream extends XStream and installs a SafeReflectionProvider, which delegates
  visitSerializableFields to the JVM reflection provider — and that provider skips transient
  (and static) fields.
• Nothing in that path looks at java.io.Serializable at all.

So removing a modifier this rule points at is not a cleanup: it changes what gets written into the
stored job status. okf/sonarqube/index.md already lists java:S2065 as not worth fixing for this
exact reason ("load-bearing in XWiki … XWiki serializes job statuses and requests with XStream,
which honours transient. Removing it changes what gets persisted"), but that reasoning lived only
in the LLM knowledge base, so SonarCloud kept reporting the issues and the next developer to open
one of these files saw nothing. The resolution the XWiki Java Code Style prescribes for a false
positive is @SuppressWarnings("java:SXXXX") plus a // comment saying why, in the code; SonarCloud
then closes the issue at the next analysis because the rule stops being raised. The idiom is already
established here — master carries 26 @SuppressWarnings("java:S…") in xwiki-commons today.

The four arguments

What a reviewer has to check is four sentences plus which group each field belongs to.

A — an object the job status store writes out. These fields are runtime state of a job status; transient is what keeps them out of the stored status. Several already say so in their own Javadoc — "We don't want to serialize it" (AbstractJobStatus#parentJobStatus), "Don't serialize the XDOM because it can lead to a huge XML" (PDFExportJobStatus.DocumentRenderingResult#xdom) — and DefaultExtensionPlan#tree carries a TODO about serializing it.

B — the job progress. AbstractJobStatus#progress is not transient, so the progress tree is part of what the store serializes; these fields are the walk state inside it.

D — an extension repository. AbstractExtension#repository is not transient, so any Extension serialized inside a job status reaches its repository; transient is what stops the injected components and the repository caches being dragged into that graph.

E — a derived-value cache. A memoized toString()/hashCode()/URL that is rebuilt on demand and must not be written out when the instance travels inside a serialized job status or request.

Scope of each annotation

Following okf/conventions/code-style.md ("prefer the narrowest scope"), the annotation is on the
field where a file has one or two flagged fields, and on the class where it has three or
more and they all share one reason — repeating the same four-line comment on every field of such a class would be worse for the reader than one statement about the class.

Clarifications

• AbstractJobStatus and XWikiExtensionRepository already carried
  @SuppressWarnings("checkstyle:ClassFanOutComplexity"); since Java does not allow two
  @SuppressWarnings on one element, the key was merged into the existing annotation rather
  than added next to it.

• This is the "judgement" half of the sweep in the sense that it asks for an opinion on suppressing
  rather than fixing. Closing it costs nothing: the code keeps behaving exactly as it does today and
  the issues simply stay open.

Screenshots & Video

N/A

Executed Tests

Whole-repo reactor (the batch touches xwiki-commons-component-api, which reports
Tests run: 0 and then fails jacoco:check when built inside a -pl subset):

cd xwiki-commons && mvn install -Plegacy,quality -fae

BUILD SUCCESS — 2394 tests, 0 failures, 0 errors, 16:56, revapi:check green in every module.

The three repos were built in dependency order (commons → rendering → platform) in one chained run,
so the later legs verify against the modified commons/rendering jars rather than a downloaded
SNAPSHOT. Note that @SuppressWarnings has SOURCE retention, so no .class file in this diff
differs from master's — the build is a Checkstyle/Revapi gate here rather than a behaviour one.

Expected merging strategy

Squash and merge, no backport needed (master only).

Related

One sweep of the same rule across the three repos, split one PR per repo:

• platform — xwiki/xwiki-platform#6378 (20 issues)
• rendering — xwiki/xwiki-rendering#437 (3 issues)

The argument and its verification are identical in all three; each repo can be merged or closed independently.

🤖 Generated with Claude Code

https://claude.ai/code/session_01N15Bod7dV3iC7kjtwhK81E

Generated by Claude Code