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

Make phpdoc types correct #8547

Merged
merged 1 commit into from Mar 21, 2021
Merged

Conversation

greg0ire
Copy link
Member

Fixes #8503

@greg0ire greg0ire added the Bug label Mar 16, 2021
* @psalm-param array{entityListeners: array<class-string, array<string, array{string}>>} $array
* @psalm-return array{entityListeners: array<class-string, array<string, array{string}>>}
* @psalm-param array<string, mixed> $array
* @psalm-return array<string, mixed>&array{entityListeners: array<class-string, array<string, array{string}>>}
Copy link
Member Author

@greg0ire greg0ire Mar 16, 2021

Choose a reason for hiding this comment

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

This is the best way I have found to say that I'm not super sure about the shape of the array except that it will have an entityListeners key with that form. Can and should be improved in subsequent PRs with more details.

Copy link
Contributor

Choose a reason for hiding this comment

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

That's a perfectly valid syntax :)

@greg0ire
Copy link
Member Author

@benjamintoussaint @stlrnz @orklah @simonberger please give this a review 🙏

@greg0ire greg0ire changed the title Make phpdoc type correct Make phpdoc types correct Mar 16, 2021
@@ -428,7 +428,7 @@ private function getAssociationValue(ResultSetMapping $rsm, $assocAlias, $entity
* @param mixed $value
* @param array<mixed> $path
*
* @return array<object>|object|null
Copy link
Contributor

Choose a reason for hiding this comment

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

Why do you widen the type here? Was this wrong?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, or at least the recursivity line 456 makes it hard to understand for psalm.

At level 6, you get ERROR: InvalidReturnStatement - lib/Doctrine/ORM/Cache/DefaultQueryCache.php:459:16 - The inferred type 'list<array<array-key, object>|null|object>' does not match the declared return type 'array<array- key, object>|null|object' for Doctrine\ORM\Cache\DefaultQueryCache::getAssociationPathValue (see http s://psalm.dev/128).

@@ -59,7 +59,7 @@ class FileLockRegion implements ConcurrentRegion
/** @var string */
private $directory;

/** @var int */
/** @var string */
Copy link
Contributor

Choose a reason for hiding this comment

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

if ($time + $this->lockLifetime <= time()) {

Maybe you're looking for the type numeric-string?

@@ -132,7 +132,7 @@ public function count()
/**
* {@inheritdoc}
*
* @return ArrayIterator<mixed, T>
* @return ArrayIterator<int|string, T>
Copy link
Contributor

Choose a reason for hiding this comment

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

maybe array-key would be more adapted here?

@greg0ire greg0ire merged commit 3358ccd into doctrine:2.8.x Mar 21, 2021
@greg0ire greg0ire deleted the psalm-lv6-phpdoc branch March 21, 2021 21:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Incompatibility between getArrayResult() new type hint and use of indexBy()
3 participants