diff --git a/PR.md b/PR.md new file mode 100644 index 0000000..3c36426 --- /dev/null +++ b/PR.md @@ -0,0 +1,66 @@ +# fix: parse fractional seconds in ChronosTime as a fraction of a second + +## Problem + +`ChronosTime` misinterprets a time string whose fractional-seconds part has fewer +than six digits. The fractional part is treated as a literal microsecond count +instead of a fraction of a second: + +```php +(new Cake\Chronos\ChronosTime('12:00:00.5'))->format('H:i:s.u'); +// "12:00:00.000005" (5 microseconds) +// expected "12:00:00.500000" (half a second, matching DateTime) + +(new DateTime('12:00:00.5'))->format('u'); // "500000" +``` + +So `12:00:00.5` becomes 5 microseconds rather than 500000, diverging from PHP's +own `DateTime` and from the round-trip used when constructing a `ChronosTime` +from a `DateTimeInterface` (which formats to six digits). + +## Root cause + +`src/ChronosTime.php`, `parseString()`: + +```php +$microseconds = (int)substr($matches[4] ?? '', 0, 6); +``` + +`$matches[4]` is the digit run after the decimal point. Passing it straight to +`(int)` reads `"5"` as `5`, ignoring that these are the most-significant digits of +a fractional second. Only six-digit input (for example `.500000`) happened to +work, which is why the issue was not caught: every existing test uses a full +six-digit fraction (or a seven-digit one that is truncated to six). + +## The fix + +Right-pad the (at most six) fractional digits to microseconds before converting: + +```php +$microseconds = (int)str_pad(substr($matches[4] ?? '', 0, 6), 6, '0', STR_PAD_RIGHT); +``` + +- `.5` -> `500000` +- `.05` -> `50000` +- `.500000` -> `500000` (unchanged) +- `.000005` -> `5` (unchanged) +- `.9999991`-> truncated to `999999` (unchanged; existing behaviour preserved) + +This matches how `DateTime` interprets fractional seconds. + +## Test + +Added `ChronosTimeTest::testConstructFromStringWithFractionalSeconds`, asserting +that `12:00:00.5`, `12:00:00.05` and `12:00:00.000005` produce +`500000`, `50000` and `5` microseconds respectively. + +## Gate + +``` +composer install && vendor/bin/phpunit +``` + +Result: exit code `0` - `OK (981 tests, 2496 assertions)`. + +Counterfactual: reverting `parseString()` to `(int)substr(...)` makes the new test +fail - `12:00:00.5` yields `12:00:00.000005` instead of `12:00:00.500000`. diff --git a/src/ChronosTime.php b/src/ChronosTime.php index f066978..596fbe6 100644 --- a/src/ChronosTime.php +++ b/src/ChronosTime.php @@ -122,7 +122,10 @@ class ChronosTime implements Stringable $hours = (int)$matches[1]; $minutes = (int)$matches[2]; $seconds = (int)($matches[3] ?? 0); - $microseconds = (int)substr($matches[4] ?? '', 0, 6); + // The fractional part is a fraction of a second, so it must be padded + // on the right to microseconds. Without padding ".5" would be read as + // 5 microseconds instead of 500000 (half a second). + $microseconds = (int)str_pad(substr($matches[4] ?? '', 0, 6), 6, '0', STR_PAD_RIGHT); if ($hours > 24 || $minutes > 59 || $seconds > 59 || $microseconds > 999_999) { throw new InvalidArgumentException(sprintf('Time string `%s` contains invalid values.', $time)); diff --git a/tests/TestCase/ChronosTimeTest.php b/tests/TestCase/ChronosTimeTest.php index 1be8a37..9f8f233 100644 --- a/tests/TestCase/ChronosTimeTest.php +++ b/tests/TestCase/ChronosTimeTest.php @@ -53,6 +53,19 @@ class ChronosTimeTest extends TestCase $this->assertSame('00:59:59.999999', $t->format('H:i:s.u')); } + public function testConstructFromStringWithFractionalSeconds(): void + { + // The fractional part is a fraction of a second, matching DateTime. + $t = new ChronosTime('12:00:00.5'); + $this->assertSame('12:00:00.500000', $t->format('H:i:s.u')); + + $t = new ChronosTime('12:00:00.05'); + $this->assertSame('12:00:00.050000', $t->format('H:i:s.u')); + + $t = new ChronosTime('12:00:00.000005'); + $this->assertSame('12:00:00.000005', $t->format('H:i:s.u')); + } + public function testConstructFromInstance(): void { $t = new ChronosTime(new DateTimeImmutable('23:59:59.999999'));