Psalm 4 #486
Psalm 4 #486
Conversation
| @@ -308,7 +308,7 @@ private function getOriginalEntityData(object $entity): array | |||
| } | |||
|
|
|||
| /** | |||
| * @return string|int | |||
| * @return string|int|false | |||
There was a problem hiding this comment.
if you're returning false, it means you're setting false to the property too.
Also false doesn't seems to be a supported return type because it's later use as a param in the query.
Maybe we should throw an exception if the value is false ?
src/AuditReader.php
Outdated
| @@ -571,7 +571,7 @@ public function findRevisions($className, $id) | |||
| * @throws NotAuditedException | |||
| * @throws Exception | |||
| * | |||
| * @return int|string | |||
| * @return int|string|false | |||
There was a problem hiding this comment.
should we prefer null ?
or throw an exception in next major ?
| @@ -122,7 +122,7 @@ public function __construct(AuditReader $auditReader, $class, ClassMetadataInfo | |||
| } | |||
|
|
|||
| /** | |||
| * @return bool | |||
| * @return true | |||
There was a problem hiding this comment.
why this change ? this method return nothing.
There was a problem hiding this comment.
To have the same return type as the interface: https://github.com/doctrine/collections/blob/c1ec9823f9250fefb274cccf62970b5c167e6bc7/lib/Doctrine/Common/Collections/Collection.php#L34-L42
src/Metadata/MetadataFactory.php
Outdated
| * @var string[] | ||
| * | ||
| * @phpstan-var class-string[] | ||
| * @phpstan-var array<class-string, int|string> |
There was a problem hiding this comment.
array<class-string, int|string> is not fully compatible with string[], so maybe the @var need to be updated too.
| */ | ||
| private $auditedEntities = []; | ||
|
|
||
| /** | ||
| * @phpstan-param array<class-string, mixed> $auditedEntities | ||
| * @phpstan-param class-string[] $auditedEntities |
There was a problem hiding this comment.
there is something weird according to phpstan
src/Metadata/MetadataFactory.php
Outdated
| $this->auditedEntities = array_flip(array_filter($auditedEntities, static function ($record): bool { | ||
| return \is_string($record) || \is_int($record); | ||
| })); | ||
| $this->auditedEntities = array_flip($auditedEntities); |
There was a problem hiding this comment.
Apparently this was done in #143 because of SonataDoctrineORMAdminBundle, but looking at the tests there, looks like there are no null values. We could still add an , but if there are array_filter call just in casenull values, we should fix it in the doctrine orm admin bundle.
franmomu
force-pushed
the
fix_types
branch
4 times, most recently
from
891c66b
to
f492703
Compare
Subject
Raises Psalm to level 4 fixing some types on the way
I am targeting this branch, because fixes some phpdoc types.
Changelog