Avoid unnecessary casting, use PDO::PARAM_X more cautiously (#5086)

* maintenance/improvement
* fix userid casting to int
  never use bindParam with an integer variable without PDO::PARAM_INT or the variable will be casted to string,
  better use bindValue to avoid this confusion
* remove bindValue()/bindParam() default parameter PDO::PARAM_STR where possible
* less casting
* move getNextCustomId to AbstractConcreteEntity
* add some missing use statements

* use more arrow functions

src/Auth/Cookie.php

@@ -19,7 +19,6 @@
 use Elabftw\Exceptions\UnauthorizedException;
 use Elabftw\Interfaces\AuthInterface;
 use Elabftw\Services\TeamsHelper;
-use PDO;
 
 /**
  * Authenticate with the cookie
@@ -45,7 +44,7 @@ public function tryAuth(): AuthResponse
             $this->validityMinutes
         );
         $req = $this->Db->prepare($sql);
-        $req->bindValue(':token', $this->Token->getToken(), PDO::PARAM_STR);
+        $req->bindValue(':token', $this->Token->getToken());
         $this->Db->execute($req);
         if ($req->rowCount() !== 1) {
             throw new UnauthorizedException();

src/Auth/CookieToken.php

@@ -46,7 +46,7 @@ public function saveToken(int $userid): bool
     {
         $sql = 'UPDATE users SET token = :token, token_created_at = NOW() WHERE userid = :userid';
         $req = $this->Db->prepare($sql);
-        $req->bindValue(':token', $this->token, PDO::PARAM_STR);
+        $req->bindValue(':token', $this->getToken());
         $req->bindParam(':userid', $userid, PDO::PARAM_INT);
         return $this->Db->execute($req);
     }

src/Auth/External.php

@@ -68,7 +68,7 @@ public function tryAuth(): AuthResponse
             $Users = ValidatedUser::fromExternal($email, $teams, $firstname, $lastname);
             $this->log->info('New user (' . $email . ') autocreated from external auth');
         }
-        $this->AuthResponse->userid = (int) $Users->userData['userid'];
+        $this->AuthResponse->userid = $Users->userData['userid'];
         $this->AuthResponse->isValidated = true;
         $UsersHelper = new UsersHelper($this->AuthResponse->userid);
         $this->AuthResponse->setTeams($UsersHelper);

src/Auth/Ldap.php

@@ -111,7 +111,7 @@ public function tryAuth(): AuthResponse
             $Users = ValidatedUser::fromExternal($email, $teamFromLdap, $firstname, $lastname);
         }
 
-        $this->AuthResponse->userid = (int) $Users->userData['userid'];
+        $this->AuthResponse->userid = $Users->userData['userid'];
         $this->AuthResponse->mfaSecret = $Users->userData['mfa_secret'];
         $this->AuthResponse->isValidated = (bool) $Users->userData['validated'];
         $UsersHelper = new UsersHelper($this->AuthResponse->userid);

src/Enums/ApiEndpoint.php

@@ -12,6 +12,8 @@
 
 namespace Elabftw\Enums;
 
+use function array_map;
+
 enum ApiEndpoint: string
 {
     case ApiKeys = 'apikeys';
@@ -35,6 +37,6 @@ enum ApiEndpoint: string
 
     public static function getCases(): array
     {
-        return array_map(fn($case) => $case->value, ApiEndpoint::cases());
+        return array_map(fn(self $case): string => $case->value, ApiEndpoint::cases());
     }
 }

src/Enums/Language.php

@@ -12,6 +12,8 @@
 
 namespace Elabftw\Enums;
 
+use function array_map;
+
 enum Language: string
 {
     case Catalan = 'ca_ES';

src/Enums/PasswordComplexity.php

@@ -59,9 +59,10 @@ public static function getAssociativeArray(): array
     {
         $cases = self::cases();
         $values = array_column($cases, 'value');
-        $descriptions = array_map(function ($case) {
-            return $case->toHuman();
-        }, $cases);
+        $descriptions = array_map(
+            fn(self $case): string => $case->toHuman(),
+            $cases
+        );
 
         return array_combine($values, $descriptions);
     }

src/Import/AbstractImport.php

@@ -73,7 +73,7 @@ protected function setTargetUsers(string $type): void
                 return;
             case EntityType::Experiments->value:
                 // check that we can import stuff in experiments of target user
-                if ($this->targetNumber !== (int) $this->Users->userData['userid'] && $this->Users->isAdminOf($this->targetNumber) === false) {
+                if ($this->targetNumber !== $this->Users->userData['userid'] && $this->Users->isAdminOf($this->targetNumber) === false) {
                     throw new IllegalActionException('User tried to import archive in experiments of a user but they are not admin of that user');
                 }
                 // set the Users object to the target user

src/Import/Csv.php

@@ -19,6 +19,7 @@
 use Elabftw\Models\Items;
 use League\Csv\Info as CsvInfo;
 use League\Csv\Reader;
+use PDO;
 
 /**
  * Import entries from a csv file.
@@ -77,10 +78,10 @@ public function import(): void
             if ($this->Entity instanceof Items) {
                 $req->bindParam(':canbook', $this->canread);
             }
-            $req->bindParam(':team', $this->Users->userData['team']);
+            $req->bindParam(':team', $this->Users->userData['team'], PDO::PARAM_INT);
             $req->bindParam(':title', $row['title']);
             $req->bindParam(':body', $body);
-            $req->bindParam(':userid', $this->Users->userData['userid']);
+            $req->bindParam(':userid', $this->Users->userData['userid'], PDO::PARAM_INT);
             $req->bindParam(':category', $this->targetNumber);
             $req->bindParam(':status', $status);
             $req->bindParam(':custom_id', $customId);

src/Make/AbstractMakeZip.php

@@ -95,7 +95,7 @@ protected function addAttachedFiles($filesArr): array
             $realNamesSoFar[] = $realName;
             // modify the real_name in place
             $file['real_name'] = $realName;
-            $storageFs = Storage::from((int) $file['storage'])->getStorage()->getFs();
+            $storageFs = Storage::from($file['storage'])->getStorage()->getFs();
 
             // make sure we have a hash
             if (empty($file['hash'])) {

src/Make/MakeBackupZip.php

@@ -46,9 +46,9 @@ public function getStreamZip(): void
         // loop on every user
         $usersArr = $this->Entity->Users->readFromQuery('');
         foreach ($usersArr as $user) {
-            $idArr = $this->Entity->getIdFromLastchange((int) $user['userid'], $this->period);
+            $idArr = $this->Entity->getIdFromLastchange($user['userid'], $this->period);
             foreach ($idArr as $id) {
-                $this->addToZip((int) $id, $user['fullname']);
+                $this->addToZip($id, $user['fullname']);
             }
         }
         $this->Zip->finish();

src/Make/MakeEln.php

@@ -36,7 +36,7 @@ class MakeEln extends MakeStreamZip
     // name of the folder containing everything
     private string $root;
 
-    public function __construct(protected ZipStream $Zip, AbstractEntity $entity, protected array $idArr)
+    public function __construct(ZipStream $Zip, AbstractEntity $entity, array $idArr)
     {
         parent::__construct(
             Zip: $Zip,
@@ -94,7 +94,7 @@ public function getStreamZip(): void
         foreach ($this->idArr as $id) {
             $hasPart = array();
             try {
-                $this->Entity->setId((int) $id);
+                $this->Entity->setId($id);
             } catch (IllegalActionException $e) {
                 continue;
             }
@@ -121,7 +121,7 @@ public function getStreamZip(): void
             }
 
             // JSON
-            $MakeJson = new MakeJson($this->Entity, array((int) $e['id']));
+            $MakeJson = new MakeJson($this->Entity, array($e['id']));
             $json = $MakeJson->getFileContent();
             $this->Zip->addFile($this->folder . '/' . $MakeJson->getFileName(), $json);
             $jsonAtId = './' . $currentDatasetFolder . '/' . $MakeJson->getFileName();

src/Make/MakePdf.php

@@ -139,7 +139,7 @@ private function loopOverEntries(): void
     {
         $entriesCount = count($this->entityIdArr);
         foreach ($this->entityIdArr as $key => $id) {
-            $this->Entity->setId((int) $id);
+            $this->Entity->setId($id);
 
             try {
                 $permissions = $this->Entity->getPermissions();
@@ -209,7 +209,7 @@ private function getHtml(): string
 
         if ($locked) {
             // get info about the locker
-            $Locker = new Users((int) $this->Entity->entityData['lockedby']);
+            $Locker = new Users($this->Entity->entityData['lockedby']);
             $lockerName = $Locker->userData['fullname'];
 
             // separate the date and time
@@ -219,7 +219,7 @@ private function getHtml(): string
 
         // read the content of the thumbnail here to feed the template
         foreach ($this->Entity->entityData['uploads'] as $key => $upload) {
-            $storageFs = Storage::from((int) $upload['storage'])->getStorage()->getFs();
+            $storageFs = Storage::from($upload['storage'])->getStorage()->getFs();
             $thumbnail = $upload['long_name'] . '_th.jpg';
             // no need to filter on extension, just insert the thumbnail if it exists
             if ($storageFs->fileExists($thumbnail)) {
@@ -333,7 +333,7 @@ private function getAttachedPdfs(): array
         }
 
         foreach ($uploadsArr as $upload) {
-            $storageFs = Storage::from((int) $upload['storage'])->getStorage()->getFs();
+            $storageFs = Storage::from($upload['storage'])->getStorage()->getFs();
             if ($storageFs->fileExists($upload['long_name']) && strtolower(Tools::getExt($upload['real_name'])) === 'pdf') {
                 // the real_name is used in case of error appending it
                 // the content is stored in a temporary file so it can be read with appendPdfs()

src/Make/MakeQrPdf.php

@@ -69,7 +69,7 @@ private function readAll(): array
         $this->Entity->idFilter = Tools::getIdFilterSql($this->idArr);
         $entityArr = $this->Entity->readShow($DisplayParams, true);
         foreach ($entityArr as &$entity) {
-            $entity['url'] = $this->getUrl((int) $entity['id']);
+            $entity['url'] = $this->getUrl($entity['id']);
         }
         return $entityArr;
     }

src/Make/MakeReport.php

@@ -76,11 +76,11 @@ protected function getRows(): array
     {
         $allUsers = $this->Teams->Users->readFromQuery('', includeArchived: true);
         foreach ($allUsers as $key => $user) {
-            $UsersHelper = new UsersHelper((int) $user['userid']);
+            $UsersHelper = new UsersHelper($user['userid']);
             // get the teams of user
             $teams = implode(',', $UsersHelper->getTeamsNameFromUserid());
             // get disk usage for all uploaded files
-            $diskUsage = $this->getDiskUsage((int) $user['userid']);
+            $diskUsage = $this->getDiskUsage($user['userid']);
 
             // remove unused columns as they will mess up the csv
             // these columns can be null

src/Make/MakeStreamZip.php

@@ -13,15 +13,13 @@
 namespace Elabftw\Make;
 
 use DateTimeImmutable;
-
 use Elabftw\Elabftw\App;
-
 use Elabftw\Exceptions\IllegalActionException;
 use Elabftw\Models\AbstractEntity;
 use League\Flysystem\UnableToReadFile;
-
 use ZipStream\ZipStream;
 
+use function array_walk;
 use function count;
 use function json_encode;
 
@@ -39,6 +37,9 @@ public function __construct(protected ZipStream $Zip, AbstractEntity $entity, pr
             entity: $entity,
             includeChangelog: $includeChangelog
         );
+        array_walk($this->idArr, function (&$id) {
+            $id = (int) $id;
+        });
     }
 
     /**
@@ -47,7 +48,7 @@ public function __construct(protected ZipStream $Zip, AbstractEntity $entity, pr
     public function getFileName(): string
     {
         if (count($this->idArr) === 1) {
-            $this->Entity->setId((int) $this->idArr[0]);
+            $this->Entity->setId($this->idArr[0]);
             $this->Entity->canOrExplode('read');
             return $this->getBaseFileName() . $this->extension;
         }

src/classes/AuthResponse.php

@@ -56,7 +56,7 @@ public function setTeams(UsersHelper $usersHelper): void
         // if the user only has access to one team, use this one directly
         $teamCount = count($this->selectableTeams);
         if ($teamCount === 1) {
-            $this->selectedTeam = (int) $this->selectableTeams[0]['id'];
+            $this->selectedTeam = $this->selectableTeams[0]['id'];
         } elseif ($teamCount === 0) {
             $Users = new Users($this->userid);
             $this->teamSelectionRequired = true;

src/classes/DisplayParams.php

@@ -22,6 +22,7 @@
 use PDO;
 use Symfony\Component\HttpFoundation\Request;
 
+use function array_map;
 use function implode;
 use function sprintf;
 use function trim;
@@ -98,7 +99,7 @@ private function adjust(): void
         // same with an extended search: we show all
         if ($scope === Scope::User->value && empty($this->Request->query->get('owner')) && empty($this->Request->query->get('extended'))) {
             // Note: the cast to int is necessary here (not sure why)
-            $this->appendFilterSql(FilterableColumn::Owner, (int) $this->Users->userData['userid']);
+            $this->appendFilterSql(FilterableColumn::Owner, $this->Users->userData['userid']);
         }
         if ($this->Users->userData['scope_' . $this->entityType->value] === Scope::Team->value) {
             $this->appendFilterSql(FilterableColumn::Team, $this->Users->team);
@@ -111,18 +112,19 @@ private function adjust(): void
             // the HAVING COUNT is necessary to make an AND search between tags
             // Note: we cannot use a placeholder for the IN of the tags because we need the quotes
             $Db = Db::getConnection();
-            $inPlaceholders = implode(' , ', array_map(function ($key) {
-                return ":tag$key";
-            }, array_keys($tags)));
+            $inPlaceholders = implode(', ', array_map(
+                fn($key): string => ":tag$key",
+                array_keys($tags),
+            ));
             $sql = 'SELECT tags2entity.item_id FROM `tags2entity`
                 INNER JOIN (SELECT id FROM tags WHERE tags.tag IN ( ' . $inPlaceholders . ' )) tg ON tags2entity.tag_id = tg.id
                 WHERE tags2entity.item_type = :type GROUP BY item_id HAVING COUNT(DISTINCT tags2entity.tag_id) = :count';
             $req = $Db->prepare($sql);
             // bind the tags in IN clause
             foreach ($tags as $key => $tag) {
-                $req->bindValue(":tag$key", $tag, PDO::PARAM_STR);
+                $req->bindValue(":tag$key", $tag);
             }
-            $req->bindValue(':type', $this->entityType->value, PDO::PARAM_STR);
+            $req->bindValue(':type', $this->entityType->value);
             $req->bindValue(':count', count($tags), PDO::PARAM_INT);
             $req->execute();
             $this->filterSql = Tools::getIdFilterSql($req->fetchAll(PDO::FETCH_COLUMN));

src/classes/EntitySqlBuilder.php

@@ -368,7 +368,7 @@ private function canBaseUserOnly(string $can): string
      */
     private function canTeams(string $can): string
     {
-        $UsersHelper = new UsersHelper((int) $this->entity->Users->userData['userid']);
+        $UsersHelper = new UsersHelper($this->entity->Users->userData['userid']);
         $teamsOfUser = $UsersHelper->getTeamsIdFromUserid();
         if (!empty($teamsOfUser)) {
             // JSON_OVERLAPS checks for the intersection of two arrays

src/classes/IdpsHelper.php

@@ -76,7 +76,7 @@ private function getSettingsByIdp(array $idp): array
             'baseurl' => $this->Config->configArr['saml_baseurl'],
 
             // Save IdP id
-            'idp_id' => (int) $idp['id'],
+            'idp_id' => $idp['id'],
 
             // Service Provider Data that we are deploying
             'sp' => array(

src/classes/Metadata.php

@@ -57,9 +57,10 @@ public function getExtraFields(): array
         }
         // sort the elements based on the position attribute. If not set, will be at the end.
         $extraFields = $this->metadata[MetadataEnum::ExtraFields->value];
-        uasort($extraFields, function (array $a, array $b): int {
-            return ($a['position'] ?? 9999) <=> ($b['position'] ?? 9999);
-        });
+        uasort(
+            $extraFields,
+            fn(array $a, array $b): int => ($a['position'] ?? 9999) <=> ($b['position'] ?? 9999),
+        );
         return $extraFields;
     }
 

src/classes/OrderingParams.php

@@ -15,6 +15,8 @@
 use Elabftw\Enums\Orderable;
 use Elabftw\Exceptions\ImproperActionException;
 
+use function array_map;
+
 /**
  * Parameters passed for ordering stuff
  */
@@ -35,8 +37,9 @@ public function __construct(protected array $reqBody)
      */
     protected function cleanup(array $ordering): array
     {
-        return array_map(function ($el) {
-            return (int) explode('_', $el)[1];
-        }, $ordering);
+        return array_map(
+            fn(string $el): int => (int) explode('_', $el)[1],
+            $ordering,
+        );
     }
 }