-
-
Notifications
You must be signed in to change notification settings - Fork 505
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
Fix more psalm findings #2338
Fix more psalm findings #2338
Changes from all commits
3e9d22a
783744b
17b4d76
9f9c8ee
e9cdeb3
4213692
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -124,7 +124,7 @@ private function createInitializer( | |
} | ||
|
||
/** | ||
* @return string[] | ||
* @return array<int, string> | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Change to be in sync with |
||
*/ | ||
private function skippedFieldsFqns(ClassMetadata $metadata): array | ||
{ | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -742,7 +742,7 @@ public function enableShardingForDbByDocumentName(string $documentName): void | |
} | ||
} | ||
|
||
private function runShardCollectionCommand(string $documentName, ?WriteConcern $writeConcern = null): array | ||
private function runShardCollectionCommand(string $documentName, ?WriteConcern $writeConcern = null): void | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This value was used prior to #1848, but after that it is not used anymore. |
||
{ | ||
$class = $this->dm->getClassMetadata($documentName); | ||
$dbName = $this->dm->getDocumentDatabase($documentName)->getDatabaseName(); | ||
|
@@ -765,15 +765,15 @@ private function runShardCollectionCommand(string $documentName, ?WriteConcern $ | |
$shardKeyPart[$fieldName] = $order; | ||
} | ||
|
||
return $adminDb->command( | ||
$adminDb->command( | ||
array_merge( | ||
[ | ||
'shardCollection' => $dbName . '.' . $class->getCollection(), | ||
'key' => $shardKeyPart, | ||
], | ||
$this->getWriteOptions(null, $writeConcern) | ||
) | ||
)->toArray()[0]; | ||
); | ||
} | ||
|
||
private function ensureGridFSIndexes(ClassMetadata $class, ?int $maxTimeMs = null, ?WriteConcern $writeConcern = null, bool $background = false): void | ||
|
Original file line number | Diff line number | Diff line change | ||
---|---|---|---|---|
|
@@ -213,7 +213,7 @@ final class UnitOfWork implements PropertyChangedListener | |||
* At the end of the UnitOfWork all these collections will make new snapshots | ||||
* of their data. | ||||
* | ||||
* @psalm-var array<string, PersistentCollectionInterface<array-key, object>> | ||||
* @psalm-var array<string, array<PersistentCollectionInterface<array-key, object>>> | ||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. my fault, it was an array containing another one:
|
||||
*/ | ||||
private $visitedCollections = []; | ||||
|
||||
|
@@ -1626,6 +1626,8 @@ public function removeFromIdentityMap(object $document): bool | |||
* @throws InvalidArgumentException If the class does not have an identifier. | ||||
* | ||||
* @template T of object | ||||
* | ||||
* @psalm-suppress InvalidReturnStatement, InvalidReturnType because of the inability of defining a generic property map | ||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. these two suppresses are because the same cause than in #2328 (comment) |
||||
*/ | ||||
public function getById($id, ClassMetadata $class): object | ||||
{ | ||||
|
@@ -1653,6 +1655,8 @@ public function getById($id, ClassMetadata $class): object | |||
* @throws InvalidArgumentException If the class does not have an identifier. | ||||
* | ||||
* @template T of object | ||||
* | ||||
* @psalm-suppress InvalidReturnStatement, InvalidReturnType because of the inability of defining a generic property map | ||||
*/ | ||||
public function tryGetById($id, ClassMetadata $class) | ||||
{ | ||||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I added this
assert
in another PR to make SA tools happy, but since this is a performance sensitive method, I think it's better to just ignore the issue which is related to vimeo/psalm#5788There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
People are meant to disable
assert
in production mode anyway :)There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
But given we need
@psalm-suppress
for two other things I'm fine with this 👍