PHP 8.2 compatibility + phpstan fixes #944
Conversation
Around one-third of our errors were from non-matching baselines
|
Figure we can just solve the Stan errors while we're at this, since there are "only" 676 of them across 90 files. |
…etRouters() will break if provided with an empty array
…or message if it's missing
…tead of silently failing
Php82 phpstan updates
PHPStan fixes: 27
…eption on PUT event
…methods Also simplifies redundant !isset || empty calls
…alksByTitleSearch
There was a problem hiding this comment.
Amazing work! Looks good to me, so I definitely approve that.
There are some comments that should be addressed in one or more future PRs but that would not stop me from merging this!
Only in the EventCommentMapper there seems to be something missing! Before merging someone should have a look at that.
| @@ -67,7 +64,7 @@ public static function build(array $config, $rebuild = false): ContainerInterfac | |||
| return new ContactEmailService($config); | |||
| }; | |||
|
|
|||
| $container[ContactController::class] = $container->factory(static function (Container $c) use ($config) { | |||
| $container[ContactController::class] = $container->factory(static function ($c) use ($config) { | |||
There was a problem hiding this comment.
Why are we no longer typehinting $c?
| @@ -10,7 +10,7 @@ | |||
|
|
|||
| class ApplicationsController extends BaseApiController | |||
| { | |||
| public function getApplication(Request $request, PDO $db) | |||
| public function getApplication(Request $request, PDO $db): array | |||
There was a problem hiding this comment.
Should we not add a typehint as to the array-shape?
| @@ -55,7 +55,7 @@ public function usernameReminder(Request $request, PDO $db) | |||
|
|
|||
| $list = $user_mapper->getUserByEmail($email); | |||
|
|
|||
| if (!is_array($list['users']) || !count($list['users'])) { | |||
| if (!is_array($list) || !count($list['users'])) { | |||
There was a problem hiding this comment.
| if (!is_array($list) || !count($list['users'])) { | |
| if (!is_array($list) || $list['users'] === []) { |
src/Model/EventCommentMapper.php
Outdated
| */ | ||
| $row = $stmt->fetch(PDO::FETCH_ASSOC); | ||
|
|
||
| while ($row) { |
There was a problem hiding this comment.
while loop! What am I missing?
There was a problem hiding this comment.
Yep, that's a bug added in cba1ffa
There was a problem hiding this comment.
hey, since I messed this up, feel free to ping me :) I'll fix it. this happens if you only look blindly at phpstan errors and mix some "tiny" refactoring in...
There was a problem hiding this comment.
@herndlm PR in the fix and I'll happily take it :). I would've either fixed or reached out but, well, I'm still awake in UTC-5 for work reasons 🙃
There was a problem hiding this comment.
Sorry @herndlm. Didn't check who "messed that up" (was a team effort after all 🙈) but feel free to fix it 😁
| if (!$response) { | ||
| throw new \RuntimeException('Could not retrieve talks', Http::INTERNAL_SERVER_ERROR); | ||
| } |
| * | ||
| * @return bool | ||
| */ | ||
| public function tokenBelongsToTrustedApplication($accessToken) | ||
| public function tokenBelongsToTrustedApplication(?string $accessToken) |
There was a problem hiding this comment.
| public function tokenBelongsToTrustedApplication(?string $accessToken) | |
| public function tokenBelongsToTrustedApplication(?string $accessToken): bool |
| @@ -55,7 +55,7 @@ public function buildOutput($content) | |||
| * | |||
| * @return null | |||
| */ | |||
| protected function printArray(array $content) | |||
| protected function printArray(array $content): null | |||
There was a problem hiding this comment.
I couldn't find a place where the returned value of this method actually is used. So shouldn't this be rather void?
There was a problem hiding this comment.
| protected function printArray(array $content): null | |
| protected function printArray(array $content): void |
|
|
||
| return null; |
There was a problem hiding this comment.
| return null; |
…-mapper Fix row handling in EventCommentMapper
|
This PR has been hanging around a bit too long, please update/rebase or comment as needed. |
|
Merging this now. Will deploy over lunch in a few hours...and then fix whatever breaks. |
Since this is a big PR (+1889,-2089 on PHP files) that I expect we'll have multiple folks reviewing, here's a list of all of its PHP files so we can make sure everything gets reviewed by at least one person:
Misc
Controllers
Exceptions + Header
Models
Request + Router
Services
View
Tests for Controllers
Tests for headers, model, request
Tests for Router
Tests for services, view