fix(api-core): properly implement Clock.withZone() in MonotonicClock

[poliglots]

Fix issue #12608: MonotonicClock.withZone() was ignoring the zone parameter and returning 'this', violating the java.time.Clock contract.

Changes:

• Add a 'zone' field to track timezone per instance
• Implement withZone() to create a new clock variant with the requested timezone, preserving monotonic timing
• Update getZone() to return the instance's actual zone
• Throw IllegalArgumentException on null zone (per Clock spec)
• Add tests verifying withZone() returns a new instance with correct zone and that monotonic timing is preserved

The singleton instance continues to use UTC. Creating timezone variants does not compromise monotonicity since Instant is timezone-agnostic and monotonicity derives from System.nanoTime().

• I hereby declare this contribution to be licenced under the Apache License Version 2.0, January 2004

[gnodet]

Review — Good fix for the Clock.withZone() contract, a few documentation nits

The core approach is sound: since Instant is timezone-agnostic and monotonicity derives from System.nanoTime(), creating timezone variants via withZone() doesn't compromise monotonic behavior. Good test coverage for the new behavior.

Findings

1. 🔴 [high] Contradictory Javadoc on withZone() — The @param tag says "or {@code null} to use UTC" (implying null is accepted), while the @throws tag says "IllegalArgumentException if zone is {@code null}" (rejecting null). The implementation rejects null, so the @param text is misleading — should read @param zone the target timezone, not null.

2. [medium] Exception type — The JDK Clock.withZone() contract specifies @param zone ... not null and all standard JDK Clock implementations use Objects.requireNonNull(zone, "zone") which throws NullPointerException. Consider using Objects.requireNonNull for consistency with the JDK contract.

3. [medium] Outdated class-level Javadoc — Lines 35-36 still say "This implementation is singleton-based and always uses UTC timezone. The clock cannot be adjusted to different timezones" — this is no longer accurate after this PR.

4. [low] Same-zone optimization — withZone() always creates a new instance, even when the requested zone equals the current zone. The JDK's Clock.SystemClock.withZone() returns this when zone.equals(this.zone). Adding this short-circuit avoids unnecessary allocations.

---

This review was generated by an AI agent (Claude Code) and may contain inaccuracies. Please verify all suggestions before applying.

Claude Code on behalf of Guillaume Nodet

[gnodet]

Contradictory Javadoc + wrong exception type: The @param says "or {@code null} to use UTC" but the code throws on null — the @param should say @param zone the target timezone, not null.

Also, the JDK Clock.withZone() convention uses NullPointerException (via Objects.requireNonNull), not IllegalArgumentException. Consider:

Suggested change

	* @throws IllegalArgumentException if zone is {@code null}
	public Clock withZone(ZoneId zone) {
	java.util.Objects.requireNonNull(zone, "zone");
	if (zone.equals(this.zone)) {
	return this;
	}
	return new MonotonicClock(startNanos, startInstant, zone);
	}

This also adds the same-zone optimization (returns this when the zone matches, matching JDK convention).

[poliglots]

Fixed review findings in the commit 41cc260

[gnodet]

All four findings from the previous review have been thoroughly addressed — great work, @poliglots! 👍

Previous findings → resolution:

1. ✅ Contradictory Javadoc → Fixed. @param now says "not null" and @throws NullPointerException is documented.
2. ✅ Wrong exception type → No longer applicable — withZone() now properly creates timezone variants instead of ignoring the parameter.
3. ✅ Stale class-level Javadoc → Fixed. Now correctly describes that timezone variants can be created via withZone().
4. ✅ Missing short-circuit optimization → Implemented with zone.equals(this.zone).

The new implementation is well-designed — sharing the monotonic timing base across zone variants via a private constructor is the right approach. The test coverage is comprehensive (new zone, same zone, null zone, instant consistency).

One very minor style note: java.util.Objects.requireNonNull(zone, "zone") uses a fully qualified class name rather than an import — most Maven code imports java.util.Objects. This should be caught by the formatter, so not blocking.

---

This review was generated by an AI agent (Claude Code) and may contain inaccuracies. Please verify all suggestions before applying.

Claude Code on behalf of Guillaume Nodet