diff --git a/.github/workflows/psalm.yml b/.github/workflows/psalm.yml new file mode 100644 index 00000000..ab0b2f0f --- /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 51f5c17e..7a08fa69 100644 --- a/.gitignore +++ b/.gitignore @@ -9,3 +9,5 @@ .vscode-upload.json .*.sw* /node_modules +/composer.lock +.php*.cache diff --git a/.l10nignore b/.l10nignore new file mode 100644 index 00000000..d8fb3c97 --- /dev/null +++ b/.l10nignore @@ -0,0 +1,3 @@ +# compiled vue templates +js/ +vendor/ diff --git a/composer.json b/composer.json index 13a99430..8cfca54f 100644 --- a/composer.json +++ b/composer.json @@ -13,7 +13,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/Activity/ActivityManager.php b/lib/Activity/ActivityManager.php index 0b5be148..99543e3f 100644 --- a/lib/Activity/ActivityManager.php +++ b/lib/Activity/ActivityManager.php @@ -149,12 +149,13 @@ class ActivityManager { case self::SUBJECT_BILL_UPDATE: case self::SUBJECT_BILL_DELETE: $subjectParams = $this->findDetailsForBill($object); + /** @var Bill $object */ $objectName = $object->getWhat(); $eventType = 'cospend_bill_event'; break; case self::SUBJECT_PROJECT_SHARE: case self::SUBJECT_PROJECT_UNSHARE: - $subjectParams = $this->findDetailsForProject($entity->getId()); + $subjectParams = $this->findDetailsForProject((string) $entity->getId()); $objectName = $object->getId(); break; default: @@ -166,13 +167,15 @@ class ActivityManager { $event->setApp('cospend') ->setType($eventType) ->setAuthor($author === null ? $this->userId ?? '' : $author) - ->setObject($objectType, (int)$object->getId(), $objectName) + ->setObject($objectType, (int)$object->getId(), (string) $objectName) ->setSubject($subject, array_merge($subjectParams, $additionalParams)) ->setTimestamp(time()); + /* if ($message !== null) { $event->setMessage($message); } + */ return $event; } diff --git a/lib/Activity/CospendProvider.php b/lib/Activity/CospendProvider.php index d8145700..70359a41 100644 --- a/lib/Activity/CospendProvider.php +++ b/lib/Activity/CospendProvider.php @@ -38,6 +38,8 @@ use OCP\IUserManager; class CospendProvider implements IProvider { + private array $projectNames; + public function __construct( private IURLGenerator $urlGenerator, private ActivityManager $activityManager, diff --git a/lib/Activity/Setting.php b/lib/Activity/Setting.php index a6f16bbf..0a2c0aba 100644 --- a/lib/Activity/Setting.php +++ b/lib/Activity/Setting.php @@ -28,7 +28,7 @@ use OCP\IL10N; class Setting implements ISetting { - public function __construct(private IL10N $l) { + public function __construct(protected IL10N $l) { } /** diff --git a/lib/Command/DeleteBills.php b/lib/Command/DeleteBills.php index 1a543f17..e3e40cbb 100644 --- a/lib/Command/DeleteBills.php +++ b/lib/Command/DeleteBills.php @@ -13,14 +13,14 @@ namespace OCA\Cospend\Command; use DateTime; +use OC\Core\Command\Base; use OCA\Cospend\Db\BillMapper; -use Symfony\Component\Console\Command\Command; use Symfony\Component\Console\Input\InputArgument; use Symfony\Component\Console\Input\InputInterface; use Symfony\Component\Console\Input\InputOption; use Symfony\Component\Console\Output\OutputInterface; -class DeleteBills extends Command { +class DeleteBills extends Base { public function __construct(private BillMapper $billMapper) { parent::__construct(); diff --git a/lib/Command/ExportProject.php b/lib/Command/ExportProject.php index 9e807789..951327b3 100644 --- a/lib/Command/ExportProject.php +++ b/lib/Command/ExportProject.php @@ -12,14 +12,13 @@ namespace OCA\Cospend\Command; +use OC\Core\Command\Base; use OCA\Cospend\Service\ProjectService; -use Symfony\Component\Console\Command\Command; use Symfony\Component\Console\Input\InputArgument; use Symfony\Component\Console\Input\InputInterface; - use Symfony\Component\Console\Output\OutputInterface; -class ExportProject extends Command { +class ExportProject extends Base { public function __construct(private ProjectService $projectService) { parent::__construct(); diff --git a/lib/Command/RepeatBills.php b/lib/Command/RepeatBills.php index d512e200..026f5e21 100644 --- a/lib/Command/RepeatBills.php +++ b/lib/Command/RepeatBills.php @@ -12,13 +12,12 @@ namespace OCA\Cospend\Command; +use OC\Core\Command\Base; use OCA\Cospend\Service\ProjectService; -use Symfony\Component\Console\Command\Command; use Symfony\Component\Console\Input\InputInterface; - use Symfony\Component\Console\Output\OutputInterface; -class RepeatBills extends Command { +class RepeatBills extends Base { public function __construct(private ProjectService $projectService) { parent::__construct(); diff --git a/lib/Controller/ApiController.php b/lib/Controller/ApiController.php index 92dc0425..134329ce 100644 --- a/lib/Controller/ApiController.php +++ b/lib/Controller/ApiController.php @@ -802,10 +802,7 @@ class ApiController extends OCSController { #[CospendUserPermissions(minimumLevel: Application::ACCESS_LEVEL_MAINTAINER)] public function createPaymentMode(string $projectId, string $name, ?string $icon, string $color, ?int $order = 0): DataResponse { $result = $this->projectService->createPaymentMode($projectId, $name, $icon, $color, $order); - if (is_numeric($result)) { - return new DataResponse($result); - } - return new DataResponse($result, Http::STATUS_BAD_REQUEST); + return new DataResponse($result); } /** @@ -825,7 +822,7 @@ class ApiController extends OCSController { string $projectId, int $pmId, ?string $name = null, ?string $icon = null, ?string $color = null ): DataResponse { $result = $this->projectService->editPaymentMode($projectId, $pmId, $name, $icon, $color); - if (is_array($result)) { + if (isset($result['name'])) { return new DataResponse($result); } return new DataResponse($result, Http::STATUS_BAD_REQUEST); @@ -882,10 +879,7 @@ class ApiController extends OCSController { #[CospendUserPermissions(minimumLevel: Application::ACCESS_LEVEL_MAINTAINER)] public function createCategory(string $projectId, string $name, ?string $icon, string $color, ?int $order = 0): DataResponse { $result = $this->projectService->createCategory($projectId, $name, $icon, $color, $order); - if (is_numeric($result)) { - return new DataResponse($result); - } - return new DataResponse($result, Http::STATUS_BAD_REQUEST); + return new DataResponse($result); } /** @@ -906,7 +900,7 @@ class ApiController extends OCSController { string $projectId, int $categoryId, ?string $name = null, ?string $icon = null, ?string $color = null ): DataResponse { $result = $this->projectService->editCategory($projectId, $categoryId, $name, $icon, $color); - if (is_array($result)) { + if (isset($result['name'])) { return new DataResponse($result); } return new DataResponse($result, Http::STATUS_BAD_REQUEST); @@ -964,10 +958,7 @@ class ApiController extends OCSController { #[CospendUserPermissions(minimumLevel: Application::ACCESS_LEVEL_MAINTAINER)] public function createCurrency(string $projectId, string $name, float $rate): DataResponse { $result = $this->projectService->createCurrency($projectId, $name, $rate); - if (is_numeric($result)) { - return new DataResponse($result); - } - return new DataResponse($result, Http::STATUS_BAD_REQUEST); + return new DataResponse($result); } /** @@ -1075,10 +1066,7 @@ class ApiController extends OCSController { #[CospendUserPermissions(minimumLevel: Application::ACCESS_LEVEL_PARTICIPANT)] public function createPublicShare(string $projectId): DataResponse { $result = $this->projectService->createPublicShare($projectId); - if (is_array($result)) { - return new DataResponse($result); - } - return new DataResponse($result, Http::STATUS_BAD_REQUEST); + return new DataResponse($result); } /** diff --git a/lib/Controller/OldApiController.php b/lib/Controller/OldApiController.php index 05d50ffe..50e5e782 100644 --- a/lib/Controller/OldApiController.php +++ b/lib/Controller/OldApiController.php @@ -845,10 +845,7 @@ class OldApiController extends ApiController { $result = $this->projectService->createPaymentMode( $publicShareInfo['projectid'], $name, $icon, $color, $order ); - if (is_numeric($result)) { - return new DataResponse($result); - } - return new DataResponse($result, Http::STATUS_BAD_REQUEST); + return new DataResponse($result); } #[NoAdminRequired] @@ -857,10 +854,7 @@ class OldApiController extends ApiController { #[CospendUserPermissions(minimumLevel: Application::ACCESS_LEVEL_MAINTAINER)] public function apiPrivAddPaymentMode(string $projectId, string $name, ?string $icon = null, ?string $color = null): DataResponse { $result = $this->projectService->createPaymentMode($projectId, $name, $icon, $color); - if (is_numeric($result)) { - return new DataResponse($result); - } - return new DataResponse($result, Http::STATUS_BAD_REQUEST); + return new DataResponse($result); } #[NoAdminRequired] @@ -875,7 +869,7 @@ class OldApiController extends ApiController { $result = $this->projectService->editPaymentMode( $publicShareInfo['projectid'], $pmid, $name, $icon, $color ); - if (is_array($result)) { + if (isset($result['name'])) { return new DataResponse($result); } return new DataResponse($result, Http::STATUS_FORBIDDEN); @@ -902,7 +896,7 @@ class OldApiController extends ApiController { public function apiPrivEditPaymentMode(string $projectId, int $pmid, ?string $name = null, ?string $icon = null, ?string $color = null): DataResponse { $result = $this->projectService->editPaymentMode($projectId, $pmid, $name, $icon, $color); - if (is_array($result)) { + if (isset($result['name'])) { return new DataResponse($result); } return new DataResponse($result, Http::STATUS_FORBIDDEN); @@ -946,11 +940,7 @@ class OldApiController extends ApiController { $result = $this->projectService->createCategory( $publicShareInfo['projectid'], $name, $icon, $color, $order ); - if (is_numeric($result)) { - // inserted category id - return new DataResponse($result); - } - return new DataResponse($result, Http::STATUS_BAD_REQUEST); + return new DataResponse($result); } #[NoAdminRequired] @@ -959,11 +949,7 @@ class OldApiController extends ApiController { #[CospendUserPermissions(minimumLevel: Application::ACCESS_LEVEL_MAINTAINER)] public function apiPrivAddCategory(string $projectId, string $name, ?string $icon = null, ?string $color = null): DataResponse { $result = $this->projectService->createCategory($projectId, $name, $icon, $color); - if (is_numeric($result)) { - // inserted category id - return new DataResponse($result); - } - return new DataResponse($result, Http::STATUS_BAD_REQUEST); + return new DataResponse($result); } #[NoAdminRequired] @@ -978,7 +964,7 @@ class OldApiController extends ApiController { $result = $this->projectService->editCategory( $publicShareInfo['projectid'], $categoryid, $name, $icon, $color ); - if (is_array($result)) { + if (isset($result['name'])) { return new DataResponse($result); } return new DataResponse($result, Http::STATUS_FORBIDDEN); @@ -1005,7 +991,7 @@ class OldApiController extends ApiController { public function apiPrivEditCategory(string $projectId, int $categoryid, ?string $name = null, ?string $icon = null, ?string $color = null): DataResponse { $result = $this->projectService->editCategory($projectId, $categoryid, $name, $icon, $color); - if (is_array($result)) { + if (isset($result['name'])) { return new DataResponse($result); } return new DataResponse($result, Http::STATUS_FORBIDDEN); @@ -1047,11 +1033,7 @@ class OldApiController extends ApiController { public function apiAddCurrency(string $token, string $name, float $rate): DataResponse { $publicShareInfo = $this->projectService->getProjectInfoFromShareToken($token); $result = $this->projectService->createCurrency($publicShareInfo['projectid'], $name, $rate); - if (is_numeric($result)) { - // inserted currency id - return new DataResponse($result); - } - return new DataResponse($result, Http::STATUS_BAD_REQUEST); + return new DataResponse($result); } #[NoAdminRequired] @@ -1060,11 +1042,7 @@ class OldApiController extends ApiController { #[CospendUserPermissions(minimumLevel: Application::ACCESS_LEVEL_MAINTAINER)] public function apiPrivAddCurrency(string $projectId, string $name, float $rate): DataResponse { $result = $this->projectService->createCurrency($projectId, $name, $rate); - if (is_numeric($result)) { - // inserted bill id - return new DataResponse($result); - } - return new DataResponse($result, Http::STATUS_BAD_REQUEST); + return new DataResponse($result); } #[NoAdminRequired] diff --git a/lib/Controller/PageController.php b/lib/Controller/PageController.php index f48e95fc..58aeb675 100644 --- a/lib/Controller/PageController.php +++ b/lib/Controller/PageController.php @@ -103,7 +103,7 @@ class PageController extends Controller { $svg = file_get_contents($path); - if ($svg === null) { + if ($svg === false) { return new NotFoundResponse(); } @@ -161,7 +161,7 @@ class PageController extends Controller { /** * @param string $token - * @return PublicTemplateResponse + * @return TemplateResponse */ #[NoAdminRequired] #[NoCSRFRequired] @@ -204,7 +204,7 @@ class PageController extends Controller { /** * @param string $token * @param string|null $password - * @return PublicTemplateResponse + * @return TemplateResponse */ #[NoAdminRequired] #[NoCSRFRequired] diff --git a/lib/Controller/PublicApiController.php b/lib/Controller/PublicApiController.php index 7d85417a..05e5709a 100644 --- a/lib/Controller/PublicApiController.php +++ b/lib/Controller/PublicApiController.php @@ -718,11 +718,7 @@ class PublicApiController extends OCSController { $result = $this->projectService->createPaymentMode( $publicShareInfo['projectid'], $name, $icon, $color, $order ); - if (is_numeric($result)) { - return new DataResponse($result); - } else { - return new DataResponse($result, Http::STATUS_BAD_REQUEST); - } + return new DataResponse($result); } /** @@ -747,7 +743,7 @@ class PublicApiController extends OCSController { $result = $this->projectService->editPaymentMode( $publicShareInfo['projectid'], $pmId, $name, $icon, $color ); - if (is_array($result)) { + if (isset($result['name'])) { return new DataResponse($result); } else { return new DataResponse($result, Http::STATUS_FORBIDDEN); @@ -818,12 +814,7 @@ class PublicApiController extends OCSController { $result = $this->projectService->createCategory( $publicShareInfo['projectid'], $name, $icon, $color, $order ); - if (is_numeric($result)) { - // inserted category id - return new DataResponse($result); - } else { - return new DataResponse($result, Http::STATUS_BAD_REQUEST); - } + return new DataResponse($result); } /** @@ -850,7 +841,7 @@ class PublicApiController extends OCSController { $result = $this->projectService->editCategory( $publicShareInfo['projectid'], $categoryId, $name, $icon, $color ); - if (is_array($result)) { + if (isset($result['name'])) { return new DataResponse($result); } else { return new DataResponse($result, Http::STATUS_FORBIDDEN); @@ -919,12 +910,7 @@ class PublicApiController extends OCSController { public function publicCreateCurrency(string $token, string $name, float $rate): DataResponse { $publicShareInfo = $this->projectService->getProjectInfoFromShareToken($token); $result = $this->projectService->createCurrency($publicShareInfo['projectid'], $name, $rate); - if (is_numeric($result)) { - // inserted currency id - return new DataResponse($result); - } else { - return new DataResponse($result, Http::STATUS_BAD_REQUEST); - } + return new DataResponse($result); } /** diff --git a/lib/Db/BillMapper.php b/lib/Db/BillMapper.php index 6b7786d6..e1724aa8 100644 --- a/lib/Db/BillMapper.php +++ b/lib/Db/BillMapper.php @@ -19,6 +19,9 @@ use OCP\AppFramework\Db\QBMapper; use OCP\DB\QueryBuilder\IQueryBuilder; use OCP\IDBConnection; +/** + * @extends QBMapper + */ class BillMapper extends QBMapper { public const TABLE_NAME = 'cospend_bills'; @@ -453,7 +456,7 @@ class BillMapper extends QBMapper { $dbWhat = $row['what']; $dbComment = $row['comment']; $dbTimestamp = (int) $row['timestamp']; - $dbDate = DateTime::createFromFormat('U', $dbTimestamp); + $dbDate = DateTime::createFromFormat('U', $row['timestamp']); $dbRepeat = $row['repeat']; $dbPayerId = (int) $row['payerid']; $dbPaymentMode = $row['paymentmode']; @@ -695,7 +698,7 @@ class BillMapper extends QBMapper { $dbWhat = $row['what']; $dbComment = $row['comment']; $dbTimestamp = (int) $row['timestamp']; - $dbDate = DateTime::createFromFormat('U', $dbTimestamp); + $dbDate = DateTime::createFromFormat('U', $row['timestamp']); $dbRepeat = $row['repeat']; $dbPayerId = (int) $row['payerid']; $dbPaymentMode = $row['paymentmode']; diff --git a/lib/Db/ProjectMapper.php b/lib/Db/ProjectMapper.php index 6c2abe53..f2500b21 100644 --- a/lib/Db/ProjectMapper.php +++ b/lib/Db/ProjectMapper.php @@ -19,6 +19,9 @@ use OCP\DB\QueryBuilder\IQueryBuilder; use OCP\IDBConnection; use OCP\IL10N; +/** + * @extends QBMapper + */ class ProjectMapper extends QBMapper { public const TABLENAME = 'cospend_projects'; @@ -136,7 +139,9 @@ class ProjectMapper extends QBMapper { throw new Exception('Project ' . $id . ' not found'); } - return $this->mapRowToEntity($row); + /** @var Project $project */ + $project = $this->mapRowToEntity($row); + return $project; } /** diff --git a/lib/Middleware/UserPermissionMiddleware.php b/lib/Middleware/UserPermissionMiddleware.php index 9c3d0429..5295bead 100644 --- a/lib/Middleware/UserPermissionMiddleware.php +++ b/lib/Middleware/UserPermissionMiddleware.php @@ -6,6 +6,8 @@ namespace OCA\Cospend\Middleware; use Exception; use OCA\Cospend\Attribute\CospendUserPermissions; +use OCA\Cospend\Controller\ApiController; +use OCA\Cospend\Controller\OldApiController; use OCA\Cospend\Exception\CospendUserPermissionsException; use OCA\Cospend\Service\ProjectService; use OCP\AppFramework\Controller; @@ -28,6 +30,9 @@ class UserPermissionMiddleware extends Middleware { } public function beforeController($controller, $methodName): void { + if (!$controller instanceof ApiController && !$controller instanceof OldApiController) { + return; + } $reflectionMethod = new ReflectionMethod($controller, $methodName); $attributes = $reflectionMethod->getAttributes(CospendUserPermissions::class); diff --git a/lib/Migration/Version010314Date20210828143421.php b/lib/Migration/Version010314Date20210828143421.php index 688fea34..1fd9224d 100644 --- a/lib/Migration/Version010314Date20210828143421.php +++ b/lib/Migration/Version010314Date20210828143421.php @@ -20,6 +20,7 @@ class Version010314Date20210828143421 extends SimpleMigrationStep { /** @var IDBConnection */ private $connection; + private IL10N $trans; /** * @param IDBConnection $connection diff --git a/lib/Service/ProjectService.php b/lib/Service/ProjectService.php index f3ae0bb1..c44b6572 100644 --- a/lib/Service/ProjectService.php +++ b/lib/Service/ProjectService.php @@ -30,7 +30,7 @@ use OCA\Cospend\Utils; use OCP\App\IAppManager; use OCP\DB\QueryBuilder\IQueryBuilder; -use OCP\Files\FileInfo; +use OCP\Files\File; use OCP\Files\Folder; use OCP\Files\IRootFolder; use OCP\Files\NotFoundException; @@ -541,7 +541,7 @@ class ProjectService { * @param int|null $categoryId * @param float|null $amountMin * @param float|null $amountMax - * @param bool|null $showDisabled + * @param bool $showDisabled * @param int|null $currencyId * @param int|null $payerId * @return array @@ -996,7 +996,7 @@ class ProjectService { $repeatuntil = null; } // priority to timestamp (moneybuster might send both for a moment) - if ($timestamp === null || !is_numeric($timestamp)) { + if ($timestamp === null) { if ($date === null || $date === '') { return ['message' => $this->l10n->t('Timestamp (or date) field is required')]; } else { @@ -1007,7 +1007,7 @@ class ProjectService { $dateTs = $datetime->getTimestamp(); } } else { - $dateTs = (int) $timestamp; + $dateTs = $timestamp; } if ($what === null) { $what = ''; @@ -1023,14 +1023,14 @@ class ProjectService { } // check owers $owerIds = explode(',', $payed_for); - if ($payed_for === null || $payed_for === '' || count($owerIds) === 0) { + if ($payed_for === null || $payed_for === '' || empty($owerIds)) { return ['payed_for' => $this->l10n->t('Invalid value')]; } foreach ($owerIds as $owerId) { if (!is_numeric($owerId)) { return ['payed_for' => $this->l10n->t('Invalid value')]; } - if ($this->getMemberById($projectId, $owerId) === null) { + if ($this->getMemberById($projectId, (int) $owerId) === null) { return ['payed_for' => $this->l10n->t('Not a valid choice')]; } } @@ -1538,7 +1538,11 @@ class ProjectService { $qb->executeStatement(); $qb->resetQueryParts(); - return $this->getMemberById($projectId, $memberId); + $member = $this->getMemberById($projectId, $memberId); + if ($member === null) { + return ['error' => $this->l10n->t('Impossible to get the edited member')]; + } + return $member; } else { return ['name' => $this->l10n->t('This project have no such member')]; } @@ -1619,7 +1623,7 @@ class ProjectService { string $projectId, string $name, ?float $weight = 1.0, bool $active = true, ?string $color = null, ?string $userId = null ): array { - if ($name !== null && $name !== '') { + if ($name !== '') { if ($this->getMemberByName($projectId, $name) === null && $this->getMemberByUserid($projectId, $userId) === null) { if (strpos($name, '/') !== false) { return ['error' => $this->l10n->t('Invalid member name')]; @@ -1662,7 +1666,11 @@ class ProjectService { $qb->executeStatement(); $qb->resetQueryParts(); - return $this->getMemberByName($projectId, $name); + $member = $this->getMemberByName($projectId, $name); + if ($member === null) { + return ['error' => $this->l10n->t('Impossible to get the created member')]; + } + return $member; } else { return ['error' => $this->l10n->t('This project already has this member')]; } @@ -2155,7 +2163,7 @@ class ProjectService { // fallback order is more than max order $elements[$cid]['order'] = $mostUsedOrder[$cid] ?? $order; } - } elseif ($sortMethod === Application::SORT_ORDER_RECENTLY_USED) { + } else { // sort by most recently used $mostUsedOrder = []; $qb->select($alias . '.id') @@ -2778,14 +2786,14 @@ class ProjectService { // check owers if ($payed_for !== null && $payed_for !== '') { $owerIds = explode(',', $payed_for); - if (count($owerIds) === 0) { + if (empty($owerIds)) { return ['payed_for' => $this->l10n->t('Invalid value')]; } else { foreach ($owerIds as $owerId) { if (!is_numeric($owerId)) { return ['payed_for' => $this->l10n->t('Invalid value')]; } - if ($this->getMemberById($projectId, $owerId) === null) { + if ($this->getMemberById($projectId, (int) $owerId) === null) { return ['payed_for' => $this->l10n->t('Not a valid choice')]; } } @@ -3381,7 +3389,11 @@ class ProjectService { $qb->executeStatement(); $qb->resetQueryParts(); - return $this->getPaymentMode($projectId, $pmId); + $pm = $this->getPaymentMode($projectId, $pmId); + if ($pm === null) { + return ['message' => $this->l10n->t('Impossible to get the edited payment mode')]; + } + return $pm; } else { return ['message' => $this->l10n->t('This project has no such payment mode')]; } @@ -3561,7 +3573,11 @@ class ProjectService { $qb->executeStatement(); $qb->resetQueryParts(); - return $this->getCategory($projectId, $categoryId); + $category = $this->getCategory($projectId, $categoryId); + if ($category === null) { + return ['message' => $this->l10n->t('Impossible to get the edited category')]; + } + return $category; } else { return ['message' => $this->l10n->t('This project has no such category')]; } @@ -3687,7 +3703,11 @@ class ProjectService { $qb->executeStatement(); $qb->resetQueryParts(); - return $this->getCurrency($projectId, $currencyId); + $currency = $this->getCurrency($projectId, $currencyId); + if ($currency === null) { + return ['message' => $this->l10n->t('Impossible to get the edited currency')]; + } + return $currency; } else { return ['message' => $this->l10n->t('This project have no such currency')]; } @@ -4448,6 +4468,9 @@ class ProjectService { return ['message' => $msg]; } $folder = $userFolder->get($outPath); + if (!$folder instanceof Folder) { + return ['message' => $outPath . ' is not a directory']; + } // create file if ($folder->nodeExists($projectId.'-settlement.csv')) { @@ -4501,7 +4524,7 @@ class ProjectService { } if ($userFolder->nodeExists($outPath)) { $folder = $userFolder->get($outPath); - if ($folder->getType() !== FileInfo::TYPE_FOLDER) { + if (!$folder instanceof Folder) { return $this->l10n->t('%1$s is not a folder', [$outPath]); } elseif (!$folder->isCreatable()) { return $this->l10n->t('%1$s is not writeable', [$outPath]); @@ -4544,6 +4567,9 @@ class ProjectService { return ['message' => $msg]; } $folder = $userFolder->get($outPath); + if (!$folder instanceof Folder) { + return ['message' => $outPath . ' is not a directory']; + } // create file if ($folder->nodeExists($projectId.'-stats.csv')) { @@ -4601,7 +4627,9 @@ class ProjectService { return ['message' => $msg]; } $folder = $userFolder->get($outPath); - + if (!$folder instanceof Folder) { + return ['message' => $outPath . ' is not a directory']; + } // create file $filename = $projectId.'.csv'; @@ -4775,7 +4803,7 @@ class ProjectService { $userFolder = $this->root->getUserFolder($userId); if ($userFolder->nodeExists($cleanPath)) { $file = $userFolder->get($cleanPath); - if ($file->getType() === FileInfo::TYPE_FILE) { + if ($file instanceof File) { if (($handle = $file->fopen('r')) !== false) { $projectName = preg_replace('/\.csv$/', '', $file->getName()); return $this->importCsvProjectAtomicWrapper($handle, $userId, $projectName); @@ -4868,8 +4896,12 @@ class ProjectService { $icon = null; } $color = $data[$columns['color']]; - $categoryid = $data[$columns['categoryid']]; $categoryname = $data[$columns['categoryname']]; + if (!is_numeric($data[$columns['categoryid']])) { + fclose($handle); + return ['message' => $this->l10n->t('Error when adding category %1$s', [$categoryname])]; + } + $categoryid = (int) $data[$columns['categoryid']]; $categories[] = [ 'icon' => $icon, 'color' => $color, @@ -4882,9 +4914,13 @@ class ProjectService { } else { $icon = null; } - $color = $data[$columns['color']]; - $paymentmodeid = $data[$columns['paymentmodeid']]; $paymentmodename = $data[$columns['paymentmodename']]; + if (!is_numeric($data[$columns['paymentmodeid']])) { + fclose($handle); + return ['message' => $this->l10n->t('Error when adding payment mode %1$s', [$paymentmodename])]; + } + $color = $data[$columns['color']]; + $paymentmodeid = (int) $data[$columns['paymentmodeid']]; $paymentModes[] = [ 'icon' => $icon, 'color' => $color, @@ -4893,8 +4929,12 @@ class ProjectService { ]; } elseif ($currentSection === 'currencies') { $name = $data[$columns['currencyname']]; - $exchange_rate = $data[$columns['exchange_rate']]; - if (((float) $exchange_rate) === 1.0) { + if (!is_numeric($data[$columns['exchange_rate']])) { + fclose($handle); + return ['message' => $this->l10n->t('Error when adding currency %1$s', [$name])]; + } + $exchange_rate = (float) $data[$columns['exchange_rate']]; + if (($exchange_rate) === 1.0) { $mainCurrencyName = $name; } else { $currencies[] = [ @@ -4904,17 +4944,19 @@ class ProjectService { } } elseif ($currentSection === 'members') { $name = trim($data[$columns['name']]); - $weight = $data[$columns['weight']]; - $active = $data[$columns['active']]; + if (!is_numeric($data[$columns['weight']]) || !is_numeric($data[$columns['active']])) { + fclose($handle); + return ['message' => $this->l10n->t('Error when adding member %1$s', [$name])]; + } + $weight = (float) $data[$columns['weight']]; + $active = (int) $data[$columns['active']]; $color = $data[$columns['color']]; if (strlen($name) > 0 - && is_numeric($weight) - && is_numeric($active) && preg_match('/^#[0-9A-Fa-f]+$/', $color) !== false ) { $membersByName[$name] = [ 'weight' => $weight, - 'active' => (int) $active !== 0, + 'active' => $active !== 0, 'color' => $color, ]; } else { @@ -4923,11 +4965,15 @@ class ProjectService { } } elseif ($currentSection === 'bills') { $what = $data[$columns['what']]; - $amount = $data[$columns['amount']]; + if (!is_numeric($data[$columns['amount']])) { + fclose($handle); + return ['message' => $this->l10n->t('Malformed CSV, invalid amount on line %1$s', [$row + 1])]; + } + $amount = (float) $data[$columns['amount']]; $timestamp = null; // priority to timestamp if (array_key_exists('timestamp', $columns)) { - $timestamp = $data[$columns['timestamp']]; + $timestamp = (int) $data[$columns['timestamp']]; } elseif (array_key_exists('date', $columns)) { $date = $data[$columns['date']]; $datetime = DateTime::createFromFormat('Y-m-d', $date); @@ -4946,12 +4992,12 @@ class ProjectService { $repeat = array_key_exists('repeat', $columns) ? $data[$columns['repeat']] : Application::FREQUENCY_NO; $categoryid = array_key_exists('categoryid', $columns) ? (int) $data[$columns['categoryid']] : null; $paymentmode = array_key_exists('paymentmode', $columns) ? $data[$columns['paymentmode']] : null; - $paymentmodeid = array_key_exists('paymentmodeid', $columns) ? $data[$columns['paymentmodeid']] : null; - $repeatallactive = array_key_exists('repeatallactive', $columns) ? $data[$columns['repeatallactive']] : 0; + $paymentmodeid = array_key_exists('paymentmodeid', $columns) ? (int) $data[$columns['paymentmodeid']] : null; + $repeatallactive = array_key_exists('repeatallactive', $columns) ? (int) $data[$columns['repeatallactive']] : 0; $repeatuntil = array_key_exists('repeatuntil', $columns) ? $data[$columns['repeatuntil']] : null; - $repeatfreq = array_key_exists('repeatfreq', $columns) ? $data[$columns['repeatfreq']] : 1; + $repeatfreq = array_key_exists('repeatfreq', $columns) ? (int) $data[$columns['repeatfreq']] : 1; $comment = array_key_exists('comment', $columns) ? urldecode($data[$columns['comment']] ?? '') : null; - $deleted = array_key_exists('deleted', $columns) ? $data[$columns['deleted']] : 0; + $deleted = array_key_exists('deleted', $columns) ? (int) $data[$columns['deleted']] : 0; // manage members if (!isset($membersByName[$payer_name])) { @@ -4985,10 +5031,6 @@ class ProjectService { $membersByName[$strippedOwer]['color'] = null; } } - if (!is_numeric($amount)) { - fclose($handle); - return ['message' => $this->l10n->t('Malformed CSV, invalid amount on line %1$s', [$row + 1])]; - } $bills[] = [ 'what' => $what, 'comment' => $comment, @@ -5034,38 +5076,22 @@ class ProjectService { // add payment modes foreach ($paymentModes as $pm) { $insertedPmId = $this->createPaymentMode($projectid, $pm['name'], $pm['icon'], $pm['color']); - if (!is_numeric($insertedPmId)) { - $this->deleteProject($projectid); - return ['message' => $this->l10n->t('Error when adding payment mode %1$s', [$pm['name']])]; - } $paymentModeIdConv[$pm['id']] = $insertedPmId; } // add categories foreach ($categories as $cat) { $insertedCatId = $this->createCategory($projectid, $cat['name'], $cat['icon'], $cat['color']); - if (!is_numeric($insertedCatId)) { - $this->deleteProject($projectid); - return ['message' => $this->l10n->t('Error when adding category %1$s', [$cat['name']])]; - } $categoryIdConv[$cat['id']] = $insertedCatId; } // add currencies foreach ($currencies as $cur) { $insertedCurId = $this->createCurrency($projectid, $cur['name'], $cur['exchange_rate']); - if (!is_numeric($insertedCurId)) { - $this->deleteProject($projectid); - return ['message' => $this->l10n->t('Error when adding currency %1$s', [$cur['name']])]; - } } // add members foreach ($membersByName as $memberName => $member) { $insertedMember = $this->createMember( $projectid, $memberName, $member['weight'], $member['active'], $member['color'] ?? null ); - if (!is_array($insertedMember)) { - $this->deleteProject($projectid); - return ['message' => $this->l10n->t('Error when adding member %1$s', [$memberName])]; - } $memberNameToId[$memberName] = $insertedMember['id']; } $dbPaymentModes = $this->getCategoriesOrPaymentModes($projectid, false); @@ -5073,12 +5099,12 @@ class ProjectService { foreach ($bills as $bill) { // manage category id if this is a custom category $catId = $bill['categoryid']; - if (is_numeric($catId) && (int) $catId > 0) { + if ($catId !== null && $catId > 0) { $catId = $categoryIdConv[$catId]; } // manage payment mode id if this is a custom payment mode $pmId = $bill['paymentmodeid']; - if (is_numeric($pmId) && (int) $pmId > 0) { + if ($pmId !== null && $pmId > 0) { $pmId = $paymentModeIdConv[$pmId]; } $payerId = $memberNameToId[$bill['payer_name']]; @@ -5094,7 +5120,7 @@ class ProjectService { $bill['paymentmode'], $pmId, $catId, $bill['repeatallactive'], $bill['repeatuntil'], $bill['timestamp'], $bill['comment'], $bill['repeatfreq'], - $dbPaymentModes, $bill['deleted'] + $dbPaymentModes, $bill['deleted'] ?? 0 ); if (!isset($addBillResult['inserted_id'])) { $this->deleteProject($projectid); @@ -5120,7 +5146,7 @@ class ProjectService { $userFolder = $this->root->getUserFolder($userId); if ($userFolder->nodeExists($cleanPath)) { $file = $userFolder->get($cleanPath); - if ($file->getType() === FileInfo::TYPE_FILE) { + if ($file instanceof File) { if (($handle = $file->fopen('r')) !== false) { $columns = []; $membersWeight = []; @@ -5200,11 +5226,11 @@ class ProjectService { // get those with a negative value, they will be the owers in generated bills $negativeCols = []; for ($c = 5; $c < $nbCol; $c++) { - $amount = $data[$c]; - if (!is_numeric($amount)) { + if (!is_numeric($data[$c])) { fclose($handle); return ['message' => $this->l10n->t('Malformed CSV, bad amount on line %1$s', [$row])]; } + $amount = (float) $data[$c]; if ($amount < 0) { $negativeCols[] = $c; } @@ -5214,7 +5240,7 @@ class ProjectService { }, $negativeCols); // each positive one: bill with member-specific amount (not the full amount), owers are the negative ones for ($c = 5; $c < $nbCol; $c++) { - $amount = $data[$c]; + $amount = (float) $data[$c]; if ($amount > 0) { $payer_name = $owersArray[$c - 5]; if (empty($payer_name)) { @@ -5263,19 +5289,23 @@ class ProjectService { $catNameToId = []; foreach ($categoryNames as $categoryName) { $insertedCatId = $this->createCategory($projectid, $categoryName, null, '#000000'); + /* if (!is_numeric($insertedCatId)) { $this->deleteProject($projectid); return ['message' => $this->l10n->t('Error when adding category %1$s', [$categoryName])]; } + */ $catNameToId[$categoryName] = $insertedCatId; } // add members foreach ($membersWeight as $memberName => $weight) { $insertedMember = $this->createMember($projectid, $memberName, $weight); + /* if (!is_array($insertedMember)) { $this->deleteProject($projectid); return ['message' => $this->l10n->t('Error when adding member %1$s', [$memberName])]; } + */ $memberNameToId[$memberName] = $insertedMember['id']; } // add bills diff --git a/psalm-baseline.xml b/psalm-baseline.xml deleted file mode 100644 index c768a071..00000000 --- a/psalm-baseline.xml +++ /dev/null @@ -1,26 +0,0 @@ - - - - - $this->getContainer() - - - addServiceListener - addServiceListener - addServiceListener - addServiceListener - addServiceListener - addServiceListener - addServiceListener - addServiceListener - addServiceListener - addServiceListener - - - Principal::class - - - Principal - - - diff --git a/psalm.xml b/psalm.xml index 76912589..3b348392 100644 --- a/psalm.xml +++ b/psalm.xml @@ -1,48 +1,60 @@ - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + diff --git a/tests/psalm-baseline.xml b/tests/psalm-baseline.xml new file mode 100644 index 00000000..84dda457 --- /dev/null +++ b/tests/psalm-baseline.xml @@ -0,0 +1,8 @@ + + + + + (int) $strId + + + diff --git a/tests/stubs/oc_core_command_base.php b/tests/stubs/oc_core_command_base.php new file mode 100644 index 00000000..bbae1a65 --- /dev/null +++ b/tests/stubs/oc_core_command_base.php @@ -0,0 +1,63 @@ +