[TOOLS] Raise PHPStan level to 5 and fix associated error, fixing some bugs in the process

components/LeftPanel/Controller/EditFeeds.php

@@ -104,7 +104,6 @@ class EditFeeds extends Controller
                     $feed->setUrl($fd[$md5 . '-url']);
                     $feed->setOrdering($fd[$md5 . '-order']);
                     $feed->setTitle($fd[$md5 . '-title']);
-                    DB::merge($feed);
                 }
                 DB::flush();
                 Cache::delete($key);

phpstan.neon

@@ -1,5 +1,5 @@
 parameters:
-    level: 4
+    level: 5
     bootstrapFiles:
         - config/bootstrap.php
     paths:

plugins/Cover/Controller/Cover.php

@@ -112,7 +112,9 @@ class Cover
             DB::flush();
             // Only delete files if the commit went through
             if ($old_file != null) {
-                @unlink($old_file);
+                foreach ($old_file as $f) {
+                    @unlink($f);
+                }
             }
             throw new RedirectException();
         }
@@ -128,7 +130,9 @@ class Cover
                 $old_file = $cover->delete();
                 DB::remove($cover);
                 DB::flush();
-                @unlink($old_file);
+                foreach ($old_file as $f) {
+                    @unlink($f);
+                }
                 throw new RedirectException();
             }
             $removeForm = $form2->createView();

plugins/Oomox/Controller/Oomox.php

@@ -170,7 +170,7 @@ class Oomox
                 if ($reset_button->isClicked()) {
                     DB::remove(EntityOomox::getByPK($actor_id));
                 } else {
-                    DB::merge($current_oomox_settings);
+                    DB::persist($current_oomox_settings);
                 }
                 DB::flush();
             }
@@ -219,7 +219,7 @@ class Oomox
                 if ($reset_button->isClicked()) {
                     DB::remove(EntityOomox::getByPK($actor_id));
                 } else {
-                    DB::merge($current_oomox_settings);
+                    DB::persist($current_oomox_settings);
                 }
                 DB::flush();
             }

plugins/Pinboard/Controller/APIv1.php

@@ -73,7 +73,7 @@ class APIv1 extends Controller
             // dd($tag_names, $keys, $result, $xml, (string) $xml);
             // $xml->addChild();
             // dd($xml);
-            return new Response(content: $xml, status: $status);
+            return new Response(content: (string) $xml, status: $status);
         } else {
             throw new InvalidRequestException;
         }

plugins/TagBasedFiltering/Controller/EditBlocked.php

@@ -78,6 +78,7 @@ class EditBlocked extends Controller
                             Cache::delete(TagFilerPlugin::cacheKeys($user->getId())[$type_name]);
                             switch ($type) {
                                 case 'toggle-canon':
+                                    /** @var NoteTagBlock $ntb */
                                     $ntb = DB::getReference($block_class, ['blocker' => $user->getId(), 'canonical' => $canon]);
                                     $ntb->setUseCanonical(!$ntb->getUseCanonical());
                                     DB::flush();

src/Core/Cache.php

@@ -23,7 +23,6 @@ declare(strict_types = 1);
 
 namespace App\Core;
 
-use App\Core\DB;
 use App\Entity\Actor;
 use App\Entity\LocalUser;
 use App\Entity\Note;
@@ -32,6 +31,7 @@ use App\Util\Exception\ConfigurationException;
 use App\Util\Exception\NotImplementedException;
 use Functional as F;
 use InvalidArgumentException;
+use Memcached;
 use Redis;
 use RedisCluster;
 use Symfony\Component\Cache\Adapter;
@@ -77,12 +77,13 @@ abstract class Cache
                             // @codeCoverageIgnoreStart
                             // This requires extra server configuration, but the code was tested
                             // manually and works, so it'll be excluded from automatic tests, for now, at least
-                        if (F\Every($dsns, function ($str) { [$scheme, $rest] = explode('://', $str); return str_contains($rest, ':'); }) == false) {
+                            if (F\Every($dsns, function ($str) { [$scheme, $rest] = explode('://', $str);
+                            return str_contains($rest, ':'); }) == false) {
                                 throw new ConfigurationException('The configuration of a redis cluster requires specifying the ports to use');
                             }
                             $class = RedisCluster::class; // true for persistent connection
                             $seeds = F\Map($dsns, fn ($str) => explode('://', $str)[1]);
-                        $r     = new RedisCluster(name: null, seeds: $seeds, timeout: null, readTimeout: null, persistent: true);
+                            $r     = new RedisCluster(name: null, seeds: $seeds, timeout: 0.0, readTimeout: 0.0, persistent: true);
                             // Distribute reads randomly
                             $r->setOption($class::OPT_SLAVE_FAILOVER, $class::FAILOVER_DISTRIBUTE);
                             // @codeCoverageIgnoreEnd
@@ -101,8 +102,12 @@ abstract class Cache
                         // These all are excluded from automatic testing, as they require an unreasonable amount
                         // of configuration in the testing environment. The code is really simple, so it should work
                         // memcached can also have multiple servers
+                        // host:port:weight?
                         $dsns      = explode(';', $dsn);
-                    $adapters[$pool][] = new Adapter\MemcachedAdapter($dsns);
+                        $servers   = F\map($dsns, fn ($dsn) => explode(':', $dsn));
+                        $memcached = new Memcached();
+                        $memcached->addServers($servers);
+                        $adapters[$pool][] = new Adapter\MemcachedAdapter($memcached);
                         break;
                     case 'filesystem':
                         $adapters[$pool][] = new Adapter\FilesystemAdapter($rest);
@@ -434,6 +439,7 @@ abstract class Cache
      * for redis and others. Different from lists, works with string map_keys
      *
      * @param callable(?CacheItem $item, bool &$save): (string|object|array<string,mixed>) $calculate
+     *
      * @TODO cleanup
      */
     public static function getHashMap(string $map_key, callable $calculate, string $pool = 'default', float $beta = 1.0): array

src/Core/DB.php

@@ -40,17 +40,18 @@ use App\Util\Formatting;
 use Closure;
 use Doctrine\Common\Collections\Criteria;
 use Doctrine\Common\Collections\ExpressionBuilder;
+use Doctrine\DBAL\Connection;
 use Doctrine\ORM\EntityManager;
 use Doctrine\ORM\EntityManagerInterface;
 use Doctrine\ORM\EntityRepository;
+use Doctrine\ORM\Mapping\ClassMetadata;
 use Doctrine\ORM\Query;
 use Doctrine\ORM\Query\ResultSetMappingBuilder;
+use Doctrine\ORM\QueryBuilder;
 use Exception;
 use Functional as F;
 
 /**
- * @mixin EntityManager
- *
  * @template T of Entity
  *
  * Finds an Entity by its identifier. You probably want to use DB::findBy instead.
@@ -77,10 +78,14 @@ use Functional as F;
  * @method static void flush()
  *
  * Executes a function in a transaction. Warning: suppresses exceptions. Returns the result of the callable.
- * @method mixed wrapInTransaction(callable $func)
+ * @method static mixed wrapInTransaction(callable $func)
  *
  * Refetch the given entity
  * @method static T                refetch(T $entity)
+ * @method static QueryBuilder     createQueryBuilder()
+ * @method static Connection       getConnection()
+ * @method static EntityRepository getRepository(string $table)
+ * @method static ClassMetadata<T> getClassMetadata(string $table)
  */
 class DB
 {
@@ -317,8 +322,10 @@ class DB
         $seqName  = $metadata->getSequenceName($conn->getDatabasePlatform());
         self::persist($owner);
         $id = (int) $conn->lastInsertId($seqName);
-        F\map(\is_array($others) ? $others : [$others], function ($o) use ($id) { $o->setId($id);
+        F\map(\is_array($others) ? $others : [$others], function ($o) use ($id) {
-        self::persist($o); });
+            $o->setId($id);
+            self::persist($o);
+        });
         if (!\is_null($extra)) {
             $extra($id);
         }

src/Core/Entity.php

@@ -37,7 +37,7 @@ use Functional as F;
 /**
  * Base class to all entities, with some utilities
  *
- * @method int getId() // Not strictly true
+ * @method int getId() // Not strictly true FIXME
  */
 abstract class Entity
 {
@@ -67,12 +67,14 @@ abstract class Entity
      * Create an instance of the called class or fill in the
      * properties of $obj with the associative array $args. Doesn't
      * persist the result
+     *
+     * @return static
      */
-    public static function create(array $args, bool $_delegated_call = false): static
+    public static function create(array $args, bool $_delegated_call = false): self
     {
         $date  = new DateTime();
         $class = static::class;
-        $obj   = new $class();
+        $obj   = new $class;
         foreach (['created', 'modified'] as $prop) {
             if (property_exists($class, $prop) && !isset($args[$prop])) {
                 $args[$prop] = $date;
@@ -87,9 +89,9 @@ abstract class Entity
     }
 
     /**
-     * @param ?static $obj
+     * @return static
      */
-    public static function createOrUpdate(?self $obj, array $args, bool $_delegated_call = false): static
+    public static function createOrUpdate(?self $obj, array $args, bool $_delegated_call = false): self
     {
         $date  = new DateTime();
         $class = static::class;
@@ -100,7 +102,7 @@ abstract class Entity
         }
 
         if (\is_null($obj)) {
-            $obj = static::create($args, _delegated_call: true);
+            $obj = $class::create($args, _delegated_call: true);
         }
 
         foreach ($args as $prop => $val) {
@@ -112,6 +114,7 @@ abstract class Entity
             }
         }
 
+        // @phpstan-ignore-next-line
         return $obj;
     }
 

src/Twig/Extension.php

@@ -64,7 +64,6 @@ class Extension extends AbstractExtension
             new TwigFunction('active', [Runtime::class, 'isCurrentRouteActive']),
             new TwigFunction('config', [Runtime::class, 'getConfig']),
             new TwigFunction('dd', 'dd'),
-            new TwigFunction('die', 'die'),
             new TwigFunction('get_profile_actions', [Runtime::class, 'getProfileActions']),
             new TwigFunction('get_extra_note_actions', [Runtime::class, 'getExtraNoteActions']),
             new TwigFunction('get_feeds', [Runtime::class, 'getFeeds']),

src/Util/Formatting.php

@@ -206,12 +206,11 @@ abstract class Formatting
 
     /**
      * Convert a user supplied string to array and return whether the conversion was successful
+     *
+     * @param static::SPLIT_BY_BOTH|static::SPLIT_BY_COMMA|static::SPLIT_BY_SPACE $split_type
      */
     public static function toArray(string $input, &$output, string $split_type = self::SPLIT_BY_COMMA): bool
     {
-        if (!\in_array($split_type, [static::SPLIT_BY_SPACE, static::SPLIT_BY_COMMA, static::SPLIT_BY_BOTH])) {
-            throw new Exception('Formatting::toArray received invalid split option');
-        }
         if ($input == '') {
             $output = [];
             return true;
@@ -226,7 +225,7 @@ abstract class Formatting
                     $arr = preg_split('/, ?/', $matches[1]);
                     break;
                 default:
-                    $arr = explode($split_type[0], $matches[1]);
+                    $arr = explode(self::SPLIT_BY_SPACE, $matches[1]);
             }
             $output = str_replace([' \'', '\'', ' "', '"'], '', $arr);
             $output = F\map($output, F\ary('trim', 1));

src/Util/TemporaryFile.php

@@ -207,7 +207,13 @@ class TemporaryFile extends SplFileInfo
             // @codeCoverageIgnoreEnd
         }
 
-        set_error_handler(function ($type, $msg) use (&$error) { $error = $msg; });
+        set_error_handler(
+            function (int $errno, string $msg, string $errfile, int $errline, array $errcontext = []) use (&$error): bool {
+                $error = $msg;
+                return $msg === '';
+            },
+        );
+
         $renamed = rename($this->getPathname(), $destpath);
         $chmoded = chmod($destpath, $filemode);
         restore_error_handler();

tests/Core/CacheTest.php

@@ -31,6 +31,13 @@ use Symfony\Component\DependencyInjection\ParameterBag\ContainerBagInterface;
 
 class CacheTest extends GNUsocialTestCase
 {
+    // Needs to return something to conform to the interface
+    private function notCalled(mixed $i): array
+    {
+        static::assertFalse('should not be called');
+        return [];
+    }
+
     private function doTest(array $adapters, $result_pool, $throws = null, $recompute = \INF)
     {
         static::bootKernel();
@@ -105,15 +112,15 @@ class CacheTest extends GNUsocialTestCase
         static::assertSame([], Cache::getList($key . '0', fn ($i) => []));
         static::assertSame(['foo'], Cache::getList($key . '1', fn ($i) => ['foo']));
         static::assertSame(['foo', 'bar'], Cache::getList($key, fn ($i) => ['foo', 'bar']));
-        static::assertSame(['foo', 'bar'], Cache::getList($key, function () { $this->assertFalse('should not be called'); })); // Repeat to test no recompute lrange
+        static::assertSame(['foo', 'bar'], Cache::getList($key, [$this, 'notCalled'])); // Repeat to test no recompute lrange
         Cache::listPushLeft($key, 'quux');
-        static::assertSame(['quux', 'foo', 'bar'], Cache::getList($key, function ($i) { $this->assertFalse('should not be called'); }));
+        static::assertSame(['quux', 'foo', 'bar'], Cache::getList($key, [$this, 'notCalled']));
         Cache::listPushLeft($key, 'foobar', max_count: 2);
-        static::assertSame(['foobar', 'quux'], Cache::getList($key, function ($i) { $this->assertFalse('should not be called'); }));
+        static::assertSame(['foobar', 'quux'], Cache::getList($key, [$this, 'notCalled']));
         Cache::listPushRight($key, 'foo');
-        static::assertSame(['foobar', 'quux', 'foo'], Cache::getList($key, function ($i) { $this->assertFalse('should not be called'); }));
+        static::assertSame(['foobar', 'quux', 'foo'], Cache::getList($key, [$this, 'notCalled']));
         Cache::listPushRight($key, 'bar', max_count: 2);
-        static::assertSame(['foo', 'bar'], Cache::getList($key, function ($i) { $this->assertFalse('should not be called'); }));
+        static::assertSame(['foo', 'bar'], Cache::getList($key, [$this, 'notCalled']));
         static::assertTrue(Cache::deleteList($key));
     }
 
@@ -135,15 +142,15 @@ class CacheTest extends GNUsocialTestCase
         static::assertSame([], Cache::getList($key . '0', fn ($i) => [], pool: 'file'));
         static::assertSame(['foo'], Cache::getList($key . '1', fn ($i) => ['foo'], pool: 'file'));
         static::assertSame(['foo', 'bar'], Cache::getList($key, fn ($i) => ['foo', 'bar'], pool: 'file'));
-        static::assertSame(['foo', 'bar'], Cache::getList($key, function () { $this->assertFalse('should not be called'); }, pool: 'file')); // Repeat to test no recompute lrange
+        static::assertSame(['foo', 'bar'], Cache::getList($key, [$this, 'notCalled'], pool: 'file')); // Repeat to test no recompute lrange
         Cache::listPushLeft($key, 'quux', pool: 'file');
-        static::assertSame(['quux', 'foo', 'bar'], Cache::getList($key, function ($i) { $this->assertFalse('should not be called'); }, pool: 'file'));
+        static::assertSame(['quux', 'foo', 'bar'], Cache::getList($key, [$this, 'notCalled'], pool: 'file'));
         Cache::listPushLeft($key, 'foobar', max_count: 2, pool: 'file');
-        static::assertSame(['foobar', 'quux'], Cache::getList($key, function ($i) { $this->assertFalse('should not be called'); }, pool: 'file'));
+        static::assertSame(['foobar', 'quux'], Cache::getList($key, [$this, 'notCalled'], pool: 'file'));
         Cache::listPushRight($key, 'foo', pool: 'file');
-        static::assertSame(['foobar', 'quux', 'foo'], Cache::getList($key, function ($i) { $this->assertFalse('should not be called'); }, pool: 'file'));
+        static::assertSame(['foobar', 'quux', 'foo'], Cache::getList($key, [$this, 'notCalled'], pool: 'file'));
         Cache::listPushRight($key, 'bar', max_count: 2, pool: 'file');
-        static::assertSame(['foo', 'bar'], Cache::getList($key, function ($i) { $this->assertFalse('should not be called'); }, pool: 'file'));
+        static::assertSame(['foo', 'bar'], Cache::getList($key, [$this, 'notCalled'], pool: 'file'));
         static::assertTrue(Cache::deleteList($key, pool: 'file'));
     }
 }

tests/Util/Form/ArrayTransformerTest.php

@@ -34,13 +34,13 @@ class ArrayTransformerTest extends WebTestCase
     {
         static::assertSame('', (new ArrayTransformer)->transform([]));
         static::assertSame('foo bar quux', (new ArrayTransformer)->transform(['foo', 'bar', 'quux']));
-        static::assertThrows(TransformationFailedException::class, fn () => (new ArrayTransformer)->transform(''));
+        static::assertThrows(TransformationFailedException::class, fn () => (new ArrayTransformer)->transform('')); // @phpstan-ignore-line
     }
 
     public function testReverseTransform()
     {
         static::assertSame([], (new ArrayTransformer)->reverseTransform(''));
         static::assertSame(['foo', 'bar', 'quux'], (new ArrayTransformer)->reverseTransform('foo bar quux'));
-        static::assertThrows(TransformationFailedException::class, fn () => (new ArrayTransformer)->reverseTransform(1));
+        static::assertThrows(TransformationFailedException::class, fn () => (new ArrayTransformer)->reverseTransform(1)); // @phpstan-ignore-line
     }
 }

tests/Util/FormattingTest.php

@@ -170,7 +170,7 @@ class FormattingTest extends WebTestCase
         static::assertSame('  foo', Formatting::indent('foo', level: 1, count: 2));
         static::assertSame("  foo\n  bar", Formatting::indent("foo\nbar"));
         static::assertSame("  foo\n  bar", Formatting::indent(['foo', 'bar']));
-        static::assertThrows(InvalidArgumentException::class, fn () => Formatting::indent(1));
+        static::assertThrows(InvalidArgumentException::class, fn () => Formatting::indent(1)); // @phpstan-ignore-line
     }
 
     public function testToString()
@@ -185,7 +185,7 @@ class FormattingTest extends WebTestCase
 
     public function testToArray()
     {
-        static::assertThrows(Exception::class, fn () => Formatting::toArray('foo', $a, ''));
+        static::assertThrows(Exception::class, fn () => Formatting::toArray('foo', $a, '')); // @phpstan-ignore-line
 
         static::assertTrue(Formatting::toArray('', $a));
         static::assertSame([], $a);

tests/Util/HTMLTest.php

@@ -41,7 +41,7 @@ class HTMLTest extends WebTestCase
         static::assertSame('<a><p>foo</p><br></a>', HTML::html(['a' => ['p' => 'foo', 'br' => 'empty']]));
         static::assertSame("<div>\n  <a><p>foo</p><br></a>\n</div>", HTML::html(['div' => ['a' => ['p' => 'foo', 'br' => 'empty']]]));
         static::assertSame('<div><a><p>foo</p><br></a></div>', HTML::html(['div' => ['a' => ['p' => 'foo', 'br' => 'empty']]], options: ['indent' => false]));
-        static::assertThrows(TypeError::class, fn () => HTML::html(1));
+        static::assertThrows(TypeError::class, fn () => HTML::html(1)); // @phpstan-ignore-line
         static::assertSame('<a href="test">foo</a>', HTML::tag('a', ['href' => 'test'], content: 'foo', options: ['empty' => false]));
         static::assertSame('<br>', HTML::tag('br', attrs: null, content: null, options: ['empty' => true]));
     }