From c2243a593483ca2634fe7d3907b268401276e0a1 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?J=C3=A9r=C3=B4me=20Gamez?= <jerome@gamez.name>
Date: Sun, 23 Aug 2026 19:37:53 +0200
Subject: [PATCH] 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.
---
 src/WrappingClock.php       | 30 ++++++++++------
 tests/WrappingClockTest.php | 71 ++++++++++++++++++++++++++++++++++++-
 2 files changed, 89 insertions(+), 12 deletions(-)

diff --git a/src/WrappingClock.php b/src/WrappingClock.php
index b04f4f3..c60c921 100644
--- a/src/WrappingClock.php
+++ b/src/WrappingClock.php
@@ -4,9 +4,12 @@ declare(strict_types=1);
 
 namespace Beste\Clock;
 
+use Closure;
 use DateTimeImmutable;
 use InvalidArgumentException;
 use Psr\Clock\ClockInterface;
+use ReflectionMethod;
+use UnexpectedValueException;
 
 final class WrappingClock implements ClockInterface
 {
@@ -17,6 +20,9 @@ final class WrappingClock implements ClockInterface
         $this->wrappedClock = $wrappedClock;
     }
 
+    /**
+     * @throws InvalidArgumentException when the given object doesn't behave like a clock.
+     */
     public static function wrapping(object $clock): self
     {
         if ($clock instanceof ClockInterface) {
@@ -24,28 +30,30 @@ final class WrappingClock implements ClockInterface
         }
 
         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)) {
-            throw new InvalidArgumentException('$clock->now() must return a DateTimeImmutable');
+        $method = new ReflectionMethod($clock, 'now');
+
+        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 {
-            private object $clock;
+        $wrappedClock = new class($method->getClosure($clock)) implements ClockInterface {
+            private Closure $now;
 
-            public function __construct(object $clock)
+            public function __construct(Closure $now)
             {
-                $this->clock = $clock;
+                $this->now = $now;
             }
 
             public function now(): DateTimeImmutable
             {
-                assert(method_exists($this->clock, 'now'));
+                $now = ($this->now)();
 
-                $now = $this->clock->now();
-
-                assert($now instanceof DateTimeImmutable);
+                if (!$now instanceof DateTimeImmutable) {
+                    throw new UnexpectedValueException('$clock->now() must return a DateTimeImmutable');
+                }
 
                 return $now;
             }
diff --git a/tests/WrappingClockTest.php b/tests/WrappingClockTest.php
index 9146439..98c02ba 100644
--- a/tests/WrappingClockTest.php
+++ b/tests/WrappingClockTest.php
@@ -7,11 +7,13 @@ namespace Beste\Clock\Tests;
 use Beste\Clock\FrozenClock;
 use Beste\Clock\WrappingClock;
 use DateTimeImmutable;
+use DateTimeZone;
 use InvalidArgumentException;
 use PHPUnit\Framework\Attributes\CoversClass;
 use PHPUnit\Framework\Attributes\Test;
 use PHPUnit\Framework\Attributes\UsesClass;
 use PHPUnit\Framework\TestCase;
+use UnexpectedValueException;
 
 /**
  * @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]
     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');
 
+        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);
     }
 }