Better merging of collection types for 2.x (liip#43)

--------- Co-authored-by: David Buchmann <david@liip.ch>
rebuy-de · Nov 8, 2023 · 9ec658c · 9ec658c
1 parent fe48494
commit 9ec658c
Show file tree

Hide file tree

Showing 6 changed files with 89 additions and 231 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -2,6 +2,14 @@
 
 # Version 2.x
 
+# 2.0.0 (unreleased)
+
+* Removed `PropertyTypeArray`, which is superseeded by `PropertyTypeIterable`.
+* Removed the deprecated `PropertyTypeIterable::getCollectionClass`. Use `PropertyTypeIterable::getTraversableClass`
+* Removed the deprecated `PropertyTypeIterable::isCollection`. Use `PropertyTypeIterable::isTraversable`
+* `JMSTypeParser::getTraversableClass` returns `Traversable::class` instead of `Doctrine\Common\Collections\Collection` for general traversable properties.
+
+
 # Version 1.x
 
 # 1.2.0

diff --git a/src/Metadata/PropertyTypeArray.php b/src/Metadata/PropertyTypeArray.php
diff --git a/src/Metadata/PropertyTypeIterable.php b/src/Metadata/PropertyTypeIterable.php
@@ -4,16 +4,24 @@
 
 namespace Liip\MetadataParser\Metadata;
 
-use Doctrine\Common\Collections\Collection;
-
 /**
- * This property type can be merged with PropertyTypeClass<T>, provided that T is, inherits from, or is a parent class of $this->collectionClass
+ * This property type can be merged with PropertyTypeClass<T>, provided that T is, inherits from, or is a parent class of {@see PropertyTypeIterable::traversableClass}
  * This property type can be merged with PropertyTypeIterable, if :
  *  - we're not merging a plain array PropertyTypeIterable into a hashmap one,
- *  - and the collection classes of each are either not present on both sides, or are the same, or parent-child of one another
+ *  - and the traversable classes of each are either not present on either sides, or are the same, or parent-child of one another
  */
-final class PropertyTypeIterable extends PropertyTypeArray
+final class PropertyTypeIterable extends AbstractPropertyType
 {
+    /**
+     * @var PropertyType
+     */
+    private $subType;
+
+    /**
+     * @var bool
+     */
+    private $hashmap;
+
     /**
      * @var string
      */
@@ -24,8 +32,10 @@ final class PropertyTypeIterable extends PropertyTypeArray
      */
     public function __construct(PropertyType $subType, bool $hashmap, bool $nullable, string $traversableClass = null)
     {
-        parent::__construct($subType, $hashmap, $nullable, null != $traversableClass);
+        parent::__construct($nullable);
 
+        $this->subType = $subType;
+        $this->hashmap = $hashmap;
         $this->traversableClass = $traversableClass;
     }
 
@@ -41,25 +51,20 @@ public function __toString(): string
             $array .= sprintf('|\\%s<%s%s>', $this->traversableClass, $this->subType, $collectionType);
         }
 
-        return ((string) $this->subType).$array.AbstractPropertyType::__toString();
+        return ((string) $this->subType).$array.parent::__toString();
     }
 
-    /**
-     * @deprecated Please prefer using {@link getTraversableClass}
-     *
-     * @return class-string<Collection>|null
-     */
-    public function getCollectionClass(): ?string
+    public function isHashmap(): bool
     {
-        return $this->isCollection() ? null : $this->traversableClass;
+        return $this->hashmap;
     }
 
     /**
-     * @deprecated Please prefer using {@link isTraversable}
+     * Returns the type of the next level, which could be an array or hashmap or another type.
      */
-    public function isCollection(): bool
+    public function getSubType(): PropertyType
     {
-        return (null != $this->traversableClass) && is_a($this->traversableClass, Collection::class, true);
+        return $this->subType;
     }
 
     /**
@@ -79,18 +84,31 @@ public function isTraversable(): bool
         return null != $this->traversableClass;
     }
 
+    /**
+     * Goes down the type until it is not an array or hashmap anymore.
+     */
+    public function getLeafType(): PropertyType
+    {
+        $type = $this->getSubType();
+        while ($type instanceof self) {
+            $type = $type->getSubType();
+        }
+
+        return $type;
+    }
+
     public function merge(PropertyType $other): PropertyType
     {
         $nullable = $this->isNullable() && $other->isNullable();
         $thisTraversableClass = $this->isTraversable() ? $this->getTraversableClass() : null;
 
         if ($other instanceof PropertyTypeUnknown) {
-            return new self($this->subType, $this->isHashmap(), $nullable, $thisTraversableClass);
+            return new self($this->getSubType(), $this->isHashmap(), $nullable, $thisTraversableClass);
         }
         if ($this->isTraversable() && (($other instanceof PropertyTypeClass) && is_a($other->getClassName(), \Traversable::class, true))) {
-            return new self($this->getSubType(), $this->isHashmap(), $nullable, $this->findCommonCollectionClass($thisTraversableClass, $other->getClassName()));
+            return new self($this->getSubType(), $this->isHashmap(), $nullable, $this->findCommonTraversableClass($thisTraversableClass, $other->getClassName()));
         }
-        if (!$other instanceof parent) {
+        if (!$other instanceof self) {
             throw new \UnexpectedValueException(sprintf('Can\'t merge type %s with %s, they must be the same or unknown', self::class, \get_class($other)));
         }
 
@@ -105,7 +123,7 @@ public function merge(PropertyType $other): PropertyType
 
         $otherTraversableClass = $other->isTraversable() ? $other->getTraversableClass() : null;
         $hashmap = $this->isHashmap() || $other->isHashmap();
-        $commonClass = $this->findCommonCollectionClass($thisTraversableClass, $otherTraversableClass);
+        $commonClass = $this->findCommonTraversableClass($thisTraversableClass, $otherTraversableClass);
 
         if ($other->getSubType() instanceof PropertyTypeUnknown) {
             return new self($this->getSubType(), $hashmap, $nullable, $commonClass);
@@ -121,7 +139,7 @@ public function merge(PropertyType $other): PropertyType
      * Find the most derived class that doesn't deny both class hints, meaning the most derived
      * between left and right if one is a child of the other
      */
-    private function findCommonCollectionClass(?string $left, ?string $right): ?string
+    private function findCommonTraversableClass(?string $left, ?string $right): ?string
     {
         if (null === $right) {
             return $left;

diff --git a/src/TypeParser/JMSTypeParser.php b/src/TypeParser/JMSTypeParser.php
@@ -5,7 +5,6 @@
 namespace Liip\MetadataParser\TypeParser;
 
 use Doctrine\Common\Collections\ArrayCollection;
-use Doctrine\Common\Collections\Collection;
 use JMS\Serializer\Type\Parser;
 use Liip\MetadataParser\Exception\InvalidTypeException;
 use Liip\MetadataParser\Metadata\DateTimeOptions;
@@ -20,6 +19,9 @@ final class JMSTypeParser
 {
     private const TYPE_ARRAY = 'array';
     private const TYPE_ARRAY_COLLECTION = 'ArrayCollection';
+    private const TYPE_GENERATOR = 'Generator';
+    private const TYPE_ARRAY_ITERATOR = 'ArrayIterator';
+    private const TYPE_ITERATOR = 'Iterator';
     private const TYPE_DATETIME_INTERFACE = 'DateTimeInterface';
 
     /**
@@ -69,13 +71,13 @@ private function parseType(array $typeInfo, bool $isSubType = false): PropertyTy
             return new PropertyTypeClass($typeInfo['name'], $nullable);
         }
 
-        $collectionClass = $this->getCollectionClass($typeInfo['name']);
-        if (self::TYPE_ARRAY === $typeInfo['name'] || $collectionClass) {
+        $traversableClass = $this->getTraversableClass($typeInfo['name']);
+        if (self::TYPE_ARRAY === $typeInfo['name'] || $traversableClass) {
             if (1 === \count($typeInfo['params'])) {
-                return new PropertyTypeIterable($this->parseType($typeInfo['params'][0], true), false, $nullable, $collectionClass);
+                return new PropertyTypeIterable($this->parseType($typeInfo['params'][0], true), false, $nullable, $traversableClass);
             }
             if (2 === \count($typeInfo['params'])) {
-                return new PropertyTypeIterable($this->parseType($typeInfo['params'][1], true), true, $nullable, $collectionClass);
+                return new PropertyTypeIterable($this->parseType($typeInfo['params'][1], true), true, $nullable, $traversableClass);
             }
 
             throw new InvalidTypeException(sprintf('JMS property type array can\'t have more than 2 parameters (%s)', var_export($typeInfo, true)));
@@ -105,13 +107,18 @@ private function parseType(array $typeInfo, bool $isSubType = false): PropertyTy
         throw new InvalidTypeException(sprintf('Unknown JMS property found (%s)', var_export($typeInfo, true)));
     }
 
-    private function getCollectionClass(string $name): ?string
+    private function getTraversableClass(string $name): ?string
     {
         switch ($name) {
             case self::TYPE_ARRAY_COLLECTION:
                 return ArrayCollection::class;
+            case self::TYPE_GENERATOR:
+                return \Generator::class;
+            case self::TYPE_ARRAY_ITERATOR:
+            case self::TYPE_ITERATOR:
+                return \ArrayIterator::class;
             default:
-                return is_a($name, Collection::class, true) ? $name : null;
+                return is_a($name, \Traversable::class, true) ? $name : null;
         }
     }
 }