From 399a56ac791a27ebaf7153f0d5b43367aff55fe7 Mon Sep 17 00:00:00 2001 From: Shish Date: Sat, 31 Aug 2024 21:22:54 +0100 Subject: [PATCH] [dev] bump phpstan strictness no more null surprises --- core/basepage.php | 4 ++-- core/block.php | 1 - ext/graphql/test.php | 2 +- ext/image_hash_ban/main.php | 4 ++-- ext/index/main.php | 1 + ext/ipban/test.php | 4 ++-- ext/link_image/theme.php | 2 +- ext/log_db/main.php | 3 +++ ext/ouroboros_api/main.php | 40 ++++++++++++++++--------------------- ext/pools/main.php | 4 +++- ext/pools/theme.php | 4 ++-- ext/post_source/main.php | 6 +++--- ext/random_image/test.php | 6 +++--- ext/rating/test.php | 4 +++- ext/ratings_blur/test.php | 2 +- ext/s3/main.php | 10 +++++++--- ext/user/events.php | 15 +++++++++++++- ext/user/main.php | 28 +++++++++++++------------- ext/user_config/main.php | 1 - ext/user_config/theme.php | 2 +- ext/word_filter/main.php | 1 + tests/phpstan.neon | 2 +- themes/lite/page.class.php | 2 +- 23 files changed, 83 insertions(+), 65 deletions(-) diff --git a/core/basepage.php b/core/basepage.php index a31aad5d5..68b4536cb 100644 --- a/core/basepage.php +++ b/core/basepage.php @@ -245,14 +245,14 @@ public function add_block(Block $block): void * Find a block which contains the given text * (Useful for unit tests) */ - public function find_block(string $text): ?Block + public function find_block(string $text): Block { foreach ($this->blocks as $block) { if ($block->header == $text) { return $block; } } - return null; + throw new \Exception("Block not found: $text"); } // ============================================== diff --git a/core/block.php b/core/block.php index 65b115b57..23fc3b585 100644 --- a/core/block.php +++ b/core/block.php @@ -56,7 +56,6 @@ public function __construct(string $header = null, string|\MicroHTML\HTMLElement $id = (empty($header) ? md5($this->body ?? '') : $header) . $section; } $str_id = preg_replace_ex('/[^\w-]/', '', str_replace(' ', '_', $id)); - assert(is_string($str_id)); $this->id = $str_id; } diff --git a/ext/graphql/test.php b/ext/graphql/test.php index f8a79ca07..b5b4c2e26 100644 --- a/ext/graphql/test.php +++ b/ext/graphql/test.php @@ -30,7 +30,7 @@ public function testQuery(): void { $this->log_in_as_user(); $image_id = $this->post_image("tests/pbx_screenshot.jpg", "test"); - $image = Image::by_id($image_id); + $image = Image::by_id_ex($image_id); $result = $this->graphql('{ posts(limit: 3, offset: 0) { diff --git a/ext/image_hash_ban/main.php b/ext/image_hash_ban/main.php index 08c66a823..c34b83cad 100644 --- a/ext/image_hash_ban/main.php +++ b/ext/image_hash_ban/main.php @@ -91,8 +91,8 @@ public function onPageRequest(PageRequestEvent $event): void if ($event->page_matches("image_hash_ban/add", method: "POST", permission: Permissions::BAN_IMAGE)) { $input = validate_input(["c_hash" => "optional,string", "c_reason" => "string", "c_image_id" => "optional,int"]); - $image = isset($input['c_image_id']) ? Image::by_id($input['c_image_id']) : null; - $hash = isset($input["c_hash"]) ? $input["c_hash"] : $image->hash; + $image = isset($input['c_image_id']) ? Image::by_id_ex($input['c_image_id']) : null; + $hash = isset($input["c_hash"]) ? $input["c_hash"] : ($image ? $image->hash : null); $reason = isset($input['c_reason']) ? $input['c_reason'] : "DNP"; if ($hash) { diff --git a/ext/index/main.php b/ext/index/main.php index 6d874ebd2..482c93f25 100644 --- a/ext/index/main.php +++ b/ext/index/main.php @@ -243,6 +243,7 @@ public function onSearchTermParse(SearchTermParseEvent $event): void // If we've reached this far, and nobody else has done anything with this term, then treat it as a tag if ($event->order === null && $event->img_conditions == [] && $event->tag_conditions == []) { + assert(is_string($event->term)); $event->add_tag_condition(new TagCondition($event->term, $event->positive)); } } diff --git a/ext/ipban/test.php b/ext/ipban/test.php index 3cbe5e131..5ba5c0385 100644 --- a/ext/ipban/test.php +++ b/ext/ipban/test.php @@ -37,7 +37,7 @@ public function testIPBan(): void $page = $this->get_page('ip_ban/list'); $this->assertStringContainsString( "42.42.42.42", - $page->find_block("Edit IP Bans")->body + $page->find_block("Edit IP Bans")->body ?? "" ); // Delete ban @@ -48,7 +48,7 @@ public function testIPBan(): void $page = $this->get_page('ip_ban/list'); $this->assertStringNotContainsString( "42.42.42.42", - $page->find_block("Edit IP Bans")->body + $page->find_block("Edit IP Bans")->body ?? "" ); } diff --git a/ext/link_image/theme.php b/ext/link_image/theme.php index 1305df4ed..a03ffc287 100644 --- a/ext/link_image/theme.php +++ b/ext/link_image/theme.php @@ -61,7 +61,7 @@ public function links_block(Page $page, array $data): void )); } - protected function url(string $url, string $content, string $type): string + protected function url(string $url, ?string $content, string $type): string { if (empty($content)) { $content = $url; diff --git a/ext/log_db/main.php b/ext/log_db/main.php index 1d1a1efd6..9549f8c48 100644 --- a/ext/log_db/main.php +++ b/ext/log_db/main.php @@ -200,8 +200,11 @@ public function display(array $row): HTMLElement protected function scan_entities(string $line): string { $line = preg_replace_callback("/Image #(\d+)/s", [$this, "link_image"], $line); + assert(is_string($line)); $line = preg_replace_callback("/Post #(\d+)/s", [$this, "link_image"], $line); + assert(is_string($line)); $line = preg_replace_callback("/>>(\d+)/s", [$this, "link_image"], $line); + assert(is_string($line)); return $line; } diff --git a/ext/ouroboros_api/main.php b/ext/ouroboros_api/main.php index 6aae49fec..8fbb16596 100644 --- a/ext/ouroboros_api/main.php +++ b/ext/ouroboros_api/main.php @@ -201,7 +201,6 @@ public function __construct(array $tag) class OuroborosAPI extends Extension { - private ?PageRequestEvent $event; private ?string $type; public const HEADER_HTTP_200 = 'OK'; @@ -246,7 +245,6 @@ public function onPageRequest(PageRequestEvent $event): void global $page, $user; if (preg_match("%\.(xml|json)$%", implode('/', $event->args), $matches) === 1) { - $this->event = $event; $this->type = $matches[1]; if ($this->type == 'json') { $page->set_mime('application/json; charset=utf-8'); @@ -257,7 +255,7 @@ public function onPageRequest(PageRequestEvent $event): void $this->tryAuth(); if ($event->page_matches('post')) { - if ($this->match('create')) { + if ($this->match($event, 'create')) { // Create if ($user->can(Permissions::CREATE_IMAGE)) { $md5 = !empty($_REQUEST['md5']) ? filter_var_ex($_REQUEST['md5'], FILTER_SANITIZE_STRING) : null; @@ -265,13 +263,13 @@ public function onPageRequest(PageRequestEvent $event): void } else { $this->sendResponse(403, 'You cannot create new posts'); } - } elseif ($this->match('update')) { + } elseif ($this->match($event, 'update')) { throw new ServerError("update not implemented"); - } elseif ($this->match('show')) { + } elseif ($this->match($event, 'show')) { // Show $id = !empty($_REQUEST['id']) ? (int)filter_var_ex($_REQUEST['id'], FILTER_SANITIZE_NUMBER_INT) : null; $this->postShow($id); - } elseif ($this->match('index') || $this->match('list')) { + } elseif ($this->match($event, 'index') || $this->match($event, 'list')) { // List $limit = !empty($_REQUEST['limit']) ? intval( filter_var_ex($_REQUEST['limit'], FILTER_SANITIZE_NUMBER_INT) @@ -286,7 +284,7 @@ public function onPageRequest(PageRequestEvent $event): void $this->postIndex($limit, $p, $tags); } } elseif ($event->page_matches('tag')) { - if ($this->match('index') || $this->match('list')) { + if ($this->match($event, 'index') || $this->match($event, 'list')) { $limit = !empty($_REQUEST['limit']) ? intval( filter_var_ex($_REQUEST['limit'], FILTER_SANITIZE_NUMBER_INT) ) : 50; @@ -297,18 +295,12 @@ public function onPageRequest(PageRequestEvent $event): void $_REQUEST['order'], FILTER_SANITIZE_STRING ) : 'date'; - $id = !empty($_REQUEST['id']) ? intval( - filter_var_ex($_REQUEST['id'], FILTER_SANITIZE_NUMBER_INT) - ) : null; - $after_id = !empty($_REQUEST['after_id']) ? intval( - filter_var_ex($_REQUEST['after_id'], FILTER_SANITIZE_NUMBER_INT) - ) : null; $name = !empty($_REQUEST['name']) ? filter_var_ex($_REQUEST['name'], FILTER_SANITIZE_STRING) : ''; $name_pattern = !empty($_REQUEST['name_pattern']) ? filter_var_ex( $_REQUEST['name_pattern'], FILTER_SANITIZE_STRING ) : ''; - $this->tagIndex($limit, $p, $order, $id, $after_id, $name, $name_pattern); + $this->tagIndex($limit, $p, $order, $name, $name_pattern); } } } elseif ($event->page_matches('post/show')) { @@ -337,17 +329,18 @@ protected function postCreate(OuroborosPost $post, ?string $md5 = ''): void return; } } + /** @var array $meta */ $meta = []; $meta['tags'] = $post->tags; - $meta['source'] = $post->source; + $meta['source'] = $post->source ?? ''; if (Extension::is_enabled(RatingsInfo::KEY) !== false) { $meta['rating'] = $post->rating; } // Check where we should try for the file - if (empty($post->file) && !empty($post->file_url) && filter_var_ex( - $post->file_url, - FILTER_VALIDATE_URL - ) !== false + if ( + empty($post->file) && + !empty($post->file_url) && + filter_var_ex($post->file_url, FILTER_VALIDATE_URL) !== false ) { // Transload from source $meta['file'] = shm_tempnam('transload_' . $config->get_string(UploadConfig::TRANSLOAD_ENGINE)); @@ -361,6 +354,7 @@ protected function postCreate(OuroborosPost $post, ?string $md5 = ''): void $meta['hash'] = \Safe\md5_file($meta['file']); } else { // Use file + assert(!is_null($post->file)); $meta['file'] = $post->file['tmp_name']; $meta['filename'] = $post->file['name']; $meta['hash'] = \Safe\md5_file($meta['file']); @@ -379,7 +373,7 @@ protected function postCreate(OuroborosPost $post, ?string $md5 = ''): void send_event(new TagSetEvent($img, $merged)); // This is really the only thing besides tags we should care - if (isset($meta['source'])) { + if (!empty($meta['source'])) { send_event(new SourceSetEvent($img, $meta['source'])); } $this->sendResponse(200, self::OK_POST_CREATE_UPDATE . ' ID: ' . $img->id); @@ -437,7 +431,7 @@ protected function postIndex(int $limit, int $page, array $tags): void * Tag */ - protected function tagIndex(int $limit, int $page, string $order, int $id, int $after_id, string $name, string $name_pattern): void + protected function tagIndex(int $limit, int $page, string $order, string $name, string $name_pattern): void { global $database, $config; $start = ($page - 1) * $limit; @@ -624,8 +618,8 @@ private function tryAuth(): void /** * Helper for matching API methods from event */ - private function match(string $page): bool + private function match(PageRequestEvent $event, string $page): bool { - return (preg_match("%{$page}\.(xml|json)$%", implode('/', $this->event->args), $matches) === 1); + return (preg_match("%{$page}\.(xml|json)$%", implode('/', $event->args), $matches) === 1); } } diff --git a/ext/pools/main.php b/ext/pools/main.php index 62c91a729..8d889449e 100644 --- a/ext/pools/main.php +++ b/ext/pools/main.php @@ -304,7 +304,9 @@ public function onPageRequest(PageRequestEvent $event): void WHERE pool_id=:pid AND i.id=:iid", ["pid" => $pool_id, "iid" => (int) $row['image_id']] ); - $images[] = ($image ? new Image($image) : null); + if ($image) { + $images[] = new Image($image); + } } $this->theme->edit_order($page, $pool, $images); diff --git a/ext/pools/theme.php b/ext/pools/theme.php index 4c67f0c38..f5eb6a9ca 100644 --- a/ext/pools/theme.php +++ b/ext/pools/theme.php @@ -58,7 +58,7 @@ public function list_pools(Page $page, array $pools, string $search, int $pageNu $pool_rows = []; foreach ($pools as $pool) { $pool_link = SHM_A("pool/view/" . $pool->id, $pool->title); - $user_link = SHM_A("user/" . url_escape($pool->user_name), $pool->user_name); + $user_link = SHM_A("user/" . url_escape($pool->user_name), $pool->user_name ?? "No Name"); $pool_rows[] = TR( TD(["class" => "left"], $pool_link), @@ -75,7 +75,7 @@ public function list_pools(Page $page, array $pools, string $search, int $pageNu ); $order_arr = ['created' => 'Recently created', 'updated' => 'Last updated', 'name' => 'Name', 'count' => 'Post Count']; - $order_selected = $page->get_cookie('ui-order-pool'); + $order_selected = $page->get_cookie('ui-order-pool') ?? ""; $order_sel = SHM_SELECT("order_pool", $order_arr, selected_options: [$order_selected], attrs: ["id" => "order_pool"]); $this->display_top(null, "Pools"); diff --git a/ext/post_source/main.php b/ext/post_source/main.php index a0cd43300..6cfe2c374 100644 --- a/ext/post_source/main.php +++ b/ext/post_source/main.php @@ -7,9 +7,9 @@ class SourceSetEvent extends Event { public Image $image; - public ?string $source; + public string $source; - public function __construct(Image $image, string $source = null) + public function __construct(Image $image, string $source) { parent::__construct(); $this->image = $image; @@ -93,7 +93,7 @@ public function onTagTermCheck(TagTermCheckEvent $event): void public function onTagTermParse(TagTermParseEvent $event): void { if (preg_match("/^source[=|:](.*)$/i", $event->term, $matches)) { - $source = ($matches[1] !== "none" ? $matches[1] : null); + $source = ($matches[1] !== "none" ? $matches[1] : ""); send_event(new SourceSetEvent(Image::by_id_ex($event->image_id), $source)); } } diff --git a/ext/random_image/test.php b/ext/random_image/test.php index 44a00077a..6d45b79ce 100644 --- a/ext/random_image/test.php +++ b/ext/random_image/test.php @@ -32,7 +32,7 @@ public function testPostListBlock(): void # enabled, no image = no text $config->set_bool("show_random_block", true); $page = $this->get_page("post/list"); - $this->assertNull($page->find_block("Random Post")); + $this->assertException(\Exception::class, function () use ($page) {$page->find_block("Random Post");}); # enabled, image = text $image_id = $this->post_image("tests/pbx_screenshot.jpg", "test"); @@ -42,11 +42,11 @@ public function testPostListBlock(): void # disabled, image = no text $config->set_bool("show_random_block", false); $page = $this->get_page("post/list"); - $this->assertNull($page->find_block("Random Post")); + $this->assertException(\Exception::class, function () use ($page) {$page->find_block("Random Post");}); # disabled, no image = no image $this->delete_image($image_id); $page = $this->get_page("post/list"); - $this->assertNull($page->find_block("Random Post")); + $this->assertException(\Exception::class, function () use ($page) {$page->find_block("Random Post");}); } } diff --git a/ext/rating/test.php b/ext/rating/test.php index eff467d3f..79061e4d2 100644 --- a/ext/rating/test.php +++ b/ext/rating/test.php @@ -68,7 +68,9 @@ public function testUserConfig(): void // If user prefers to see all images, going to the safe image // and clicking next should show the explicit image $user_config->set_array(RatingsConfig::USER_DEFAULTS, ["s", "q", "e"]); - $this->assertEquals($image_s->get_next()->id, $image_id_e); + $next = $image_s->get_next(); + $this->assertNotNull($next); + $this->assertEquals($next->id, $image_id_e); // If the user prefers to see only safe images by default, then // going to the safe image and clicking next should not show diff --git a/ext/ratings_blur/test.php b/ext/ratings_blur/test.php index abdac37b2..46db667dc 100644 --- a/ext/ratings_blur/test.php +++ b/ext/ratings_blur/test.php @@ -102,7 +102,7 @@ public function testRatingBlurUserConfig(): void private function create_test_user(string $username): void { $uce = send_event(new UserCreationEvent($username, $username, $username, "$username@test.com", false)); - send_event(new UserLoginEvent($uce->user)); + send_event(new UserLoginEvent($uce->get_user())); } private function delete_test_user(string $username): void diff --git a/ext/s3/main.php b/ext/s3/main.php index 904d4c134..80c6d4548 100644 --- a/ext/s3/main.php +++ b/ext/s3/main.php @@ -63,7 +63,9 @@ public function onAdminAction(AdminActionEvent $event): void ) as $row) { if ($row['action'] == "S") { $image = Image::by_hash($row['hash']); - $this->sync_post($image); + if ($image) { + $this->sync_post($image); + } } elseif ($row['action'] == "D") { $this->remove_file($row['hash']); } @@ -87,8 +89,10 @@ public function onCliGen(CliGenEvent $event): void ) as $row) { if ($row['action'] == "S") { $image = Image::by_hash($row['hash']); - $output->writeln("SYN {$row['hash']} ($image->id)"); - $this->sync_post($image); + if ($image) { + $output->writeln("SYN {$row['hash']} ($image->id)"); + $this->sync_post($image); + } } elseif ($row['action'] == "D") { $output->writeln("DEL {$row['hash']}"); $this->remove_file($row['hash']); diff --git a/ext/user/events.php b/ext/user/events.php index 52850fac1..82042efe9 100644 --- a/ext/user/events.php +++ b/ext/user/events.php @@ -44,7 +44,7 @@ public function __construct( class UserCreationEvent extends Event { - public ?User $user; + private ?User $user; public function __construct( public string $username, @@ -55,6 +55,19 @@ public function __construct( ) { parent::__construct(); } + + public function set_user(User $user): void + { + $this->user = $user; + } + + public function get_user(): User + { + if (is_null($this->user)) { + throw new \Exception("User not created"); + } + return $this->user; + } } class UserLoginEvent extends Event diff --git a/ext/user/main.php b/ext/user/main.php index 0e32a3782..45658cc05 100644 --- a/ext/user/main.php +++ b/ext/user/main.php @@ -113,8 +113,8 @@ public static function create_user(string $username, string $password1, string $ try { $uce = send_event(new UserCreationEvent($username, $password1, $password2, $email, true)); return new LoginResult( - $uce->user, - $uce->user->get_session_id(), + $uce->get_user(), + $uce->get_user()->get_session_id(), null ); } catch (UserCreationException $ex) { @@ -196,7 +196,7 @@ public function onPageRequest(PageRequestEvent $event): void true ) ); - $uce->user->set_login_cookie(); + $uce->get_user()->set_login_cookie(); $page->set_mode(PageMode::REDIRECT); $page->set_redirect(make_link("user")); } catch (UserCreationException $ex) { @@ -334,27 +334,27 @@ public function onUserPageBuilding(UserPageBuildingEvent $event): void { global $user, $config; - $h_join_date = autodate($event->display_user->join_date); - if ($event->display_user->can(Permissions::HELLBANNED)) { - $h_class = $event->display_user->class->parent->name; + $duser = $event->display_user; + $h_join_date = autodate($duser->join_date); + $class = $duser->class; + if ($duser->can(Permissions::HELLBANNED) && $class->parent) { + $h_class = $class->parent->name; } else { - $h_class = $event->display_user->class->name; + $h_class = $class->name; } $event->add_part("Joined: $h_join_date", 10); - if ($user->name == $event->display_user->name) { + if ($user->name == $duser->name) { $event->add_part("Current IP: " . get_real_ip(), 80); } $event->add_part("Class: $h_class", 90); - $av = $event->display_user->get_avatar_html(); + $av = $duser->get_avatar_html(); if ($av) { $event->add_part($av, 0); } elseif ( - ( - $config->get_string("avatar_host") == "gravatar" - ) && - ($user->id == $event->display_user->id) + ($config->get_string("avatar_host") == "gravatar") && + ($user->id == $duser->id) ) { $event->add_part( "No avatar? This gallery uses Gravatar for avatar hosting, use the" . @@ -560,7 +560,7 @@ public function onUserCreation(UserCreationEvent $event): void send_event(new UserLoginEvent($new_user)); } - $event->user = $new_user; + $event->set_user($new_user); } public const USER_SEARCH_REGEX = "/^(?:poster|user)(!?)[=|:](.*)$/i"; diff --git a/ext/user_config/main.php b/ext/user_config/main.php index 537fd9ced..8affdc4f3 100644 --- a/ext/user_config/main.php +++ b/ext/user_config/main.php @@ -64,7 +64,6 @@ public function onUserLogin(UserLoginEvent $event): void public static function get_for_user(int $id): Config { global $database; - $user = User::by_id($id); $user_config = new DatabaseConfig($database, "user_config", "user_id", "$id"); send_event(new InitUserConfigEvent($user, $user_config)); diff --git a/ext/user_config/theme.php b/ext/user_config/theme.php index 46c1a866d..b6af8298d 100644 --- a/ext/user_config/theme.php +++ b/ext/user_config/theme.php @@ -75,7 +75,7 @@ protected function sb_to_html(SetupBlock $block): string { $h = $block->header; $b = $block->body; - $i = preg_replace_ex('/[^a-zA-Z0-9]/', '_', $h) . "-setup"; + $i = preg_replace_ex('/[^a-zA-Z0-9]/', '_', $h ?? "") . "-setup"; $html = "
$h diff --git a/ext/word_filter/main.php b/ext/word_filter/main.php index b73a35d58..c2f070b6c 100644 --- a/ext/word_filter/main.php +++ b/ext/word_filter/main.php @@ -37,6 +37,7 @@ private function filter(string $text): string $search = "/\\b" . str_replace("/", "\\/", $search) . "\\b/i"; $text = preg_replace_ex($search, $replace, $text); } + assert(is_string($text)); } return $text; } diff --git a/tests/phpstan.neon b/tests/phpstan.neon index d80d46f14..c7e2f9e2a 100644 --- a/tests/phpstan.neon +++ b/tests/phpstan.neon @@ -1,6 +1,6 @@ parameters: errorFormat: raw - level: 7 + level: 8 paths: - ../core - ../ext diff --git a/themes/lite/page.class.php b/themes/lite/page.class.php index 9f2ef8bf4..efef0cbf3 100644 --- a/themes/lite/page.class.php +++ b/themes/lite/page.class.php @@ -114,7 +114,7 @@ public function block_to_html(Block $block, bool $hidable = false): string } $html .= "
"; } - return $html; + return $html ?? ""; } public function navlinks(Link $link, HTMLElement|string $desc, bool $active): ?string