From b84315c95b9f9f55f0602117b10ba5868b138907 Mon Sep 17 00:00:00 2001 From: Hugo Sales Date: Sun, 26 Dec 2021 22:17:26 +0000 Subject: [PATCH] [TOOLS] Fix errors reported by PHPStan at level 4 --- components/Search/Util/Parser.php | 11 +++++---- phpstan.neon | 8 ++++++- plugins/Embed/Embed.php | 2 +- plugins/Oomox/Controller/Oomox.php | 10 ++++---- plugins/RepeatNote/RepeatNote.php | 16 ++++--------- plugins/StoreRemoteMedia/StoreRemoteMedia.php | 1 - src/Controller/UserPanel.php | 2 +- src/Core/Cache.php | 3 +++ src/Core/Controller.php | 17 +++++++------ src/Core/DB/DB.php | 18 +++++++------- src/Core/VisibilityScope.php | 2 +- .../Compiler/SchemaDefDriver.php | 1 + src/Entity/Note.php | 2 +- ...DynamicStaticMethodReturnTypeExtension.php | 11 +++++---- src/Util/GNUsocialTestCase.php | 5 ++++ src/Util/Nickname.php | 1 - src/Util/Notification/Notification.php | 24 ++++++++++--------- tests/Controller/FeedsTest.php | 3 +-- tests/Entity/NoteTest.php | 4 ++-- 19 files changed, 75 insertions(+), 66 deletions(-) diff --git a/components/Search/Util/Parser.php b/components/Search/Util/Parser.php index 8b8cfe8364..899cedeb3e 100644 --- a/components/Search/Util/Parser.php +++ b/components/Search/Util/Parser.php @@ -79,16 +79,16 @@ abstract class Parser $note_res = null; $actor_res = null; Event::handle('SearchCreateExpression', [$eb, $term, $language, $actor, &$note_res, &$actor_res]); - if (\is_null($note_res) && \is_null($actor_res)) { + if (\is_null($note_res) && \is_null($actor_res)) { // @phpstan-ignore-line throw new ServerException("No one claimed responsibility for a match term: {$term}"); } - if (!\is_null($note_res) && !empty($note_res)) { + if (!\is_null($note_res) && !empty($note_res)) { // @phpstan-ignore-line if (\is_array($note_res)) { $note_res = $eb->orX(...$note_res); } $note_parts[] = $note_res; } - if (!\is_null($actor_res) && !empty($note_res)) { + if (!\is_null($actor_res) && !empty($actor_res)) { if (\is_array($actor_res)) { $actor_res = $eb->orX(...$actor_res); } @@ -103,10 +103,11 @@ abstract class Parser $last_op = $delimiter; } $match = true; - continue 2; + break; } } - if (!$match) { + // TODO + if (!$match) { // @phpstan-ignore-line ++$right; } } diff --git a/phpstan.neon b/phpstan.neon index 69c3f117dd..527c2f02f0 100644 --- a/phpstan.neon +++ b/phpstan.neon @@ -1,5 +1,5 @@ parameters: - level: 3 + level: 4 bootstrapFiles: - config/bootstrap.php paths: @@ -9,6 +9,7 @@ parameters: - plugins excludePaths: - plugins/ActivityPub + - plugins/Poll - components/FreeNetwork earlyTerminatingMethodCalls: App\Core\Log: @@ -18,6 +19,11 @@ parameters: message: '/Access to an undefined property App\\Util\\Bitmap::\$\w+/' paths: - * + + - + message: '/^Property App\\PHPStan\\ClassFromTableNameDynamicStaticMethodReturnTypeExtension::\$provider is never read, only written\./' + path: src/PHPStan/ClassFromTableNameDynamicStaticMethodReturnTypeExtension.php + # - # message: '/has no return typehint specified/' # paths: diff --git a/plugins/Embed/Embed.php b/plugins/Embed/Embed.php index adbcc4065f..ebb9d04a0f 100644 --- a/plugins/Embed/Embed.php +++ b/plugins/Embed/Embed.php @@ -285,7 +285,7 @@ class Embed extends Plugin $root_url = "{$root_url['scheme']}://{$root_url['host']}"; $metadata['author_url'] = $info->authorUrl ? (string) $info->authorUrl : $root_url; $metadata['provider_name'] = $info->providerName; - $metadata['provider_url'] = (string) $info->providerUrl ?? $metadata['author_name']; + $metadata['provider_url'] = (string) ($info->providerUrl ?? $metadata['author_name']); if (!\is_null($info->image)) { $thumbnail_url = (string) $info->image; diff --git a/plugins/Oomox/Controller/Oomox.php b/plugins/Oomox/Controller/Oomox.php index 11e7a3d91f..467cd7500d 100644 --- a/plugins/Oomox/Controller/Oomox.php +++ b/plugins/Oomox/Controller/Oomox.php @@ -61,7 +61,7 @@ class Oomox * * @throws ServerException */ - public function getOomoxForm(?EntityOomox $current_oomox_settings, bool $is_light): FormInterface + public static function getOomoxForm(?EntityOomox $current_oomox_settings, bool $is_light): FormInterface { $theme = $is_light ? 'light' : 'dark'; $foreground = 'colour_foreground_' . $theme; @@ -144,15 +144,15 @@ class Oomox $actor_id = $user->getId(); $current_oomox_settings = PluginOomox::getEntity($user); - $form_light = (new self)->getOomoxForm($current_oomox_settings, true); + $form_light = self::getOomoxForm($current_oomox_settings, true); $form_light->handleRequest($request); if ($form_light->isSubmitted() && $form_light->isValid()) { /** @var SubmitButton $reset_button */ $reset_button = $form_light->get('colour_reset_light'); if ($reset_button->isClicked()) { - if (isset($current_oomox_settings)) { - $current_oomox_settings?->resetTheme(true); + if (!\is_null($current_oomox_settings)) { + $current_oomox_settings->resetTheme(true); } } else { $data = $form_light->getData(); @@ -190,7 +190,7 @@ class Oomox $actor_id = $user->getId(); $current_oomox_settings = PluginOomox::getEntity($user); - $form_dark = (new self)->getOomoxForm($current_oomox_settings, false); + $form_dark = self::getOomoxForm($current_oomox_settings, false); $form_dark->handleRequest($request); if ($form_dark->isSubmitted() && $form_dark->isValid()) { diff --git a/plugins/RepeatNote/RepeatNote.php b/plugins/RepeatNote/RepeatNote.php index b36363a4cb..7af1e70ade 100644 --- a/plugins/RepeatNote/RepeatNote.php +++ b/plugins/RepeatNote/RepeatNote.php @@ -74,17 +74,11 @@ class RepeatNote extends NoteHandlerPlugin process_note_content_extra_args: $extra_args, ); - // Find the id of the note we just created - $repeat_id = $repeat?->getId(); - - // Add it to note_repeat table - if (!\is_null($repeat_id)) { - DB::persist(NoteRepeat::create([ - 'note_id' => $repeat_id, - 'actor_id' => $actor_id, - 'repeat_of' => $og_id, - ])); - } + DB::persist(NoteRepeat::create([ + 'note_id' => $repeat->getId(), + 'actor_id' => $actor_id, + 'repeat_of' => $og_id, + ])); // Log an activity $repeat_activity = Activity::create([ diff --git a/plugins/StoreRemoteMedia/StoreRemoteMedia.php b/plugins/StoreRemoteMedia/StoreRemoteMedia.php index 1de558eca7..670117c844 100644 --- a/plugins/StoreRemoteMedia/StoreRemoteMedia.php +++ b/plugins/StoreRemoteMedia/StoreRemoteMedia.php @@ -109,7 +109,6 @@ class StoreRemoteMedia extends Plugin } // Have we handled it already? - /** @var AttachmentToLink */ $attachment_to_link = DB::find( 'attachment_to_link', ['link_id' => $link->getId()], diff --git a/src/Controller/UserPanel.php b/src/Controller/UserPanel.php index 9ec684d29e..139fdd168e 100644 --- a/src/Controller/UserPanel.php +++ b/src/Controller/UserPanel.php @@ -224,7 +224,7 @@ class UserPanel extends Controller unset($form_defs['placeholder']); $tabbed_forms = []; - foreach ($form_defs as $transport_name => $f) { + foreach ($form_defs as $transport_name => $f) { // @phpstan-ignore-line unset($f['save']); $form = Form::create($f); $tabbed_forms[$transport_name]['title'] = $transport_name; diff --git a/src/Core/Cache.php b/src/Core/Cache.php index 48320e7417..f0f20f4169 100644 --- a/src/Core/Cache.php +++ b/src/Core/Cache.php @@ -179,6 +179,9 @@ abstract class Cache } } + /** + * @param callable(mixed, bool &$save): mixed $calculate + */ public static function get(string $key, callable $calculate, string $pool = 'default', float $beta = 1.0) { if (isset(self::$redis[$pool])) { diff --git a/src/Core/Controller.php b/src/Core/Controller.php index 8ef512d73b..30d8fee0bc 100644 --- a/src/Core/Controller.php +++ b/src/Core/Controller.php @@ -35,6 +35,7 @@ namespace App\Core; use function App\Core\I18n\_m; use App\Util\Common; +use App\Util\Exception\BugFoundException; use App\Util\Exception\ClientException; use App\Util\Exception\RedirectException; use App\Util\Exception\ServerException; @@ -54,11 +55,11 @@ use Symfony\Component\HttpKernel\KernelEvents; use Symfony\Component\Validator\Exception\ValidatorException; /** - * @method int int(string $param) - * @method bool bool(string $param) - * @method string string(string $param) - * @method string params(string $param) - * @method mixed handle(Request $request, mixed ...$extra) + * @method ?int int(string $param) + * @method ?bool bool(string $param) + * @method ?string string(string $param) + * @method ?string params(string $param) + * @method mixed handle(Request $request, mixed ...$extra) */ abstract class Controller extends AbstractController implements EventSubscriberInterface { @@ -176,11 +177,9 @@ abstract class Controller extends AbstractController implements EventSubscriberI } } else { if (\is_null($potential_response)) { - // TODO BugFoundException - Log::critical($m = "ControllerResponseInFormat for route '{$route}' returned Event::stop but didn't provide a response"); - throw new ServerException(_m($m, ['route' => $route])); + throw new BugFoundException("ControllerResponseInFormat for route '{$route}' returned Event::stop but didn't provide a response"); } - $event->setResponse($potential_response); + $event->setResponse($potential_response); // @phpstan-ignore-line } Event::handle('CleanupModule'); diff --git a/src/Core/DB/DB.php b/src/Core/DB/DB.php index 2a16692b8f..e3eca85ecb 100644 --- a/src/Core/DB/DB.php +++ b/src/Core/DB/DB.php @@ -51,14 +51,14 @@ use Functional as F; * @mixin EntityManager * @template T * - * @method static T find(string $class, array $values) // Finds an Entity by its identifier. - * @method static T getReference(string $class, array $values) // Special cases: It's like find but does not load the object if it has not been loaded yet, it only returns a proxy to the object. (https://www.doctrine-project.org/projects/doctrine-orm/en/2.10/reference/unitofwork.html) - * @method static void remove(object $entity) // Removes an entity instance. - * @method static T merge(object $entity) // Merges the state of a detached entity into the persistence context - * @method static void persist(object $entity) // Tells the EntityManager to make an instance managed and persistent. - * @method static bool contains(object $entity) // Determines whether an entity instance is managed in this EntityManager. - * @method static void flush() // Flushes the in-memory state of persisted objects to the database. - * @method mixed wrapInTransaction(callable $func) // Executes a function in a transaction. Warning: suppresses exceptions + * @method static ?T find(string $class, array $values) // Finds an Entity by its identifier. + * @method static ?T getReference(string $class, array $values) // Special cases: It's like find but does not load the object if it has not been loaded yet, it only returns a proxy to the object. (https://www.doctrine-project.org/projects/doctrine-orm/en/2.10/reference/unitofwork.html) + * @method static void remove(object $entity) // Removes an entity instance. + * @method static T merge(object $entity) // Merges the state of a detached entity into the persistence context + * @method static void persist(object $entity) // Tells the EntityManager to make an instance managed and persistent. + * @method static bool contains(object $entity) // Determines whether an entity instance is managed in this EntityManager. + * @method static void flush() // Flushes the in-memory state of persisted objects to the database. + * @method mixed wrapInTransaction(callable $func) // Executes a function in a transaction. Warning: suppresses exceptions */ class DB { @@ -206,8 +206,6 @@ class DB $expr = self::buildExpression($eb, $exp); if (\is_array($expr)) { $expressions[] = $eb->{$method}(...$expr); - } else { - $expressions[] = $eb->{$method}($expr); } } elseif ($op == 'is_null') { $expressions[] = $eb->isNull($exp); diff --git a/src/Core/VisibilityScope.php b/src/Core/VisibilityScope.php index 3c158d7243..20c4e30bc8 100644 --- a/src/Core/VisibilityScope.php +++ b/src/Core/VisibilityScope.php @@ -29,4 +29,4 @@ enum VisibilityScope: int // having an int is just convenient case GROUP = 8; // Only in the Group feed case COLLECTION = 16; // Only for the collection to see (same as addressee but not available in feeds, notifications only) case MESSAGE = 32; // Direct Message (same as Collection, but also with dedicated plugin) -} +}; diff --git a/src/DependencyInjection/Compiler/SchemaDefDriver.php b/src/DependencyInjection/Compiler/SchemaDefDriver.php index dfeaa4a601..351db20acf 100644 --- a/src/DependencyInjection/Compiler/SchemaDefDriver.php +++ b/src/DependencyInjection/Compiler/SchemaDefDriver.php @@ -116,6 +116,7 @@ class SchemaDefDriver extends StaticPHPDriver implements CompilerPassInterface } } + // @phpstan-ignore-next-line if (false && $opts['foreign key'] ?? false) { // @codeCoverageIgnoreStart // TODO: Get foreign keys working diff --git a/src/Entity/Note.php b/src/Entity/Note.php index 5ca45bfb3e..a675a84a5f 100644 --- a/src/Entity/Note.php +++ b/src/Entity/Note.php @@ -394,7 +394,7 @@ class Note extends Entity // Only for the collection to see return !\is_null($actor) && in_array($actor->getId(), $this->getNotificationTargetIds()); default: - Log::error("Unknown scope found: {$this->getScope()}."); + Log::error("Unknown scope found: {$this->getScope()->value}."); } return false; } diff --git a/src/PHPStan/ClassFromTableNameDynamicStaticMethodReturnTypeExtension.php b/src/PHPStan/ClassFromTableNameDynamicStaticMethodReturnTypeExtension.php index 40abde8c6a..5ec17b998c 100644 --- a/src/PHPStan/ClassFromTableNameDynamicStaticMethodReturnTypeExtension.php +++ b/src/PHPStan/ClassFromTableNameDynamicStaticMethodReturnTypeExtension.php @@ -11,13 +11,13 @@ use PhpParser\Node\Scalar\String_; use PHPStan\Analyser\Scope; use PHPStan\Reflection\MethodReflection; use PHPStan\Type\DynamicStaticMethodReturnTypeExtension; +use PHPStan\Type\NullType; +use PHPStan\Type\UnionType; class ClassFromTableNameDynamicStaticMethodReturnTypeExtension implements DynamicStaticMethodReturnTypeExtension { - private ?GNUsocialProvider $provider = null; - public function __construct(GNUsocialProvider $provider) + public function __construct(private ?GNUsocialProvider $provider = null) { - $this->provider = $provider; } public function getClass(): string @@ -43,7 +43,10 @@ class ClassFromTableNameDynamicStaticMethodReturnTypeExtension implements Dynami ): \PHPStan\Type\Type { if (isset($_ENV['PHPSTAN_BOOT_KERNEL']) && \count($staticCall->args) >= 1 && ($arg = $staticCall->args[0]->value) instanceof String_) { // If called with the first argument as a string, it's a table name - return $scope->resolveTypeByName(new Name(DB::filterTableName($staticCall->name->toString(), [$arg->value]))); + return new UnionType([ + $scope->resolveTypeByName(new Name(DB::filterTableName($staticCall->name->toString(), [$arg->value]))), + new NullType(), + ]); } else { // Let PHPStan handle it normally return \PHPStan\Reflection\ParametersAcceptorSelector::selectFromArgs($scope, $staticCall->args, $methodReflection->getVariants())->getReturnType(); diff --git a/src/Util/GNUsocialTestCase.php b/src/Util/GNUsocialTestCase.php index 48da6aecbc..3c4d076738 100644 --- a/src/Util/GNUsocialTestCase.php +++ b/src/Util/GNUsocialTestCase.php @@ -69,4 +69,9 @@ class GNUsocialTestCase extends WebTestCase ); self::$social = new GNUsocial(...$services); } + + protected function getGNUsocial(): GNUsocial + { + return self::$social; + } } diff --git a/src/Util/Nickname.php b/src/Util/Nickname.php index 53732bf41e..39103558b1 100644 --- a/src/Util/Nickname.php +++ b/src/Util/Nickname.php @@ -161,7 +161,6 @@ class Nickname // @codeCoverageIgnoreStart case self::CHECK_LOCAL_GROUP: throw new NotImplementedException(); - break; default: throw new InvalidArgumentException(); // @codeCoverageIgnoreEnd diff --git a/src/Util/Notification/Notification.php b/src/Util/Notification/Notification.php index e1bf8c3743..97fe0fc97c 100644 --- a/src/Util/Notification/Notification.php +++ b/src/Util/Notification/Notification.php @@ -45,18 +45,20 @@ class Notification public const DM = 7; /** - * One of the above constants + * @param int $type One of the above constants + * @param Actor $actor Who caused this notification */ - private int $type; - - /** - * Who caused this notification - */ - private Actor $actor; - - public function __construct(int $type, Actor $actor) + public function __construct(private int $type, private Actor $actor) { - $this->type = $type; - $this->actor = $actor; + } + + public function getType(): int + { + return $this->type; + } + + public function getActor(): Actor + { + return $this->actor; } } diff --git a/tests/Controller/FeedsTest.php b/tests/Controller/FeedsTest.php index 6f903ec7c7..99af81547b 100644 --- a/tests/Controller/FeedsTest.php +++ b/tests/Controller/FeedsTest.php @@ -26,7 +26,6 @@ namespace App\Tests\Controller; use App\Controller\Feeds; use App\Core\DB\DB; use App\Core\Security; -use App\Core\VisibilityScope; use App\Entity\Note; use App\Util\Common; use App\Util\Exception\ClientException; @@ -86,7 +85,7 @@ class FeedsTest extends GNUsocialTestCase $notes = Common::flattenNoteArray($result['notes']); foreach ($notes as $n) { static::assertTrue(\get_class($n) == Note::class); - $vis = VisibilityScope::create($n->getScope()); + $vis = $n->getScope(); static::assertTrue($visibility($vis)); } } diff --git a/tests/Entity/NoteTest.php b/tests/Entity/NoteTest.php index a55b2f80ae..344c7a7f33 100644 --- a/tests/Entity/NoteTest.php +++ b/tests/Entity/NoteTest.php @@ -52,7 +52,7 @@ class NoteTest extends GNUsocialTestCase $actor2 = DB::findOneBy('actor', ['nickname' => 'taken_group']); $actor3 = DB::findOneBy('actor', ['nickname' => 'some_user']); - $note_visible_to_1 = DB::findBy('note', ['actor_id' => $actor1->getId(), 'content' => 'private note', 'scope' => VisibilityScope::COLLECTION], limit: 1)[0]; + $note_visible_to_1 = DB::findBy('note', ['actor_id' => $actor1->getId(), 'content' => 'private note', 'scope' => VisibilityScope::COLLECTION->value], limit: 1)[0]; static::assertTrue($note_visible_to_1->isVisibleTo($actor1)); static::assertFalse($note_visible_to_1->isVisibleTo($actor2)); static::assertFalse($note_visible_to_1->isVisibleTo($actor3)); @@ -62,7 +62,7 @@ class NoteTest extends GNUsocialTestCase static::assertTrue($note_public->isVisibleTo($actor2)); static::assertTrue($note_public->isVisibleTo($actor3)); - $group_note = DB::findBy('note', ['actor_id' => $actor1->getId(), 'content' => 'group note', 'scope' => VisibilityScope::GROUP], limit: 1)[0]; + $group_note = DB::findBy('note', ['actor_id' => $actor1->getId(), 'content' => 'group note', 'scope' => VisibilityScope::GROUP->value], limit: 1)[0]; static::assertTrue($group_note->isVisibleTo($actor3)); static::assertFalse($group_note->isVisibleTo($actor2)); static::assertFalse($group_note->isVisibleTo($actor1));