thephpleague-period/PR.md

fix: apply the sign of chronometer/time strings to the whole Duration

Problem

Duration::fromChronoString() and Duration::fromTimeString() accept an optional leading sign so callers can build negative durations (the grammar defines a (?<sign>\+|-)? group). The sign was mishandled in two ways:

1. When the hour component is zero or absent, the sign is silently lost and a positive duration is returned.
2. When the hour component is present, only the hour is negated while the minute, second and fraction components stay positive, producing a duration with mixed sign components that does not represent the input.

Examples on the current code:

Duration::fromChronoString('-05:30');   // minute:second -> +5m30s  (sign lost)
Duration::fromChronoString('-1:05:30'); // -1h +5m +30s = -3270s, not -3930s
Duration::fromTimeString('-00:30');     // -> +30m       (sign lost)
Duration::fromTimeString('-01:30');     // -1h +30m = -1800s, not -5400s

Applying these durations to a date point therefore moves the date in the wrong direction or by the wrong amount.

Root cause

src/Duration.php, Duration::fromUnits(). The negative sign was concatenated onto the hour string only:

if ('-' === $units['sign']) {
    $units['hour'] = '-'.$units['hour'];
}

Because the string handed to DateInterval::createFromDateString() only carried a sign on the hours token, -0 hours 30 minutes normalized to a positive 30-minute interval, and -1 hours 5 minutes 30 seconds kept the minute and second tokens positive.

The fix

Prefix every unit token with the sign so the whole duration is negated consistently:

$sign = '-' === $units['sign'] ? '-' : '';

return self::fromDateString(
    $sign.$units['hour'].' hours '
    .$sign.$units['minute'].' minutes '
    .$sign.$units['second'].' seconds '
    .$sign.$units['fraction'].' microseconds'
);

DateInterval::createFromDateString() accepts a per-token sign, so this produces a DateInterval whose components all carry the correct sign. Verified against a reference date:

2021-01-01 00:10:00 + fromChronoString('-05:30')   = 2021-01-01 00:04:30
2021-01-01 02:00:00 + fromChronoString('-1:05:30')  = 2021-01-01 00:54:30
2021-01-01 00:20:00 + fromChronoString('-12:28.5')  = 2021-01-01 00:07:31.500000
2021-01-01 01:00:00 + fromTimeString('-00:30')      = 2021-01-01 00:30:00
2021-01-01 05:00:00 + fromTimeString('-01:30')      = 2021-01-01 03:30:00

Tests

New data provider signedDurationProvider with the test testSignedStringsApplySignToWholeDuration asserts the real elapsed effect of the signed strings by applying them to a reference DateTimeImmutable (helper independent, covers the zero-hour, full h:m:s and fractional-second cases).

Two existing data-provider rows pinned the buggy output and encode the defect, so they were corrected:

• fromChronoProvider negative chrono: -12:28.5 => PT12M28.5S (sign dropped) updated to a fully negative case -12:28 => PT-12M-28S.
• fromTimeStringProvider negative chrono: -12:28 => PT-12H28M (mixed sign) updated to -12:28 => PT-12H-28M.
• fromTimeStringProvider negative chrono with seconds: -00:00:28.5 => PT28.5S (sign dropped) updated to -01:02:28 => PT-1H-2M-28S.

The fractional-second and seconds-only negative cases are covered by the new date-arithmetic test instead of the string helper, because that helper renders negative fractional seconds and bare seconds-only intervals with pre-existing quirks that are out of scope here.

Gate

composer install && vendor/bin/phpunit --no-coverage

Result: exit code 0 - OK (562 tests, 933 assertions).

Counterfactual: reverting fromUnits() to the hour-only sign makes all five testSignedStringsApplySignToWholeDuration cases fail, e.g. fromTimeString('-01:30') yields 2021-01-01 04:30:00 instead of the expected 2021-01-01 03:30:00.