From 085d4dba9bc42f7a7f3bbca514529572b5ddea63 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Gr=C3=A9goire=20Paris?= Date: Wed, 24 Aug 2022 15:00:16 +0200 Subject: [PATCH 1/2] Extract read-only part of Collection This allows us to mark the extracted interface as covariant, thus allowing consumers to ensure through static analysis that a collection will not be altered in a way that would no longer make it a collection of items of a specific type. Closes #298 --- docs/en/index.rst | 5 + .../Collections/AbstractLazyCollection.php | 4 + .../Common/Collections/ArrayCollection.php | 8 + .../Common/Collections/Collection.php | 185 +-------------- .../Common/Collections/ReadableCollection.php | 213 ++++++++++++++++++ psalm.xml.dist | 7 + 6 files changed, 241 insertions(+), 181 deletions(-) create mode 100644 lib/Doctrine/Common/Collections/ReadableCollection.php diff --git a/docs/en/index.rst b/docs/en/index.rst index 936f9efa3..8ca97229b 100644 --- a/docs/en/index.rst +++ b/docs/en/index.rst @@ -32,6 +32,11 @@ explicitly retrieve an iterator though ``getIterator()`` which can then be used to iterate over the collection. You can not rely on the internal iterator of the collection being at a certain position unless you explicitly positioned it before. +Methods that do not alter the collection or have template types +appearing in invariant or contravariant positions are not directly +defined in ``Doctrine\Common\Collections\Collection``, but are inherited +from the ``Doctrine\Common\Collections\ReadableCollection`` interface. + The methods available on the interface are: add diff --git a/lib/Doctrine/Common/Collections/AbstractLazyCollection.php b/lib/Doctrine/Common/Collections/AbstractLazyCollection.php index 6b0743e03..baab4d599 100644 --- a/lib/Doctrine/Common/Collections/AbstractLazyCollection.php +++ b/lib/Doctrine/Common/Collections/AbstractLazyCollection.php @@ -61,6 +61,8 @@ public function clear() /** * {@inheritDoc} + * + * @template TMaybeContained */ public function contains($element) { @@ -260,6 +262,8 @@ public function partition(Closure $p) /** * {@inheritDoc} + * + * @template TMaybeContained */ public function indexOf($element) { diff --git a/lib/Doctrine/Common/Collections/ArrayCollection.php b/lib/Doctrine/Common/Collections/ArrayCollection.php index 8637c9286..186f6ec4c 100644 --- a/lib/Doctrine/Common/Collections/ArrayCollection.php +++ b/lib/Doctrine/Common/Collections/ArrayCollection.php @@ -231,6 +231,8 @@ public function containsKey($key) /** * {@inheritDoc} + * + * @template TMaybeContained */ public function contains($element) { @@ -253,6 +255,12 @@ public function exists(Closure $p) /** * {@inheritDoc} + * + * @psalm-param TMaybeContained $element + * + * @psalm-return (TMaybeContained is T ? TKey|false : false) + * + * @template TMaybeContained */ public function indexOf($element) { diff --git a/lib/Doctrine/Common/Collections/Collection.php b/lib/Doctrine/Common/Collections/Collection.php index d3f2061ad..fb091923c 100644 --- a/lib/Doctrine/Common/Collections/Collection.php +++ b/lib/Doctrine/Common/Collections/Collection.php @@ -4,8 +4,6 @@ use ArrayAccess; use Closure; -use Countable; -use IteratorAggregate; /** * The missing (SPL) Collection/Array/OrderedMap interface. @@ -26,10 +24,10 @@ * * @psalm-template TKey of array-key * @psalm-template T - * @template-extends IteratorAggregate + * @template-extends ReadableCollection * @template-extends ArrayAccess */ -interface Collection extends Countable, IteratorAggregate, ArrayAccess +interface Collection extends ReadableCollection, ArrayAccess { /** * Adds an element at the end of the collection. @@ -48,24 +46,6 @@ public function add($element); */ public function clear(); - /** - * Checks whether an element is contained in the collection. - * This is an O(n) operation, where n is the size of the collection. - * - * @param mixed $element The element to search for. - * @psalm-param T $element - * - * @return bool TRUE if the collection contains the element, FALSE otherwise. - */ - public function contains($element); - - /** - * Checks whether the collection is empty (contains no elements). - * - * @return bool TRUE if the collection is empty, FALSE otherwise. - */ - public function isEmpty(); - /** * Removes the element at the specified index from the collection. * @@ -87,46 +67,6 @@ public function remove($key); */ public function removeElement($element); - /** - * Checks whether the collection contains an element with the specified key/index. - * - * @param string|int $key The key/index to check for. - * @psalm-param TKey $key - * - * @return bool TRUE if the collection contains an element with the specified key/index, - * FALSE otherwise. - */ - public function containsKey($key); - - /** - * Gets the element at the specified key/index. - * - * @param string|int $key The key/index of the element to retrieve. - * @psalm-param TKey $key - * - * @return mixed - * @psalm-return T|null - */ - public function get($key); - - /** - * Gets all keys/indices of the collection. - * - * @return int[]|string[] The keys/indices of the collection, in the order of the corresponding - * elements in the collection. - * @psalm-return TKey[] - */ - public function getKeys(); - - /** - * Gets all values of the collection. - * - * @return mixed[] The values of all elements in the collection, in the - * order they appear in the collection. - * @psalm-return list - */ - public function getValues(); - /** * Sets an element in the collection at the specified key/index. * @@ -140,69 +80,7 @@ public function getValues(); public function set($key, $value); /** - * Gets a native PHP array representation of the collection. - * - * @return mixed[] - * @psalm-return array - */ - public function toArray(); - - /** - * Sets the internal iterator to the first element in the collection and returns this element. - * - * @return mixed - * @psalm-return T|false - */ - public function first(); - - /** - * Sets the internal iterator to the last element in the collection and returns this element. - * - * @return mixed - * @psalm-return T|false - */ - public function last(); - - /** - * Gets the key/index of the element at the current iterator position. - * - * @return int|string|null - * @psalm-return TKey|null - */ - public function key(); - - /** - * Gets the element of the collection at the current iterator position. - * - * @return mixed - * @psalm-return T|false - */ - public function current(); - - /** - * Moves the internal iterator position to the next element and returns this element. - * - * @return mixed - * @psalm-return T|false - */ - public function next(); - - /** - * Tests for the existence of an element that satisfies the given predicate. - * - * @param Closure $p The predicate. - * @psalm-param Closure(TKey, T):bool $p - * - * @return bool TRUE if the predicate is TRUE for at least one element, FALSE otherwise. - */ - public function exists(Closure $p); - - /** - * Returns all the elements of this collection that satisfy the predicate p. - * The order of the elements is preserved. - * - * @param Closure $p The predicate used for filtering. - * @psalm-param Closure(T):bool $p + * {@inheritdoc} * * @return Collection A collection with the results of the filter operation. * @psalm-return Collection @@ -210,34 +88,7 @@ public function exists(Closure $p); public function filter(Closure $p); /** - * Tests whether the given predicate p holds for all elements of this collection. - * - * @param Closure $p The predicate. - * @psalm-param Closure(TKey, T):bool $p - * - * @return bool TRUE, if the predicate yields TRUE for all elements, FALSE otherwise. - */ - public function forAll(Closure $p); - - /** - * Applies the given function to each element in the collection and returns - * a new collection with the elements returned by the function. - * - * @psalm-param Closure(T):U $func - * - * @return Collection - * @psalm-return Collection - * - * @psalm-template U - */ - public function map(Closure $func); - - /** - * Partitions this collection in two collections according to a predicate. - * Keys are preserved in the resulting collections. - * - * @param Closure $p The predicate on which to partition. - * @psalm-param Closure(TKey, T):bool $p + * {@inheritdoc} * * @return Collection[] An array with two elements. The first element contains the collection * of elements where the predicate returned TRUE, the second element @@ -245,32 +96,4 @@ public function map(Closure $func); * @psalm-return array{0: Collection, 1: Collection} */ public function partition(Closure $p); - - /** - * Gets the index/key of a given element. The comparison of two elements is strict, - * that means not only the value but also the type must match. - * For objects this means reference equality. - * - * @param mixed $element The element to search for. - * @psalm-param T $element - * - * @return int|string|bool The key/index of the element or FALSE if the element was not found. - * @psalm-return TKey|false - */ - public function indexOf($element); - - /** - * Extracts a slice of $length elements starting at position $offset from the Collection. - * - * If $length is null it returns all elements from $offset to the end of the Collection. - * Keys have to be preserved by this method. Calling this method will only return the - * selected slice and NOT change the elements contained in the collection slice is called on. - * - * @param int $offset The offset to start from. - * @param int|null $length The maximum number of elements to return, or null for no limit. - * - * @return mixed[] - * @psalm-return array - */ - public function slice($offset, $length = null); } diff --git a/lib/Doctrine/Common/Collections/ReadableCollection.php b/lib/Doctrine/Common/Collections/ReadableCollection.php new file mode 100644 index 000000000..aa36087de --- /dev/null +++ b/lib/Doctrine/Common/Collections/ReadableCollection.php @@ -0,0 +1,213 @@ + + */ +interface ReadableCollection extends Countable, IteratorAggregate +{ + /** + * Checks whether an element is contained in the collection. + * This is an O(n) operation, where n is the size of the collection. + * + * @param mixed $element The element to search for. + * @psalm-param TMaybeContained $element + * + * @return bool TRUE if the collection contains the element, FALSE otherwise. + * @psalm-return (TMaybeContained is T ? bool : false) + * + * @template TMaybeContained + */ + public function contains($element); + + /** + * Checks whether the collection is empty (contains no elements). + * + * @return bool TRUE if the collection is empty, FALSE otherwise. + */ + public function isEmpty(); + + /** + * Checks whether the collection contains an element with the specified key/index. + * + * @param string|int $key The key/index to check for. + * @psalm-param TKey $key + * + * @return bool TRUE if the collection contains an element with the specified key/index, + * FALSE otherwise. + */ + public function containsKey($key); + + /** + * Gets the element at the specified key/index. + * + * @param string|int $key The key/index of the element to retrieve. + * @psalm-param TKey $key + * + * @return mixed + * @psalm-return T|null + */ + public function get($key); + + /** + * Gets all keys/indices of the collection. + * + * @return int[]|string[] The keys/indices of the collection, in the order of the corresponding + * elements in the collection. + * @psalm-return TKey[] + */ + public function getKeys(); + + /** + * Gets all values of the collection. + * + * @return mixed[] The values of all elements in the collection, in the + * order they appear in the collection. + * @psalm-return list + */ + public function getValues(); + + /** + * Gets a native PHP array representation of the collection. + * + * @return mixed[] + * @psalm-return array + */ + public function toArray(); + + /** + * Sets the internal iterator to the first element in the collection and returns this element. + * + * @return mixed + * @psalm-return T|false + */ + public function first(); + + /** + * Sets the internal iterator to the last element in the collection and returns this element. + * + * @return mixed + * @psalm-return T|false + */ + public function last(); + + /** + * Gets the key/index of the element at the current iterator position. + * + * @return int|string|null + * @psalm-return TKey|null + */ + public function key(); + + /** + * Gets the element of the collection at the current iterator position. + * + * @return mixed + * @psalm-return T|false + */ + public function current(); + + /** + * Moves the internal iterator position to the next element and returns this element. + * + * @return mixed + * @psalm-return T|false + */ + public function next(); + + /** + * Extracts a slice of $length elements starting at position $offset from the Collection. + * + * If $length is null it returns all elements from $offset to the end of the Collection. + * Keys have to be preserved by this method. Calling this method will only return the + * selected slice and NOT change the elements contained in the collection slice is called on. + * + * @param int $offset The offset to start from. + * @param int|null $length The maximum number of elements to return, or null for no limit. + * + * @return mixed[] + * @psalm-return array + */ + public function slice($offset, $length = null); + + /** + * Tests for the existence of an element that satisfies the given predicate. + * + * @param Closure $p The predicate. + * @psalm-param Closure(TKey, T):bool $p + * + * @return bool TRUE if the predicate is TRUE for at least one element, FALSE otherwise. + */ + public function exists(Closure $p); + + /** + * Returns all the elements of this collection that satisfy the predicate p. + * The order of the elements is preserved. + * + * @param Closure $p The predicate used for filtering. + * @psalm-param Closure(T):bool $p + * + * @return ReadableCollection A collection with the results of the filter operation. + * @psalm-return ReadableCollection + */ + public function filter(Closure $p); + + /** + * Applies the given function to each element in the collection and returns + * a new collection with the elements returned by the function. + * + * @psalm-param Closure(T):U $func + * + * @return Collection + * @psalm-return Collection + * + * @psalm-template U + */ + public function map(Closure $func); + + /** + * Partitions this collection in two collections according to a predicate. + * Keys are preserved in the resulting collections. + * + * @param Closure $p The predicate on which to partition. + * @psalm-param Closure(TKey, T):bool $p + * + * @return ReadableCollection[] An array with two elements. The first element contains the collection + * of elements where the predicate returned TRUE, the second element + * contains the collection of elements where the predicate returned FALSE. + * @psalm-return array{0: ReadableCollection, 1: ReadableCollection} + */ + public function partition(Closure $p); + + /** + * Tests whether the given predicate p holds for all elements of this collection. + * + * @param Closure $p The predicate. + * @psalm-param Closure(TKey, T):bool $p + * + * @return bool TRUE, if the predicate yields TRUE for all elements, FALSE otherwise. + */ + public function forAll(Closure $p); + + /** + * Gets the index/key of a given element. The comparison of two elements is strict, + * that means not only the value but also the type must match. + * For objects this means reference equality. + * + * @param mixed $element The element to search for. + * @psalm-param TMaybeContained $element + * + * @return int|string|bool The key/index of the element or FALSE if the element was not found. + * @psalm-return (TMaybeContained is T ? TKey|false : false) + * + * @template TMaybeContained + */ + public function indexOf($element); +} diff --git a/psalm.xml.dist b/psalm.xml.dist index e54717882..53ad4f2f4 100644 --- a/psalm.xml.dist +++ b/psalm.xml.dist @@ -37,5 +37,12 @@ + + + + + + + From 60f59cec1935224ea24b47ac75662229c94fc584 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Gr=C3=A9goire=20Paris?= Date: Sat, 27 Aug 2022 18:28:24 +0200 Subject: [PATCH 2/2] Make phpdoc of getKeys() more precise --- lib/Doctrine/Common/Collections/ReadableCollection.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/Doctrine/Common/Collections/ReadableCollection.php b/lib/Doctrine/Common/Collections/ReadableCollection.php index aa36087de..44b374969 100644 --- a/lib/Doctrine/Common/Collections/ReadableCollection.php +++ b/lib/Doctrine/Common/Collections/ReadableCollection.php @@ -61,7 +61,7 @@ public function get($key); * * @return int[]|string[] The keys/indices of the collection, in the order of the corresponding * elements in the collection. - * @psalm-return TKey[] + * @psalm-return list */ public function getKeys();