From 79f2f94ee0d18796ed5601361001ed18cc9da1b9 Mon Sep 17 00:00:00 2001 From: Vincent Langlet Date: Sun, 18 Dec 2022 21:18:50 +0100 Subject: [PATCH 1/4] Add impure annotation in some Collection methods --- stubs/Collections/Collection.stub | 15 ++++++++++++++ .../IsEmptyTypeSpecifyingExtensionTest.php | 20 +++++++++++++++++-- .../Doctrine/Collection/data/collection.php | 14 +++++++++++++ 3 files changed, 47 insertions(+), 2 deletions(-) diff --git a/stubs/Collections/Collection.stub b/stubs/Collections/Collection.stub index dbf36bd6..4741be04 100644 --- a/stubs/Collections/Collection.stub +++ b/stubs/Collections/Collection.stub @@ -16,4 +16,19 @@ use IteratorAggregate; interface Collection extends Countable, IteratorAggregate, ArrayAccess, ReadableCollection { + /** + * @phpstan-impure + */ + public function add($element) {} + + /** + * @phpstan-impure + */ + public function remove($key) {} + + /** + * @phpstan-impure + */ + public function removeElement($element) {} + } diff --git a/tests/Type/Doctrine/Collection/IsEmptyTypeSpecifyingExtensionTest.php b/tests/Type/Doctrine/Collection/IsEmptyTypeSpecifyingExtensionTest.php index 1678253b..d3a8117c 100644 --- a/tests/Type/Doctrine/Collection/IsEmptyTypeSpecifyingExtensionTest.php +++ b/tests/Type/Doctrine/Collection/IsEmptyTypeSpecifyingExtensionTest.php @@ -35,13 +35,29 @@ public function testExtension(): void 'Variable $false2 is: false', 28, ], + [ + 'Variable $result1 is: MyEntity|false', + 32, + ], + [ + 'Variable $result2 is: MyEntity|false', + 35, + ], [ 'Variable $entity1 is: MyEntity', - 33, + 40, ], [ 'Variable $entity2 is: MyEntity', - 36, + 43, + ], + [ + 'Variable $result3 is: MyEntity|false', + 47, + ], + [ + 'Variable $result4 is: MyEntity|false', + 50, ], ]); } diff --git a/tests/Type/Doctrine/Collection/data/collection.php b/tests/Type/Doctrine/Collection/data/collection.php index 249a8152..17d0089a 100644 --- a/tests/Type/Doctrine/Collection/data/collection.php +++ b/tests/Type/Doctrine/Collection/data/collection.php @@ -26,6 +26,13 @@ class MyEntity $false2 = $collection->last(); $false2; + + $collection->add($new); + $result1 = $collection->first(); + $result1; + + $result2 = $collection->last(); + $result2; } if (!$collection->isEmpty()) { @@ -34,4 +41,11 @@ class MyEntity $entity2 = $collection->last(); $entity2; + + $collection->removeElement($new); + $result3 = $collection->first(); + $result3; + + $result4 = $collection->last(); + $result4; } From 90ad1289ed988967e441f476c8a1a8368c6211cc Mon Sep 17 00:00:00 2001 From: Vincent Langlet Date: Sun, 18 Dec 2022 21:24:29 +0100 Subject: [PATCH 2/4] Add phpdoc --- stubs/Collections/Collection.stub | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/stubs/Collections/Collection.stub b/stubs/Collections/Collection.stub index 4741be04..f6aafc8c 100644 --- a/stubs/Collections/Collection.stub +++ b/stubs/Collections/Collection.stub @@ -18,16 +18,28 @@ interface Collection extends Countable, IteratorAggregate, ArrayAccess, Readable /** * @phpstan-impure + * + * @param T $element + * + * @return true */ public function add($element) {} /** * @phpstan-impure + * + * @param TKey $key + * + * @return T|null */ public function remove($key) {} /** * @phpstan-impure + * + * @param T $element + * + * @return bool */ public function removeElement($element) {} From e6adce15798fbdaad780148d56fc6de4c1715259 Mon Sep 17 00:00:00 2001 From: Vincent Langlet Date: Wed, 21 Dec 2022 23:39:08 +0100 Subject: [PATCH 3/4] Rework --- .../IsEmptyTypeSpecifyingExtensionTest.php | 20 +++++++++---------- .../Doctrine/Collection/data/collection.php | 16 +++++++++------ 2 files changed, 20 insertions(+), 16 deletions(-) diff --git a/tests/Type/Doctrine/Collection/IsEmptyTypeSpecifyingExtensionTest.php b/tests/Type/Doctrine/Collection/IsEmptyTypeSpecifyingExtensionTest.php index d3a8117c..45defd24 100644 --- a/tests/Type/Doctrine/Collection/IsEmptyTypeSpecifyingExtensionTest.php +++ b/tests/Type/Doctrine/Collection/IsEmptyTypeSpecifyingExtensionTest.php @@ -36,28 +36,28 @@ public function testExtension(): void 28, ], [ - 'Variable $result1 is: MyEntity|false', - 32, + 'Variable $entity1 is: MyEntity', + 33, ], [ - 'Variable $result2 is: MyEntity|false', - 35, + 'Variable $entity2 is: MyEntity', + 36, ], [ - 'Variable $entity1 is: MyEntity', - 40, + 'Variable $result1 is: MyEntity|false', + 42, ], [ - 'Variable $entity2 is: MyEntity', - 43, + 'Variable $result2 is: MyEntity|false', + 45, ], [ 'Variable $result3 is: MyEntity|false', - 47, + 51, ], [ 'Variable $result4 is: MyEntity|false', - 50, + 54, ], ]); } diff --git a/tests/Type/Doctrine/Collection/data/collection.php b/tests/Type/Doctrine/Collection/data/collection.php index 17d0089a..c4db7daf 100644 --- a/tests/Type/Doctrine/Collection/data/collection.php +++ b/tests/Type/Doctrine/Collection/data/collection.php @@ -26,7 +26,17 @@ class MyEntity $false2 = $collection->last(); $false2; +} + +if (!$collection->isEmpty()) { + $entity1 = $collection->first(); + $entity1; + + $entity2 = $collection->last(); + $entity2; +} +if ($collection->isEmpty()) { $collection->add($new); $result1 = $collection->first(); $result1; @@ -36,12 +46,6 @@ class MyEntity } if (!$collection->isEmpty()) { - $entity1 = $collection->first(); - $entity1; - - $entity2 = $collection->last(); - $entity2; - $collection->removeElement($new); $result3 = $collection->first(); $result3; From 5013e0fd6ae21faa189230a4110bc8bf5db23edd Mon Sep 17 00:00:00 2001 From: Vincent Langlet Date: Wed, 21 Dec 2022 23:43:12 +0100 Subject: [PATCH 4/4] Refacto test --- .../IsEmptyTypeSpecifyingExtensionTest.php | 66 +++++-------------- .../Doctrine/Collection/data/collection.php | 22 ++++--- 2 files changed, 27 insertions(+), 61 deletions(-) diff --git a/tests/Type/Doctrine/Collection/IsEmptyTypeSpecifyingExtensionTest.php b/tests/Type/Doctrine/Collection/IsEmptyTypeSpecifyingExtensionTest.php index 45defd24..83fc20c9 100644 --- a/tests/Type/Doctrine/Collection/IsEmptyTypeSpecifyingExtensionTest.php +++ b/tests/Type/Doctrine/Collection/IsEmptyTypeSpecifyingExtensionTest.php @@ -2,64 +2,28 @@ namespace PHPStan\Type\Doctrine\Collection; -use PHPStan\Rules\Rule; -use PHPStan\Testing\RuleTestCase; +use PHPStan\Testing\TypeInferenceTestCase; -/** - * @extends RuleTestCase - */ -class IsEmptyTypeSpecifyingExtensionTest extends RuleTestCase +class IsEmptyTypeSpecifyingExtensionTest extends TypeInferenceTestCase { - protected function getRule(): Rule + /** @return iterable */ + public function dataFileAsserts(): iterable { - return new VariableTypeReportingRule(); + yield from $this->gatherAssertTypes(__DIR__ . '/data/collection.php'); } - public function testExtension(): void + /** + * @dataProvider dataFileAsserts + * @param mixed ...$args + */ + public function testFileAsserts( + string $assertType, + string $file, + ...$args + ): void { - $this->analyse([__DIR__ . '/data/collection.php'], [ - [ - 'Variable $entityOrFalse1 is: MyEntity|false', - 18, - ], - [ - 'Variable $entityOrFalse2 is: MyEntity|false', - 21, - ], - [ - 'Variable $false1 is: false', - 25, - ], - [ - 'Variable $false2 is: false', - 28, - ], - [ - 'Variable $entity1 is: MyEntity', - 33, - ], - [ - 'Variable $entity2 is: MyEntity', - 36, - ], - [ - 'Variable $result1 is: MyEntity|false', - 42, - ], - [ - 'Variable $result2 is: MyEntity|false', - 45, - ], - [ - 'Variable $result3 is: MyEntity|false', - 51, - ], - [ - 'Variable $result4 is: MyEntity|false', - 54, - ], - ]); + $this->assertFileAsserts($assertType, $file, ...$args); } public static function getAdditionalConfigFiles(): array diff --git a/tests/Type/Doctrine/Collection/data/collection.php b/tests/Type/Doctrine/Collection/data/collection.php index c4db7daf..14a6c95b 100644 --- a/tests/Type/Doctrine/Collection/data/collection.php +++ b/tests/Type/Doctrine/Collection/data/collection.php @@ -2,6 +2,8 @@ use Doctrine\Common\Collections\ArrayCollection; +use function PHPStan\Testing\assertType; + class MyEntity { @@ -15,41 +17,41 @@ class MyEntity $collection = new ArrayCollection(); $entityOrFalse1 = $collection->first(); -$entityOrFalse1; +assertType('MyEntity|false', $entityOrFalse1); $entityOrFalse2 = $collection->last(); -$entityOrFalse2; +assertType('MyEntity|false', $entityOrFalse2); if ($collection->isEmpty()) { $false1 = $collection->first(); - $false1; + assertType('false', $false1); $false2 = $collection->last(); - $false2; + assertType('false', $false2); } if (!$collection->isEmpty()) { $entity1 = $collection->first(); - $entity1; + assertType('MyEntity', $entity1); $entity2 = $collection->last(); - $entity2; + assertType('MyEntity', $entity2); } if ($collection->isEmpty()) { $collection->add($new); $result1 = $collection->first(); - $result1; + assertType('MyEntity|false', $result1); $result2 = $collection->last(); - $result2; + assertType('MyEntity|false', $result2); } if (!$collection->isEmpty()) { $collection->removeElement($new); $result3 = $collection->first(); - $result3; + assertType('MyEntity|false', $result3); $result4 = $collection->last(); - $result4; + assertType('MyEntity|false', $result4); }