Fix psalm issues related to the user backend

- Reflect the actual return value returned by the implementation in the
  the interface. E.g. IUser|bool -> IUser|false
- Remove $hasLoggedIn parameter from private countUser implementation.
  Replace the two call with the equivalent countSeenUser
- getBackend is nuallable, add this to the interface
- Use backend interface to make psalm happy about call to undefined
  methods. Also helps with getting rid at some point of the old
  implementActions

Signed-off-by: Carl Schwan <carl@carlschwan.eu>

diff --git a/build/psalm-baseline.xml b/build/psalm-baseline.xml
index b2953298fa986493296b35de36121d130190fd0c..21da75d5608c2c25e8f6cd8891c4e22087865c09 100644 (file)
--- a/build/psalm-baseline.xml
+++ b/build/psalm-baseline.xml
@@ -2310,11 +2310,6 @@
       <code>searchCollections</code>
     </UndefinedInterfaceMethod>
   </file>
-  <file src="core/Controller/SvgController.php">
-    <TypeDoesNotContainNull occurrences="1">
-      <code>$svg === null</code>
-    </TypeDoesNotContainNull>
-  </file>
   <file src="core/Controller/UnifiedSearchController.php">
     <NullArgument occurrences="1">
       <code>null</code>
@@ -4354,28 +4349,11 @@
     <FalsableReturnStatement occurrences="1">
       <code>false</code>
     </FalsableReturnStatement>
-    <ImplicitToStringCast occurrences="1">
-      <code>$query-&gt;func()-&gt;lower('displayname')</code>
-    </ImplicitToStringCast>
   </file>
   <file src="lib/private/User/Manager.php">
-    <ImplementedReturnTypeMismatch occurrences="1">
-      <code>array|int</code>
-    </ImplementedReturnTypeMismatch>
-    <InvalidArgument occurrences="1">
-      <code>$callback</code>
-    </InvalidArgument>
-    <InvalidNullableReturnType occurrences="1">
-      <code>bool|IUser</code>
-    </InvalidNullableReturnType>
-    <NullableReturnStatement occurrences="2">
-      <code>$this-&gt;createUserFromBackend($uid, $password, $backend)</code>
-      <code>$this-&gt;createUserFromBackend($uid, $password, $backend)</code>
-    </NullableReturnStatement>
-    <UndefinedInterfaceMethod occurrences="5">
-      <code>checkPassword</code>
-      <code>checkPassword</code>
-      <code>countUsers</code>
+    <ImplementedReturnTypeMismatch occurrences="1"/>
+    <InvalidArgument occurrences="1"/>
+    <UndefinedInterfaceMethod occurrences="2">
       <code>createUser</code>
       <code>getUsersForUserValueCaseInsensitive</code>
     </UndefinedInterfaceMethod>
@@ -4387,23 +4365,13 @@
     <InvalidArgument occurrences="1">
       <code>IUser::class . '::firstLogin'</code>
     </InvalidArgument>
-    <InvalidScalarArgument occurrences="2">
-      <code>$this-&gt;timeFactory-&gt;getTime()</code>
-      <code>$this-&gt;timeFactory-&gt;getTime()</code>
-    </InvalidScalarArgument>
     <NoInterfaceProperties occurrences="2">
       <code>$request-&gt;server</code>
       <code>$request-&gt;server</code>
     </NoInterfaceProperties>
-    <NullableReturnStatement occurrences="1">
-      <code>null</code>
-    </NullableReturnStatement>
     <TooManyArguments occurrences="1">
       <code>dispatch</code>
     </TooManyArguments>
-    <UndefinedMethod occurrences="1">
-      <code>getByEmail</code>
-    </UndefinedMethod>
   </file>
   <file src="lib/private/User/User.php">
     <InvalidArgument occurrences="5">
@@ -4413,22 +4381,6 @@
       <code>IUser::class . '::preDelete'</code>
       <code>IUser::class . '::preSetPassword'</code>
     </InvalidArgument>
-    <InvalidNullableReturnType occurrences="1">
-      <code>getBackend</code>
-    </InvalidNullableReturnType>
-    <InvalidReturnStatement occurrences="1">
-      <code>$image</code>
-    </InvalidReturnStatement>
-    <InvalidReturnType occurrences="1">
-      <code>IImage|null</code>
-    </InvalidReturnType>
-    <InvalidScalarArgument occurrences="2">
-      <code>$quota</code>
-      <code>$this-&gt;lastLogin</code>
-    </InvalidScalarArgument>
-    <NullableReturnStatement occurrences="1">
-      <code>$this-&gt;backend</code>
-    </NullableReturnStatement>
     <TooManyArguments occurrences="5">
       <code>dispatch</code>
       <code>dispatch</code>
@@ -4436,13 +4388,6 @@
       <code>dispatch</code>
       <code>dispatch</code>
     </TooManyArguments>
-    <UndefinedInterfaceMethod occurrences="5">
-      <code>canChangeAvatar</code>
-      <code>deleteUserAvatar</code>
-      <code>getHome</code>
-      <code>setDisplayName</code>
-      <code>setPassword</code>
-    </UndefinedInterfaceMethod>
   </file>
   <file src="lib/private/legacy/OC_API.php">
     <InvalidNullableReturnType occurrences="1">

diff --git a/lib/private/DB/QueryBuilder/QueryBuilder.php b/lib/private/DB/QueryBuilder/QueryBuilder.php
index fc436383b04f80bc32ae3f6ad58ca67348a90094..71dd18fd68b9fb73a6dbd2fb44d56a27def4df0d 100644 (file)
--- a/lib/private/DB/QueryBuilder/QueryBuilder.php
+++ b/lib/private/DB/QueryBuilder/QueryBuilder.php
@@ -1096,7 +1096,7 @@ class QueryBuilder implements IQueryBuilder {
         * Specifies an ordering for the query results.
         * Replaces any previously specified orderings, if any.
         *
-        * @param string $sort The ordering expression.
+        * @param string|IQueryFunction|ILiteral|IParameter $sort The ordering expression.
         * @param string $order The ordering direction.
         *
         * @return $this This QueryBuilder instance.

diff --git a/lib/private/User/Database.php b/lib/private/User/Database.php
index a9464c27085674147cbf2977e782fb4355c00605..4821a2fc632cbddcb469dae8db308cee2f278b86 100644 (file)
--- a/lib/private/User/Database.php
+++ b/lib/private/User/Database.php
@@ -275,7 +275,7 @@ class Database extends ABackend implements
                        ->setMaxResults($limit)
                        ->setFirstResult($offset);
 
-               $result = $query->execute();
+               $result = $query->executeQuery();
                $displayNames = [];
                while ($row = $result->fetch()) {
                        $displayNames[(string)$row['uid']] = (string)$row['displayname'];

diff --git a/lib/private/User/LazyUser.php b/lib/private/User/LazyUser.php
index 8b98b112731e4bfb780ab4f15e6a8ad5f628a8f2..8e93d6481ab20c780516da6686b5327e173f29cf 100644 (file)
--- a/lib/private/User/LazyUser.php
+++ b/lib/private/User/LazyUser.php
@@ -25,6 +25,7 @@ namespace OC\User;
 
 use OCP\IUser;
 use OCP\IUserManager;
+use OCP\UserInterface;
 
 class LazyUser implements IUser {
        private ?IUser $user = null;
@@ -83,7 +84,7 @@ class LazyUser implements IUser {
                return $this->getUser()->getBackendClassName();
        }
 
-       public function getBackend() {
+       public function getBackend(): ?UserInterface {
                return $this->getUser()->getBackend();
        }
 

diff --git a/lib/private/User/Manager.php b/lib/private/User/Manager.php
index c59cbaa7b2055395e7803f107e21b26883081bca..a6f5658532536280530701a1291f7a090f784b42 100644 (file)
--- a/lib/private/User/Manager.php
+++ b/lib/private/User/Manager.php
@@ -48,6 +48,8 @@ use OCP\Notification\IManager;
 use OCP\Support\Subscription\IRegistry;
 use OCP\User\Backend\IGetRealUIDBackend;
 use OCP\User\Backend\ISearchKnownUsersBackend;
+use OCP\User\Backend\ICheckPasswordBackend;
+use OCP\User\Backend\ICountUsersBackend;
 use OCP\User\Events\BeforeUserCreatedEvent;
 use OCP\User\Events\UserCreatedEvent;
 use OCP\UserInterface;
@@ -223,7 +225,7 @@ class Manager extends PublicEmitter implements IUserManager {
         *
         * @param string $loginName
         * @param string $password
-        * @return mixed the User object on success, false otherwise
+        * @return IUser|false the User object on success, false otherwise
         */
        public function checkPassword($loginName, $password) {
                $result = $this->checkPasswordNoLogging($loginName, $password);
@@ -254,7 +256,8 @@ class Manager extends PublicEmitter implements IUserManager {
                        $backends = $this->backends;
                }
                foreach ($backends as $backend) {
-                       if ($backend->implementsActions(Backend::CHECK_PASSWORD)) {
+                       if ($backend instanceof ICheckPasswordBackend || $backend->implementsActions(Backend::CHECK_PASSWORD)) {
+                               /** @var ICheckPasswordBackend $backend */
                                $uid = $backend->checkPassword($loginName, $password);
                                if ($uid !== false) {
                                        return $this->getUserObject($uid, $backend);
@@ -268,7 +271,8 @@ class Manager extends PublicEmitter implements IUserManager {
                $password = urldecode($password);
 
                foreach ($backends as $backend) {
-                       if ($backend->implementsActions(Backend::CHECK_PASSWORD)) {
+                       if ($backend instanceof ICheckPasswordBackend || $backend->implementsActions(Backend::CHECK_PASSWORD)) {
+                               /** @var ICheckPasswordBackend|UserInterface $backend */
                                $uid = $backend->checkPassword($loginName, $password);
                                if ($uid !== false) {
                                        return $this->getUserObject($uid, $backend);
@@ -376,7 +380,7 @@ class Manager extends PublicEmitter implements IUserManager {
         * @param string $uid
         * @param string $password
         * @throws \InvalidArgumentException
-        * @return bool|IUser the created user or false
+        * @return false|IUser the created user or false
         */
        public function createUser($uid, $password) {
                // DI injection is not used here as IRegistry needs the user manager itself for user count and thus it would create a cyclic dependency
@@ -415,7 +419,7 @@ class Manager extends PublicEmitter implements IUserManager {
         * @param string $uid
         * @param string $password
         * @param UserInterface $backend
-        * @return IUser|null
+        * @return IUser|false
         * @throws \InvalidArgumentException
         */
        public function createUserFromBackend($uid, $password, UserInterface $backend) {
@@ -469,8 +473,9 @@ class Manager extends PublicEmitter implements IUserManager {
                        /** @deprecated 21.0.0 use UserCreatedEvent event with the IEventDispatcher instead */
                        $this->emit('\OC\User', 'postCreateUser', [$user, $password]);
                        $this->eventDispatcher->dispatchTyped(new UserCreatedEvent($user, $password));
+                       return $user;
                }
-               return $user;
+               return false;
        }
 
        /**
@@ -478,16 +483,13 @@ class Manager extends PublicEmitter implements IUserManager {
         *
         * @param boolean $hasLoggedIn when true only users that have a lastLogin
         *                entry in the preferences table will be affected
-        * @return array|int an array of backend class as key and count number as value
-        *                if $hasLoggedIn is true only an int is returned
+        * @return array<string, int> an array of backend class as key and count number as value
         */
-       public function countUsers($hasLoggedIn = false) {
-               if ($hasLoggedIn) {
-                       return $this->countSeenUsers();
-               }
+       public function countUsers() {
                $userCountStatistics = [];
                foreach ($this->backends as $backend) {
-                       if ($backend->implementsActions(Backend::COUNT_USERS)) {
+                       if ($backend instanceof ICountUsersBackend || $backend->implementsActions(Backend::COUNT_USERS)) {
+                               /** @var ICountUsersBackend|IUserBackend $backend */
                                $backendUsers = $backend->countUsers();
                                if ($backendUsers !== false) {
                                        if ($backend instanceof IUserBackend) {
@@ -528,7 +530,7 @@ class Manager extends PublicEmitter implements IUserManager {
         * The callback is executed for each user on each backend.
         * If the callback returns false no further users will be retrieved.
         *
-        * @param \Closure $callback
+        * @psalm-param \Closure(\OCP\IUser):?bool $callback
         * @param string $search
         * @param boolean $onlySeen when true only users that have a lastLogin entry
         *                in the preferences table will be affected

diff --git a/lib/private/User/Session.php b/lib/private/User/Session.php
index 365a01c4595353fe10a0fcca0f97f9897672185e..626ddca2dadd84c6da723ed073721abe0dee2eb5 100644 (file)
--- a/lib/private/User/Session.php
+++ b/lib/private/User/Session.php
@@ -90,7 +90,7 @@ use Symfony\Component\EventDispatcher\GenericEvent;
  */
 class Session implements IUserSession, Emitter {
 
-       /** @var Manager|PublicEmitter $manager */
+       /** @var Manager $manager */
        private $manager;
 
        /** @var ISession $session */
@@ -288,9 +288,9 @@ class Session implements IUserSession, Emitter {
        }
 
        /**
-        * get the login name of the current user
+        * Get the login name of the current user
         *
-        * @return string
+        * @return ?string
         */
        public function getLoginName() {
                if ($this->activeUser) {
@@ -870,7 +870,7 @@ class Session implements IUserSession, Emitter {
                // replace successfully used token with a new one
                $this->config->deleteUserValue($uid, 'login_token', $currentToken);
                $newToken = $this->random->generate(32);
-               $this->config->setUserValue($uid, 'login_token', $newToken, $this->timeFactory->getTime());
+               $this->config->setUserValue($uid, 'login_token', $newToken, (string)$this->timeFactory->getTime());
 
                try {
                        $sessionId = $this->session->getId();
@@ -905,7 +905,7 @@ class Session implements IUserSession, Emitter {
         */
        public function createRememberMeToken(IUser $user) {
                $token = $this->random->generate(32);
-               $this->config->setUserValue($user->getUID(), 'login_token', $token, $this->timeFactory->getTime());
+               $this->config->setUserValue($user->getUID(), 'login_token', $token, (string)$this->timeFactory->getTime());
                $this->setMagicInCookie($user->getUID(), $token);
        }
 

diff --git a/lib/private/User/User.php b/lib/private/User/User.php
index e7aa72fafbafc1780f3c271e0a7a5ebd9d05bbe1..de9af35f541830a6cc9065d088ed95b5a67d4429 100644 (file)
--- a/lib/private/User/User.php
+++ b/lib/private/User/User.php
@@ -52,6 +52,10 @@ use OCP\IUserBackend;
 use OCP\User\Events\BeforeUserDeletedEvent;
 use OCP\User\Events\UserDeletedEvent;
 use OCP\User\GetQuotaEvent;
+use OCP\User\Backend\ISetDisplayNameBackend;
+use OCP\User\Backend\ISetPasswordBackend;
+use OCP\User\Backend\IProvideAvatarBackend;
+use OCP\User\Backend\IGetHomeBackend;
 use OCP\UserInterface;
 use Symfony\Component\EventDispatcher\EventDispatcherInterface;
 use Symfony\Component\EventDispatcher\GenericEvent;
@@ -155,7 +159,9 @@ class User implements IUser {
                $displayName = trim($displayName);
                $oldDisplayName = $this->getDisplayName();
                if ($this->backend->implementsActions(Backend::SET_DISPLAYNAME) && !empty($displayName) && $displayName !== $oldDisplayName) {
-                       $result = $this->backend->setDisplayName($this->uid, $displayName);
+                       /** @var ISetDisplayNameBackend $backend */
+                       $backend = $this->backend;
+                       $result = $backend->setDisplayName($this->uid, $displayName);
                        if ($result) {
                                $this->displayName = $displayName;
                                $this->triggerChange('displayName', $displayName, $oldDisplayName);
@@ -241,7 +247,7 @@ class User implements IUser {
                $firstTimeLogin = ($this->getLastLogin() === 0);
                $this->lastLogin = time();
                $this->config->setUserValue(
-                       $this->uid, 'login', 'lastLogin', $this->lastLogin);
+                       $this->uid, 'login', 'lastLogin', (string)$this->lastLogin);
 
                return $firstTimeLogin;
        }
@@ -280,7 +286,7 @@ class User implements IUser {
                        \OC::$server->getCommentsManager()->deleteReferencesOfActor('users', $this->uid);
                        \OC::$server->getCommentsManager()->deleteReadMarksFromUser($this);
 
-                       /** @var IAvatarManager $avatarManager */
+                       /** @var AvatarManager $avatarManager */
                        $avatarManager = \OC::$server->query(AvatarManager::class);
                        $avatarManager->deleteUserAvatar($this->uid);
 
@@ -319,7 +325,9 @@ class User implements IUser {
                        $this->emitter->emit('\OC\User', 'preSetPassword', [$this, $password, $recoveryPassword]);
                }
                if ($this->backend->implementsActions(Backend::SET_PASSWORD)) {
-                       $result = $this->backend->setPassword($this->uid, $password);
+                       /** @var ISetPasswordBackend $backend */
+                       $backend = $this->backend;
+                       $result = $backend->setPassword($this->uid, $password);
 
                        if ($result !== false) {
                                $this->legacyDispatcher->dispatch(IUser::class . '::postSetPassword', new GenericEvent($this, [
@@ -344,7 +352,8 @@ class User implements IUser {
         */
        public function getHome() {
                if (!$this->home) {
-                       if ($this->backend->implementsActions(Backend::GET_HOME) and $home = $this->backend->getHome($this->uid)) {
+                       /** @psalm-suppress UndefinedInterfaceMethod Once we get rid of the legacy implementsActions, psalm won't complain anymore */
+                       if (($this->backend instanceof IGetHomeBackend || $this->backend->implementsActions(Backend::GET_HOME)) && $home = $this->backend->getHome($this->uid)) {
                                $this->home = $home;
                        } elseif ($this->config) {
                                $this->home = $this->config->getSystemValue('datadirectory', \OC::$SERVERROOT . '/data') . '/' . $this->uid;
@@ -367,18 +376,20 @@ class User implements IUser {
                return get_class($this->backend);
        }
 
-       public function getBackend() {
+       public function getBackend(): ?UserInterface {
                return $this->backend;
        }
 
        /**
-        * check if the backend allows the user to change his avatar on Personal page
+        * Check if the backend allows the user to change his avatar on Personal page
         *
         * @return bool
         */
        public function canChangeAvatar() {
-               if ($this->backend->implementsActions(Backend::PROVIDE_AVATAR)) {
-                       return $this->backend->canChangeAvatar($this->uid);
+               if ($this->backend instanceof IProvideAvatarBackend || $this->backend->implementsActions(Backend::PROVIDE_AVATAR)) {
+                       /** @var IProvideAvatarBackend $backend */
+                       $backend = $this->backend;
+                       return $backend->canChangeAvatar($this->uid);
                }
                return true;
        }
@@ -501,7 +512,7 @@ class User implements IUser {
                $oldQuota = $this->config->getUserValue($this->uid, 'files', 'quota', '');
                if ($quota !== 'none' and $quota !== 'default') {
                        $quota = OC_Helper::computerFileSize($quota);
-                       $quota = OC_Helper::humanFileSize($quota);
+                       $quota = OC_Helper::humanFileSize((int)$quota);
                }
                if ($quota !== $oldQuota) {
                        $this->config->setUserValue($this->uid, 'files', 'quota', $quota);

diff --git a/lib/public/DB/QueryBuilder/IQueryBuilder.php b/lib/public/DB/QueryBuilder/IQueryBuilder.php
index e3257e82bca9e80ff0d01d8bbe5160cde639ef61..9b074db563ed3b4856f2cb9518a86d91b7e7d753 100644 (file)
--- a/lib/public/DB/QueryBuilder/IQueryBuilder.php
+++ b/lib/public/DB/QueryBuilder/IQueryBuilder.php
@@ -820,7 +820,7 @@ interface IQueryBuilder {
         * Specifies an ordering for the query results.
         * Replaces any previously specified orderings, if any.
         *
-        * @param string $sort The ordering expression.
+        * @param string|IQueryFunction $sort The ordering expression.
         * @param string $order The ordering direction.
         *
         * @return $this This QueryBuilder instance.

diff --git a/lib/public/IAvatar.php b/lib/public/IAvatar.php
index 8a1bc79245007172b26c8b45546a368a6849d288..218ef2582505c487ba4d1d780ab0c294d50a757e 100644 (file)
--- a/lib/public/IAvatar.php
+++ b/lib/public/IAvatar.php
@@ -38,7 +38,7 @@ interface IAvatar {
        /**
         * get the users avatar
         * @param int $size size in px of the avatar, avatars are square, defaults to 64, -1 can be used to not scale the image
-        * @return boolean|\OCP\IImage containing the avatar or false if there's no image
+        * @return false|\OCP\IImage containing the avatar or false if there's no image
         * @since 6.0.0 - size of -1 was added in 9.0.0
         */
        public function get($size = 64);

diff --git a/lib/public/IUser.php b/lib/public/IUser.php
index 1a1d1e44d8aaf727dc80dd93dfff9d9f0ee33473..ce9cf0096ec37de9d2f233f0d7d92c62ee4825ff 100644 (file)
--- a/lib/public/IUser.php
+++ b/lib/public/IUser.php
@@ -113,10 +113,9 @@ interface IUser {
        /**
         * Get the backend for the current user object
         *
-        * @return UserInterface
         * @since 15.0.0
         */
-       public function getBackend();
+       public function getBackend(): ?UserInterface;
 
        /**
         * check if the backend allows the user to change his avatar on Personal page

diff --git a/lib/public/IUserManager.php b/lib/public/IUserManager.php
index e5c220af40c60f9963dc5a11bc7ed5a94f0e6a95..77e2fc21a220fe95c5ebc20498e7b80be6e3fbe6 100644 (file)
--- a/lib/public/IUserManager.php
+++ b/lib/public/IUserManager.php
@@ -98,7 +98,7 @@ interface IUserManager {
         *
         * @param string $loginName
         * @param string $password
-        * @return mixed the User object on success, false otherwise
+        * @return IUser|false the User object on success, false otherwise
         * @since 8.0.0
         */
        public function checkPassword($loginName, $password);
@@ -141,7 +141,7 @@ interface IUserManager {
         * @param string $uid
         * @param string $password
         * @throws \InvalidArgumentException
-        * @return bool|\OCP\IUser the created user or false
+        * @return false|\OCP\IUser the created user or false
         * @since 8.0.0
         */
        public function createUser($uid, $password);
@@ -157,9 +157,9 @@ interface IUserManager {
        public function createUserFromBackend($uid, $password, UserInterface $backend);
 
        /**
-        * returns how many users per backend exist (if supported by backend)
+        * Get how many users per backend exist (if supported by backend)
         *
-        * @return array an array of backend class as key and count number as value
+        * @return array<string, int> an array of backend class name as key and count number as value
         * @since 8.0.0
         */
        public function countUsers();

diff --git a/lib/public/User/Backend/ICheckPasswordBackend.php b/lib/public/User/Backend/ICheckPasswordBackend.php
index b3eaf7cedb0857665a4536d6b99bb9aa01775ab5..0d4026be85966f52a54e5f2d23ac20b7fcb6e5df 100644 (file)
--- a/lib/public/User/Backend/ICheckPasswordBackend.php
+++ b/lib/public/User/Backend/ICheckPasswordBackend.php
@@ -35,7 +35,7 @@ interface ICheckPasswordBackend {
         *
         * @param string $loginName The loginname
         * @param string $password The password
-        * @return string|bool The uid on success false on failure
+        * @return string|false The uid on success false on failure
         */
        public function checkPassword(string $loginName, string $password);
 }

diff --git a/tests/lib/User/ManagerTest.php b/tests/lib/User/ManagerTest.php
index c8c1430d583e4742b5f280a9126b7dbb440167fd..2536eee8441e6c8912f2941880ffd4a388748e20 100644 (file)
--- a/tests/lib/User/ManagerTest.php
+++ b/tests/lib/User/ManagerTest.php
@@ -609,7 +609,7 @@ class ManagerTest extends TestCase {
        public function testCountUsersOnlySeen() {
                $manager = \OC::$server->getUserManager();
                // count other users in the db before adding our own
-               $countBefore = $manager->countUsers(true);
+               $countBefore = $manager->countSeenUsers();
 
                //Add test users
                $user1 = $manager->createUser('testseencount1', 'testseencount1');
@@ -623,7 +623,7 @@ class ManagerTest extends TestCase {
                $user4 = $manager->createUser('testseencount4', 'testseencount4');
                $user4->updateLastLoginTimestamp();
 
-               $this->assertEquals($countBefore + 3, $manager->countUsers(true));
+               $this->assertEquals($countBefore + 3, $manager->countSeenUsers());
 
                //cleanup
                $user1->delete();