From 12d086551e1dd993d32752923a475b5719156fda Mon Sep 17 00:00:00 2001 From: "Alexander M. Turek" Date: Fri, 26 Aug 2022 00:09:37 +0200 Subject: [PATCH 1/3] Fix static analysis errors for Collections 1.7 --- lib/Doctrine/ORM/PersistentCollection.php | 46 ++++++++++++----------- phpstan-baseline.neon | 12 +----- phpstan-dbal2.neon | 2 - phpstan-persistence2.neon | 3 -- phpstan.neon | 3 -- psalm-baseline.xml | 40 ++++++++++---------- 6 files changed, 44 insertions(+), 62 deletions(-) diff --git a/lib/Doctrine/ORM/PersistentCollection.php b/lib/Doctrine/ORM/PersistentCollection.php index aa8cb92b65a..6a1b186030d 100644 --- a/lib/Doctrine/ORM/PersistentCollection.php +++ b/lib/Doctrine/ORM/PersistentCollection.php @@ -18,6 +18,7 @@ use function array_map; use function array_values; use function array_walk; +use function assert; use function get_class; use function is_object; use function spl_object_id; @@ -33,7 +34,8 @@ * * @psalm-template TKey of array-key * @psalm-template T - * @template-implements Collection + * @template-extends AbstractLazyCollection + * @template-implements Selectable */ final class PersistentCollection extends AbstractLazyCollection implements Selectable { @@ -71,7 +73,7 @@ final class PersistentCollection extends AbstractLazyCollection implements Selec * The name of the field on the target entities that points to the owner * of the collection. This is only set if the association is bi-directional. * - * @var string + * @var string|null */ private $backRefFieldName; @@ -148,7 +150,7 @@ public function getTypeClass(): Mapping\ClassMetadataInfo */ public function hydrateAdd($element): void { - $this->collection->add($element); + $this->unwrap()->add($element); // If _backRefFieldName is set and its a one-to-many association, // we need to set the back reference. @@ -176,7 +178,7 @@ public function hydrateAdd($element): void */ public function hydrateSet($key, $element): void { - $this->collection->set($key, $element); + $this->unwrap()->set($key, $element); // If _backRefFieldName is set, then the association is bidirectional // and we need to set the back reference. @@ -210,7 +212,7 @@ public function initialize(): void */ public function takeSnapshot(): void { - $this->snapshot = $this->collection->toArray(); + $this->snapshot = $this->unwrap()->toArray(); $this->isDirty = false; } @@ -233,7 +235,7 @@ public function getSnapshot(): array */ public function getDeleteDiff(): array { - $collectionItems = $this->collection->toArray(); + $collectionItems = $this->unwrap()->toArray(); return array_values(array_diff_key( array_combine(array_map('spl_object_id', $this->snapshot), $this->snapshot), @@ -249,7 +251,7 @@ public function getDeleteDiff(): array */ public function getInsertDiff(): array { - $collectionItems = $this->collection->toArray(); + $collectionItems = $this->unwrap()->toArray(); return array_values(array_diff_key( array_combine(array_map('spl_object_id', $collectionItems), $collectionItems), @@ -322,8 +324,6 @@ public function setInitialized($bool): void /** * {@inheritdoc} - * - * @return object */ public function remove($key) { @@ -387,7 +387,7 @@ public function containsKey($key): bool ) { $persister = $this->em->getUnitOfWork()->getCollectionPersister($this->association); - return $this->collection->containsKey($key) || $persister->containsKey($this, $key); + return $this->unwrap()->containsKey($key) || $persister->containsKey($this, $key); } return parent::containsKey($key); @@ -401,7 +401,7 @@ public function contains($element): bool if (! $this->initialized && $this->association['fetch'] === ClassMetadata::FETCH_EXTRA_LAZY) { $persister = $this->em->getUnitOfWork()->getCollectionPersister($this->association); - return $this->collection->contains($element) || $persister->contains($this, $element); + return $this->unwrap()->contains($element) || $persister->contains($this, $element); } return parent::contains($element); @@ -432,7 +432,7 @@ public function count(): int if (! $this->initialized && $this->association !== null && $this->association['fetch'] === ClassMetadata::FETCH_EXTRA_LAZY) { $persister = $this->em->getUnitOfWork()->getCollectionPersister($this->association); - return $persister->count($this) + ($this->isDirty ? $this->collection->count() : 0); + return $persister->count($this) + ($this->isDirty ? $this->unwrap()->count() : 0); } return parent::count(); @@ -457,7 +457,7 @@ public function set($key, $value): void */ public function add($value): bool { - $this->collection->add($value); + $this->unwrap()->add($value); $this->changed(); @@ -514,13 +514,13 @@ public function offsetUnset($offset) public function isEmpty(): bool { - return $this->collection->isEmpty() && $this->count() === 0; + return $this->unwrap()->isEmpty() && $this->count() === 0; } public function clear(): void { if ($this->initialized && $this->isEmpty()) { - $this->collection->clear(); + $this->unwrap()->clear(); return; } @@ -536,12 +536,12 @@ public function clear(): void // hence for event listeners we need the objects in memory. $this->initialize(); - foreach ($this->collection as $element) { + foreach ($this->unwrap() as $element) { $uow->scheduleOrphanRemoval($element); } } - $this->collection->clear(); + $this->unwrap()->clear(); $this->initialized = true; // direct call, {@link initialize()} is too expensive @@ -633,7 +633,7 @@ public function matching(Criteria $criteria): Collection } if ($this->initialized) { - return $this->collection->matching($criteria); + return $this->unwrap()->matching($criteria); } if ($this->association['type'] === ClassMetadata::MANY_TO_MANY) { @@ -665,6 +665,8 @@ public function matching(Criteria $criteria): Collection */ public function unwrap(): Collection { + assert($this->collection !== null); + return $this->collection; } @@ -674,10 +676,10 @@ protected function doInitialize(): void $newlyAddedDirtyObjects = []; if ($this->isDirty) { - $newlyAddedDirtyObjects = $this->collection->toArray(); + $newlyAddedDirtyObjects = $this->unwrap()->toArray(); } - $this->collection->clear(); + $this->unwrap()->clear(); $this->em->getUnitOfWork()->loadCollection($this); $this->takeSnapshot(); @@ -696,14 +698,14 @@ protected function doInitialize(): void */ private function restoreNewObjectsInDirtyCollection(array $newObjects): void { - $loadedObjects = $this->collection->toArray(); + $loadedObjects = $this->unwrap()->toArray(); $newObjectsByOid = array_combine(array_map('spl_object_id', $newObjects), $newObjects); $loadedObjectsByOid = array_combine(array_map('spl_object_id', $loadedObjects), $loadedObjects); $newObjectsThatWereNotLoaded = array_diff_key($newObjectsByOid, $loadedObjectsByOid); if ($newObjectsThatWereNotLoaded) { // Reattach NEW objects added through add(), if any. - array_walk($newObjectsThatWereNotLoaded, [$this->collection, 'add']); + array_walk($newObjectsThatWereNotLoaded, [$this->unwrap(), 'add']); $this->isDirty = true; } diff --git a/phpstan-baseline.neon b/phpstan-baseline.neon index fdac67256d1..b686cc94dcd 100644 --- a/phpstan-baseline.neon +++ b/phpstan-baseline.neon @@ -251,12 +251,7 @@ parameters: path: lib/Doctrine/ORM/NativeQuery.php - - message: "#^Call to an undefined method Doctrine\\\\Common\\\\Collections\\\\Collection\\<\\(int\\|string\\), mixed\\>\\:\\:matching\\(\\)\\.$#" - count: 1 - path: lib/Doctrine/ORM/PersistentCollection.php - - - - message: "#^Method Doctrine\\\\ORM\\\\PersistentCollection\\:\\:remove\\(\\) should return object but returns array\\|float\\|int\\|string\\|false\\|null\\.$#" + message: "#^Call to an undefined method Doctrine\\\\Common\\\\Collections\\\\Collection\\\\:\\:matching\\(\\)\\.$#" count: 1 path: lib/Doctrine/ORM/PersistentCollection.php @@ -265,11 +260,6 @@ parameters: count: 2 path: lib/Doctrine/ORM/PersistentCollection.php - - - message: "#^The @implements tag of class Doctrine\\\\ORM\\\\PersistentCollection describes Doctrine\\\\Common\\\\Collections\\\\Collection but the class implements\\: Doctrine\\\\Common\\\\Collections\\\\Selectable$#" - count: 1 - path: lib/Doctrine/ORM/PersistentCollection.php - - message: "#^Method Doctrine\\\\ORM\\\\Persisters\\\\Collection\\\\OneToManyPersister\\:\\:delete\\(\\) should return int\\|null but empty return statement found\\.$#" count: 1 diff --git a/phpstan-dbal2.neon b/phpstan-dbal2.neon index b262dad9f96..51b44346945 100644 --- a/phpstan-dbal2.neon +++ b/phpstan-dbal2.neon @@ -6,8 +6,6 @@ parameters: reportUnmatchedIgnoredErrors: false ignoreErrors: - # https://github.com/doctrine/collections/pull/282 - - '/Variable \$offset in isset\(\) always exists and is not nullable\./' # PHPStan doesn't understand our method_exists() safeguards. - '/Call to an undefined method Doctrine\\DBAL\\Connection::createSchemaManager\(\)\./' # Class name will change in DBAL 3. diff --git a/phpstan-persistence2.neon b/phpstan-persistence2.neon index 0a98da1db76..4447d0db94e 100644 --- a/phpstan-persistence2.neon +++ b/phpstan-persistence2.neon @@ -32,9 +32,6 @@ parameters: path: lib/Doctrine/ORM/Internal/Hydration/AbstractHydrator.php # False positive - - - message: '/^Variable \$offset in isset\(\) always exists and is not nullable\.$/' - path: lib/Doctrine/ORM/PersistentCollection.php - message: '/^Call to an undefined method Doctrine\\Common\\Cache\\Cache::deleteAll\(\)\.$/' count: 1 diff --git a/phpstan.neon b/phpstan.neon index 78ef3526f2b..c2025fdde4b 100644 --- a/phpstan.neon +++ b/phpstan.neon @@ -32,9 +32,6 @@ parameters: path: lib/Doctrine/ORM/Internal/Hydration/AbstractHydrator.php # False positive - - - message: '/^Variable \$offset in isset\(\) always exists and is not nullable\.$/' - path: lib/Doctrine/ORM/PersistentCollection.php - message: '/^Call to an undefined method Doctrine\\Common\\Cache\\Cache::deleteAll\(\)\.$/' count: 1 diff --git a/psalm-baseline.xml b/psalm-baseline.xml index d4708a9af78..fdb56c2b6a4 100644 --- a/psalm-baseline.xml +++ b/psalm-baseline.xml @@ -83,6 +83,8 @@ $assoc['joinColumnFieldNames'] + $assoc['targetToSourceKeyColumns'] + $owningAssociation['targetToSourceKeyColumns'] getCacheRegion @@ -472,11 +474,6 @@ $class - - - LazyCriteriaCollection - - $repositoryClassName @@ -1070,28 +1067,25 @@ - - isset($offset) - - + + Collection<TKey, T> object|null - - final class PersistentCollection extends AbstractLazyCollection implements Selectable - - - - $key + + $this->em->find($this->typeClass->name, $key) + + $offset $value - + $this->association $this->association $this->association $this->association['targetEntity'] + $this->backRefFieldName $this->association['fetch'] @@ -1111,13 +1105,9 @@ setValue setValue - - $backRefFieldName - - + $this->em $this->em - is_object($this->collection) is_object($value) && $this->em is_object($value) && $this->em @@ -1353,6 +1343,8 @@ $association['joinTable'] $association['joinTable'] $association['joinTable'] + $owningAssoc['targetToSourceKeyColumns'] + $owningAssoc['targetToSourceKeyColumns'] $this->class->associationMappings[$fieldName]['joinColumns'] $this->class->associationMappings[$idField]['joinColumns'] @@ -1384,6 +1376,7 @@ executeInserts + $assoc['targetToSourceKeyColumns'] $mapping['joinColumns'] $mapping['joinColumns'] @@ -1703,6 +1696,7 @@ $owningAssoc['joinTable'] + $owningAssoc['targetToSourceKeyColumns'] $collectionPathExpression @@ -2428,8 +2422,11 @@ $assoc['joinColumns'] $assoc['joinColumns'] $assoc['sourceToTargetKeyColumns'] + $assoc['targetToSourceKeyColumns'] $association['sourceToTargetKeyColumns'] + $association['targetToSourceKeyColumns'] $owningAssoc['joinTable'] + $owningAssoc['targetToSourceKeyColumns'] $whereClause !== null @@ -3092,6 +3089,7 @@ $assoc['joinColumns'] $assoc['orphanRemoval'] + $assoc['targetToSourceKeyColumns'] unwrap From f287b74470e344920cda482ac77f5665f39d9a5c Mon Sep 17 00:00:00 2001 From: "Alexander M. Turek" Date: Fri, 26 Aug 2022 00:28:54 +0200 Subject: [PATCH 2/3] Fix tests for doctrine/common 3.4 --- lib/Doctrine/ORM/Tools/Setup.php | 8 +++++++- psalm-baseline.xml | 3 ++- 2 files changed, 9 insertions(+), 2 deletions(-) diff --git a/lib/Doctrine/ORM/Tools/Setup.php b/lib/Doctrine/ORM/Tools/Setup.php index 3dc43d49c64..51930d95ee9 100644 --- a/lib/Doctrine/ORM/Tools/Setup.php +++ b/lib/Doctrine/ORM/Tools/Setup.php @@ -28,7 +28,9 @@ use Symfony\Component\Cache\Adapter\RedisAdapter; use function class_exists; +use function dirname; use function extension_loaded; +use function file_exists; use function md5; use function sys_get_temp_dir; @@ -52,7 +54,11 @@ class Setup public static function registerAutoloadDirectory($directory) { if (! class_exists('Doctrine\Common\ClassLoader', false)) { - require_once $directory . '/Doctrine/Common/ClassLoader.php'; + if (file_exists($directory . '/Doctrine/Common/ClassLoader.php')) { + require_once $directory . '/Doctrine/Common/ClassLoader.php'; + } elseif (file_exists(dirname($directory) . '/src/ClassLoader.php')) { + require_once dirname($directory) . '/src/ClassLoader.php'; + } } $loader = new ClassLoader('Doctrine', $directory); diff --git a/psalm-baseline.xml b/psalm-baseline.xml index fdb56c2b6a4..82ae8e9b472 100644 --- a/psalm-baseline.xml +++ b/psalm-baseline.xml @@ -2992,8 +2992,9 @@ new ClassLoader('Doctrine', $directory) new ClassLoader('Symfony\Component', $directory . '/Doctrine') - + require_once $directory . '/Doctrine/Common/ClassLoader.php' + require_once dirname($directory) . '/src/ClassLoader.php' From 46ec86557e69b7fb0e7f5de0b2188cf9d76de8f7 Mon Sep 17 00:00:00 2001 From: "Alexander M. Turek" Date: Fri, 26 Aug 2022 12:27:20 +0200 Subject: [PATCH 3/3] Bump coding standard to 9.0.2 --- composer.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/composer.json b/composer.json index 0f4e12893b6..bd6d2b2cffb 100644 --- a/composer.json +++ b/composer.json @@ -40,7 +40,7 @@ }, "require-dev": { "doctrine/annotations": "^1.13", - "doctrine/coding-standard": "^9.0", + "doctrine/coding-standard": "^9.0.2", "phpbench/phpbench": "^0.16.10 || ^1.0", "phpstan/phpstan": "~1.4.10 || 1.8.2", "phpunit/phpunit": "^7.5 || ^8.5 || ^9.5",