Harden WrappingClock validation

Wrapping an object should not call it just to validate the return value.
Instead, we check the method signature when the object is wrapped, and
the return value at runtime. We were implicitly checking it already
with the `assert()` calls.
This commit is contained in:
Jérôme Gamez
2026-08-23 20:03:38 +02:00
parent e8f20c112e
commit c2243a5934
2 changed files with 89 additions and 12 deletions
+19 -11
View File
@@ -4,9 +4,12 @@ declare(strict_types=1);
namespace Beste\Clock; namespace Beste\Clock;
use Closure;
use DateTimeImmutable; use DateTimeImmutable;
use InvalidArgumentException; use InvalidArgumentException;
use Psr\Clock\ClockInterface; use Psr\Clock\ClockInterface;
use ReflectionMethod;
use UnexpectedValueException;
final class WrappingClock implements ClockInterface final class WrappingClock implements ClockInterface
{ {
@@ -17,6 +20,9 @@ final class WrappingClock implements ClockInterface
$this->wrappedClock = $wrappedClock; $this->wrappedClock = $wrappedClock;
} }
/**
* @throws InvalidArgumentException when the given object doesn't behave like a clock.
*/
public static function wrapping(object $clock): self public static function wrapping(object $clock): self
{ {
if ($clock instanceof ClockInterface) { if ($clock instanceof ClockInterface) {
@@ -24,28 +30,30 @@ final class WrappingClock implements ClockInterface
} }
if (!method_exists($clock, 'now')) { if (!method_exists($clock, 'now')) {
throw new InvalidArgumentException('$clock must implement StellaMaris\Clock\ClockInterface or have a now() method'); throw new InvalidArgumentException('$clock must implement Psr\Clock\ClockInterface or have a now() method');
} }
if (!($clock->now() instanceof DateTimeImmutable)) { $method = new ReflectionMethod($clock, 'now');
throw new InvalidArgumentException('$clock->now() must return a DateTimeImmutable');
if (!$method->isPublic() || $method->getNumberOfRequiredParameters() > 0) {
throw new InvalidArgumentException('$clock->now() must be public and accept no required parameters');
} }
$wrappedClock = new class($clock) implements ClockInterface { $wrappedClock = new class($method->getClosure($clock)) implements ClockInterface {
private object $clock; private Closure $now;
public function __construct(object $clock) public function __construct(Closure $now)
{ {
$this->clock = $clock; $this->now = $now;
} }
public function now(): DateTimeImmutable public function now(): DateTimeImmutable
{ {
assert(method_exists($this->clock, 'now')); $now = ($this->now)();
$now = $this->clock->now(); if (!$now instanceof DateTimeImmutable) {
throw new UnexpectedValueException('$clock->now() must return a DateTimeImmutable');
assert($now instanceof DateTimeImmutable); }
return $now; return $now;
} }
+70 -1
View File
@@ -7,11 +7,13 @@ namespace Beste\Clock\Tests;
use Beste\Clock\FrozenClock; use Beste\Clock\FrozenClock;
use Beste\Clock\WrappingClock; use Beste\Clock\WrappingClock;
use DateTimeImmutable; use DateTimeImmutable;
use DateTimeZone;
use InvalidArgumentException; use InvalidArgumentException;
use PHPUnit\Framework\Attributes\CoversClass; use PHPUnit\Framework\Attributes\CoversClass;
use PHPUnit\Framework\Attributes\Test; use PHPUnit\Framework\Attributes\Test;
use PHPUnit\Framework\Attributes\UsesClass; use PHPUnit\Framework\Attributes\UsesClass;
use PHPUnit\Framework\TestCase; use PHPUnit\Framework\TestCase;
use UnexpectedValueException;
/** /**
* @internal * @internal
@@ -60,6 +62,29 @@ final class WrappingClockTest extends TestCase
); );
} }
#[Test]
public function itDoesNotCallNowWhileWrapping(): void
{
$clock = new class() {
public int $calls = 0;
public function now(): DateTimeImmutable
{
++$this->calls;
return new DateTimeImmutable();
}
};
$wrappedClock = WrappingClock::wrapping($clock);
self::assertSame(0, $clock->calls);
$wrappedClock->now();
self::assertSame(1, $clock->calls);
}
#[Test] #[Test]
public function itRejectsObjectsWithANowMethodReturningANonDateTimeImmutable(): void public function itRejectsObjectsWithANowMethodReturningANonDateTimeImmutable(): void
{ {
@@ -70,9 +95,53 @@ final class WrappingClockTest extends TestCase
} }
}; };
$this->expectException(InvalidArgumentException::class); $this->expectException(UnexpectedValueException::class);
$this->expectExceptionMessage('$clock->now() must return a DateTimeImmutable'); $this->expectExceptionMessage('$clock->now() must return a DateTimeImmutable');
WrappingClock::wrapping($clock)->now();
}
#[Test]
public function itRejectsObjectsWithoutANowMethod(): void
{
$clock = new class() {
};
$this->expectException(InvalidArgumentException::class);
$this->expectExceptionMessage('$clock must implement Psr\Clock\ClockInterface or have a now() method');
WrappingClock::wrapping($clock);
}
#[Test]
public function itRejectsObjectsWithANonPublicNowMethod(): void
{
$clock = new class() {
protected function now(): DateTimeImmutable
{
return new DateTimeImmutable();
}
};
$this->expectException(InvalidArgumentException::class);
$this->expectExceptionMessage('$clock->now() must be public and accept no required parameters');
WrappingClock::wrapping($clock);
}
#[Test]
public function itRejectsObjectsWithANowMethodRequiringArguments(): void
{
$clock = new class() {
public function now(string $timeZone): DateTimeImmutable
{
return new DateTimeImmutable('now', new DateTimeZone($timeZone));
}
};
$this->expectException(InvalidArgumentException::class);
$this->expectExceptionMessage('$clock->now() must be public and accept no required parameters');
WrappingClock::wrapping($clock); WrappingClock::wrapping($clock);
} }
} }