Make some more things null-safe

(in preparation for bumping up the phpstan strictness to disallow null-unsafe code)

core/basepage.php

@@ -303,7 +303,7 @@ class BasePage
                 if (!is_null($this->filename)) {
                     header('Content-Disposition: ' . $this->disposition . '; filename=' . $this->filename);
                 }
-                assert($this->file, "file should not be null with PageMode::FILE");
+                assert(!is_null($this->file), "file should not be null with PageMode::FILE");
 
                 // https://gist.github.com/codler/3906826
                 $size = \Safe\filesize($this->file); // File size

core/config.php

@@ -112,6 +112,10 @@ interface Config
 
     /**
      * Pick a value out of the table by name, cast to the appropriate data type.
+     *
+     * @template T of string|null
+     * @param T $default
+     * @return T|string
      */
     public function get_string(string $name, ?string $default = null): ?string;
 

core/database.php

@@ -71,8 +71,10 @@ class Database
         if (is_null($this->db)) {
             $this->db = new PDO($this->dsn);
             $this->connect_engine();
+            assert(!is_null($this->db));
             $this->get_engine()->init($this->db);
             $this->begin_transaction();
+            assert(!is_null($this->db));
         }
         return $this->db;
     }
@@ -155,6 +157,7 @@ class Database
     {
         if (is_null($this->engine)) {
             $this->connect_engine();
+            assert(!is_null($this->engine));
         }
         return $this->engine;
     }
@@ -182,7 +185,8 @@ class Database
         global $_tracer, $tracer_enabled;
         $dur = ftime() - $start;
         // trim whitespace
-        $query = preg_replace('/[\n\t ]+/m', ' ', $query);
+        $query = \Safe\preg_replace('/[\n\t ]+/m', ' ', $query);
+        assert(is_string($query));
         $query = trim($query);
         if ($tracer_enabled) {
             $_tracer->complete($start * 1000000, $dur * 1000000, "DB Query", ["query" => $query, "args" => $args, "method" => $method]);

core/extension.php

@@ -181,14 +181,16 @@ abstract class ExtensionInfo
     {
         if ($this->supported === null) {
             $this->check_support();
+            assert(!is_null($this->supported));
         }
         return $this->supported;
     }
 
     public function get_support_info(): string
     {
-        if ($this->supported === null) {
+        if ($this->support_info === null) {
             $this->check_support();
+            assert(!is_null($this->support_info));
         }
         return $this->support_info;
     }
@@ -373,12 +375,10 @@ abstract class DataHandlerExtension extends Extension
 
             // If everything is OK, then move the file to the archive
             $filename = warehouse_path(Image::IMAGE_DIR, $event->hash);
-            if (!@copy($event->tmpname, $filename)) {
+            try {
-                $errors = error_get_last();
+                \Safe\copy($event->tmpname, $filename);
-                throw new UploadException(
+            } catch (\Exception $e) {
-                    "Failed to copy file from uploads ({$event->tmpname}) to archive ($filename): ".
+                throw new UploadException("Failed to copy file from uploads ({$event->tmpname}) to archive ($filename): ".$e->getMessage());
-                    "{$errors['type']} / {$errors['message']}"
-                );
             }
 
             $event->images[] = $iae->image;

core/polyfills.php

@@ -693,7 +693,9 @@ function validate_input(array $inputs): array
  */
 function sanitize_path(string $path): string
 {
-    return preg_replace('|[\\\\/]+|S', DIRECTORY_SEPARATOR, $path);
+    $r = \Safe\preg_replace('|[\\\\/]+|S', DIRECTORY_SEPARATOR, $path);
+    assert(is_string($r));
+    return $r;
 }
 
 /**

core/testcase.php

@@ -44,7 +44,7 @@ if (class_exists("\\PHPUnit\\Framework\\TestCase")) {
             $database->execute("SAVEPOINT test_start");
             self::log_out();
             foreach ($database->get_col("SELECT id FROM images") as $image_id) {
-                send_event(new ImageDeletionEvent(Image::by_id((int)$image_id), true));
+                send_event(new ImageDeletionEvent(Image::by_id_ex((int)$image_id), true));
             }
 
             $_tracer->end();  # setUp
@@ -223,38 +223,36 @@ if (class_exists("\\PHPUnit\\Framework\\TestCase")) {
             $this->assertEquals($results, $ids);
         }
 
-        protected function assertException(string $type, callable $function): \Exception|null
+        protected function assertException(string $type, callable $function): \Exception
         {
-            $exception = null;
             try {
                 call_user_func($function);
-            } catch (\Exception $e) {
+                self::fail("Expected exception of type $type, but none was thrown");
-                $exception = $e;
+            } catch (\Exception $exception) {
+                self::assertThat(
+                    $exception,
+                    new \PHPUnit\Framework\Constraint\Exception($type),
+                    "Expected exception of type $type, but got " . get_class($exception)
+                );
+                return $exception;
             }
-
-            self::assertThat(
-                $exception,
-                new \PHPUnit\Framework\Constraint\Exception($type),
-                "Expected exception of type $type, but got " . ($exception ? get_class($exception) : "none")
-            );
-            return $exception;
         }
 
         // user things
         protected static function log_in_as_admin(): void
         {
-            send_event(new UserLoginEvent(User::by_name(self::$admin_name)));
+            send_event(new UserLoginEvent(User::by_name_ex(self::$admin_name)));
         }
 
         protected static function log_in_as_user(): void
         {
-            send_event(new UserLoginEvent(User::by_name(self::$user_name)));
+            send_event(new UserLoginEvent(User::by_name_ex(self::$user_name)));
         }
 
         protected static function log_out(): void
         {
             global $config;
-            send_event(new UserLoginEvent(User::by_id($config->get_int("anon_id", 0))));
+            send_event(new UserLoginEvent(User::by_id_ex($config->get_int("anon_id", 0))));
         }
 
         // post things

core/urls.php

@@ -6,10 +6,10 @@ namespace Shimmie2;
 
 class Link
 {
-    public ?string $page;
+    public string $page;
     public ?string $query;
 
-    public function __construct(?string $page = null, ?string $query = null)
+    public function __construct(string $page, ?string $query = null)
     {
         $this->page = $page;
         $this->query = $query;
@@ -61,8 +61,12 @@ function make_link(?string $page = null, ?string $query = null, ?string $fragmen
         $parts['path'] = "$install_dir/index.php";
         $query = empty($query) ? "q=$page" : "q=$page&$query";
     }
-    $parts['query'] = $query;  // http_build_query($query);
+    if (!is_null($query)) {
-    $parts['fragment'] = $fragment;  // http_build_query($hash);
+        $parts['query'] = $query;  // http_build_query($query);
+    }
+    if (!is_null($fragment)) {
+        $parts['fragment'] = $fragment;  // http_build_query($hash);
+    }
 
     return unparse_url($parts);
 }

core/user.php

@@ -86,6 +86,9 @@ class User
         $user = $cache->get("user-session-obj:$name-$session");
         if (is_null($user)) {
             $user_by_name = User::by_name($name);
+            if (is_null($user_by_name)) {
+                return null;
+            }
             if ($user_by_name->get_session_id() === $session) {
                 $user = $user_by_name;
             }
@@ -110,6 +113,16 @@ class User
         return is_null($row) ? null : new User($row);
     }
 
+    public static function by_id_ex(int $id): User
+    {
+        $u = User::by_id($id);
+        if (is_null($u)) {
+            throw new UserNotFound("Can't find any user with ID $id");
+        } else {
+            return $u;
+        }
+    }
+
     #[Query(name: "user")]
     public static function by_name(string $name): ?User
     {
@@ -118,6 +131,16 @@ class User
         return is_null($row) ? null : new User($row);
     }
 
+    public static function by_name_ex(string $name): User
+    {
+        $u = User::by_name($name);
+        if (is_null($u)) {
+            throw new UserNotFound("Can't find any user named $name");
+        } else {
+            return $u;
+        }
+    }
+
     public static function name_to_id(string $name): int
     {
         $u = User::by_name($name);
@@ -142,6 +165,7 @@ class User
                 log_info("core-user", "Migrating from md5 to bcrypt for $name");
                 $my_user->set_password($pass);
             }
+            assert(!is_null($my_user->passhash));
             if (password_verify($pass, $my_user->passhash)) {
                 log_info("core-user", "Logged in as $name ({$my_user->class->name})");
                 return $my_user;