-
Notifications
You must be signed in to change notification settings - Fork 87
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
Stricter typing to match psalm level 2 #4584
Conversation
2537d6c
to
e908731
Compare
Passing run #11337 ↗︎
Details:
This comment has been generated by cypress-bot as a result of this project's GitHub integration settings. |
9fe178a
to
1b300ce
Compare
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.
Thanks a lot for looking into this 🐝
Apart from two small comments changes all look sensible to me 😊
@@ -77,6 +77,7 @@ private function detectUtfBom(string $string): ?string { | |||
*/ | |||
private function getEncodings(): array { | |||
$mbOrder = mb_detect_order() ?: []; |
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.
Seems like this line can go, no?
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.
The return value is still needed, unfortunately the return value of mb_detect_order is strange as it is documented to return a bool|array while when calling it without an argument it will always return an array. I've updated the relevant line to require the duplicate variable assignment
@@ -127,6 +124,8 @@ public function create($fileId = null, $filePath = null, ?string $token = null, | |||
return new DataResponse('Failed to create the document session', 500); | |||
} | |||
|
|||
/** @var Document $document */ | |||
|
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.
Nitpicking, but this empty line is superfluous, no?
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.
This is not the var of the left line but an indicator for psalm not detecting that document is never undefined in this case (which seems like a false positive).
Signed-off-by: Julius Härtl <[email protected]>
Signed-off-by: Julius Härtl <[email protected]>
Signed-off-by: Julius Härtl <[email protected]>
Signed-off-by: Julius Härtl <[email protected]>
1b300ce
to
7b24775
Compare
Did some work towards higher psalm levels during todays travel, let's see what CI thinks about that