[3.0] Let SMF\Time do arithmetic with an SMF\TimeInterval

[albertlast]

Description

The calendar is a fatal error on a stock install. Not on a forum that has had an event added to it — on every forum, because the installer seeds 28 holidays, and drawing any of them goes through Calendar\Event::__set():

$end = (clone $this->start)->add($this->duration);   // Event.php:1125

$this->duration is an SMF\TimeInterval, and PHP refuses it:

Object of type SMF\TimeInterval (inheriting DateInterval) has not been correctly
initialized by calling parent::__construct() in its constructor

This is the bug reported in #9384. It has been sitting there since, so here is a fix that does not require settling the design question first.

Why the constructor is not simply restored

PHP's date arithmetic reads a \DateInterval's internal state directly, not the property hooks. TimeInterval keeps its values in a private \DateInterval and serves them through hooks precisely so that $days can work — as its own docblock says, that is the only reliable way to get a usable $days in a subclass.

Calling parent::__construct() fixes the arithmetic and breaks that, because PHP then serves every hooked property from the internal state and ignores the hooks. Confirmed on the image's PHP 8.4.24:

class TI extends DateInterval {
	public mixed $days { get => $this->base->days; }   // base->days === 45
	private DateInterval $base;
	public function __construct(string $d) { …; parent::__construct($d); }
}

$ti = new TI('P45D');
// y=0 m=0 d=45 days=false      ← the hook is declared, and never consulted
// $dt->add($ti) → works

Assigning $this->days afterwards does not help either — it creates a deprecated dynamic property that shadows nothing, and reads still return false. Calendar\Holiday.php:214 reads ->days off a Time::diff() result, so losing it is not free.

What this does instead

TimeInterval::toDateInterval() hands out the instance it is wrapping, and SMF\Time::add() / ::sub() unwrap before calling PHP:

public function add(\DateInterval $interval): static
{
	parent::add($interval instanceof TimeInterval ? $interval->toDateInterval() : $interval);

	return $this;
}

Every caller that passes a TimeInterval — Event.php ×2, EventOccurrence.php ×3, Actions/Calendar.php — has an SMF\Time on the left-hand side, so they are all covered. A plain \DateInterval takes exactly the path it took before. TimeInterval's public API is unchanged, and $days still works.

It is a fix at the boundary rather than in the type, and I would rather you had the choice: the alternative is to make TimeInterval a genuine \DateInterval and give up the hooked $days, which changes a public property's behaviour and is your call, not mine. This unblocks the calendar either way.

Checked

On the running forum, before: action=calendar returns the fatal error page, Event.php:1125 in smf_log_errors. After: renders, and December 2026 marks the 21st and the 25th as class="days windowbg holidays", November the 11th and the 26th. Nothing new in the error log.

Rebased on the merged #9383 — the fractional duration fix — which this needs to construct the seeded holidays in the first place.

Issues References (Fixes|Related|Closes)

Fixes #9384
Related to #7933

[albertlast]

Pushed a second commit: the first one was incomplete.

Calendar\RecurrenceIterator holds its view start as a plain \DateTime or \DateTimeImmutable, depending on the recurrence type, rather than an SMF\Time — so it does not get the unwrapping that SMF\Time::add() does, and it threw the same error at RecurrenceIterator.php:342. Posting a calendar event (?action=calendar;sa=post) was still a fatal after the first commit. It now unwraps the view duration itself, with a comment saying why that one is different.

Found by re-sweeping the calendar's own pages after the first fix rather than stopping at ?action=calendar. All of these now render with nothing in the error log:

?action=calendar
?action=calendar;viewlist
?action=calendar;viewweek
?action=calendar;year=2026;month=12
?action=calendar;sa=post

This is the leak in fixing it at the boundary rather than in the type, and it is worth being plain about: every \DateTime/\DateTimeImmutable that meets a TimeInterval needs the same unwrapping. I checked the rest — Event.php ×2, EventOccurrence.php ×3, Actions/Calendar.php all have an SMF\Time on the left, and the remaining ->add()/->sub() calls in RecurrenceIterator pass $this->frequency_interval, which is a plain \DateInterval. So the set is closed today, but nothing stops it reopening.

Separately, ?action=admin;area=managecalendar still fails after this — but on something unrelated: Holiday.php:278, "Failed to parse time string (FREQ=YEARLY)". That is its own bug and I will raise it separately rather than widen this PR.

[Sesquipedalian]

We're not going to use this solution. I will submit a PR soon that restores full compatibility of SMF\TimeInterval with \DateInterval.