Skip to content

Add impure annotation in some Collection methods #401

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 4 commits into from
Dec 30, 2022
Merged

Add impure annotation in some Collection methods #401

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
79f2f94
Add impure annotation in some Collection methods
VincentLanglet Dec 18, 2022
90ad128
Add phpdoc
VincentLanglet Dec 18, 2022
e6adce1
Rework
VincentLanglet Dec 21, 2022
5013e0f
Refacto test
VincentLanglet Dec 21, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
27 changes: 27 additions & 0 deletions stubs/Collections/Collection.stub
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,4 +16,31 @@ use IteratorAggregate;
interface Collection extends Countable, IteratorAggregate, ArrayAccess, ReadableCollection
{

/**
* @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) {}

}
50 changes: 15 additions & 35 deletions tests/Type/Doctrine/Collection/IsEmptyTypeSpecifyingExtensionTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,48 +2,28 @@

namespace PHPStan\Type\Doctrine\Collection;

use PHPStan\Rules\Rule;
use PHPStan\Testing\RuleTestCase;
use PHPStan\Testing\TypeInferenceTestCase;

/**
* @extends RuleTestCase<VariableTypeReportingRule>
*/
class IsEmptyTypeSpecifyingExtensionTest extends RuleTestCase
class IsEmptyTypeSpecifyingExtensionTest extends TypeInferenceTestCase
{

protected function getRule(): Rule
/** @return iterable<mixed> */
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,
],
]);
$this->assertFileAsserts($assertType, $file, ...$args);
}

public static function getAdditionalConfigFiles(): array
Expand Down
32 changes: 26 additions & 6 deletions tests/Type/Doctrine/Collection/data/collection.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@

use Doctrine\Common\Collections\ArrayCollection;

use function PHPStan\Testing\assertType;

class MyEntity
{

Expand All @@ -15,23 +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();
assertType('MyEntity|false', $result1);

$result2 = $collection->last();
assertType('MyEntity|false', $result2);
}
Copy link
Member

@ondrejmirtes ondrejmirtes Dec 19, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please do not modify the test like that, the diff of the rule test case is unreadable. Also, it'd make sense to refactor this into a TypeInferenceTestCase :) Thank you.

Copy link
Contributor Author

@VincentLanglet VincentLanglet Dec 21, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The test is now refactored into a TypeInferenceTestCase ;)


if (!$collection->isEmpty()) {
$collection->removeElement($new);
$result3 = $collection->first();
assertType('MyEntity|false', $result3);

$result4 = $collection->last();
assertType('MyEntity|false', $result4);
}