Merge pull request #316 from doctrine/1.7.x-merge-up-into-2.0.x_vd8MiHqn

composer.json

@@ -33,7 +33,8 @@
     ],
     "homepage": "https://www.doctrine-project.org/projects/collections.html",
     "require": {
-        "php": "^8.1"
+        "php": "^8.1",
+        "doctrine/deprecations": "^1"
     },
     "require-dev": {
         "ext-json": "*",

lib/Doctrine/Common/Collections/AbstractLazyCollection.php

@@ -5,6 +5,7 @@
 namespace Doctrine\Common\Collections;
 
 use Closure;
+use LogicException;
 use Traversable;
 
 /**
@@ -19,10 +20,10 @@ abstract class AbstractLazyCollection implements Collection
     /**
      * The backed collection to use
      *
-     * @psalm-var Collection<TKey,T>
-     * @var Collection<mixed>
+     * @psalm-var Collection<TKey,T>|null
+     * @var Collection<mixed>|null
      */
-    protected Collection $collection;
+    protected ?Collection $collection;
 
     protected bool $initialized = false;
 
@@ -320,6 +321,8 @@ public function isInitialized(): bool
 
     /**
      * Initialize the collection
+     *
+     * @psalm-assert Collection<TKey,T> $this->collection
      */
     protected function initialize(): void
     {
@@ -329,6 +332,10 @@ protected function initialize(): void
 
         $this->doInitialize();
         $this->initialized = true;
+
+        if ($this->collection === null) {
+            throw new LogicException('You must initialize the collection property in the doInitialize() method.');
+        }
     }
 
     /**

lib/Doctrine/Common/Collections/ArrayCollection.php

@@ -387,7 +387,7 @@ public function slice(int $offset, ?int $length = null): array
     }
 
     /**
-     * @psalm-return Collection<TKey, T>
+     * @psalm-return Collection<TKey, T>&Selectable<TKey,T>
      */
     public function matching(Criteria $criteria): Collection
     {

lib/Doctrine/Common/Collections/Criteria.php

@@ -6,8 +6,10 @@
 
 use Doctrine\Common\Collections\Expr\CompositeExpression;
 use Doctrine\Common\Collections\Expr\Expression;
+use Doctrine\Deprecations\Deprecation;
 
 use function array_map;
+use function func_num_args;
 use function strtoupper;
 
 /**
@@ -59,6 +61,17 @@ public function __construct(
         ?int $firstResult = null,
         ?int $maxResults = null,
     ) {
+        $this->expression = $expression;
+
+        if ($firstResult === null && func_num_args() > 2) {
+            Deprecation::trigger(
+                'doctrine/collections',
+                'https://github.com/doctrine/collections/pull/311',
+                'Passing null as $firstResult to the constructor of %s is deprecated. Pass 0 instead or omit the argument.',
+                self::class
+            );
+        }
+
         $this->setFirstResult($firstResult);
         $this->setMaxResults($maxResults);
 
@@ -178,6 +191,15 @@ public function getFirstResult(): ?int
      */
     public function setFirstResult(?int $firstResult): static
     {
+        if ($firstResult === null) {
+            Deprecation::triggerIfCalledFromOutside(
+                'doctrine/collections',
+                'https://github.com/doctrine/collections/pull/311',
+                'Passing null to %s() is deprecated, pass 0 instead.',
+                __METHOD__
+            );
+        }
+
         $this->firstResult = $firstResult;
 
         return $this;

lib/Doctrine/Common/Collections/Selectable.php

@@ -25,8 +25,8 @@ interface Selectable
      * Selects all elements from a selectable that match the expression and
      * returns a new collection containing these elements and preserved keys.
      *
-     * @return Collection<mixed>
-     * @psalm-return Collection<TKey,T>
+     * @return Collection<mixed>&Selectable<mixed>
+     * @psalm-return Collection<TKey,T>&Selectable<TKey,T>
      */
     public function matching(Criteria $criteria): Collection;
 }

phpstan.neon.dist

@@ -11,6 +11,9 @@ parameters:
         -
             message: '~Parameter #1 \$key of method Doctrine\\Common\\Collections\\ArrayCollection<TKey of \(int\|string\),T>::set\(\) expects TKey of \(int\|string\), int\|string given\.~'
             path: 'lib/Doctrine/Common/Collections/ArrayCollection.php'
+        -
+            message: '~Cannot call method .* on Doctrine\\Common\\Collections\\Collection<TKey of \(int\|string\), T>\|null\.~'
+            path: 'lib/Doctrine/Common/Collections/AbstractLazyCollection.php'
 
 includes:
     - vendor/phpstan/phpstan-phpunit/extension.neon

tests/Doctrine/Tests/Common/Collections/BaseCollectionTest.php

@@ -5,13 +5,16 @@
 namespace Doctrine\Tests\Common\Collections;
 
 use Doctrine\Common\Collections\Collection;
+use Doctrine\Common\Collections\Criteria;
+use Doctrine\Common\Collections\Selectable;
 use PHPUnit\Framework\TestCase;
 use stdClass;
 
 use function count;
 use function is_array;
 use function is_numeric;
 use function is_string;
+use function sprintf;
 
 abstract class BaseCollectionTest extends TestCase
 {
@@ -250,4 +253,16 @@ public function testCanVerifyExistingKeysWithNullValues(): void
         $this->collection->set('key', null);
         self::assertTrue($this->collection->containsKey('key'));
     }
+
+    public function testMatchingAlwaysReturnsCollection(): void
+    {
+        if (! $this->collection instanceof Selectable) {
+            self::markTestSkipped(sprintf('Collection does not implement %s', Selectable::class));
+        }
+
+        $criteria = Criteria::create();
+
+        self::assertInstanceOf(Collection::class, $this->collection->matching($criteria));
+        self::assertInstanceOf(Selectable::class, $this->collection->matching($criteria));
+    }
 }

tests/Doctrine/Tests/Common/Collections/CriteriaTest.php

@@ -8,10 +8,13 @@
 use Doctrine\Common\Collections\Expr\Comparison;
 use Doctrine\Common\Collections\Expr\CompositeExpression;
 use Doctrine\Common\Collections\ExpressionBuilder;
+use Doctrine\Deprecations\PHPUnit\VerifyDeprecations;
 use PHPUnit\Framework\TestCase;
 
 class CriteriaTest extends TestCase
 {
+    use VerifyDeprecations;
+
     public function testCreate(): void
     {
         $criteria = Criteria::create();
@@ -25,9 +28,33 @@ public function testConstructor(): void
         $criteria = new Criteria($expr, ['foo' => 'ASC'], 10, 20);
 
         self::assertSame($expr, $criteria->getWhereExpression());
-        self::assertEquals(['foo' => 'ASC'], $criteria->getOrderings());
-        self::assertEquals(10, $criteria->getFirstResult());
-        self::assertEquals(20, $criteria->getMaxResults());
+        self::assertSame(['foo' => 'ASC'], $criteria->getOrderings());
+        self::assertSame(10, $criteria->getFirstResult());
+        self::assertSame(20, $criteria->getMaxResults());
+    }
+
+    public function testDeprecatedNullOffset(): void
+    {
+        $expr = new Comparison('field', '=', 'value');
+
+        $this->expectDeprecationWithIdentifier('https://github.com/doctrine/collections/pull/311');
+        $criteria = new Criteria($expr, ['foo' => 'ASC'], null, 20);
+
+        self::assertSame($expr, $criteria->getWhereExpression());
+        self::assertSame(['foo' => 'ASC'], $criteria->getOrderings());
+        self::assertNull($criteria->getFirstResult());
+        self::assertSame(20, $criteria->getMaxResults());
+    }
+
+    public function testDefaultConstructor(): void
+    {
+        $this->expectNoDeprecationWithIdentifier('https://github.com/doctrine/collections/pull/311');
+        $criteria = new Criteria();
+
+        self::assertNull($criteria->getWhereExpression());
+        self::assertSame([], $criteria->getOrderings());
+        self::assertNull($criteria->getFirstResult());
+        self::assertNull($criteria->getMaxResults());
     }
 
     public function testWhere(): void