Fix Cloning

src/Attributes.php

@@ -4,8 +4,10 @@
 
 use ArrayAccess;
 use ArrayIterator;
+use Closure;
 use InvalidArgumentException;
 use IteratorAggregate;
+use ReflectionFunction;
 use Traversable;
 
 use function ipl\Stdlib\get_php_type;
@@ -365,29 +367,19 @@ public function setPrefix($prefix)
     /**
      * Register callback for an attribute
      *
-     * @param string   $name            Name of the attribute to register the callback for
-     * @param callable $callback        Callback to call when retrieving the attribute
-     * @param callable $setterCallback  Callback to call when setting the attribute
+     * @param string    $name           Name of the attribute to register the callback for
+     * @param ?callable $callback       Callback to call when retrieving the attribute
+     * @param ?callable $setterCallback Callback to call when setting the attribute
      *
      * @return $this
-     *
-     * @throws InvalidArgumentException If $callback is not callable or if $setterCallback is set and not callable
      */
-    public function registerAttributeCallback($name, $callback, $setterCallback = null)
+    public function registerAttributeCallback(string $name, ?callable $callback, ?callable $setterCallback = null): self
     {
         if ($callback !== null) {
-            if (! is_callable($callback)) {
-                throw new InvalidArgumentException(__METHOD__ . ' expects a callable callback');
-            }
-
             $this->callbacks[$name] = $callback;
         }
 
         if ($setterCallback !== null) {
-            if (! is_callable($setterCallback)) {
-                throw new InvalidArgumentException(__METHOD__ . ' expects a callable setterCallback');
-            }
-
             $this->setterCallbacks[$name] = $setterCallback;
         }
 
@@ -518,4 +510,58 @@ public function getIterator(): Traversable
     {
         return new ArrayIterator($this->attributes);
     }
+
+    /**
+     * Rebind all callbacks that point to `$oldThisId` to `$newThis`
+     *
+     * @param int    $oldThisId
+     * @param object $newThis
+     */
+    public function rebind(int $oldThisId, object $newThis): void
+    {
+        $this->rebindCallbacks($this->callbacks, $oldThisId, $newThis);
+        $this->rebindCallbacks($this->setterCallbacks, $oldThisId, $newThis);
+    }
+
+    /**
+     * Loops over all `$callbacks`, binds them to `$newThis` only where `$oldThisId` matches. The callbacks are
+     * modified directly on the `$callbacks` reference.
+     *
+     * @param callable[] $callbacks
+     * @param int        $oldThisId
+     * @param object     $newThis
+     */
+    private function rebindCallbacks(array &$callbacks, int $oldThisId, object $newThis): void
+    {
+        foreach ($callbacks as &$callback) {
+            if (! $callback instanceof Closure) {
+                if (is_array($callback) && ! is_string($callback[0])) {
+                    if (spl_object_id($callback[0]) === $oldThisId) {
+                        $callback[0] = $newThis;
+                    }
+                }
+
+                continue;
+            }
+
+            $closureThis = (new ReflectionFunction($callback))
+                ->getClosureThis();
+
+            // Closure is most likely static
+            if ($closureThis === null) {
+                continue;
+            }
+
+            if (spl_object_id($closureThis) === $oldThisId) {
+                $callback = $callback->bindTo($newThis);
+            }
+        }
+    }
+
+    public function __clone()
+    {
+        foreach ($this->attributes as &$attribute) {
+            $attribute = clone $attribute;
+        }
+    }
 }

src/BaseHtmlElement.php

@@ -75,6 +75,9 @@ abstract class BaseHtmlElement extends HtmlDocument
     /** @var string Tag of element. Set this property in order to provide the element's tag when extending this class */
     protected $tag;
 
+    /** @var int Holds an ID to identify itself, used to get the ID of the Object for comparison when cloning */
+    private $thisRefId;
+
     /**
      * Get the attributes of the element
      *
@@ -83,6 +86,8 @@ abstract class BaseHtmlElement extends HtmlDocument
     public function getAttributes()
     {
         if ($this->attributes === null) {
+            $this->thisRefId = spl_object_id($this);
+
             $default = $this->getDefaultAttributes();
             if (empty($default)) {
                 $this->attributes = new Attributes();
@@ -105,6 +110,8 @@ public function getAttributes()
      */
     public function setAttributes($attributes)
     {
+        $this->thisRefId = spl_object_id($this);
+
         $this->attributes = Attributes::wantAttributes($attributes);
 
         $this->attributeCallbacksRegistered = false;
@@ -352,4 +359,18 @@ public function renderUnwrapped()
             $tag
         );
     }
+
+    public function __clone()
+    {
+        parent::__clone();
+
+        if ($this->attributes !== null) {
+            $this->attributes = clone $this->attributes;
+
+            // `$this->thisRefId` is the ID to this Object prior of cloning, `$this` is the newly cloned Object
+            $this->attributes->rebind($this->thisRefId, $this);
+
+            $this->thisRefId = spl_object_id($this);
+        }
+    }
 }

tests/AttributesTest.php

@@ -113,4 +113,75 @@ function ($v) use (&$value) {
 
         $this->assertEquals(' foo="bar rab" bar="foo"', $attributes->render());
     }
+
+    public function testAttributesAreDeepCloned()
+    {
+        $attributes = Attributes::create(['class' => 'one']);
+
+        $clone = clone $attributes;
+        $clone->add('class', 'two');
+
+        $this->assertNotSame(
+            $attributes->get('class'),
+            $clone->get('class'),
+            'Attribute instances are not cloned'
+        );
+        $this->assertSame(
+            'one',
+            $attributes->get('class')->getValue(),
+            'Attribute instances are not cloned correctly'
+        );
+        $this->assertSame(
+            ['one', 'two'],
+            $clone->get('class')->getValue(),
+            'Attribute instances are not cloned correctly'
+        );
+    }
+
+    public function testCallbacksOfClonedAttributesPointToTheirClone()
+    {
+        $element = new class extends BaseHtmlElement {
+            protected $value;
+
+            protected $noGetterOrSetter;
+
+            public function setValue($value)
+            {
+                $this->value = $value;
+            }
+
+            public function getValue()
+            {
+                return $this->value;
+            }
+
+            protected function registerAttributeCallbacks(Attributes $attributes)
+            {
+                $attributes->registerAttributeCallback('value', [$this, 'getValue'], [$this, 'setValue']);
+                $attributes->registerAttributeCallback('data-ngos', function () {
+                    return $this->noGetterOrSetter;
+                }, function ($value) {
+                    $this->noGetterOrSetter = $value;
+                });
+            }
+        };
+
+        $element->setAttribute('value', 'foo');
+
+        $clone = clone $element;
+
+        $clone->setAttribute('value', 'bar')
+            ->setAttribute('data-ngos', true);
+
+        $this->assertSame(
+            ' value="foo"',
+            $element->getAttributes()->render(),
+            'Attribute callbacks are not rebound to their new owner'
+        );
+        $this->assertSame(
+            ' value="bar" data-ngos',
+            $clone->getAttributes()->render(),
+            'Attribute callbacks are not rebound to their new owner'
+        );
+    }
 }

tests/CloneTest.php

@@ -0,0 +1,155 @@
+<?php
+
+namespace ipl\Tests\Html;
+
+use Closure;
+use ipl\Html\Attribute;
+use ipl\Html\Attributes;
+use ipl\Tests\Html\Lib\CloningDummyElement;
+use ReflectionFunction;
+use ReflectionProperty;
+
+class CloneTest extends TestCase
+{
+    public function testHtmlOutput(): void
+    {
+        $original = new CloningDummyElement();
+        $original->getAttributes()->set('class', 'original_class');
+
+        $firstClone = clone $original;
+        $firstClone->getAttributes()->set('class', 'first_clone_class');
+
+        $secondClone = clone $firstClone;
+        $secondClone->getAttributes()->set('class', 'second_clone_class');
+
+        $originalHtml = <<<'HTML'
+<p class="original_class"
+    test-instance-scope-noop-inline="inline"
+    test-instance-noop-attribute="static_callback"
+    test-closure-static-scope-noop="static_callback"
+    test-closure-instance-scope-noop="static_callback">
+</p>
+HTML;
+
+        $firstCloneHtml = <<<'HTML'
+<p class="first_clone_class"
+    test-instance-scope-noop-inline="inline"
+    test-instance-noop-attribute="static_callback"
+    test-closure-static-scope-noop="static_callback"
+    test-closure-instance-scope-noop="static_callback">
+</p>
+HTML;
+
+
+        $secondCloneHtml = <<<'HTML'
+<p class="second_clone_class"
+    test-instance-scope-noop-inline="inline"
+    test-instance-noop-attribute="static_callback"
+    test-closure-static-scope-noop="static_callback"
+    test-closure-instance-scope-noop="static_callback">
+</p>
+HTML;
+
+        $this->assertHtml($originalHtml, $original);
+        $this->assertHtml($firstCloneHtml, $firstClone);
+        $this->assertHtml($secondCloneHtml, $secondClone);
+    }
+
+    public function testElementCallbacksCloning(): void
+    {
+        $element = new CloningDummyElement();
+        $element->getAttributes();
+
+        $clone = clone $element;
+
+        $this->assertCallbacksFor($element);
+        $this->assertCallbacksFor($clone);
+    }
+
+    public function testCloningAttributes(): void
+    {
+        $original = Attributes::create([Attribute::create('class', 'class01')]);
+
+        $clone = clone $original;
+        foreach ($clone->getAttributes() as $attribute) {
+            if ($attribute->getName() === 'class') {
+                $attribute->setValue('class02');
+            }
+        }
+
+        $this->assertSame($original->get('class')->getValue(), 'class01');
+        $this->assertSame($clone->get('class')->getValue(), 'class02');
+    }
+
+    protected function getCallbackThis(callable $callback): ?object
+    {
+        if (! $callback instanceof Closure) {
+            if (is_array($callback) && ! is_string($callback[0])) {
+                return $callback[0];
+            } else {
+                return null;
+            }
+        }
+
+        return (new ReflectionFunction($callback))
+            ->getClosureThis();
+    }
+
+    protected function isCallbackGlobalOrStatic(callable $callback): bool
+    {
+        if (! $callback instanceof Closure) {
+            if (is_array($callback) && ! is_string($callback[0])) {
+                return false;
+            }
+        } else {
+            $closureThis = (new ReflectionFunction($callback))
+                ->getClosureThis();
+
+            if ($closureThis) {
+                return false;
+            }
+        }
+
+        return true;
+    }
+
+    protected function getAttributeCallback(Attributes $attributes, string $name): callable
+    {
+        $callbacksProperty = new ReflectionProperty(get_class($attributes), 'callbacks');
+        $callbacksProperty->setAccessible(true);
+        $callbacks = $callbacksProperty->getValue($attributes);
+
+        return $callbacks[$name];
+    }
+
+    protected function assertCallbacksFor(CloningDummyElement $element)
+    {
+        $this->assertCallbackBelongsTo($element->getAttributes(), 'test-instance-scope-noop-inline', $element);
+        $this->assertCallbackBelongsTo(
+            $element->getAttributes(),
+            'test-instance-noop-attribute',
+            $element
+        );
+        $this->assertGlobalOrStaticCallback(
+            $element->getAttributes(),
+            'test-closure-static-scope-noop'
+        );
+        $this->assertGlobalOrStaticCallback(
+            $element->getAttributes(),
+            'test-closure-instance-scope-noop'
+        );
+    }
+
+    protected function assertGlobalOrStaticCallback(Attributes $attributes, string $callbackName)
+    {
+        $callback = $this->getAttributeCallback($attributes, $callbackName);
+        $this->assertTrue($this->isCallbackGlobalOrStatic($callback));
+    }
+
+    protected function assertCallbackBelongsTo(Attributes $attributes, string $callbackName, object $owner)
+    {
+        $callback = $this->getAttributeCallback($attributes, $callbackName);
+        $callbackThis = $this->getCallbackThis($callback);
+        $this->assertSame($callbackThis, $owner);
+    }
+}