Merge pull request #1330 from nextcloud/fix/add-psalm

fix: Add psalm configuration, fix some of the errors, add stubs, add baseline

.github/workflows/psalm.yml

@@ -0,0 +1,41 @@
+# 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: Static analysis
+
+on: pull_request
+
+concurrency:
+  group: psalm-${{ github.head_ref || github.run_id }}
+  cancel-in-progress: true
+
+jobs:
+  static-analysis:
+    runs-on: ubuntu-latest
+
+    name: static-psalm-analysis
+    steps:
+      - name: Checkout
+        uses: actions/checkout@b4ffde65f46336ab88eb53be808477a3936bae11 # v4.1.1
+
+      - name: Get php version
+        id: versions
+        uses: icewind1991/nextcloud-version-matrix@7d433286e92318f51ed0537b6c77374759e12f46 # v1.3.0
+
+      - name: Set up php${{ steps.versions.outputs.php-available }}
+        uses: shivammathur/setup-php@6d7209f44a25a59e904b1ee9f3b0c33ab2cd888d # v2
+        with:
+          php-version: ${{ steps.versions.outputs.php-available }}
+          extensions: bz2, ctype, curl, dom, fileinfo, gd, iconv, intl, json, libxml, mbstring, openssl, pcntl, posix, session, simplexml, xmlreader, xmlwriter, zip, zlib, sqlite, pdo_sqlite
+          coverage: none
+          ini-file: development
+        env:
+          GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
+
+      - name: Install dependencies
+        run: composer i
+
+      - name: Run coding standards check
+        run: composer run psalm

.php-cs-fixer.dist.php

@@ -10,9 +10,10 @@
 $config
 	->getFinder()
 	->ignoreVCSIgnored(true)
-	->notPath('build')
+	->notPath('tests/stubs')
 	->notPath('l10n')
 	->notPath('src')
 	->notPath('vendor')
+	->notPath('vendor-bin')
 	->in(__DIR__);
 return $config;

composer.json

@@ -10,7 +10,11 @@
 	"scripts": {
 		"cs:fix": "php-cs-fixer fix",
 		"cs:check": "php-cs-fixer fix --dry-run --diff",
-		"lint": "find . -name \\*.php -not -path './vendor/*' -print0 | xargs -0 -n1 php -l",
+		"lint": "find . -name \\*.php -not -path './vendor/*' -not -path './vendor-bin/*' -not -path './tests/stubs/*' -print0 | xargs -0 -n1 php -l",
+		"psalm": "psalm.phar --threads=1",
+		"psalm:update-baseline": "psalm.phar --threads=1 --update-baseline",
+		"psalm:clear": "psalm.phar --clear-cache && psalm.phar --clear-global-cache",
+		"psalm:fix": "psalm.phar --alter --issues=InvalidReturnType,InvalidNullableReturnType,MissingParamType,InvalidFalsableReturnType",
 		"test:integration": "phpunit -c tests/phpunit.integration.xml --fail-on-warning",
 		"test:integration:dev": "phpunit -c tests/phpunit.integration.xml --no-coverage --order-by=defects --stop-on-defect --fail-on-warning --stop-on-error --stop-on-failure",
 		"test:unit": "phpunit -c tests/phpunit.unit.xml --fail-on-warning",

lib/AppInfo/Application.php

@@ -74,9 +74,8 @@ public function boot(IBootContext $context): void {
 		$context->getAppContainer()->registerService('FileHooks', function ($c) {
 			return new FileHooks(
 				$c->query(IServerContainer::class)->getRootFolder(),
-				\OC::$server->query(PhotofilesService::class),
-				\OC::$server->query(TracksService::class),
-				$c->query(IServerContainer::class)->getLogger(),
+				\OCP\Server::get(PhotofilesService::class),
+				\OCP\Server::get(TracksService::class),
 				$c->query('AppName'),
 				$c->query(IServerContainer::class)->getLockingProvider()
 			);

lib/BackgroundJob/AddPhotoJob.php

@@ -42,27 +42,29 @@ class AddPhotoJob extends QueuedJob {
 	 * @param ITimeFactory $timeFactory
 	 * @param PhotofilesService $photofilesService
 	 */
-	public function __construct(ITimeFactory $timeFactory,
+	public function __construct(
+		ITimeFactory $timeFactory,
 		IRootFolder $root,
 		PhotofilesService $photofilesService,
-		ICacheFactory $cacheFactory) {
+		ICacheFactory $cacheFactory,
+	) {
 		parent::__construct($timeFactory);
 		$this->photofilesService = $photofilesService;
 		$this->root = $root;
 		$this->cacheFactory = $cacheFactory;
 		$this->backgroundJobCache = $this->cacheFactory->createDistributed('maps:background-jobs');
 	}
 
-	public function run($arguments) {
-		$userFolder = $this->root->getUserFolder($arguments['userId']);
-		$files = $userFolder->getById($arguments['photoId']);
+	public function run($argument) {
+		$userFolder = $this->root->getUserFolder($argument['userId']);
+		$files = $userFolder->getById($argument['photoId']);
 		if (empty($files)) {
 			return;
 		}
 		$file = array_shift($files);
-		$this->photofilesService->addPhotoNow($file, $arguments['userId']);
+		$this->photofilesService->addPhotoNow($file, $argument['userId']);
 
-		$counter = $this->backgroundJobCache->get('recentlyAdded:'.$arguments['userId']) ?? 0;
-		$this->backgroundJobCache->set('recentlyAdded:'.$arguments['userId'], (int)$counter + 1, 60 * 60 * 3);
+		$counter = $this->backgroundJobCache->get('recentlyAdded:'.$argument['userId']) ?? 0;
+		$this->backgroundJobCache->set('recentlyAdded:'.$argument['userId'], (int)$counter + 1, 60 * 60 * 3);
 	}
 }

lib/BackgroundJob/LaunchUsersInstallScanJob.php

@@ -17,26 +17,27 @@
 use OCP\BackgroundJob\QueuedJob;
 use OCP\IUser;
 use OCP\IUserManager;
+use Psr\Log\LoggerInterface;
 
 class LaunchUsersInstallScanJob extends QueuedJob {
 
-	private $jobList;
-
 	/**
 	 * LaunchUsersInstallScanJob constructor.
 	 *
 	 * A QueuedJob to launch a scan job for each user
 	 *
 	 * @param IJobList $jobList
 	 */
-	public function __construct(ITimeFactory $timeFactory, IJobList $jobList, IUserManager $userManager) {
+	public function __construct(
+		ITimeFactory $timeFactory,
+		private IJobList $jobList,
+		private IUserManager $userManager,
+	) {
 		parent::__construct($timeFactory);
-		$this->jobList = $jobList;
-		$this->userManager = $userManager;
 	}
 
-	public function run($arguments) {
-		\OC::$server->getLogger()->debug('Launch users install scan jobs cronjob executed');
+	public function run($argument) {
+		\OCP\Server::get(LoggerInterface::class)->debug('Launch users install scan jobs cronjob executed');
 		$this->userManager->callForSeenUsers(function (IUser $user) {
 			$this->jobList->add(UserInstallScanJob::class, ['userId' => $user->getUID()]);
 		});

lib/BackgroundJob/LookupMissingGeoJob.php

@@ -16,31 +16,25 @@
 use OCP\AppFramework\Utility\ITimeFactory;
 use OCP\BackgroundJob\IJobList;
 use OCP\BackgroundJob\QueuedJob;
+use Psr\Log\LoggerInterface;
 
 class LookupMissingGeoJob extends QueuedJob {
 
-	/** @var AddressService */
-	private $addressService;
-
-	/** @var AddressService */
-	private $jobList;
-
 	/**
 	 * LookupMissingGeoJob constructor.
 	 *
 	 * A QueuedJob to lookup missing geo information of addresses
-	 *
-	 * @param AddressService $service
-	 * @param IJobList $jobList
 	 */
-	public function __construct(ITimeFactory $timeFactory, AddressService $service, IJobList $jobList) {
+	public function __construct(
+		ITimeFactory $timeFactory,
+		private AddressService $addressService,
+		private IJobList $jobList,
+	) {
 		parent::__construct($timeFactory);
-		$this->addressService = $service;
-		$this->jobList = $jobList;
 	}
 
-	public function run($arguments) {
-		\OC::$server->getLogger()->debug('Maps address lookup cronjob executed');
+	public function run($argument) {
+		\OCP\Server::get(LoggerInterface::class)->debug('Maps address lookup cronjob executed');
 		// lookup at most 200 addresses
 		if (!$this->addressService->lookupMissingGeo(200)) {
 			// if not all addresses where looked up successfully add a new job for next time

lib/BackgroundJob/UpdatePhotoByFileJob.php

@@ -39,30 +39,30 @@ class UpdatePhotoByFileJob extends QueuedJob {
 	 *
 	 * A QueuedJob to scan user storage for photos and tracks
 	 *
-	 * @param ITimeFactory $timeFactory
-	 * @param PhotofilesService $photofilesService
 	 */
-	public function __construct(ITimeFactory $timeFactory,
+	public function __construct(
+		ITimeFactory $timeFactory,
 		IRootFolder $root,
 		PhotofilesService $photofilesService,
-		ICacheFactory $cacheFactory) {
+		ICacheFactory $cacheFactory,
+	) {
 		parent::__construct($timeFactory);
 		$this->photofilesService = $photofilesService;
 		$this->root = $root;
 		$this->cacheFactory = $cacheFactory;
 		$this->backgroundJobCache = $this->cacheFactory->createDistributed('maps:background-jobs');
 	}
 
-	public function run($arguments) {
-		$userFolder = $this->root->getUserFolder($arguments['userId']);
-		$files = $userFolder->getById($arguments['fileId']);
+	public function run($argument) {
+		$userFolder = $this->root->getUserFolder($argument['userId']);
+		$files = $userFolder->getById($argument['fileId']);
 		if (empty($files)) {
 			return;
 		}
 		$file = array_shift($files);
 		$this->photofilesService->updateByFileNow($file);
 
-		$counter = $this->backgroundJobCache->get('recentlyUpdated:'.$arguments['userId']) ?? 0;
-		$this->backgroundJobCache->set('recentlyUpdated:'.$arguments['userId'], (int)$counter + 1, 60 * 60 * 3);
+		$counter = $this->backgroundJobCache->get('recentlyUpdated:'.$argument['userId']) ?? 0;
+		$this->backgroundJobCache->set('recentlyUpdated:'.$argument['userId'], (int)$counter + 1, 60 * 60 * 3);
 	}
 }

lib/BackgroundJob/UserInstallScanJob.php

@@ -20,6 +20,7 @@
 
 use OCP\IConfig;
 use OCP\IUserManager;
+use Psr\Log\LoggerInterface;
 
 class UserInstallScanJob extends QueuedJob {
 
@@ -49,9 +50,9 @@ public function __construct(ITimeFactory $timeFactory, IJobList $jobList,
 		$this->tracksService = $tracksService;
 	}
 
-	public function run($arguments) {
-		$userId = $arguments['userId'];
-		\OC::$server->getLogger()->debug('Launch user install scan job for '.$userId.' cronjob executed');
+	public function run($argument) {
+		$userId = $argument['userId'];
+		\OCP\Server::get(LoggerInterface::class)->debug('Launch user install scan job for '.$userId.' cronjob executed');
 		// scan photos and tracks for given user
 		$this->rescanUserPhotos($userId);
 		$this->rescanUserTracks($userId);

lib/Command/RegisterMimetypes.php

@@ -40,7 +40,6 @@ protected function configure() {
 	 * @return int
 	 */
 	protected function execute(InputInterface $input, OutputInterface $output): int {
-		$this->output = $output;
 		$output->writeln('Register mimetypes for existing files');
 		$this->mimetypeService->registerForExistingFiles();
 		$output->writeln('Register mimetypes for new files');

lib/Controller/ContactsController.php

@@ -12,7 +12,6 @@
 
 namespace OCA\Maps\Controller;
 
-use OC\Files\Node\Node;
 use OCA\DAV\CardDAV\CardDavBackend;
 use OCA\Maps\Service\AddressService;
 use OCP\AppFramework\Controller;
@@ -21,18 +20,17 @@
 use OCP\Contacts\IManager;
 use OCP\DB\QueryBuilder\IQueryBuilder;
 use OCP\Files\IRootFolder;
+use OCP\Files\Node;
 use OCP\Files\NotFoundException;
 use OCP\IAvatarManager;
 use OCP\IDBConnection;
-use OCP\ILogger;
 use OCP\IRequest;
 use OCP\IURLGenerator;
 use Sabre\VObject\Property\Text;
 use Sabre\VObject\Reader;
 
 class ContactsController extends Controller {
 	private $userId;
-	private $logger;
 	private $contactsManager;
 	private $addressService;
 	private $dbconnection;
@@ -45,7 +43,6 @@ class ContactsController extends Controller {
 
 	/**
 	 * @param $AppName
-	 * @param ILogger $logger
 	 * @param IRequest $request
 	 * @param IDBConnection $dbconnection
 	 * @param IManager $contactsManager
@@ -57,7 +54,6 @@ class ContactsController extends Controller {
 	 */
 	public function __construct(
 		$AppName,
-		ILogger $logger,
 		IRequest $request,
 		IDBConnection $dbconnection,
 		IManager $contactsManager,
@@ -68,7 +64,6 @@ public function __construct(
 		IRootFolder $root,
 		IURLGenerator $urlGenerator) {
 		parent::__construct($AppName, $request);
-		$this->logger = $logger;
 		$this->userId = $UserId;
 		$this->avatarManager = $avatarManager;
 		$this->contactsManager = $contactsManager;
@@ -83,7 +78,7 @@ public function __construct(
 	/**
 	 * Converts a geo string as a float array
 	 * @param string formatted as "lat;lon"
-	 * @return float array containing [lat;lon]
+	 * @return float[] array containing [lat;lon]
 	 */
 	private function geoAsFloatArray($geo) {
 		$res = array_map(function ($value) {return floatval($value);}, explode(';', $geo));
@@ -116,7 +111,7 @@ private function isNewAddress($prevGeo, $geo) {
 	 * get distance between two geo points
 	 * @param GPS coordinates of first point
 	 * @param GPS coordinates of second point
-	 * @return Distance in meters between these two points
+	 * @return float Distance in meters between these two points
 	 */
 	private function getDistance($coordsA, $coordsB) {
 		if (empty($coordsA) || empty($coordsB)) {

lib/Controller/DevicesApiController.php

@@ -12,22 +12,16 @@
 namespace OCA\Maps\Controller;
 
 use OCA\Maps\Service\DevicesService;
-
 use OCP\App\IAppManager;
 use OCP\AppFramework\ApiController;
-
 use OCP\AppFramework\Http;
-
-
 use OCP\AppFramework\Http\DataResponse;
 use OCP\IConfig;
 use OCP\IGroupManager;
 use OCP\IL10N;
-use OCP\ILogger;
 use OCP\IRequest;
 use OCP\IServerContainer;
 use OCP\IUserManager;
-
 use OCP\Share\IManager;
 
 class DevicesApiController extends ApiController {
@@ -43,7 +37,6 @@ class DevicesApiController extends ApiController {
 	private $dbdblquotes;
 	private $defaultDeviceId;
 	private $l;
-	private $logger;
 	private $devicesService;
 	protected $appName;
 
@@ -56,15 +49,13 @@ public function __construct($AppName,
 		IUserManager $userManager,
 		IGroupManager $groupManager,
 		IL10N $l,
-		ILogger $logger,
 		DevicesService $devicesService,
 		$UserId) {
 		parent::__construct($AppName, $request,
 			'PUT, POST, GET, DELETE, PATCH, OPTIONS',
 			'Authorization, Content-Type, Accept',
 			1728000);
 		$this->devicesService = $devicesService;
-		$this->logger = $logger;
 		$this->appName = $AppName;
 		$this->appVersion = $config->getAppValue('maps', 'installed_version');
 		$this->userId = $UserId;