From daaddbd1f9b2dcf594d2c1a7e27b7fab453fa4eb Mon Sep 17 00:00:00 2001 From: Julien Veyssier Date: Fri, 12 Jan 2024 18:02:45 +0100 Subject: [PATCH] add psalm action, fix all psalm errors Signed-off-by: Julien Veyssier --- .github/workflows/psalm.yml | 57 ++++++++++ .gitignore | 3 +- composer.json | 2 +- lib/Controller/ComparisonController.php | 13 ++- lib/Controller/OldComparisonController.php | 14 ++- lib/Controller/OldPageController.php | 102 +++++++++++------- lib/Controller/PageController.php | 2 +- lib/Controller/UtilsController.php | 3 +- lib/Db/DirectoryMapper.php | 23 +++- lib/Db/PictureMapper.php | 3 + lib/Db/TileServerMapper.php | 15 ++- lib/Db/TrackMapper.php | 23 +++- lib/Listener/AddFilesScriptsListener.php | 3 + .../FilesSharingAddScriptsListener.php | 3 + lib/Service/ConversionService.php | 30 +++--- lib/Service/MapService.php | 9 +- lib/Service/OpenElevationService.php | 2 +- lib/Service/ProcessService.php | 68 +++++++----- lib/Service/SRTMGeoTIFFReader.php | 31 +++--- lib/Service/SrtmGeotiffElevationService.php | 6 +- lib/Service/ToolsService.php | 2 +- lib/Settings/AdminSection.php | 4 +- psalm.xml | 64 +++++++++++ tests/psalm-baseline.xml | 3 + tests/stubs/oc_core_command_base.php | 63 +++++++++++ tests/stubs/oc_filesystem.php | 8 ++ tests/stubs/oc_hooks_emitter.php | 11 ++ tests/stubs/oca_events.php | 21 ++++ 28 files changed, 454 insertions(+), 134 deletions(-) create mode 100644 .github/workflows/psalm.yml create mode 100644 psalm.xml create mode 100644 tests/psalm-baseline.xml create mode 100644 tests/stubs/oc_core_command_base.php create mode 100644 tests/stubs/oc_filesystem.php create mode 100644 tests/stubs/oc_hooks_emitter.php create mode 100644 tests/stubs/oca_events.php diff --git a/.github/workflows/psalm.yml b/.github/workflows/psalm.yml new file mode 100644 index 000000000..ab0b2f0f0 --- /dev/null +++ b/.github/workflows/psalm.yml @@ -0,0 +1,57 @@ +# This workflow is provided via the organization template repository +# +# https://github.com/nextcloud/.github +# https://docs.github.com/en/actions/learn-github-actions/sharing-workflows-with-your-organization + +name: Psalm static analysis + +on: + pull_request: + paths: + - .github/workflows/psalm.yml + - appinfo/** + - composer.* + - lib/** + - templates/** + - tests/** + push: + branches: + - main + - stable* + - test + paths: + - .github/workflows/psalm.yml + - appinfo/** + - composer.* + - lib/** + - templates/** + - tests/** + +concurrency: + group: psalm-${{ github.head_ref || github.run_id }} + cancel-in-progress: true + +jobs: + static-analysis: + runs-on: ubuntu-latest + + name: Psalm check + steps: + - name: Checkout + uses: actions/checkout@8e5e7e5ab8b370d6c329ec480221332ada57f0ab # v3.5.2 + + - name: Set up php + uses: shivammathur/setup-php@c5fc0d8281aba02c7fda07d3a70cc5371548067d # v2 + with: + php-version: 8.2 + coverage: none + ini-file: development + extensions: mbstring, iconv, fileinfo, intl, sqlite, pdo_sqlite, gd, zip + env: + GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} + + - name: Install dependencies + run: composer i + + - name: Run coding standards check + run: composer run psalm diff --git a/.gitignore b/.gitignore index 3b19fa7e7..aedcfad04 100644 --- a/.gitignore +++ b/.gitignore @@ -4,4 +4,5 @@ /.idea /css/fontawesome-free /vendor -/.php-cs-fixer.cache +.php*.cache +/composer.lock diff --git a/composer.json b/composer.json index 4f4c27d4b..06a1bd8bb 100644 --- a/composer.json +++ b/composer.json @@ -20,7 +20,7 @@ "lint": "find . -name \\*.php -not -path './vendor/*' -print0 | xargs -0 -n1 php -l", "cs:check": "php-cs-fixer fix --dry-run --diff", "cs:fix": "php-cs-fixer fix", - "psalm": "psalm.phar", + "psalm": "psalm.phar --no-cache", "test:unit": "phpunit --config tests/phpunit.xml" }, "require-dev": { diff --git a/lib/Controller/ComparisonController.php b/lib/Controller/ComparisonController.php index ccf2ad078..55639020a 100644 --- a/lib/Controller/ComparisonController.php +++ b/lib/Controller/ComparisonController.php @@ -23,6 +23,7 @@ use OCP\DB\Exception; +use OCP\Files\File; use OCP\Files\IRootFolder; use OCP\IConfig; use OCP\IRequest; @@ -62,8 +63,10 @@ public function comparePageGet(): TemplateResponse { if (isset($_GET['path' . $i]) && $_GET['path' . $i] !== '') { $cleanPath = str_replace(array('../', '..\\'), '', $_GET['path' . $i]); $file = $userFolder->get($cleanPath); - $content = $file->getContent(); - $gpxs[$cleanPath] = $content; + if ($file instanceof File) { + $content = $file->getContent(); + $gpxs[$cleanPath] = $content; + } } } } @@ -756,7 +759,7 @@ private function getStats(array $contents, array &$process_errors): array { if (empty($point->time)) { $pointtime = null; } else { - $pointtime = new \DateTime($point->time); + $pointtime = new \DateTime((string) $point->time); } if ($lastPoint !== null && (!empty($lastPoint->ele))) { $lastPointele = (float)$lastPoint->ele; @@ -764,7 +767,7 @@ private function getStats(array $contents, array &$process_errors): array { $lastPointele = null; } if ($lastPoint !== null && (!empty($lastPoint->time))) { - $lastTime = new \DateTime($lastPoint->time); + $lastTime = new \DateTime((string) $lastPoint->time); } else { $lastTime = null; } @@ -860,7 +863,7 @@ private function getStats(array $contents, array &$process_errors): array { $lastPointele = null; } if ($lastPoint !== null && (!empty($lastPoint->time))) { - $lastTime = new \DateTime($lastPoint->time); + $lastTime = new \DateTime((string) $lastPoint->time); } else { $lastTime = null; } diff --git a/lib/Controller/OldComparisonController.php b/lib/Controller/OldComparisonController.php index ba054220f..ab453a761 100644 --- a/lib/Controller/OldComparisonController.php +++ b/lib/Controller/OldComparisonController.php @@ -18,6 +18,7 @@ use OCP\AppFramework\Http\TemplateResponse; use OCP\AppFramework\Services\IInitialState; +use OCP\Files\File; use OCP\Files\IRootFolder; use OCP\IConfig; @@ -91,8 +92,10 @@ public function gpxvcomp(): TemplateResponse { if (isset($_GET['path'.$i]) && $_GET['path'.$i] !== '') { $cleanpath = str_replace(array('../', '..\\'), '', $_GET['path'.$i]); $file = $userFolder->get($cleanpath); - $content = $file->getContent(); - $gpxs[$cleanpath] = $content; + if ($file instanceof File) { + $content = $file->getContent(); + $gpxs[$cleanpath] = $content; + } } } } @@ -222,6 +225,7 @@ private function processTrackComparison($contents, &$process_errors): array { } // comparison of each pair of input file + /** @var array $names */ $names = array_keys($contents); $i = 0; while ($i < count($names)) { @@ -832,7 +836,7 @@ private function getStats($contents, &$process_errors): array { $lastPointele = null; } if ($lastPoint !== null && (!empty($lastPoint->time))) { - $lastTime = new \DateTime($lastPoint->time); + $lastTime = new \DateTime((string) $lastPoint->time); } else { $lastTime = null; } @@ -928,7 +932,7 @@ private function getStats($contents, &$process_errors): array { $lastPointele = null; } if ($lastPoint !== null && (!empty($lastPoint->time))) { - $lastTime = new \DateTime($lastPoint->time); + $lastTime = new \DateTime((string) $lastPoint->time); } else { $lastTime = null; } @@ -1021,7 +1025,7 @@ private function getStats($contents, &$process_errors): array { $moving_avg_speed = $total_distance / $moving_time; $moving_avg_speed = $moving_avg_speed / 1000; $moving_avg_speed = $moving_avg_speed * 3600; - $moving_avg_speed = sprintf('%.2f', $moving_avg_speed); + $moving_avg_speed = (float) sprintf('%.2f', $moving_avg_speed); } if ($date_begin === null) { diff --git a/lib/Controller/OldPageController.php b/lib/Controller/OldPageController.php index dd522a34d..f669a1440 100644 --- a/lib/Controller/OldPageController.php +++ b/lib/Controller/OldPageController.php @@ -13,7 +13,7 @@ use \OCP\IL10N; use Exception; -use OC\Files\Node\File; +use OC\User\NoUserException; use OCA\GpxPod\AppInfo\Application; use OCA\GpxPod\Service\ConversionService; use OCA\GpxPod\Service\ProcessService; @@ -25,14 +25,17 @@ use OCP\AppFramework\Http\TemplateResponse; use OCP\AppFramework\Services\IInitialState; use OCP\DB\QueryBuilder\IQueryBuilder; +use OCP\Files\File; use OCP\Files\Folder; - use OCP\Files\IRootFolder; + +use OCP\Files\NotPermittedException; use OCP\IConfig; use OCP\IDBConnection; use OCP\IRequest; use OCP\Share\IManager; +use OCP\Share\IShare; use Psr\Log\LoggerInterface; use Throwable; @@ -384,7 +387,7 @@ public function getgpx($path) { $gpxContent = ''; if ($userFolder->nodeExists($cleanpath)) { $file = $userFolder->get($cleanpath); - if ($file->getType() === \OCP\Files\FileInfo::TYPE_FILE) { + if ($file instanceof File) { if (str_ends_with($file->getName(), '.GPX') || str_ends_with($file->getName(), '.gpx')) { $gpxContent = $this->toolsService->remove_utf8_bom($file->getContent()); } @@ -406,7 +409,7 @@ public function getpublicgpx($path, $username) { if ($userFolder->nodeExists($cleanpath)) { $file = $userFolder->get($cleanpath); - if ($file->getType() === \OCP\Files\FileInfo::TYPE_FILE) { + if ($file instanceof File) { if (str_ends_with($file->getName(), '.GPX') || str_ends_with($file->getName(), '.gpx')) { // we check the file is actually shared by public link $dl_url = $this->getPublinkDownloadURL($file, $username); @@ -433,7 +436,7 @@ public function getTrackMarkersText(string $directoryPath, bool $processAll = fa $userFolder = $this->root->getUserFolder($this->userId); $qb = $this->dbconnection->getQueryBuilder(); - if ($directoryPath === null || !$userFolder->nodeExists($directoryPath) || $this->getDirectoryByPath($this->userId, $directoryPath) === null) { + if (!$userFolder->nodeExists($directoryPath) || $this->getDirectoryByPath($this->userId, $directoryPath) === null) { return new DataResponse('No such directory', 400); } @@ -457,13 +460,16 @@ public function getTrackMarkersText(string $directoryPath, bool $processAll = fa } if (!$recursive) { - foreach ($userFolder->get($directoryPath)->getDirectoryListing() as $ff) { - if ($ff->getType() === \OCP\Files\FileInfo::TYPE_FILE) { - $ffext = '.'.strtolower(pathinfo($ff->getName(), PATHINFO_EXTENSION)); - if (in_array($ffext, array_keys(ConversionService::fileExtToGpsbabelFormat))) { - // if shared files are allowed or it is not shared - if ($sharedAllowed || !$ff->isShared()) { - $filesByExtension[$ffext][] = $ff; + $subfolderNode = $userFolder->get($directoryPath); + if ($subfolderNode instanceof Folder) { + foreach ($subfolderNode->getDirectoryListing() as $ff) { + if ($ff instanceof File) { + $ffext = '.' . strtolower(pathinfo($ff->getName(), PATHINFO_EXTENSION)); + if (in_array($ffext, array_keys(ConversionService::fileExtToGpsbabelFormat))) { + // if shared files are allowed or it is not shared + if ($sharedAllowed || !$ff->isShared()) { + $filesByExtension[$ffext][] = $ff; + } } } } @@ -591,13 +597,16 @@ private function processGpxFiles(Folder $userFolder, string $subfolder, string $ $gpxfiles = []; if (!$recursive) { - foreach ($userFolder->get($subfolder)->getDirectoryListing() as $ff) { - if ($ff->getType() === \OCP\Files\FileInfo::TYPE_FILE) { - $ffext = '.'.strtolower(pathinfo($ff->getName(), PATHINFO_EXTENSION)); - if ($ffext === '.gpx') { - // if shared files are allowed or it is not shared - if ($sharedAllowed || !$ff->isShared()) { - $gpxfiles[] = $ff; + $subfolderNode = $userFolder->get($subfolder); + if ($subfolderNode instanceof Folder) { + foreach ($subfolderNode->getDirectoryListing() as $ff) { + if ($ff->getType() === \OCP\Files\FileInfo::TYPE_FILE) { + $ffext = '.' . strtolower(pathinfo($ff->getName(), PATHINFO_EXTENSION)); + if ($ffext === '.gpx') { + // if shared files are allowed or it is not shared + if ($sharedAllowed || !$ff->isShared()) { + $gpxfiles[] = $ff; + } } } } @@ -615,9 +624,9 @@ private function processGpxFiles(Folder $userFolder, string $subfolder, string $ $gpx_relative_path = str_replace('//', '/', $gpx_relative_path); $newCRC[$gpx_relative_path] = $gg->getMTime().'.'.$gg->getSize(); // if the file is not in the DB or if its content hash has changed - if ((! array_key_exists($gpx_relative_path, $gpxs_in_db)) or - $gpxs_in_db[$gpx_relative_path] !== $newCRC[$gpx_relative_path] or - $processAll === 'true' + if ( + (!array_key_exists($gpx_relative_path, $gpxs_in_db)) + || $gpxs_in_db[$gpx_relative_path] !== $newCRC[$gpx_relative_path] ) { // not in DB or hash changed $gpxs_to_process[] = $gg; @@ -711,15 +720,18 @@ private function getGeoPicsFromFolder($subfolder, $recursive, $user = null) { if ($recursive) { $picfiles = $this->processService->searchFilesWithExt($userFolder->get($subfolder), $sharedAllowed, $mountedAllowed, ['.jpg']); } else { - foreach ($userFolder->get($subfolder)->search('.jpg') as $picfile) { - if ($picfile->getType() === \OCP\Files\FileInfo::TYPE_FILE - && dirname($picfile->getPath()) === $subfolder_path - && ( - str_ends_with($picfile->getName(), '.jpg') - || str_ends_with($picfile->getName(), '.JPG') - ) - ) { - $picfiles[] = $picfile; + $subfolderNode = $userFolder->get($subfolder); + if ($subfolderNode instanceof Folder) { + foreach ($subfolderNode->search('.jpg') as $picfile) { + if ($picfile instanceof File + && dirname($picfile->getPath()) === $subfolder_path + && ( + str_ends_with($picfile->getName(), '.jpg') + || str_ends_with($picfile->getName(), '.JPG') + ) + ) { + $picfiles[] = $picfile; + } } } } @@ -1021,7 +1033,11 @@ private function cleanDbFromAbsentFiles($subfolder) { * method to get the URL to download a public file with OC/NC File system * from the file object and the user who shares the file * - * @return null if the file is not shared or inside a shared folder + * @param $file + * @param $username + * @return string|null null if the file is not shared or inside a shared folder + * @throws NoUserException + * @throws NotPermittedException */ private function getPublinkDownloadURL($file, $username) { $uf = $this->root->getUserFolder($username); @@ -1029,7 +1045,7 @@ private function getPublinkDownloadURL($file, $username) { // CHECK if file is shared $shares = $this->shareManager->getSharesBy($username, - \OCP\Share::SHARE_TYPE_LINK, $file, false, 1, 0); + IShare::TYPE_LINK, $file, false, 1, 0); if (count($shares) > 0) { foreach($shares as $share) { if ($share->getPassword() === null) { @@ -1045,7 +1061,7 @@ private function getPublinkDownloadURL($file, $username) { while ($tmpfolder->getPath() !== $uf->getPath() and $tmpfolder->getPath() !== "/" && $dl_url === null) { $shares_folder = $this->shareManager->getSharesBy($username, - \OCP\Share::SHARE_TYPE_LINK, $tmpfolder, false, 1, 0); + IShare::TYPE_LINK, $tmpfolder, false, 1, 0); if (count($shares_folder) > 0) { foreach($shares_folder as $share) { if ($share->getPassword() === null) { @@ -1067,7 +1083,11 @@ private function getPublinkDownloadURL($file, $username) { } /** - * @return null if the file is not shared or inside a shared folder + * @param $file + * @param $username + * @return array|null null if the file is not shared or inside a shared folder + * @throws NotPermittedException + * @throws NoUserException */ private function getPublinkParameters($file, $username) { $uf = $this->root->getUserFolder($username); @@ -1075,7 +1095,7 @@ private function getPublinkParameters($file, $username) { // CHECK if file is shared $shares = $this->shareManager->getSharesBy($username, - \OCP\Share::SHARE_TYPE_LINK, $file, false, 1, 0); + IShare::TYPE_LINK, $file, false, 1, 0); if (count($shares) > 0) { foreach($shares as $share) { if ($share->getPassword() === null) { @@ -1091,7 +1111,7 @@ private function getPublinkParameters($file, $username) { while ($tmpfolder->getPath() !== $uf->getPath() and $tmpfolder->getPath() !== "/" && $paramArray === null) { $shares_folder = $this->shareManager->getSharesBy($username, - \OCP\Share::SHARE_TYPE_LINK, $tmpfolder, false, 1, 0); + IShare::TYPE_LINK, $tmpfolder, false, 1, 0); if (count($shares_folder) > 0) { foreach($shares_folder as $share) { if ($share->getPassword() === null) { @@ -1293,7 +1313,7 @@ private function getPubfolderDownloadURL($dir, $username) { // check that this is a directory if ($dir->getType() === \OCP\Files\FileInfo::TYPE_FOLDER) { $shares_folder = $this->shareManager->getSharesBy($username, - \OCP\Share::SHARE_TYPE_LINK, $dir, false, 1, 0); + IShare::TYPE_LINK, $dir, false, 1, 0); // check that this directory is publicly shared if (count($shares_folder) > 0) { foreach($shares_folder as $share) { @@ -1314,7 +1334,7 @@ private function getPubfolderDownloadURL($dir, $username) { while ($tmpfolder->getPath() !== $uf->getPath() and $tmpfolder->getPath() !== "/" && $dl_url === null) { $shares_folder = $this->shareManager->getSharesBy($username, - \OCP\Share::SHARE_TYPE_LINK, $tmpfolder, false, 1, 0); + IShare::TYPE_LINK, $tmpfolder, false, 1, 0); if (count($shares_folder) > 0) { foreach($shares_folder as $share) { if ($share->getPassword() === null) { @@ -1343,7 +1363,7 @@ private function getPubfolderParameters($dir, $username) { // check that this is a directory if ($dir->getType() === \OCP\Files\FileInfo::TYPE_FOLDER) { $shares_folder = $this->shareManager->getSharesBy($username, - \OCP\Share::SHARE_TYPE_LINK, $dir, false, 1, 0); + IShare::TYPE_LINK, $dir, false, 1, 0); // check that this directory is publicly shared if (count($shares_folder) > 0) { foreach($shares_folder as $share) { @@ -1361,7 +1381,7 @@ private function getPubfolderParameters($dir, $username) { while ($tmpfolder->getPath() !== $uf->getPath() and $tmpfolder->getPath() !== "/" && $paramArray === null) { $shares_folder = $this->shareManager->getSharesBy($username, - \OCP\Share::SHARE_TYPE_LINK, $tmpfolder, false, 1, 0); + IShare::TYPE_LINK, $tmpfolder, false, 1, 0); if (count($shares_folder) > 0) { foreach($shares_folder as $share) { if ($share->getPassword() === null) { diff --git a/lib/Controller/PageController.php b/lib/Controller/PageController.php index 954f66326..3f5fe6e4c 100644 --- a/lib/Controller/PageController.php +++ b/lib/Controller/PageController.php @@ -1048,7 +1048,7 @@ public function getTrackMarkersJson(int $id, string $directoryPath, bool $proces } catch (\OCP\DB\Exception | DoesNotExistException $e) { return new DataResponse(['error' => 'No such directory'], Http::STATUS_NOT_FOUND); } - if ($directoryPath === null || !$userFolder->nodeExists($directoryPath)) { + if (!$userFolder->nodeExists($directoryPath)) { return new DataResponse(['error' => 'No such directory'], Http::STATUS_NOT_FOUND); } $folder = $userFolder->get($directoryPath); diff --git a/lib/Controller/UtilsController.php b/lib/Controller/UtilsController.php index 001642009..2b498d959 100644 --- a/lib/Controller/UtilsController.php +++ b/lib/Controller/UtilsController.php @@ -21,6 +21,7 @@ use OCP\DB\QueryBuilder\IQueryBuilder; use OCP\Files\FileInfo; +use OCP\Files\Folder; use OCP\Files\IRootFolder; use OCP\IConfig; use OCP\IDBConnection; @@ -358,7 +359,7 @@ public function moveTracks($trackpaths, $destination): DataResponse { if ($uf->nodeExists($cleanDest)) { $destNode = $uf->get($cleanDest); - if ($destNode->getType() === FileInfo::TYPE_FOLDER + if ($destNode instanceof Folder && $destNode->isCreatable() ) { $done = true; diff --git a/lib/Db/DirectoryMapper.php b/lib/Db/DirectoryMapper.php index 878a66f0a..6bb9ad655 100644 --- a/lib/Db/DirectoryMapper.php +++ b/lib/Db/DirectoryMapper.php @@ -34,6 +34,9 @@ use OCP\IDBConnection; +/** + * @extends QBMapper + */ class DirectoryMapper extends QBMapper { public function __construct(IDBConnection $db) { parent::__construct($db, 'gpxpod_directories', Directory::class); @@ -54,7 +57,9 @@ public function getDirectory(int $id): Directory { $qb->expr()->eq('id', $qb->createNamedParameter($id, IQueryBuilder::PARAM_INT)) ); - return $this->findEntity($qb); + /** @var Directory $directory */ + $directory = $this->findEntity($qb); + return $directory; } /** @@ -77,7 +82,9 @@ public function getDirectoryOfUser(int $id, string $userId): Directory { $qb->expr()->eq('user', $qb->createNamedParameter($userId, IQueryBuilder::PARAM_STR)) ); - return $this->findEntity($qb); + /** @var Directory $directory */ + $directory = $this->findEntity($qb); + return $directory; } /** @@ -117,7 +124,9 @@ public function getDirectoryOfUserByPath(string $path, string $userId): Director $qb->expr()->eq('user', $qb->createNamedParameter($userId, IQueryBuilder::PARAM_STR)) ); - return $this->findEntity($qb); + /** @var Directory $directory */ + $directory = $this->findEntity($qb); + return $directory; } /** @@ -171,7 +180,9 @@ public function createDirectory(string $path, string $user, bool $isOpen = false $dir->setSortOrder($sortOrder); $dir->setSortAsc($sortAsc); $dir->setRecursive($recursive); - return $this->insert($dir); + /** @var Directory $directory */ + $createdDirectory = $this->insert($dir); + return $createdDirectory; } /** @@ -213,6 +224,8 @@ public function updateDirectory( if ($recursive !== null) { $dir->setRecursive($recursive); } - return $this->update($dir); + /** @var Directory $directory */ + $updatedDirectory = $this->update($dir); + return $updatedDirectory; } } diff --git a/lib/Db/PictureMapper.php b/lib/Db/PictureMapper.php index 1a6d9c1a0..477c2868b 100644 --- a/lib/Db/PictureMapper.php +++ b/lib/Db/PictureMapper.php @@ -30,6 +30,9 @@ use OCP\DB\QueryBuilder\IQueryBuilder; use OCP\IDBConnection; +/** + * @extends QBMapper + */ class PictureMapper extends QBMapper { public function __construct(IDBConnection $db) { parent::__construct($db, 'gpxpod_pictures', Picture::class); diff --git a/lib/Db/TileServerMapper.php b/lib/Db/TileServerMapper.php index 7b81ce40b..1c822dce1 100644 --- a/lib/Db/TileServerMapper.php +++ b/lib/Db/TileServerMapper.php @@ -34,6 +34,9 @@ use OCP\IDBConnection; +/** + * @extends QBMapper + */ class TileServerMapper extends QBMapper { public function __construct(IDBConnection $db) { parent::__construct($db, 'gpxpod_tileservers', TileServer::class); @@ -55,7 +58,9 @@ public function getTileServer(int $id): TileServer { $qb->expr()->eq('id', $qb->createNamedParameter($id, IQueryBuilder::PARAM_INT)) ); - return $this->findEntity($qb); + /** @var TileServer $tileServer */ + $tileServer = $this->findEntity($qb); + return $tileServer; } /** @@ -84,7 +89,9 @@ public function getTileServerOfUser(int $id, ?string $userId): TileServer { ); } - return $this->findEntity($qb); + /** @var TileServer $tileServer */ + $tileServer = $this->findEntity($qb); + return $tileServer; } /** @@ -156,7 +163,9 @@ public function createTileServer(?string $userId, int $type, string $name, strin $tileServer->setAttribution($attribution); $tileServer->setMinZoom($minZoom); $tileServer->setMaxZoom($maxZoom); - return $this->insert($tileServer); + /** @var TileServer $createdTileServer */ + $createdTileServer = $this->insert($tileServer); + return $createdTileServer; } /** diff --git a/lib/Db/TrackMapper.php b/lib/Db/TrackMapper.php index 50f325165..d2d4b62e3 100644 --- a/lib/Db/TrackMapper.php +++ b/lib/Db/TrackMapper.php @@ -33,6 +33,9 @@ use OCP\IDBConnection; +/** + * @extends QBMapper + */ class TrackMapper extends QBMapper { public function __construct(IDBConnection $db) { parent::__construct($db, 'gpxpod_tracks', Track::class); @@ -54,7 +57,9 @@ public function getTrack(int $id): Track { $qb->expr()->eq('id', $qb->createNamedParameter($id, IQueryBuilder::PARAM_INT)) ); - return $this->findEntity($qb); + /** @var Track $track */ + $track = $this->findEntity($qb); + return $track; } /** @@ -77,7 +82,9 @@ public function getTrackOfUser(int $id, string $userId): Track { $qb->expr()->eq('user', $qb->createNamedParameter($userId, IQueryBuilder::PARAM_STR)) ); - return $this->findEntity($qb); + /** @var Track $track */ + $track = $this->findEntity($qb); + return $track; } /** @@ -144,7 +151,9 @@ public function getTrackOfUserByPath(string $userId, string $trackPath, ?int $di ); } - return $this->findEntity($qb); + /** @var Track $track */ + $track = $this->findEntity($qb); + return $track; } /** @@ -242,7 +251,9 @@ public function createTrack( $track->setIsEnabled($isEnabled ? 1 : 0); $track->setColor($color); $track->setColorCriteria($colorCriteria); - return $this->insert($track); + /** @var Track $createdTrack */ + $createdTrack = $this->insert($track); + return $createdTrack; } /** @@ -287,7 +298,9 @@ public function updateTrack(int $id, string $userId, if ($directoryId !== null) { $track->setDirectoryId($directoryId); } - return $this->update($track); + /** @var Track $updatedTrack */ + $updatedTrack = $this->update($track); + return $updatedTrack; } public function updateDirectoryTracks(int $directoryId, string $userId, ?bool $isEnabled = null): int { diff --git a/lib/Listener/AddFilesScriptsListener.php b/lib/Listener/AddFilesScriptsListener.php index a5db32592..717e1c719 100644 --- a/lib/Listener/AddFilesScriptsListener.php +++ b/lib/Listener/AddFilesScriptsListener.php @@ -29,6 +29,9 @@ use OCP\EventDispatcher\IEventListener; use OCP\Util; +/** + * @implements IEventListener + */ class AddFilesScriptsListener implements IEventListener { public function __construct(private IAppManager $appManager) { diff --git a/lib/Listener/FilesSharingAddScriptsListener.php b/lib/Listener/FilesSharingAddScriptsListener.php index e5643a07a..e5b07aa3a 100644 --- a/lib/Listener/FilesSharingAddScriptsListener.php +++ b/lib/Listener/FilesSharingAddScriptsListener.php @@ -29,6 +29,9 @@ use OCP\EventDispatcher\IEventListener; use OCP\Util; +/** + * @implements IEventListener + */ class FilesSharingAddScriptsListener implements IEventListener { public function __construct(private IInitialState $initialStateService) { diff --git a/lib/Service/ConversionService.php b/lib/Service/ConversionService.php index d50193cbb..6abe0a5d9 100644 --- a/lib/Service/ConversionService.php +++ b/lib/Service/ConversionService.php @@ -79,10 +79,10 @@ public function sanitizeGpxExtensions(string $gpxContent): string { $parentNodeName = $extensionsNode->parentNode->localName; if ($parentNodeName === 'trkpt') { foreach ($extensionsNode->childNodes as $ext) { - if ($ext instanceof DOMNode && $ext->nodeName === 'gpxtpx:TrackPointExtension') { + if ($ext->nodeName === 'gpxtpx:TrackPointExtension') { $nodesToPushUp = []; foreach ($ext->childNodes as $gpxtpxExt) { - if ($gpxtpxExt instanceof DOMNode && $gpxtpxExt->prefix !== 'gpxtpx') { + if ($gpxtpxExt->prefix !== 'gpxtpx') { $nodesToPushUp[] = $gpxtpxExt; } } @@ -99,13 +99,11 @@ public function sanitizeGpxExtensions(string $gpxContent): string { } elseif ($parentNodeName === 'trk' || $parentNodeName === 'rte') { $emptyExtensionToRemove = []; foreach ($extensionsNode->childNodes as $ext) { - if ($ext instanceof DOMNode && count($ext->childNodes) > 0) { + if (count($ext->childNodes) > 0) { $nodesToPushUp = []; foreach ($ext->childNodes as $subExt) { - if ($subExt instanceof DOMNode) { - if ($subExt instanceof DOMElement) { - $nodesToPushUp[] = $subExt; - } + if ($subExt instanceof DOMElement) { + $nodesToPushUp[] = $subExt; } } foreach ($nodesToPushUp as $node) { @@ -397,7 +395,7 @@ private function exifCoordToNumber($coordPart) { public function jpgToGpx($jpgFilePath, $fileName) { $result = ''; - $exif = \exif_read_data($jpgFilePath, 0, true); + $exif = \exif_read_data($jpgFilePath, null, true); if (isset($exif['GPS']) and isset($exif['GPS']['GPSLongitude']) and isset($exif['GPS']['GPSLatitude']) @@ -507,17 +505,17 @@ public function igcToGpx($fh, $trackOptions) { $gpx_wpt_lat = $domGpx->createAttribute('lat'); $gpx_trkpt->appendChild($gpx_wpt_lat); - $gpx_wpt_lat_text = $domGpx->createTextNode($lat); + $gpx_wpt_lat_text = $domGpx->createTextNode((string) $lat); $gpx_wpt_lat->appendChild($gpx_wpt_lat_text); $gpx_wpt_lon = $domGpx->createAttribute('lon'); $gpx_trkpt->appendChild($gpx_wpt_lon); - $gpx_wpt_lon_text = $domGpx->createTextNode($lon); + $gpx_wpt_lon_text = $domGpx->createTextNode((string) $lon); $gpx_wpt_lon->appendChild($gpx_wpt_lon_text); $gpx_ele = $domGpx->createElement('ele'); $gpx_trkpt->appendChild($gpx_ele); - $gpx_ele_text = $domGpx->createTextNode(intval(substr($line, 30, 5))); + $gpx_ele_text = $domGpx->createTextNode((string) intval(substr($line, 30, 5))); $gpx_ele->appendChild($gpx_ele_text); $gpx_time = $domGpx->createElement('time'); @@ -529,9 +527,10 @@ public function igcToGpx($fh, $trackOptions) { $gpx_time->appendChild($gpx_time_text); if ($includeBaro) { + /** @var DOMElement $gpx_trkpt_baro */ $gpx_trkpt_baro = $gpx_trkpt->cloneNode(true); $ele = $gpx_trkpt_baro->getElementsByTagName('ele')->item(0); - $ele->nodeValue = intval(substr($line, 25, 5)); + $ele->nodeValue = (string) intval(substr($line, 25, 5)); $gpx_trkseg_baro->appendChild($gpx_trkpt_baro); } } @@ -546,8 +545,11 @@ public function unicsvToGpx($csvFilePath) { $csv = array_map('str_getcsv', file($csvFilePath, FILE_SKIP_EMPTY_LINES)); $keys = array_shift($csv); - foreach ($csv as $i => $row) { - $csv[$i] = array_combine($keys, $row); + if (is_array($keys)) { + foreach ($csv as $i => $row) { + /** @var array $keys */ + $csv[$i] = array_combine($keys, $row); + } } foreach ($csv as $line) { diff --git a/lib/Service/MapService.php b/lib/Service/MapService.php index 4cc65e838..3cef30574 100644 --- a/lib/Service/MapService.php +++ b/lib/Service/MapService.php @@ -107,7 +107,14 @@ public function getRasterTile(string $service, int $x, int $y, int $z): ?string $s = 'abc'[mt_rand(0, 2)]; $url = 'https://' . $s . '.tile.openstreetmap.org/' . $z . '/' . $x . '/' . $y . '.png'; } - return $this->client->get($url)->getBody(); + $body = $this->client->get($url)->getBody(); + if (is_resource($body)) { + $content = stream_get_contents($body); + return $content === false + ? null + : $content; + } + return $body; } /** diff --git a/lib/Service/OpenElevationService.php b/lib/Service/OpenElevationService.php index 46e3b1bac..f201efbbf 100644 --- a/lib/Service/OpenElevationService.php +++ b/lib/Service/OpenElevationService.php @@ -70,7 +70,7 @@ public function correctElevations(GpxFile $gpxFile): GpxFile { /** * @param array $coordinates - * @return string[] + * @return array * @throws Exception */ private function request(array $coordinates): array { diff --git a/lib/Service/ProcessService.php b/lib/Service/ProcessService.php index a16118907..660aeb43b 100644 --- a/lib/Service/ProcessService.php +++ b/lib/Service/ProcessService.php @@ -23,6 +23,7 @@ use OCP\AppFramework\Db\MultipleObjectsReturnedException; use OCP\DB\QueryBuilder\IQueryBuilder; use OCP\Files\File; +use OCP\Files\Folder; use OCP\Files\InvalidPathException; use OCP\Files\IRootFolder; use OCP\Files\Node; @@ -60,13 +61,14 @@ public function __construct( /** * recursively search files with given extensions (case insensitive) * - * @param Node $folder + * @param Folder $folder * @param bool $sharedAllowed * @param bool $mountedAllowed * @param array $extensions * @return array|File[] + * @throws NotFoundException */ - public function searchFilesWithExt(Node $folder, bool $sharedAllowed, bool $mountedAllowed, array $extensions): array { + public function searchFilesWithExt(Folder $folder, bool $sharedAllowed, bool $mountedAllowed, array $extensions): array { $res = []; foreach ($folder->getDirectoryListing() as $node) { // top level files with matching ext @@ -77,7 +79,7 @@ public function searchFilesWithExt(Node $folder, bool $sharedAllowed, bool $moun $res[] = $node; } } - } else { + } elseif ($node instanceof Folder) { // top level folders if (($mountedAllowed || !$node->isMounted()) && ($sharedAllowed || !$node->isShared()) @@ -162,20 +164,25 @@ public function processGpxFiles( $userfolder_path = $userFolder->getPath(); // find gpx files in the directory (in the file system) - if ($recursive) { - $gpxFiles = $this->searchFilesWithExt($userFolder->get($dbDir->getPath()), $sharedAllowed, $mountedAllowed, ['.gpx']); - } else { - $gpxFiles = array_filter($userFolder->get($dbDir->getPath())->getDirectoryListing(), static function (Node $node) use ($sharedAllowed) { - if ($node instanceof File) { - $fileExtension = '.' . strtolower(pathinfo($node->getName(), PATHINFO_EXTENSION)); - if ($fileExtension === '.gpx') { - if ($sharedAllowed || !$node->isShared()) { - return true; + $dbDirNode = $userFolder->get($dbDir->getPath()); + if ($dbDirNode instanceof Folder) { + if ($recursive) { + $gpxFiles = $this->searchFilesWithExt($dbDirNode, $sharedAllowed, $mountedAllowed, ['.gpx']); + } else { + $gpxFiles = array_filter($dbDirNode->getDirectoryListing(), static function (Node $node) use ($sharedAllowed) { + if ($node instanceof File) { + $fileExtension = '.' . strtolower(pathinfo($node->getName(), PATHINFO_EXTENSION)); + if ($fileExtension === '.gpx') { + if ($sharedAllowed || !$node->isShared()) { + return true; + } } } - } - return false; - }); + return false; + }); + } + } else { + $gpxFiles = []; } // CHECK what is to be processed @@ -520,7 +527,7 @@ public function getMarkerFromFile(File $file, string $userId): ?array { private function getPointsWithCoordinates(SimpleXMLElement $points): array { $pointsWithCoords = []; foreach ($points as $point) { - if ($point !== null && !empty($point['lat']) && !empty($point['lon'])) { + if (!empty($point) && !empty($point['lat']) && !empty($point['lon'])) { $pointsWithCoords[] = $point; } } @@ -809,20 +816,23 @@ public function getGeoPicsFromFolder(string $userId, string $subfolder, int $dir if ($recursive) { $picfiles = $this->searchFilesWithExt($userFolder->get($subfolder), $sharedAllowed, $mountedAllowed, ['.jpg', '.jpeg']); } else { - foreach ($userFolder->get($subfolder)->search('.jpg') as $picfile) { - if ($picfile instanceof File - && dirname($picfile->getPath()) === $subfolder_path - && preg_match('/\.jpg$/i', $picfile->getName()) - ) { - $picfiles[] = $picfile; + $subfolderNode = $userFolder->get($subfolder); + if ($subfolderNode instanceof Folder) { + foreach ($subfolderNode->search('.jpg') as $picfile) { + if ($picfile instanceof File + && dirname($picfile->getPath()) === $subfolder_path + && preg_match('/\.jpg$/i', $picfile->getName()) + ) { + $picfiles[] = $picfile; + } } - } - foreach ($userFolder->get($subfolder)->search('.jpeg') as $picfile) { - if ($picfile instanceof File - && dirname($picfile->getPath()) === $subfolder_path - && preg_match('/\.jpeg$/i', $picfile->getName()) - ) { - $picfiles[] = $picfile; + foreach ($subfolderNode->search('.jpeg') as $picfile) { + if ($picfile instanceof File + && dirname($picfile->getPath()) === $subfolder_path + && preg_match('/\.jpeg$/i', $picfile->getName()) + ) { + $picfiles[] = $picfile; + } } } } diff --git a/lib/Service/SRTMGeoTIFFReader.php b/lib/Service/SRTMGeoTIFFReader.php index 87b8a4cf0..d4d481cce 100644 --- a/lib/Service/SRTMGeoTIFFReader.php +++ b/lib/Service/SRTMGeoTIFFReader.php @@ -34,10 +34,15 @@ class SRTMGeoTIFFReader { private $fp; // file pointer to current GeoTIFF data file private $tileRefHoriz; // the horizontal tile reference figure (01-72) private $tileRefVert; // the vertical tile reference figure (01-24) - private $latLons = array(); // the supplied lats & lons - private $elevations = array(); // the elevations values found + private $latLons = []; // the supplied lats & lons + private $elevations = []; // the elevations values found private LoggerInterface $logger; private ISimpleFolder $dataDir; + private mixed $stripOffsets; + private mixed $numDataRows; + private mixed $numDataCols; + private int|float $topleftLon; + private int|float $topleftLat; /** * Constructor: assigns data directory @@ -94,7 +99,7 @@ public function getAscentDescent() { } } } - return array("ascent" => $ascent, "descent" => $descent); + return ['ascent' => $ascent, 'descent' => $descent]; } /** @@ -257,15 +262,14 @@ public function getElevationByRowCol($tileRefHoriz, $tileRefVert, $row, $col) { * @param float $latitude * @param float $longitude */ - private function getRoundedElevation($latitude, $longitude) { - + private function getRoundedElevation(float $latitude, float $longitude) { // Returns results exactly as per http://www.geonames.org elevation API $row = round(($this->topleftLat - $latitude) / self::DEGREES_PER_TILE * ($this->numDataRows - 1)); $col = round(abs($this->topleftLon - $longitude) / self::DEGREES_PER_TILE * ($this->numDataCols - 1)); // get the elevation for the calculated row & column - return $this->getRowColumnData($row, $col); + return $this->getRowColumnData((int) $row, (int) $col); } /** @@ -276,7 +280,6 @@ private function getRoundedElevation($latitude, $longitude) { * @param float $longitude */ private function getInterpolatedElevation($latitude, $longitude) { - // calculate row & col for the data point p0 (above & left of the parameter point) $row[0] = floor(($this->topleftLat - $latitude) / self::DEGREES_PER_TILE * ($this->numDataRows - 1)); $col[0] = floor(abs($this->topleftLon - $longitude) / self::DEGREES_PER_TILE * ($this->numDataCols - 1)); @@ -329,7 +332,6 @@ private function getInterpolatedElevation($latitude, $longitude) { * @param array $pointData */ private function interpolate($x, $y, $pointData) { - // NB: x & y are expressed as a proportions of the dimension of the square side // p0------------p1 @@ -408,7 +410,7 @@ public static function getTileInfo(float $lat, float $lon): ?array { // NB: gets the values of the top left lat and lon (row 0, col 0) if (($lat > - $MAX_LAT) && ($lat <= $MAX_LAT)) { - $tileRefVert = (fmod($lat, self::DEGREES_PER_TILE) === 0) + $tileRefVert = (fmod($lat, self::DEGREES_PER_TILE) === 0.0) ? (($MAX_LAT - $lat) / self::DEGREES_PER_TILE + 1) : (ceil(($MAX_LAT - $lat) / self::DEGREES_PER_TILE)); } else { @@ -416,7 +418,7 @@ public static function getTileInfo(float $lat, float $lon): ?array { } if (($lon > - 180) && ($lon < 180)) { - $tileRefHoriz = (fmod($lon, self::DEGREES_PER_TILE) === 0) + $tileRefHoriz = (fmod($lon, self::DEGREES_PER_TILE) === 0.0) ? ((180 + $lon) / self::DEGREES_PER_TILE + 1) : (ceil((180 + $lon) / self::DEGREES_PER_TILE)); } else { @@ -439,9 +441,9 @@ public static function getTileInfo(float $lat, float $lon): ?array { private function getTileFileName($tileRefHoriz, $tileRefVert) { $fileName = "srtm_" - . str_pad($tileRefHoriz, 2, "0", STR_PAD_LEFT) + . str_pad((string) $tileRefHoriz, 2, "0", STR_PAD_LEFT) . "_" - . str_pad($tileRefVert, 2, "0", STR_PAD_LEFT) + . str_pad((string) $tileRefVert, 2, "0", STR_PAD_LEFT) . ".tif"; return $fileName; @@ -544,7 +546,7 @@ private function getSRTMFilePointer($fileName) { * @param int $row * @param int $col */ - private function getRowColumnData($row, $col) { + private function getRowColumnData(int $row, int $col) { $LEN_DATA = 2; // the number of bytes containing each item of elevation data // ( = BitsPerSample tag value / 8) @@ -610,10 +612,9 @@ private function getDistance($lat1, $lon1, $lat2, $lon2, $miles = true) { * @param string $error */ private function handleError($method, $message) { - if ($this->showErrors) { ob_start(); - var_dump($this); + // var_dump($this); $dump = ob_get_contents(); ob_end_clean(); die("Died: error in $method: $message 
$dump
"); diff --git a/lib/Service/SrtmGeotiffElevationService.php b/lib/Service/SrtmGeotiffElevationService.php index 8a5070a17..9b88d2ad6 100644 --- a/lib/Service/SrtmGeotiffElevationService.php +++ b/lib/Service/SrtmGeotiffElevationService.php @@ -146,8 +146,8 @@ private function downloadNecessaryFiles(array $coordinates): void { if ($fileInfo === null) { continue; } - $horiz = str_pad($fileInfo['horiz'], 2, '0', STR_PAD_LEFT); - $vert = str_pad($fileInfo['vert'], 2, '0', STR_PAD_LEFT); + $horiz = str_pad((string) $fileInfo['horiz'], 2, '0', STR_PAD_LEFT); + $vert = str_pad((string) $fileInfo['vert'], 2, '0', STR_PAD_LEFT); $fileName = 'srtm_' . $horiz . '_' . $vert . '.tif'; if (!isset($sizesByName[$fileName])) { $this->download($horiz, $vert, $folder, $fileName); @@ -180,7 +180,7 @@ private function download(string $horiz, string $vert, ISimpleFolder $folder, st /** * @param string $horiz * @param string $vert - * @return string[] + * @return array * @throws Exception */ private function request(string $horiz, string $vert): array { diff --git a/lib/Service/ToolsService.php b/lib/Service/ToolsService.php index 4607b8be7..ad477d6e5 100644 --- a/lib/Service/ToolsService.php +++ b/lib/Service/ToolsService.php @@ -174,7 +174,7 @@ public function getProgramPath(string $progname): ?string { $filteredPath = $pathArray; // filter path values with open_basedir $obd = ini_get('open_basedir'); - if ($obd !== null && $obd !== '') { + if ($obd !== false && $obd !== '') { $filteredPath = []; $obdArray = explode(PATH_SEPARATOR, $obd); foreach ($obdArray as $obdElem) { diff --git a/lib/Settings/AdminSection.php b/lib/Settings/AdminSection.php index 5891b2952..64197fc48 100644 --- a/lib/Settings/AdminSection.php +++ b/lib/Settings/AdminSection.php @@ -41,9 +41,9 @@ public function getPriority(): int { } /** - * @return ?string The relative path to an icon describing the section + * @return string The relative path to an icon describing the section */ - public function getIcon(): ?string { + public function getIcon(): string { return $this->urlGenerator->imagePath('gpxpod', 'app_black.svg'); } } diff --git a/psalm.xml b/psalm.xml new file mode 100644 index 000000000..983b3717d --- /dev/null +++ b/psalm.xml @@ -0,0 +1,64 @@ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + diff --git a/tests/psalm-baseline.xml b/tests/psalm-baseline.xml new file mode 100644 index 000000000..5a9a7d829 --- /dev/null +++ b/tests/psalm-baseline.xml @@ -0,0 +1,3 @@ + + + diff --git a/tests/stubs/oc_core_command_base.php b/tests/stubs/oc_core_command_base.php new file mode 100644 index 000000000..bbae1a65a --- /dev/null +++ b/tests/stubs/oc_core_command_base.php @@ -0,0 +1,63 @@ +