Improved closure serialization (#151)

* Use SplObjectStorage for priority * Added `ClosureInfo::getClosure` * Updated ClosureInfo serialization * Moved isEnum() to ClassInfo * Added refId() * Improved parent, self, static detection * Improved serialization/unserialization of arrays * Marked getFactory() as internal * Updated changelog and readme
opis · Jan 4, 2025 · 847d3c2 · 847d3c2
1 parent 1b26a1a
commit 847d3c2
Show file tree

Hide file tree

Showing 13 changed files with 188 additions and 98 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -1,6 +1,32 @@
 CHANGELOG
 ---------
 
+### v4.1.0, 2025.01.05
+
+#### Changes
+
+- Improved closure parser
+- Improved array & object serialization/deserialization
+- Added `ClosureInfo::getClosure()`
+- Fixed `ClosureInfo::getFactory()` bindings and marked the method as internal
+
+#### Internal changes
+
+Added
+
+- `ClassInfo::get()`
+- `ClassInfo::clear()`
+- `ClassInfo::isInternal()`
+- `ClassInfo::isEnum()`
+- `ClassInfo::refId()`
+- `ClassInfo` is final
+- `ClassInfo` constructor is private
+
+Removed
+
+- `Serializer::getClassInfo()` (replaced by `ClassInfo::get()`)
+- `Serializer::isEnum()` (replaced by `ClassInfo::isEnum()`)
+
 ### v4.0.1, 2025.01.04
 
 - Fixes unserialization error [#149](https://github.com/opis/closure/issues/149)

diff --git a/README.md b/README.md
@@ -59,7 +59,7 @@ Or you could directly reference it into your `composer.json` file as a dependenc
 ```json
 {
     "require": {
-        "opis/closure": "^4.0"
+        "opis/closure": "^4.1"
     }
 }
 ```

diff --git a/src/ClassInfo.php b/src/ClassInfo.php
@@ -2,12 +2,12 @@
 
 namespace Opis\Closure;
 
-use ReflectionClass;
+use UnitEnum, ReflectionClass, ReflectionReference;
 
 /**
  * @internal
  */
-class ClassInfo
+final class ClassInfo
 {
     public ReflectionClass $reflection;
     public bool $box;
@@ -24,7 +24,14 @@ class ClassInfo
      */
     public $unserialize = null;
 
-    public function __construct(string $className)
+    /**
+     * @var ClassInfo[]
+     */
+    private static array $cache = [];
+
+    private static ?bool $enumExists = null;
+
+    private function __construct(string $className)
     {
         $reflection = $this->reflection = new ReflectionClass($className);
         $this->box = empty($reflection->getAttributes(Attribute\PreventBoxing::class));
@@ -36,4 +43,31 @@ public function className(): string
     {
         return $this->reflection->name;
     }
-}
+
+    public static function get(string $class): self
+    {
+        return self::$cache[$class] ??= new self($class);
+    }
+
+    public static function clear(): void
+    {
+        self::$cache = [];
+    }
+
+    public static function isInternal(object|string $object): bool
+    {
+        return self::get(is_string($object) ? $object : get_class($object))->reflection->isInternal();
+    }
+
+    public static function isEnum(mixed $value): bool
+    {
+        // enums were added in php 8.1
+        self::$enumExists ??= interface_exists(UnitEnum::class, false);
+        return self::$enumExists && ($value instanceof UnitEnum);
+    }
+
+    public static function refId(mixed &$reference): ?string
+    {
+        return ReflectionReference::fromArrayElement([&$reference], 0)?->getId();
+    }
+}
diff --git a/src/ClosureInfo.php b/src/ClosureInfo.php
@@ -99,15 +99,36 @@ public function hasScope(): bool
         return ($this->flags & self::FLAG_HAS_SCOPE) === self::FLAG_HAS_SCOPE;
     }
 
+    public function getClosure(?array &$vars = null, ?object $thisObj = null, ?string $scope = null): Closure
+    {
+        return $this->getFactory($thisObj, $scope)($vars);
+    }
+
+    /**
+     * @internal
+     */
     public function getFactory(?object $thisObj, ?string $scope = null): Closure
     {
         $factory = ($this->factory ??= ClosureStream::factory($this));
 
+        if ($thisObj && $this->isStatic()) {
+            // closure is static, we cannot bind
+            if (!$scope) {
+                // we can extract scope
+                $scope = get_class($thisObj);
+            }
+            // remove this
+            $thisObj = null;
+        }
+
         if ($thisObj) {
-            return $factory->bindTo($thisObj, $scope ?? 'static');
+            if (ClassInfo::isInternal($thisObj)) {
+                return $factory->bindTo($thisObj);
+            }
+            return $factory->bindTo($thisObj, $thisObj);
         }
 
-        if ($scope) {
+        if ($scope && $scope !== "static" && $this->hasScope() && !ClassInfo::isInternal($scope)) {
             return $factory->bindTo(null, $scope);
         }
 
@@ -147,7 +168,7 @@ public function __serialize(): array
     {
         $data = ['key' => $this->key()];
         if ($this->header) {
-            $data['imports'] = $this->header;
+            $data['header'] = $this->header;
         }
         $data['body'] = $this->body;
         if ($this->use) {
@@ -162,7 +183,7 @@ public function __serialize(): array
     public function __unserialize(array $data): void
     {
         $this->key = $data['key'] ?? null;
-        $this->header = $data['imports'] ?? '';
+        $this->header = $data['header'] ?? $data['imports'] ?? '';
         $this->body = $data['body'];
         $this->use = $data['use'] ?? null;
         $this->flags = $data['flags'] ?? 0;

diff --git a/src/ClosureParser.php b/src/ClosureParser.php
@@ -13,6 +13,8 @@ final class ClosureParser
 
     private const MATCH_CLOSURE = [T_FN, T_FUNCTION];
 
+    private const ACCESS_PROP = [T_OBJECT_OPERATOR, T_NULLSAFE_OBJECT_OPERATOR, T_DOUBLE_COLON];
+
     /**
      * @var array Transformed file tokens cache
      */
@@ -663,6 +665,27 @@ private function walk(int $index, array $token_types, bool $match = false, int $
         } while (true);
     }
 
+    private function nextIs(int $index, array $token_types, bool $reverse = false): bool
+    {
+        $count = $this->count;
+        $tokens = $this->tokens;
+
+        $inc = $reverse ? -1 : 1;
+        for ($index += $inc; $index >= 0 && $index < $count; $index += $inc) {
+            if (is_array($tokens[$index])) {
+                if (in_array($tokens[$index][0], self::SKIP_WHITESPACE_AND_COMMENTS, true)) {
+                    continue;
+                }
+                $tok = $tokens[$index][0];
+            } else {
+                $tok = $tokens[$index];
+            }
+
+            return in_array($tok, $token_types, true);
+        }
+
+        return false;
+    }
     /**
      * @param string $hint
      * @return bool
@@ -705,9 +728,9 @@ private function checkSpecialToken(int $index): void
             $check = $checkThis = strcasecmp($token[1], '$this') === 0;
         } elseif ($token[0] === T_STRING) {
             if (strcasecmp($token[1], 'self') === 0) {
-                $check = true;
+                $check = !$this->nextIs($index, self::ACCESS_PROP, true);
             } elseif (strcasecmp($token[1], 'parent') === 0) {
-                $check = $isParent = true;
+                $check = $isParent = !$this->nextIs($index, self::ACCESS_PROP, true);
                 $checkThis = !$this->isStatic;
             }
         }

diff --git a/src/DeserializationHandler.php b/src/DeserializationHandler.php
@@ -74,28 +74,11 @@ private function handleIterable(array|object &$iterable): void
 
     private function handleArray(array &$array): void
     {
-        $visited = &$this->visitedArrays;
-
-        if ($visited) {
-            $found = false;
-            $array[Serializer::$uniqKey] = true;
-
-            for ($i = count($visited) - 1; $i >= 0; $i--) {
-                if (isset($visited[$i][Serializer::$uniqKey])) {
-                    $found = true;
-                    break;
-                }
-            }
-
-            unset($array[Serializer::$uniqKey]);
-
-            if ($found) {
-                return;
-            }
+        $id = ClassInfo::refId($array);
+        if (!isset($this->visitedArrays[$id])) {
+            $this->visitedArrays[$id] = true;
+            $this->handleIterable($array);
         }
-
-        $visited[] = &$array;
-        $this->handleIterable($array);
     }
 
     private function handleObject(object &$object): void
@@ -162,7 +145,7 @@ private function handleObject(object &$object): void
 
     private function unboxObject(Box $box): object
     {
-        $info = Serializer::getClassInfo($box->data[0]);
+        $info = ClassInfo::get($box->data[0]);
 
         /**
          * @var $data array|null
@@ -242,7 +225,7 @@ private function unboxClosure(Box $box): Closure
             $this->handleArray($data["vars"]);
         }
 
-        return $info->getFactory($data["this"], $data["scope"])($data["vars"]);
+        return $info->getClosure($data["vars"], $data["this"], $data["scope"]);
     }
 
     ///////////////////////////////////////////////////////
@@ -357,6 +340,6 @@ private function v3_unboxClosure(array &$data): Closure
             $this->handleArray($data["use"]);
         }
 
-        return $info->getFactory($data["this"], $data["scope"])($data["use"]);
+        return $info->getClosure($data["use"], $data["this"], $data["scope"]);
     }
 }
diff --git a/src/SerializationHandler.php b/src/SerializationHandler.php
@@ -2,7 +2,7 @@
 
 namespace Opis\Closure;
 
-use stdClass, Closure, WeakMap;
+use stdClass, Closure, WeakMap, SplObjectStorage;
 use function serialize;
 
 class SerializationHandler
@@ -11,39 +11,26 @@ class SerializationHandler
 
     private ?WeakMap $objectMap;
 
-    /**
-     * @var object[]|null
-     */
-    private ?array $priority;
+    private ?SplObjectStorage $priority;
 
     private ?WeakMap $shouldBox;
 
-    private int $uniqueArrayKeyValue;
-
     private bool $hasAnonymousObjects;
 
     public function serialize(mixed $data): string
     {
         $this->arrayMap = [];
         $this->objectMap = new WeakMap();
-        $this->priority = [];
+        $this->priority = new SplObjectStorage();
         $this->shouldBox = new WeakMap();
-        $this->uniqueArrayKeyValue = 0;
         $this->hasAnonymousObjects = false;
 
         try {
             // get boxed structure
             $data = $this->handle($data);
-            // remove unique key
-            foreach ($this->arrayMap as &$pair) {
-                unset($pair[0][Serializer::$uniqKey], $pair);
-            }
-            if ($this->hasAnonymousObjects) {
+            if ($this->hasAnonymousObjects && $this->priority->count()) {
                 // we only need priority when we have closures
-                $priority = array_unique($this->priority, \SORT_REGULAR);
-                if ($priority) {
-                    $data = new PriorityWrapper($priority, $data);
-                }
+                $data = new PriorityWrapper(iterator_to_array($this->priority), $data);
             }
             return serialize($data);
         } finally {
@@ -91,7 +78,7 @@ private function shouldBox(ClassInfo $info): bool
     private function handleObject(object $data): object
     {
         if (
-            Serializer::isEnum($data) ||
+            ClassInfo::isEnum($data) ||
             ($data instanceof Box) ||
             ($data instanceof ClosureInfo)
         ) {
@@ -106,7 +93,9 @@ private function handleObject(object $data): object
 
         if ($data instanceof stdClass) {
             // handle stdClass
-            return $this->priority[] = $this->handleStdClass($data);
+            $obj = $this->handleStdClass($data);
+            $this->priority->attach($obj);
+            return $obj;
         }
 
         if ($data instanceof Closure) {
@@ -116,7 +105,7 @@ private function handleObject(object $data): object
             return $this->handleClosure($data);
         }
 
-        $info = Serializer::getClassInfo(get_class($data));
+        $info = ClassInfo::get(get_class($data));
         if (!$this->shouldBox($info)) {
             // skip boxing
             return $this->objectMap[$data] = $data;
@@ -139,25 +128,28 @@ private function handleObject(object $data): object
             $box->data[1] = &$this->handleArray($vars);
         }
 
-        return $this->priority[] = $box;
+        $this->priority->attach($box);
+
+        return $box;
     }
 
     private function &handleArray(array &$data): array
     {
-        if (isset($data[Serializer::$uniqKey])) {
-            // we must grab the reference to boxed
-            return $this->arrayMap[$data[Serializer::$uniqKey]][1];
+        $id = ClassInfo::refId($data);
+
+        if (array_key_exists($id, $this->arrayMap)) {
+            return $this->arrayMap[$id];
         }
 
         $box = [];
-        $this->arrayMap[($data[Serializer::$uniqKey] ??= $this->uniqueArrayKeyValue++)] = [&$data, &$box];
+        $this->arrayMap[$id] = &$box;
 
         foreach ($data as $key => &$value) {
             if (is_object($value)) {
                 $box[$key] = $this->handleObject($value);
             } elseif (is_array($value)) {
                 $box[$key] = &$this->handleArray($value);
-            } elseif ($key !== Serializer::$uniqKey) {
+            } else {
                 $box[$key] = &$value;
             }
             unset($value);
-Original file line number
+Diff line change
@@ Expand Up @@
     ```json
     {
         "require": {
-            "opis/closure": "^4.0"
+            "opis/closure": "^4.1"
         }
     }
     ```
@@ Expand Down @@