Skip to content
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.

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

Psalm level 5 #2408

Merged
merged 1 commit into from
Feb 18, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
2 changes: 1 addition & 1 deletion composer.json
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@
"phpunit/phpunit": "^8.5 || ^9",
"squizlabs/php_codesniffer": "^3.5",
"symfony/cache": "^4.4 || ^5.0 || ^6.0",
"vimeo/psalm": "^4.8.1"
"vimeo/psalm": "^4.20.0"
},
"suggest": {
"ext-bcmath": "Decimal128 type support"
Expand Down
8 changes: 4 additions & 4 deletions lib/Doctrine/ODM/MongoDB/Iterator/PrimingIterator.php
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ final class PrimingIterator implements Iterator
/** @var ReferencePrimer */
private $referencePrimer;

/** @var array<string, callable|null> */
/** @var array<string, callable|true|null> */
private $primers;

/**
Expand All @@ -42,9 +42,9 @@ final class PrimingIterator implements Iterator
private $referencesPrimed = false;

/**
* @param \Iterator<mixed, TValue> $iterator
* @param ClassMetadata<TDocument> $class
* @param array<string, callable|null> $primers
* @param \Iterator<mixed, TValue> $iterator
* @param ClassMetadata<TDocument> $class
* @param array<string, callable|true|null> $primers
* @psalm-param Hints $unitOfWorkHints
*/
public function __construct(\Iterator $iterator, ClassMetadata $class, ReferencePrimer $referencePrimer, array $primers, array $unitOfWorkHints = [])
Expand Down
23 changes: 19 additions & 4 deletions lib/Doctrine/ODM/MongoDB/Mapping/Driver/AttributeReader.php
Original file line number Diff line number Diff line change
Expand Up @@ -20,16 +20,17 @@
*/
final class AttributeReader implements Reader
{
/**
* @param ReflectionClass<object> $class
*/
public function getClassAnnotations(ReflectionClass $class): array
{
return $this->convertToAttributeInstances($class->getAttributes());
}

/**
* @param ReflectionClass<object> $class
* @param class-string<T> $annotationName
*
* @return T|null
*
* @template T
*/
public function getClassAnnotation(ReflectionClass $class, $annotationName)
{
Expand All @@ -47,6 +48,13 @@ public function getMethodAnnotations(ReflectionMethod $method): array
return $this->convertToAttributeInstances($method->getAttributes());
}

/**
* @param class-string<T> $annotationName
*
* @return T|null
*
* @template T
*/
public function getMethodAnnotation(ReflectionMethod $method, $annotationName)
{
foreach ($this->getMethodAnnotations($method) as $annotation) {
Expand All @@ -63,6 +71,13 @@ public function getPropertyAnnotations(ReflectionProperty $property): array
return $this->convertToAttributeInstances($property->getAttributes());
}

/**
* @param class-string<T> $annotationName
*
* @return T|null
*
* @template T
*/
public function getPropertyAnnotation(ReflectionProperty $property, $annotationName)
{
foreach ($this->getPropertyAnnotations($property) as $annotation) {
Expand Down
2 changes: 1 addition & 1 deletion lib/Doctrine/ODM/MongoDB/Mapping/Driver/XmlDriver.php
Original file line number Diff line number Diff line change
Expand Up @@ -571,7 +571,7 @@ private function addIndex(ClassMetadata $class, SimpleXMLElement $xmlIndex): voi
}

/**
* @return array<string, array<string, mixed>|scalar>
* @return array<string, array<string, mixed>|scalar|null>
Copy link
Member

Choose a reason for hiding this comment

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

null doesn't look like an acceptable value for the array but I guess it comes from convertXMLElementValue so let's live with that

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yep, exactly

*/
private function getPartialFilterExpression(SimpleXMLElement $fields): array
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -428,6 +428,7 @@ public function isEmpty()

/**
* @return Traversable
* @psalm-return Traversable<TKey, T>
*/
#[ReturnTypeWillChange]
public function getIterator()
Expand Down Expand Up @@ -540,6 +541,7 @@ public function offsetExists($offset)
* @param mixed $offset
*
* @return mixed
* @psalm-return T|null
*/
#[ReturnTypeWillChange]
public function offsetGet($offset)
Expand Down Expand Up @@ -660,6 +662,11 @@ private function doAdd($value, $arrayAccess)
* @param mixed $offset
*
* @return bool|T|null
* @psalm-return (
* $arrayAccess is false
* ? T|null
* : T|null|true
Copy link
Member

Choose a reason for hiding this comment

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

oh boy, now we're programming in comments :D

Copy link
Contributor Author

Choose a reason for hiding this comment

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

😬 this can be really interesting when adding generics for example to Query, that depending on the hydrate value, maybe we change the return type of Query::execute() to just Iterator<TDocument> which would mean removing quite assert calls.

* )
*/
private function doRemove($offset, bool $arrayAccess)
{
Expand Down
17 changes: 13 additions & 4 deletions lib/Doctrine/ODM/MongoDB/Proxy/Factory/StaticProxyFactory.php
Original file line number Diff line number Diff line change
Expand Up @@ -70,8 +70,9 @@ public function generateProxyClasses(array $classes): int
->proxyFactory
->createProxy(
$metadata->getName(),
static function () {
static function (): bool {
// empty closure, serves its purpose, for now
return true;
},
[
'skippedProperties' => $this->skippedFieldsFqns($metadata),
Expand All @@ -83,10 +84,18 @@ static function () {
}

/**
* @param ClassMetadata<T> $metadata
* @param DocumentPersister<T> $documentPersister
* @param ClassMetadata<TDocument> $metadata
* @param DocumentPersister<TDocument> $documentPersister
*
* @template T of object
* @psalm-return Closure(
* TDocument&GhostObjectInterface<TDocument>,
* string,
* array<string, mixed>,
* ?Closure=,
* array<string, mixed>
* ) : bool
*
* @template TDocument of object
*/
private function createInitializer(
ClassMetadata $metadata,
Expand Down
5 changes: 0 additions & 5 deletions phpstan-baseline.neon
Original file line number Diff line number Diff line change
Expand Up @@ -153,11 +153,6 @@ parameters:
count: 1
path: tests/Doctrine/ODM/MongoDB/Tests/Functional/Ticket/MODM88Test.php

-
message: "#^Call to an undefined method Doctrine\\\\Persistence\\\\Mapping\\\\ClassMetadata\\<object\\>\\:\\:mapField\\(\\)\\.$#"
count: 1
path: tests/Doctrine/ODM/MongoDB/Tests/Mapping/ClassMetadataLoadEventTest.php

-
message: "#^Parameter \\#1 \\$primer of method Doctrine\\\\ODM\\\\MongoDB\\\\Query\\\\Builder\\:\\:prime\\(\\) expects bool\\|\\(callable\\(\\)\\: mixed\\), 1 given\\.$#"
count: 1
Expand Down
88 changes: 88 additions & 0 deletions psalm-baseline.xml
Original file line number Diff line number Diff line change
@@ -0,0 +1,88 @@
<?xml version="1.0" encoding="UTF-8"?>
<files psalm-version="4.20.0@f82a70e7edfc6cf2705e9374c8a0b6a974a779ed">
<file src="lib/Doctrine/ODM/MongoDB/Aggregation/Stage/GeoNear.php">
<InvalidReturnStatement occurrences="1">
<code>$this-&gt;num($limit)</code>
</InvalidReturnStatement>
<InvalidReturnType occurrences="1">
<code>self</code>
</InvalidReturnType>
</file>
<file src="lib/Doctrine/ODM/MongoDB/Mapping/ClassMetadata.php">
<InvalidArrayOffset occurrences="1">
<code>[$this-&gt;identifier =&gt; $this-&gt;getIdentifierValue($object)]</code>
</InvalidArrayOffset>
</file>
<file src="lib/Doctrine/ODM/MongoDB/Mapping/Driver/XmlDriver.php">
<InvalidArgument occurrences="1">
<code>$mapping</code>
</InvalidArgument>
</file>
<file src="lib/Doctrine/ODM/MongoDB/PersistentCollection/AbstractPersistentCollectionFactory.php">
<InvalidArgument occurrences="1">
<code>new PersistentCollection($coll, $dm, $dm-&gt;getUnitOfWork())</code>
</InvalidArgument>
</file>
<file src="lib/Doctrine/ODM/MongoDB/PersistentCollection/PersistentCollectionTrait.php">
<InvalidArgument occurrences="5">
<code>$func</code>
<code>$p</code>
<code>$p</code>
<code>$p</code>
<code>$p</code>
</InvalidArgument>
<InvalidNullableReturnType occurrences="1">
<code>getMapping</code>
</InvalidNullableReturnType>
<InvalidReturnType occurrences="1">
<code>getTypeClass</code>
</InvalidReturnType>
</file>
<file src="lib/Doctrine/ODM/MongoDB/Proxy/Resolver/ProxyManagerClassNameResolver.php">
<MoreSpecificImplementedParamType occurrences="1">
<code>$className</code>
</MoreSpecificImplementedParamType>
</file>
<file src="lib/Doctrine/ODM/MongoDB/Repository/DefaultGridFSRepository.php">
<InvalidReturnStatement occurrences="1">
<code>$options + ['metadata' =&gt; (object) $metadata]</code>
</InvalidReturnStatement>
</file>
<file src="lib/Doctrine/ODM/MongoDB/UnitOfWork.php">
<InvalidPropertyAssignmentValue occurrences="2">
<code>$this-&gt;identityMap</code>
<code>$this-&gt;originalDocumentData</code>
</InvalidPropertyAssignmentValue>
<NullableReturnStatement occurrences="1">
<code>$mapping['targetDocument']</code>
</NullableReturnStatement>
</file>
<file src="tests/Doctrine/ODM/MongoDB/Tests/DocumentRepositoryTest.php">
<InvalidArgument occurrences="1">
<code>new ArrayCollection([$project])</code>
</InvalidArgument>
</file>
<file src="tests/Doctrine/ODM/MongoDB/Tests/Functional/CollectionPersisterTest.php">
<InvalidArgument occurrences="3">
<code>[$user-&gt;categories[0]-&gt;children, $user-&gt;categories[1]-&gt;children]</code>
<code>[$user-&gt;categories[0]-&gt;children[0]-&gt;children, $user-&gt;categories[0]-&gt;children[1]-&gt;children]</code>
<code>[$user-&gt;categories[0]-&gt;children[0]-&gt;children, $user-&gt;categories[0]-&gt;children[1]-&gt;children]</code>
</InvalidArgument>
</file>
<file src="tests/Doctrine/ODM/MongoDB/Tests/Functional/CustomCollectionsTest.php">
<InvalidArgument occurrences="4">
<code>$i</code>
<code>$i</code>
<code>$j</code>
<code>$j</code>
</InvalidArgument>
</file>
<file src="tests/Doctrine/ODM/MongoDB/Tests/Functional/Ticket/MODM81Test.php">
<InvalidNullableReturnType occurrences="1">
<code>DocumentManager</code>
</InvalidNullableReturnType>
<NullableReturnStatement occurrences="1">
<code>$this-&gt;dm</code>
</NullableReturnStatement>
</file>
</files>
3 changes: 2 additions & 1 deletion psalm.xml
Original file line number Diff line number Diff line change
@@ -1,11 +1,12 @@
<?xml version="1.0"?>
<psalm
errorLevel="7"
errorLevel="5"
findUnusedPsalmSuppress="true"
resolveFromConfigFile="true"
xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
xmlns="https://getpsalm.org/schema/config"
xsi:schemaLocation="https://getpsalm.org/schema/config vendor/vimeo/psalm/config.xsd"
errorBaseline="psalm-baseline.xml"
>
<projectFiles>
<directory name="lib/Doctrine/ODM/MongoDB" />
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -101,17 +101,21 @@ public function testChangingCollectionInPostEventsHasNoIllEffects(): void
$this->dm->persist($user);
$this->dm->flush();

$this->assertCount(1, $user->getPhonenumbers()); // so we got a number on postPersist
$this->assertInstanceOf(PersistentCollectionInterface::class, $user->getPhonenumbers()); // so we got a number on postPersist
$this->assertTrue($user->getPhonenumbers()->isDirty()); // but they should be dirty
$phoneNumbers = $user->getPhonenumbers();
$this->assertCount(1, $phoneNumbers); // so we got a number on postPersist
$this->assertInstanceOf(PersistentCollectionInterface::class, $phoneNumbers); // so we got a number on postPersist
$this->assertTrue($phoneNumbers->isDirty()); // but they should be dirty

$collection = $this->dm->getDocumentCollection(get_class($user));
$inDb = $collection->findOne();
$this->assertArrayNotHasKey('phonenumbers', $inDb, 'Collection modified in postPersist should not be in database without recomputing change set');

$this->dm->flush();
$this->assertCount(2, $user->getPhonenumbers()); // so we got a number on postUpdate
$this->assertTrue($user->getPhonenumbers()->isDirty()); // but they should be dirty

$phoneNumbers = $user->getPhonenumbers();
$this->assertInstanceOf(PersistentCollectionInterface::class, $phoneNumbers);
$this->assertCount(2, $phoneNumbers); // so we got a number on postUpdate
$this->assertTrue($phoneNumbers->isDirty()); // but they should be dirty

$inDb = $collection->findOne();
$this->assertCount(1, $inDb['phonenumbers'], 'Collection changes from postUpdate should not be in database');
Expand Down
3 changes: 2 additions & 1 deletion tests/Doctrine/ODM/MongoDB/Tests/Functional/FilterTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -10,12 +10,13 @@
use Documents\Group;
use Documents\Profile;
use Documents\User;
use MongoDB\BSON\ObjectId;

use function sort;

class FilterTest extends BaseTest
{
/** @var array<string, string> */
/** @var array<string, ObjectId|string|null> */
private $ids;

/** @var FilterCollection */
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ class ParentObject
/**
* @ODM\EmbedOne(targetDocument=ChildEmbeddedObject::class)
*
* @var ChildEmbeddedObject|null
* @var ChildEmbeddedObject
*/
private $childEmbedded;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,8 +37,9 @@ public function testHintIsNotSetByDefault(): void
$user = $query->getSingleResult();
$this->assertInstanceOf(User::class, $user);

$this->assertInstanceOf(PersistentCollectionInterface::class, $user->getGroups());
$this->assertArrayNotHasKey(Query::HINT_READ_PREFERENCE, $user->getGroups()->getHints());
$groups = $user->getGroups();
$this->assertInstanceOf(PersistentCollectionInterface::class, $groups);
$this->assertArrayNotHasKey(Query::HINT_READ_PREFERENCE, $groups->getHints());
}

/**
Expand All @@ -58,8 +59,9 @@ public function testHintIsSetOnQuery(int $readPreference, array $tags = []): voi
$user = $query->getSingleResult();
$this->assertInstanceOf(User::class, $user);

$this->assertInstanceOf(PersistentCollectionInterface::class, $user->getGroups());
$this->assertReadPreferenceHint($readPreference, $user->getGroups()->getHints()[Query::HINT_READ_PREFERENCE], $tags);
$groups = $user->getGroups();
$this->assertInstanceOf(PersistentCollectionInterface::class, $groups);
$this->assertReadPreferenceHint($readPreference, $groups->getHints()[Query::HINT_READ_PREFERENCE], $tags);
}

public function provideReadPreferenceHints(): array
Expand Down
18 changes: 10 additions & 8 deletions tests/Doctrine/ODM/MongoDB/Tests/Functional/ReferencesTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -257,10 +257,11 @@ public function testFlushInitializesEmptyPersistentCollection(): void
$this->dm->persist($user);
$this->dm->flush();

$this->assertInstanceOf(PersistentCollectionInterface::class, $user->getGroups());
$this->assertTrue($user->getGroups()->isInitialized(), 'A flushed collection should be initialized');
$this->assertCount(2, $user->getGroups());
$this->assertCount(2, $user->getGroups()->toArray());
$groups = $user->getGroups();
$this->assertInstanceOf(PersistentCollectionInterface::class, $groups);
$this->assertTrue($groups->isInitialized(), 'A flushed collection should be initialized');
$this->assertCount(2, $groups);
$this->assertCount(2, $groups->toArray());
}

public function testFlushInitializesNotEmptyPersistentCollection(): void
Expand All @@ -280,10 +281,11 @@ public function testFlushInitializesNotEmptyPersistentCollection(): void
$this->dm->persist($user);
$this->dm->flush();

$this->assertInstanceOf(PersistentCollectionInterface::class, $user->getGroups());
$this->assertTrue($user->getGroups()->isInitialized(), 'A flushed collection should be initialized');
$this->assertCount(3, $user->getGroups());
$this->assertCount(3, $user->getGroups()->toArray());
$groups = $user->getGroups();
$this->assertInstanceOf(PersistentCollectionInterface::class, $groups);
$this->assertTrue($groups->isInitialized(), 'A flushed collection should be initialized');
$this->assertCount(3, $groups);
$this->assertCount(3, $groups->toArray());
}

public function testManyReferenceWithAddToSetStrategy(): void
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -58,11 +58,11 @@ class GH1152Parent
/**
* @ODM\EmbeddedDocument
*
* @psalm-import-type FieldMapping from ClassMetadata
* @psalm-import-type AssociationFieldMapping from ClassMetadata
*/
class GH1152Child
{
/** @psalm-var array{0: FieldMapping, 1: object|null, 2: string}|null */
/** @psalm-var array{0: AssociationFieldMapping, 1: object|null, 2: string}|null */
public $parentAssociation;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -203,7 +203,7 @@ class GH1229Child
/**
* @ODM\Field(type="int")
*
* @var int|null
* @var int
*/
public $order = 0;

Expand Down
Loading