[TOOLS][PHPStan][DocCheck] Fix errors found by PHPStan and Doc Check

plugins/Cover/Controller/Cover.php

@@ -152,5 +152,6 @@ class Cover
         //     return  new Response('Cover File not found',Response::HTTP_NOT_FOUND);
         // }
         // return GSFile::sendFile($cover->getFilePath(), $file->getMimetype(), $file->getTitle());
+        return new Response('Cover File not found', Response::HTTP_NOT_FOUND);
     }
 }

src/Controller/Security.php

@@ -1,6 +1,6 @@
 <?php
 
-declare(strict_types=1);
+declare(strict_types = 1);
 
 namespace App\Controller;
 
@@ -8,8 +8,8 @@ use App\Core\Controller;
 use App\Core\DB\DB;
 use App\Core\Event;
 use App\Core\Form;
+use function App\Core\I18n\_m;
 use App\Core\Log;
-use App\Core\Router\Router;
 use App\Core\VisibilityScope;
 use App\Entity\Actor;
 use App\Entity\Follow;
@@ -18,6 +18,8 @@ use App\Entity\Note;
 use App\Security\Authenticator;
 use App\Security\EmailVerifier;
 use App\Util\Common;
+use App\Util\Exception\DuplicateFoundException;
+use App\Util\Exception\EmailTakenException;
 use App\Util\Exception\NicknameEmptyException;
 use App\Util\Exception\NicknameException;
 use App\Util\Exception\NicknameInvalidException;
@@ -38,7 +40,6 @@ use Symfony\Component\Security\Guard\GuardAuthenticatorHandler;
 use Symfony\Component\Security\Http\Authentication\AuthenticationUtils;
 use Symfony\Component\Validator\Constraints\Length;
 use Symfony\Component\Validator\Constraints\NotBlank;
-use function App\Core\I18n\_m;
 
 class Security extends Controller
 {
@@ -57,10 +58,10 @@ class Security extends Controller
         $last_login_id = $authenticationUtils->getLastUsername();
 
         return [
-            '_template' => 'security/login.html.twig',
+            '_template'     => 'security/login.html.twig',
             'last_login_id' => $last_login_id,
-            'error' => $error,
+            'error'         => $error,
-            'notes_fn' => fn() => Note::getAllNotes(VisibilityScope::$instance_scope),
+            'notes_fn'      => fn ()      => Note::getAllNotes(VisibilityScope::$instance_scope),
         ];
     }
 
@@ -76,50 +77,45 @@ class Security extends Controller
      * Register a user, making sure the nickname is not reserved and
      * possibly sending a confirmation email
      *
-     * @param Request $request
-     * @param GuardAuthenticatorHandler $guard_handler
-     * @param Authenticator $authenticator
-     *
-     * @return null|array|Response
-     * @throws EmailTakenException
-     * @throws NicknameTakenException
-     * @throws ServerException
      * @throws DuplicateFoundException
      * @throws EmailTakenException
+     * @throws EmailTakenException
      * @throws NicknameEmptyException
+     * @throws NicknameException
      * @throws NicknameInvalidException
      * @throws NicknameNotAllowedException
      * @throws NicknameTakenException
      * @throws NicknameTooLongException
      * @throws ServerException
-     * @throws NicknameException
      */
-    public function register(Request                   $request,
+    public function register(
-                             GuardAuthenticatorHandler $guard_handler,
+        Request $request,
-                             Authenticator             $authenticator): array|Response|null
+        GuardAuthenticatorHandler $guard_handler,
+        Authenticator $authenticator,
+    ): array|Response|null
     {
         $form = Form::create([
             ['nickname', TextType::class, [
-                'label' => _m('Nickname'),
+                'label'       => _m('Nickname'),
-                'help' => _m('Your desired nickname (e.g., j0hnD03)'),
+                'help'        => _m('Your desired nickname (e.g., j0hnD03)'),
                 'constraints' => [
                     new NotBlank(['message' => _m('Please enter a nickname')]),
                     new Length([
-                        'min' => 1,
+                        'min'        => 1,
                         'minMessage' => _m(['Your nickname must be at least # characters long'], ['count' => 1]),
-                        'max' => Nickname::MAX_LEN,
+                        'max'        => Nickname::MAX_LEN,
-                        'maxMessage' => _m(['Your nickname must be at most # characters long'], ['count' => Nickname::MAX_LEN]),]),
+                        'maxMessage' => _m(['Your nickname must be at most # characters long'], ['count' => Nickname::MAX_LEN]), ]),
                 ],
-                'block_name' => 'nickname',
+                'block_name'      => 'nickname',
-                'label_attr' => ['class' => 'section-form-label'],
+                'label_attr'      => ['class' => 'section-form-label'],
                 'invalid_message' => _m('Nickname not valid. Please provide a valid nickname.'),
             ]],
             ['email', EmailType::class, [
-                'label' => _m('Email'),
+                'label'           => _m('Email'),
-                'help' => _m('Desired email for this account (e.g., john@provider.com)'),
+                'help'            => _m('Desired email for this account (e.g., john@provider.com)'),
-                'constraints' => [new NotBlank(['message' => _m('Please enter an email')])],
+                'constraints'     => [new NotBlank(['message' => _m('Please enter an email')])],
-                'block_name' => 'email',
+                'block_name'      => 'email',
-                'label_attr' => ['class' => 'section-form-label'],
+                'label_attr'      => ['class' => 'section-form-label'],
                 'invalid_message' => _m('Email not valid. Please provide a valid email.'),
             ]],
             FormFields::repeated_password(),
@@ -129,7 +125,7 @@ class Security extends Controller
         $form->handleRequest($request);
 
         if ($form->isSubmitted() && $form->isValid()) {
-            $data = $form->getData();
+            $data             = $form->getData();
             $data['password'] = $form->get('password')->getData();
 
             // TODO: ensure there's no user with this email registered already
@@ -140,17 +136,17 @@ class Security extends Controller
             try {
                 // This already checks if the nickname is being used
                 $actor = Actor::create(['nickname' => $sanitized_nickname]);
-                $user = LocalUser::create([
+                $user  = LocalUser::create([
-                    'nickname' => $sanitized_nickname,
+                    'nickname'       => $sanitized_nickname,
                     'outgoing_email' => $data['email'],
                     'incoming_email' => $data['email'],
-                    'password' => LocalUser::hashPassword($data['password']),
+                    'password'       => LocalUser::hashPassword($data['password']),
                 ]);
                 DB::persistWithSameId(
                     $actor,
                     $user,
                     // Self follow
-                    fn(int $id) => DB::persist(Follow::create(['follower' => $id, 'followed' => $id])),
+                    fn (int $id) => DB::persist(Follow::create(['follower' => $id, 'followed' => $id])),
                 );
 
                 Event::handle('SuccessfulLocalUserRegistration', [$actor, $user]);
@@ -169,7 +165,7 @@ class Security extends Controller
             if ($_ENV['APP_ENV'] !== 'dev' && Common::config('site', 'use_email')) {
                 // @codeCoverageIgnoreStart
                 EmailVerifier::sendEmailConfirmation($user);
-                // @codeCoverageIgnoreEnd
+            // @codeCoverageIgnoreEnd
             } else {
                 $user->setIsEmailVerified(true);
             }
@@ -183,9 +179,9 @@ class Security extends Controller
         }
 
         return [
-            '_template' => 'security/register.html.twig',
+            '_template'         => 'security/register.html.twig',
             'registration_form' => $form->createView(),
-            'notes_fn' => fn() => Note::getAllNotes(VisibilityScope::$instance_scope),
+            'notes_fn'          => fn ()          => Note::getAllNotes(VisibilityScope::$instance_scope),
         ];
     }
 }

src/Core/Router/RouteLoader.php

@@ -103,12 +103,12 @@ class RouteLoader extends Loader
         // XXX: This hack can definitely be optimised by actually intersecting the arrays,
         // maybe this helps: https://backbeat.tech/blog/symfony-routing-tricks-part-1
         // see: https://symfony.com/index.php/doc/3.1/components/http_foundation.html#accessing-accept-headers-data
+        $accept_header_condition = '';
         if (isset($options['accept'])) {
-            $accept_header_condition = '';
             foreach ($options['accept'] as $accept) {
-                $accept_header_condition .= "('$accept' in request.getAcceptableContentTypes()) ||";
+                $accept_header_condition .= "('{$accept}' in request.getAcceptableContentTypes()) ||";
             }
-            $accept_header_condition = substr($accept_header_condition, 0, -3);
+            $accept_header_condition = mb_substr($accept_header_condition, 0, -3);
         }
 
         $this->rc->add(

src/Core/Router/Router.php

@@ -82,7 +82,7 @@ abstract class Router
      * placeholder route values. Extra params are added as query
      * string to the URL
      */
-    public static function url(string $id, array $args, int $type = self::ABSOLUTE_PATH): string
+    public static function url(string $id, array $args = [], int $type = self::ABSOLUTE_PATH): string
     {
         return self::$url_gen->generate($id, $args, $type);
     }

src/DependencyInjection/Compiler/SchemaDefDriver.php

@@ -92,7 +92,9 @@ class SchemaDefDriver extends StaticPHPDriver implements CompilerPassInterface
 
     /**
      * Fill in the database $metadata for $class_name
-     * @param ClassMetadataInfo $metadata ClassMetadataInfo is the real type, but we need to override the method
+     *
+     * @param class-string      $class_name
+     * @param ClassMetadataInfo $metadata   ClassMetadataInfo is the real type, but we need to override the method
      */
     public function loadMetadataForClass($class_name, ClassMetadata $metadata)
     {

src/Entity/LocalUser.php

@@ -1,5 +1,7 @@
 <?php
 
+declare(strict_types = 1);
+
 // {{{ License
 
 // This file is part of GNU social - https://www.gnu.org/software/social
@@ -21,11 +23,11 @@
 
 namespace App\Entity;
 
+use App\Core\Cache;
 use App\Core\DB\DB;
 use App\Core\Entity;
 use App\Core\UserRoles;
 use App\Util\Common;
-use App\Util\Exception\DuplicateFoundException;
 use DateTimeInterface;
 use Exception;
 use libphonenumber\PhoneNumber;
@@ -64,8 +66,8 @@ class LocalUser extends Entity implements UserInterface
     private ?bool $auto_follow_back;
     private ?int $follow_policy;
     private ?bool $is_stream_private;
-    private \DateTimeInterface $created;
+    private DateTimeInterface $created;
-    private \DateTimeInterface $modified;
+    private DateTimeInterface $modified;
 
     public function setId(int $id): self
     {
@@ -280,7 +282,7 @@ class LocalUser extends Entity implements UserInterface
      * Returns the salt that was originally used to encode the password.
      * BCrypt and Argon2 generate their own salts
      */
-    public function getSalt()
+    public function getSalt(): ?string
     {
         return null;
     }
@@ -303,24 +305,17 @@ class LocalUser extends Entity implements UserInterface
     {
     }
 
+    public static function getByNickname(string $nickname): ?self
+    {
+        return Cache::get("user-nickname-{$nickname}", fn () => DB::findOneBy('local_user', ['nickname' => $nickname]));
+    }
+
     /**
-     * Is the nickname or email already in use locally?
+     * @return self Returns self if email found
-     *
-     * @return self Returns self if nickname or email found
      */
     public static function getByEmail(string $email): ?self
     {
-        $users = DB::findBy('local_user', ['or' => ['outgoing_email' => $email, 'incoming_email' => $email]]);
+        return Cache::get("user-email-{$email}", fn () => DB::findOneBy('local_user', ['or' => ['outgoing_email' => $email, 'incoming_email' => $email]]));
-        switch (count($users)) {
-        case 0:
-            return null;
-        case 1:
-            return $users[0];
-        default:
-            // @codeCoverageIgnoreStart
-            throw new DuplicateFoundException('Multiple values in table local_user match the requested criteria');
-            // @codeCoverageIgnoreEnd
-        }
     }
 
     /**
@@ -332,9 +327,11 @@ class LocalUser extends Entity implements UserInterface
         // Timing safe password verification
         if (password_verify($password_plain_text, $this->password)) {
             // Update old formats
-            if (password_needs_rehash($this->password,
+            if (password_needs_rehash(
-                                      self::algoNameToConstant(Common::config('security', 'algorithm')),
+                $this->password,
-                                      Common::config('security', 'options'))
+                self::algoNameToConstant(Common::config('security', 'algorithm')),
+                Common::config('security', 'options'),
+            )
             ) {
                 // @codeCoverageIgnoreStart
                 $this->changePassword(null, $password_plain_text, override: true);
@@ -373,9 +370,9 @@ class LocalUser extends Entity implements UserInterface
         case 'argon2i':
         case 'argon2d':
         case 'argon2id':
-            $c = 'PASSWORD_' . strtoupper($algo);
+            $c = 'PASSWORD_' . mb_strtoupper($algo);
-            if (defined($c)) {
+            if (\defined($c)) {
-                return constant($c);
+                return \constant($c);
             }
             // fallthrough
             // no break

src/Security/Authenticator.php

@@ -21,14 +21,14 @@ declare(strict_types = 1);
 
 namespace App\Security;
 
-use App\Core\DB\DB;
-use App\Util\Exception\NoSuchActorException;
-use App\Util\Exception\NotFoundException;
 use function App\Core\I18n\_m;
+use App\Core\Router\Router;
 use App\Entity\LocalUser;
 use App\Entity\User;
+use App\Util\Exception\NoSuchActorException;
 use App\Util\Nickname;
 use Exception;
+use Stringable;
 use Symfony\Component\HttpFoundation\RedirectResponse;
 use Symfony\Component\HttpFoundation\Request;
 use Symfony\Component\Routing\Generator\UrlGeneratorInterface;
@@ -36,6 +36,7 @@ use Symfony\Component\Security\Core\Authentication\Token\TokenInterface;
 use Symfony\Component\Security\Core\Exception\CustomUserMessageAuthenticationException;
 use Symfony\Component\Security\Core\Exception\InvalidCsrfTokenException;
 use Symfony\Component\Security\Core\Security;
+use Symfony\Component\Security\Core\User\UserInterface;
 use Symfony\Component\Security\Core\User\UserProviderInterface;
 use Symfony\Component\Security\Csrf\CsrfToken;
 use Symfony\Component\Security\Csrf\CsrfTokenManagerInterface;
@@ -75,12 +76,15 @@ class Authenticator extends AbstractFormLoginAuthenticator
     public function getCredentials(Request $request)
     {
         return [
-            'nickname_or_email'   => $request->request->get('nickname_or_email'),
+            'nickname_or_email' => $request->request->get('nickname_or_email'),
-            'password'   => $request->request->get('password'),
+            'password'          => $request->request->get('password'),
-            'csrf_token' => $request->request->get('_csrf_token'),
+            'csrf_token'        => $request->request->get('_csrf_token'),
         ];
     }
 
+    /**
+     * Get a user given credentials and a CSRF token
+     */
     public function getUser($credentials, UserProviderInterface $userProvider)
     {
         $token = new CsrfToken('authenticate', $credentials['csrf_token']);
@@ -89,10 +93,10 @@ class Authenticator extends AbstractFormLoginAuthenticator
         }
 
         try {
-            if (filter_var($credentials['nickname_or_email'], FILTER_VALIDATE_EMAIL) !== false) {
+            if (filter_var($credentials['nickname_or_email'], \FILTER_VALIDATE_EMAIL) !== false) {
                 $user = LocalUser::getByEmail($credentials['nickname_or_email']);
             } else {
-                $user = LocalUser::getWithPK(['nickname' => Nickname::normalize($credentials['nickname_or_email'], check_already_used: false, which: Nickname::CHECK_LOCAL_USER, check_is_allowed: false)]);
+                $user = LocalUser::getByNickname(Nickname::normalize($credentials['nickname_or_email'], check_already_used: false, which: Nickname::CHECK_LOCAL_USER, check_is_allowed: false));
             }
             if ($user === null) {
                 throw new NoSuchActorException('No such local user.');
@@ -119,21 +123,32 @@ class Authenticator extends AbstractFormLoginAuthenticator
         }
     }
 
+    /**
+     * After a successful login, redirect user to the path saved in their session or to the root of the website
+     */
     public function onAuthenticationSuccess(Request $request, TokenInterface $token, $providerKey)
     {
+        $nickname = $token->getUser();
+        if ($nickname instanceof Stringable) {
+            $nickname = (string) $nickname;
+        } elseif ($nickname instanceof UserInterface) {
+            $nickname = $nickname->getUsername();
+        }
+
         $request->getSession()->set(
             Security::LAST_USERNAME,
-            $token->getUser()->getNickname()
+            $nickname,
         );
+
         if ($targetPath = $this->getTargetPath($request->getSession(), $providerKey)) {
             return new RedirectResponse($targetPath);
         }
 
-        return new RedirectResponse($this->urlGenerator->generate('main_all'));
+        return new RedirectResponse(Router::url('/'));
     }
 
     protected function getLoginUrl()
     {
-        return $this->urlGenerator->generate(self::LOGIN_ROUTE);
+        return Router::url(self::LOGIN_ROUTE);
     }
 }

src/Util/Form/ActorArrayTransformer.php

@@ -34,7 +34,7 @@ declare(strict_types = 1);
 
 namespace App\Util\Form;
 
-use App\Entity\Actor;
+use App\Entity\LocalUser;
 
 class ActorArrayTransformer extends ArrayTransformer
 {
@@ -56,8 +56,9 @@ class ActorArrayTransformer extends ArrayTransformer
      */
     public function reverseTransform($s)
     {
+        // TODO support full mentions
         return array_map(
-            fn ($nickmame) => Actor::getFromNickname($nickmame),
+            fn ($nickmame) => LocalUser::getByNickname($nickmame),
             parent::reverseTransform($s),
         );
     }

src/Util/Form/ArrayTransformer.php

@@ -40,6 +40,11 @@ class ArrayTransformer implements DataTransformerInterface
 {
     // Can't use type annotations, to conform to interface
 
+    /**
+     * @param array $a
+     *
+     * @return string
+     */
     public function transform($a)
     {
         if (!\is_array($a)) {
@@ -48,6 +53,13 @@ class ArrayTransformer implements DataTransformerInterface
         return Formatting::toString($a, Formatting::SPLIT_BY_SPACE);
     }
 
+    /**
+     * Transform a string to an array
+     *
+     * @param string $s
+     *
+     * @return array
+     */
     public function reverseTransform($s)
     {
         if (empty($s)) {

tests/Entity/ActorTest.php

@@ -22,7 +22,6 @@ declare(strict_types = 1);
 namespace App\Tests\Entity;
 
 use App\Core\DB\DB;
-use App\Entity\Actor;
 use App\Entity\ActorTag;
 use App\Util\GNUsocialTestCase;
 use Functional as F;
@@ -38,11 +37,6 @@ class ActorTest extends GNUsocialTestCase
         static::assertSame("/{$actor->getId()}/avatar", $actor->getAvatarUrl());
     }
 
-    public function testGetFromNickname()
-    {
-        static::assertNotNull(Actor::getFromNickname('taken_user'));
-    }
-
     public function testSelfTags()
     {
         $actor = DB::findOneBy('actor', ['nickname' => 'taken_user']);

tests/Entity/LocalUserTest.php

@@ -50,9 +50,14 @@ class LocalUserTest extends GNUsocialTestCase
     public function testChangePassword()
     {
         parent::bootKernel();
-        $user = LocalUser::findByNicknameOrEmail('form_personal_info_test_user', 'some@email');
+        $user = LocalUser::getByNickname('form_personal_info_test_user');
         static::assertTrue($user->changePassword(old_password_plain_text: '', new_password_plain_text: 'password', override: true));
         static::assertTrue($user->changePassword(old_password_plain_text: 'password', new_password_plain_text: 'new_password', override: false));
         static::assertFalse($user->changePassword(old_password_plain_text: 'wrong_password', new_password_plain_text: 'new_password', override: false));
     }
+
+    public function testGetByNickname()
+    {
+        static::assertNotNull(LocalUser::getByNickname('taken_user'));
+    }
 }

tests/Util/NicknameTest.php

@@ -27,7 +27,6 @@ use App\Util\Exception\NicknameInvalidException;
 use App\Util\Exception\NicknameNotAllowedException;
 use App\Util\Exception\NicknameTakenException;
 use App\Util\Exception\NicknameTooLongException;
-use App\Util\Exception\NicknameTooShortException;
 use App\Util\GNUsocialTestCase;
 use App\Util\Nickname;
 use Jchook\AssertThrows\AssertThrows;
@@ -52,7 +51,6 @@ class NicknameTest extends GNUsocialTestCase
         static::assertSame('foobar', Nickname::normalize('  foobar  ', check_already_used: false));
         // static::assertSame('foobar', Nickname::normalize('foo_bar', check_already_used: false));
         // static::assertSame('foobar', Nickname::normalize('FooBar', check_already_used: false));
-        static::assertThrows(NicknameTooShortException::class, fn () => Nickname::normalize('foo', check_already_used: false));
         static::assertThrows(NicknameEmptyException::class, fn () => Nickname::normalize('', check_already_used: false));
         // static::assertThrows(NicknameInvalidException::class,  fn () => Nickname::normalize('FóóBár', check_already_used: false));
         static::assertThrows(NicknameNotAllowedException::class, fn () => Nickname::normalize('this_nickname_is_reserved', check_already_used: false));