From d54bec90e158892c4e1c6f0415032af57100e313 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=A9r=C3=B4me=20Tamarelle?= Date: Thu, 1 Jun 2023 23:16:24 +0200 Subject: [PATCH 1/2] PHPLIB-113 Use array_is_list --- composer.json | 3 ++- src/Operation/Aggregate.php | 11 ++++------- src/Operation/BulkWrite.php | 11 ++++------- src/Operation/CreateCollection.php | 11 ++++------- src/Operation/CreateIndexes.php | 11 ++++------- src/Operation/InsertMany.php | 11 ++++------- src/functions.php | 13 +++++-------- tests/Operation/AggregateTest.php | 2 +- tests/Operation/BulkWriteTest.php | 2 +- tests/Operation/CreateCollectionTest.php | 2 +- tests/Operation/CreateIndexesTest.php | 2 +- tests/Operation/InsertManyTest.php | 2 +- tests/Operation/WatchTest.php | 2 +- 13 files changed, 33 insertions(+), 50 deletions(-) diff --git a/composer.json b/composer.json index 772723852..fe9698d0e 100644 --- a/composer.json +++ b/composer.json @@ -15,7 +15,8 @@ "ext-mongodb": "^1.16.0", "jean85/pretty-package-versions": "^2.0.1", "symfony/polyfill-php73": "^1.27", - "symfony/polyfill-php80": "^1.27" + "symfony/polyfill-php80": "^1.27", + "symfony/polyfill-php81": "^1.27" }, "require-dev": { "doctrine/coding-standard": "^11.1", diff --git a/src/Operation/Aggregate.php b/src/Operation/Aggregate.php index 010194dff..48a892d80 100644 --- a/src/Operation/Aggregate.php +++ b/src/Operation/Aggregate.php @@ -31,6 +31,7 @@ use MongoDB\Exception\UnsupportedException; use stdClass; +use function array_is_list; use function current; use function is_array; use function is_bool; @@ -137,18 +138,14 @@ class Aggregate implements Executable, Explainable */ public function __construct(string $databaseName, ?string $collectionName, array $pipeline, array $options = []) { - $expectedIndex = 0; + if (! array_is_list($pipeline)) { + throw new InvalidArgumentException('$pipeline is not a list'); + } foreach ($pipeline as $i => $operation) { - if ($i !== $expectedIndex) { - throw new InvalidArgumentException(sprintf('$pipeline is not a list (unexpected index: "%s")', $i)); - } - if (! is_array($operation) && ! is_object($operation)) { throw InvalidArgumentException::invalidType(sprintf('$pipeline[%d]', $i), $operation, 'array or object'); } - - $expectedIndex += 1; } $options += ['useCursor' => true]; diff --git a/src/Operation/BulkWrite.php b/src/Operation/BulkWrite.php index b68daed4d..e104d5d73 100644 --- a/src/Operation/BulkWrite.php +++ b/src/Operation/BulkWrite.php @@ -26,6 +26,7 @@ use MongoDB\Exception\InvalidArgumentException; use MongoDB\Exception\UnsupportedException; +use function array_is_list; use function array_key_exists; use function count; use function current; @@ -132,13 +133,11 @@ public function __construct(string $databaseName, string $collectionName, array throw new InvalidArgumentException('$operations is empty'); } - $expectedIndex = 0; + if (! array_is_list($operations)) { + throw new InvalidArgumentException('$operations is not a list'); + } foreach ($operations as $i => $operation) { - if ($i !== $expectedIndex) { - throw new InvalidArgumentException(sprintf('$operations is not a list (unexpected index: "%s")', $i)); - } - if (! is_array($operation)) { throw InvalidArgumentException::invalidType(sprintf('$operations[%d]', $i), $operation, 'array'); } @@ -271,8 +270,6 @@ public function __construct(string $databaseName, string $collectionName, array default: throw new InvalidArgumentException(sprintf('Unknown operation type "%s" in $operations[%d]', $type, $i)); } - - $expectedIndex += 1; } $options += ['ordered' => true]; diff --git a/src/Operation/CreateCollection.php b/src/Operation/CreateCollection.php index a4cc721c0..cfbacd7e4 100644 --- a/src/Operation/CreateCollection.php +++ b/src/Operation/CreateCollection.php @@ -24,6 +24,7 @@ use MongoDB\Driver\WriteConcern; use MongoDB\Exception\InvalidArgumentException; +use function array_is_list; use function current; use function is_array; use function is_bool; @@ -241,18 +242,14 @@ public function __construct(string $databaseName, string $collectionName, array } if (isset($options['pipeline'])) { - $expectedIndex = 0; + if (! array_is_list($options['pipeline'])) { + throw new InvalidArgumentException('The "pipeline" option is not a list'); + } foreach ($options['pipeline'] as $i => $operation) { - if ($i !== $expectedIndex) { - throw new InvalidArgumentException(sprintf('The "pipeline" option is not a list (unexpected index: "%s")', $i)); - } - if (! is_array($operation) && ! is_object($operation)) { throw InvalidArgumentException::invalidType(sprintf('$options["pipeline"][%d]', $i), $operation, 'array or object'); } - - $expectedIndex += 1; } } diff --git a/src/Operation/CreateIndexes.php b/src/Operation/CreateIndexes.php index 0b3ed2728..1b8414fd8 100644 --- a/src/Operation/CreateIndexes.php +++ b/src/Operation/CreateIndexes.php @@ -26,6 +26,7 @@ use MongoDB\Exception\UnsupportedException; use MongoDB\Model\IndexInput; +use function array_is_list; use function array_map; use function is_array; use function is_integer; @@ -88,20 +89,16 @@ public function __construct(string $databaseName, string $collectionName, array throw new InvalidArgumentException('$indexes is empty'); } - $expectedIndex = 0; + if (! array_is_list($indexes)) { + throw new InvalidArgumentException('$indexes is not a list'); + } foreach ($indexes as $i => $index) { - if ($i !== $expectedIndex) { - throw new InvalidArgumentException(sprintf('$indexes is not a list (unexpected index: "%s")', $i)); - } - if (! is_array($index)) { throw InvalidArgumentException::invalidType(sprintf('$index[%d]', $i), $index, 'array'); } $this->indexes[] = new IndexInput($index); - - $expectedIndex += 1; } if (isset($options['commitQuorum']) && ! is_string($options['commitQuorum']) && ! is_integer($options['commitQuorum'])) { diff --git a/src/Operation/InsertMany.php b/src/Operation/InsertMany.php index 816315dcd..a257d069c 100644 --- a/src/Operation/InsertMany.php +++ b/src/Operation/InsertMany.php @@ -26,6 +26,7 @@ use MongoDB\Exception\UnsupportedException; use MongoDB\InsertManyResult; +use function array_is_list; use function is_array; use function is_bool; use function is_object; @@ -84,18 +85,14 @@ public function __construct(string $databaseName, string $collectionName, array throw new InvalidArgumentException('$documents is empty'); } - $expectedIndex = 0; + if (! array_is_list($documents)) { + throw new InvalidArgumentException('$documents is not a list'); + } foreach ($documents as $i => $document) { - if ($i !== $expectedIndex) { - throw new InvalidArgumentException(sprintf('$documents is not a list (unexpected index: "%s")', $i)); - } - if (! is_array($document) && ! is_object($document)) { throw InvalidArgumentException::invalidType(sprintf('$documents[%d]', $i), $document, 'array or object'); } - - $expectedIndex += 1; } $options += ['ordered' => true]; diff --git a/src/functions.php b/src/functions.php index 5d4e0add3..b3a14dd9e 100644 --- a/src/functions.php +++ b/src/functions.php @@ -34,6 +34,7 @@ use ReflectionClass; use ReflectionException; +use function array_is_list; use function array_key_first; use function assert; use function end; @@ -260,19 +261,15 @@ function is_pipeline($pipeline, bool $allowEmpty = false): bool return $allowEmpty; } - $expectedKey = 0; + if (! array_is_list($pipeline)) { + return false; + } - foreach ($pipeline as $key => $stage) { + foreach ($pipeline as $stage) { if (! is_array($stage) && ! is_object($stage)) { return false; } - if ($expectedKey !== $key) { - return false; - } - - $expectedKey++; - if (! is_first_key_operator($stage)) { return false; } diff --git a/tests/Operation/AggregateTest.php b/tests/Operation/AggregateTest.php index 3ce450ad7..7e9a66e85 100644 --- a/tests/Operation/AggregateTest.php +++ b/tests/Operation/AggregateTest.php @@ -10,7 +10,7 @@ class AggregateTest extends TestCase public function testConstructorPipelineArgumentMustBeAList(): void { $this->expectException(InvalidArgumentException::class); - $this->expectExceptionMessage('$pipeline is not a list (unexpected index: "1")'); + $this->expectExceptionMessage('$pipeline is not a list'); new Aggregate($this->getDatabaseName(), $this->getCollectionName(), [1 => ['$match' => ['x' => 1]]]); } diff --git a/tests/Operation/BulkWriteTest.php b/tests/Operation/BulkWriteTest.php index ac21e558e..444178cf7 100644 --- a/tests/Operation/BulkWriteTest.php +++ b/tests/Operation/BulkWriteTest.php @@ -17,7 +17,7 @@ public function testOperationsMustNotBeEmpty(): void public function testOperationsMustBeAList(): void { $this->expectException(InvalidArgumentException::class); - $this->expectExceptionMessage('$operations is not a list (unexpected index: "1")'); + $this->expectExceptionMessage('$operations is not a list'); new BulkWrite($this->getDatabaseName(), $this->getCollectionName(), [ 1 => [BulkWrite::INSERT_ONE => [['x' => 1]]], ]); diff --git a/tests/Operation/CreateCollectionTest.php b/tests/Operation/CreateCollectionTest.php index c3e58b0a2..5be217f8f 100644 --- a/tests/Operation/CreateCollectionTest.php +++ b/tests/Operation/CreateCollectionTest.php @@ -10,7 +10,7 @@ class CreateCollectionTest extends TestCase public function testConstructorPipelineOptionMustBeAList(): void { $this->expectException(InvalidArgumentException::class); - $this->expectExceptionMessage('The "pipeline" option is not a list (unexpected index: "1")'); + $this->expectExceptionMessage('The "pipeline" option is not a list'); new CreateCollection($this->getDatabaseName(), $this->getCollectionName(), ['pipeline' => [1 => ['$match' => ['x' => 1]]]]); } diff --git a/tests/Operation/CreateIndexesTest.php b/tests/Operation/CreateIndexesTest.php index 80db10078..74c386f30 100644 --- a/tests/Operation/CreateIndexesTest.php +++ b/tests/Operation/CreateIndexesTest.php @@ -14,7 +14,7 @@ class CreateIndexesTest extends TestCase public function testConstructorIndexesArgumentMustBeAList(): void { $this->expectException(InvalidArgumentException::class); - $this->expectExceptionMessage('$indexes is not a list (unexpected index: "1")'); + $this->expectExceptionMessage('$indexes is not a list'); new CreateIndexes($this->getDatabaseName(), $this->getCollectionName(), [1 => ['key' => ['x' => 1]]]); } diff --git a/tests/Operation/InsertManyTest.php b/tests/Operation/InsertManyTest.php index eb283a970..67d91418e 100644 --- a/tests/Operation/InsertManyTest.php +++ b/tests/Operation/InsertManyTest.php @@ -17,7 +17,7 @@ public function testConstructorDocumentsMustNotBeEmpty(): void public function testConstructorDocumentsMustBeAList(): void { $this->expectException(InvalidArgumentException::class); - $this->expectExceptionMessage('$documents is not a list (unexpected index: "1")'); + $this->expectExceptionMessage('$documents is not a list'); new InsertMany($this->getDatabaseName(), $this->getCollectionName(), [1 => ['x' => 1]]); } diff --git a/tests/Operation/WatchTest.php b/tests/Operation/WatchTest.php index 64c490fa2..30b45a13e 100644 --- a/tests/Operation/WatchTest.php +++ b/tests/Operation/WatchTest.php @@ -23,7 +23,7 @@ public function testConstructorCollectionNameShouldBeNullIfDatabaseNameIsNull(): public function testConstructorPipelineArgumentMustBeAList(): void { $this->expectException(InvalidArgumentException::class); - $this->expectExceptionMessage('$pipeline is not a list (unexpected index: "foo")'); + $this->expectExceptionMessage('$pipeline is not a list'); /* Note: Watch uses array_unshift() to prepend the $changeStream stage * to the pipeline. Since array_unshift() reindexes numeric keys, we'll From 006d1f59c47ad8446e622dc2a5bbcd55e301c455 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=A9r=C3=B4me=20Tamarelle?= Date: Tue, 13 Jun 2023 13:47:55 +0200 Subject: [PATCH 2/2] Use assert for ambuguous type --- psalm-baseline.xml | 6 +++--- src/Operation/CreateCollection.php | 5 ++++- 2 files changed, 7 insertions(+), 4 deletions(-) diff --git a/psalm-baseline.xml b/psalm-baseline.xml index b8640c4f2..dcc159ab8 100644 --- a/psalm-baseline.xml +++ b/psalm-baseline.xml @@ -417,13 +417,13 @@ - - $i + $i $this->options['typeMap'] - + $cmd[$option] + $i $options['session'] $options['writeConcern'] diff --git a/src/Operation/CreateCollection.php b/src/Operation/CreateCollection.php index cfbacd7e4..db85e172d 100644 --- a/src/Operation/CreateCollection.php +++ b/src/Operation/CreateCollection.php @@ -25,6 +25,7 @@ use MongoDB\Exception\InvalidArgumentException; use function array_is_list; +use function assert; use function current; use function is_array; use function is_bool; @@ -242,7 +243,9 @@ public function __construct(string $databaseName, string $collectionName, array } if (isset($options['pipeline'])) { - if (! array_is_list($options['pipeline'])) { + $pipeline = $options['pipeline']; + assert(is_array($pipeline)); + if (! array_is_list($pipeline)) { throw new InvalidArgumentException('The "pipeline" option is not a list'); }