Merge pull request #5344 from nextcloud/chore/static-analysis

chore: Set up static analysis
nextcloud · Jun 29, 2023 · 913aad7 · 913aad7
2 parents 976e53d + 49e6bd2
commit 913aad7
Show file tree

Hide file tree

Showing 19 changed files with 2,435 additions and 31 deletions.
diff --git a/.github/workflows/psalm-matrix.yml b/.github/workflows/psalm-matrix.yml
@@ -0,0 +1,62 @@
+# 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:
+  push:
+    branches:
+      - master
+      - main
+      - stable*
+
+concurrency:
+  group: psalm-${{ github.head_ref || github.run_id }}
+  cancel-in-progress: true
+
+jobs:
+  static-analysis:
+    runs-on: ubuntu-latest
+    strategy:
+      # do not stop on another job's failure
+      fail-fast: false
+      matrix:
+        ocp-version: [ 'dev-master', 'dev-stable27', 'dev-stable26', 'dev-stable25' ]
+
+    name: Nextcloud ${{ matrix.ocp-version }}
+    steps:
+      - name: Checkout
+        uses: actions/checkout@8e5e7e5ab8b370d6c329ec480221332ada57f0ab # v3.5.2
+
+      - name: Set up php
+        uses: shivammathur/setup-php@c5fc0d8281aba02c7fda07d3a70cc5371548067d # v2
+        with:
+          php-version: 8.0
+          coverage: none
+          ini-file: development
+        env:
+          GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
+
+      - name: Install dependencies
+        run: composer i
+
+      - name: Install dependencies
+        run: composer require --dev nextcloud/ocp:${{ matrix.ocp-version }} --ignore-platform-reqs --with-dependencies
+
+      - name: Run coding standards check
+        run: composer run psalm
+
+  summary:
+    runs-on: ubuntu-latest
+    needs: static-analysis
+
+    if: always()
+
+    name: static-psalm-analysis-summary
+
+    steps:
+      - name: Summary status
+        run: if ${{ needs.static-analysis.result != 'success' }}; then exit 1; fi
diff --git a/.nextcloudignore b/.nextcloudignore
@@ -25,6 +25,7 @@ package.json
 package-lock.json
 phpunit.unit.xml
 README.md
+/psalm.xml
 screenshots
 src
 stylelint.config.js

diff --git a/composer.json b/composer.json
@@ -23,6 +23,7 @@
         "cs:fix": "php-cs-fixer fix",
         "cs:check": "php-cs-fixer fix --dry-run --diff",
         "lint": "find . -name \\*.php -not -path './vendor/*' -not -path './tests/*' -print0 | xargs -0 -n1 php -l",
+        "psalm": "psalm",
         "test": "phpunit --configuration phpunit.unit.xml --fail-on-warning",
         "test:dev": "phpunit --configuration phpunit.unit.xml --fail-on-warning --stop-on-error --stop-on-failure",
         "post-install-cmd": [

diff --git a/lib/BackgroundJob/CleanUpOutdatedBookingsJob.php b/lib/BackgroundJob/CleanUpOutdatedBookingsJob.php
@@ -29,13 +29,12 @@
 use OCA\Calendar\Service\Appointments\BookingService;
 use OCP\AppFramework\Utility\ITimeFactory;
 use OCP\BackgroundJob\TimedJob;
-use OCP\ILogger;
 use Psr\Log\LoggerInterface;
 use function method_exists;
 
 class CleanUpOutdatedBookingsJob extends TimedJob {
-	/** @var ILogger */
-	private $logger;
+	/** @var LoggerInterface */
+	private LoggerInterface $logger;
 
 	/** @var BookingService */
 	private $service;

diff --git a/lib/Controller/BookingController.php b/lib/Controller/BookingController.php
@@ -28,7 +28,6 @@
 use DateTimeImmutable;
 use DateTimeZone;
 use InvalidArgumentException;
-use OC\URLGenerator;
 use OCA\Calendar\AppInfo\Application;
 use OCA\Calendar\Exception\ClientException;
 use OCA\Calendar\Exception\NoSlotFoundException;
@@ -44,6 +43,7 @@
 use OCP\DB\Exception;
 use OCP\IConfig;
 use OCP\IRequest;
+use OCP\IURLGenerator;
 use OCP\Mail\IMailer;
 use Psr\Log\LoggerInterface;
 
@@ -60,7 +60,7 @@ class BookingController extends Controller {
 	/** @var IInitialState */
 	private $initialState;
 
-	/** @var URLGenerator */
+	/** @var IURLGenerator */
 	private $urlGenerator;
 
 	/** @var LoggerInterface */
@@ -76,7 +76,7 @@ public function __construct(string $appName,
 		IInitialState $initialState,
 		BookingService $bookingService,
 		AppointmentConfigService $appointmentConfigService,
-		URLGenerator $urlGenerator,
+		IURLGenerator $urlGenerator,
 		LoggerInterface $logger,
 		IMailer $mailer,
 		IConfig $systemConfig) {

diff --git a/lib/Controller/PublicViewController.php b/lib/Controller/PublicViewController.php
@@ -121,7 +121,7 @@ private function publicIndex(string $token):PublicTemplateResponse {
 		$defaultShowTasks = $this->config->getAppValue($this->appName, 'showTasks', 'yes');
 		$defaultCanSubscribeLink = $this->config->getAppValue('dav', 'allow_calendar_link_subscriptions', 'yes');
 
-		$appVersion = $this->config->getAppValue($this->appName, 'installed_version', null);
+		$appVersion = $this->config->getAppValue($this->appName, 'installed_version', '');
 
 		$this->initialStateService->provideInitialState($this->appName, 'app_version', $appVersion);
 		$this->initialStateService->provideInitialState($this->appName, 'event_limit', ($defaultEventLimit === 'yes'));

diff --git a/lib/Controller/ViewController.php b/lib/Controller/ViewController.php
@@ -97,7 +97,7 @@ public function index():TemplateResponse {
 		$defaultDefaultReminder = $this->config->getAppValue($this->appName, 'defaultReminder', 'none');
 		$defaultShowTasks = $this->config->getAppValue($this->appName, 'showTasks', 'yes');
 
-		$appVersion = $this->config->getAppValue($this->appName, 'installed_version', null);
+		$appVersion = $this->config->getAppValue($this->appName, 'installed_version', '');
 		$eventLimit = $this->config->getUserValue($this->userId, $this->appName, 'eventLimit', $defaultEventLimit) === 'yes';
 		$firstRun = $this->config->getUserValue($this->userId, $this->appName, 'firstRun', 'yes') === 'yes';
 		$initialView = $this->getView($this->config->getUserValue($this->userId, $this->appName, 'currentView', $defaultInitialView));

diff --git a/lib/Dashboard/CalendarWidget.php b/lib/Dashboard/CalendarWidget.php
@@ -136,12 +136,6 @@ public function load(): void {
 		});
 	}
 
-	/**
-	 * @inheritDoc
-	 *
-	 * @param string|null $since Use any PHP DateTime allowed values to get future dates
-	 * @param int $limit  Max 14 items is the default
-	 */
 	public function getItems(string $userId, ?string $since = null, int $limit = 7): array {
 		$calendars = $this->calendarManager->getCalendarsForPrincipal('principals/users/' . $userId);
 		$count = count($calendars);

diff --git a/lib/Db/AppointmentConfigMapper.php b/lib/Db/AppointmentConfigMapper.php
@@ -114,7 +114,7 @@ public function deleteById(int $id, string $userId): int {
 			->where($qb->expr()->eq('id', $qb->createNamedParameter($id, IQueryBuilder::PARAM_INT), IQueryBuilder::PARAM_INT))
 			->andWhere($qb->expr()->eq('user_id', $qb->createNamedParameter($userId, IQueryBuilder::PARAM_STR), IQueryBuilder::PARAM_STR));
 
-		return $qb->execute();
+		return $qb->executeStatement();
 	}
 
 	public function deleteByUserId(string $uid): void {

diff --git a/lib/Db/BookingMapper.php b/lib/Db/BookingMapper.php
@@ -83,6 +83,6 @@ public function deleteOutdated(int $validFor) : int {
 		$qb = $this->db->getQueryBuilder();
 		$qb->delete($this->getTableName())
 			->where($qb->expr()->lt('created_at', $qb->createNamedParameter($limit, IQueryBuilder::PARAM_INT), IQueryBuilder::PARAM_INT));
-		return $qb->execute();
+		return $qb->executeStatement();
 	}
 }
diff --git a/lib/Listener/UserDeletedListener.php b/lib/Listener/UserDeletedListener.php
@@ -32,6 +32,9 @@
 use OCP\User\Events\UserDeletedEvent;
 use Psr\Log\LoggerInterface;
 
+/**
+ * @template-implements IEventListener<Event|UserDeletedEvent>
+ */
 class UserDeletedListener implements IEventListener {
 	/** @var AppointmentConfigService */
 	private $appointmentConfigService;

diff --git a/lib/Service/Appointments/DailyLimitFilter.php b/lib/Service/Appointments/DailyLimitFilter.php
@@ -25,8 +25,8 @@
 
 namespace OCA\Calendar\Service\Appointments;
 
-use OC\Calendar\CalendarQuery;
 use OCA\Calendar\Db\AppointmentConfig;
+use OCP\Calendar\ICalendarQuery;
 use OCP\Calendar\IManager;
 use function array_filter;
 use function array_values;
@@ -61,7 +61,7 @@ public function filter(AppointmentConfig $config, array $slots): array {
 		}
 
 		// 2. Check what days are bookable
-		/** @var CalendarQuery $query */
+		/** @var ICalendarQuery $query */
 		$query = $this->calendarManger->newQuery($config->getPrincipalUri());
 		// Note: the query is not limited to the target calendar so that the user can
 		//         1. Move the event to another calendar

diff --git a/lib/Service/Appointments/MailService.php b/lib/Service/Appointments/MailService.php
@@ -27,14 +27,13 @@
 namespace OCA\Calendar\Service\Appointments;
 
 use Exception;
-use OC\Notification\Notification;
-use OC\URLGenerator;
 use OCA\Calendar\Db\AppointmentConfig;
 use OCA\Calendar\Db\Booking;
 use OCA\Calendar\Exception\ServiceException;
 use OCP\Defaults;
 use OCP\IDateTimeFormatter;
 use OCP\IL10N;
+use OCP\IURLGenerator;
 use OCP\IUser;
 use OCP\IUserManager;
 use OCP\L10N\IFactory;
@@ -55,7 +54,7 @@ class MailService {
 	private $defaults;
 	/** @var LoggerInterface */
 	private $logger;
-	/** @var URLGenerator */
+	/** @var IURLGenerator */
 	private $urlGenerator;
 	/** @var IDateTimeFormatter */
 	private $dateFormatter;
@@ -69,7 +68,7 @@ public function __construct(IMailer $mailer,
 		IL10N $l10n,
 		Defaults $defaults,
 		LoggerInterface $logger,
-		URLGenerator $urlGenerator,
+		IURLGenerator $urlGenerator,
 		IDateTimeFormatter $dateFormatter,
 		IFactory $lFactory,
 		IManager $notificationManager) {
@@ -97,9 +96,11 @@ public function sendConfirmationEmail(Booking $booking, AppointmentConfig $confi
 		}
 
 		$fromEmail = $user->getEMailAddress();
+		if ($fromEmail === null) {
+			throw new ServiceException('Organizer has no email set');
+		}
 		$fromName = $user->getDisplayName();
 
-
 		$sys = $this->getSysEmail();
 		$message = $this->mailer->createMessage()
 			->setFrom([$sys => $fromName])
@@ -117,10 +118,10 @@ public function sendConfirmationEmail(Booking $booking, AppointmentConfig $confi
 		// Heading
 		$summary = $this->l10n->t("Dear %s, please confirm your booking", [$booking->getDisplayName()]);
 		$template->addHeading($summary);
-		
+
 		$bookingUrl = $this->urlGenerator->linkToRouteAbsolute('calendar.booking.confirmBooking', ['token' => $booking->getToken()]);
 		$template->addBodyButton($this->l10n->t('Confirm'), $bookingUrl);
-		
+
 		$template->addBodyListItem($user->getDisplayName(), 'Appointment with:');
 		if (!empty($config->getDescription())) {
 			$template->addBodyListItem($config->getDescription(), 'Description:');
@@ -168,6 +169,9 @@ public function sendBookingInformationEmail(Booking $booking, AppointmentConfig
 		}
 
 		$fromEmail = $user->getEMailAddress();
+		if ($fromEmail === null) {
+			throw new ServiceException('Organizer has no email set');
+		}
 		$fromName = $user->getDisplayName();
 
 		$sys = $this->getSysEmail();
@@ -263,12 +267,16 @@ public function sendOrganizerBookingInformationEmail(Booking $booking, Appointme
 			throw new ServiceException('Could not find organizer');
 		}
 
+		$toEmail = $user->getEMailAddress();
+		if ($toEmail === null) {
+			throw new ServiceException('Organizer has no email set');
+		}
 		$fromName = $user->getDisplayName();
 
 		$sys = $this->getSysEmail();
 		$message = $this->mailer->createMessage()
 			->setFrom([$sys => $fromName])
-			->setTo([$user->getEMailAddress() => $booking->getDisplayName()]);
+			->setTo([$toEmail => $booking->getDisplayName()]);
 
 
 		$template = $this->mailer->createEMailTemplate('calendar.confirmOrganizer');
@@ -320,7 +328,6 @@ public function sendOrganizerBookingInformationNotification(Booking $booking, Ap
 			$this->lFactory->get('calendar')
 		);
 
-		/** @var Notification $notification */
 		$notification = $this->notificationManager->createNotification();
 		$notification
 			->setApp('calendar')

diff --git a/psalm.xml b/psalm.xml
@@ -0,0 +1,53 @@
+<?xml version="1.0"?>
+<psalm
+    errorLevel="4"
+    xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
+    xmlns="https://getpsalm.org/schema/config"
+    xsi:schemaLocation="https://getpsalm.org/schema/config"
+    errorBaseline="tests/psalm-baseline.xml"
+	findUnusedBaselineEntry="true"
+	findUnusedCode="false"
+>
+    <projectFiles>
+        <directory name="lib" />
+		<ignoreFiles>
+			<directory name="vendor" />
+			<directory name="vendor-bin" />
+		</ignoreFiles>
+    </projectFiles>
+    <extraFiles>
+        <directory name="vendor" />
+		<directory name="vendor/nextcloud/ocp" />
+    </extraFiles>
+	<issueHandlers>
+		<UndefinedClass>
+			<errorLevel type="suppress">
+				<referencedClass name="Doctrine\DBAL\Platforms\MySQLPlatform" />
+				<referencedClass name="Doctrine\DBAL\Platforms\PostgreSQL94Platform" />
+				<referencedClass name="Doctrine\DBAL\Platforms\SqlitePlatform" />
+				<referencedClass name="Doctrine\DBAL\Types\Type" />
+				<referencedClass name="Doctrine\DBAL\Types\Types" />
+				<referencedClass name="OC\Security\CSP\ContentSecurityPolicyNonceManager" />
+				<referencedClass name="Psr\Http\Client\ClientExceptionInterface" />
+				<referencedClass name="Sabre\VObject\Component\VCalendar" />
+				<referencedClass name="Symfony\Component\HttpFoundation\IpUtils" />
+				<referencedClass name="Symfony\Component\Console\Command\Command" />
+				<referencedClass name="Symfony\Component\Console\Input\InputArgument" />
+				<referencedClass name="Symfony\Component\Console\Input\InputInterface" />
+				<referencedClass name="Symfony\Component\Console\Input\InputOption" />
+				<referencedClass name="Symfony\Component\Console\Output\OutputInterface" />
+			</errorLevel>
+		</UndefinedClass>
+		<UndefinedDocblockClass>
+			<errorLevel type="suppress">
+				<referencedClass name="Doctrine\DBAL\Driver\Statement" />
+				<referencedClass name="Doctrine\DBAL\Platforms\AbstractPlatform" />
+				<referencedClass name="Doctrine\DBAL\Schema\Schema" />
+				<referencedClass name="Doctrine\DBAL\Schema\SchemaException" />
+				<referencedClass name="Doctrine\DBAL\Schema\Table" />
+				<referencedClass name="OC\Security\CSP\ContentSecurityPolicyNonceManager" />
+				<referencedClass name="Symfony\Component\Console\Output\OutputInterface" />
+			</errorLevel>
+		</UndefinedDocblockClass>
+	</issueHandlers>
+</psalm>
diff --git a/tests/php/unit/Controller/PublicViewControllerTest.php b/tests/php/unit/Controller/PublicViewControllerTest.php
@@ -76,7 +76,7 @@ public function testPublicIndexWithBranding():void {
 				['calendar', 'slotDuration', '00:30:00', 'defaultSlotDuration'],
 				['calendar', 'defaultReminder', 'none', 'defaultDefaultReminder'],
 				['calendar', 'showTasks', 'yes', 'yes'],
-				['calendar', 'installed_version', null, '1.0.0']
+				['calendar', 'installed_version', '', '1.0.0']
 			]);
 
 		$this->request->expects(self::once())
@@ -154,7 +154,7 @@ public function testPublicIndexForEmbedding():void {
 				['calendar', 'slotDuration', '00:30:00', 'defaultSlotDuration'],
 				['calendar', 'defaultReminder', 'none', 'defaultDefaultReminder'],
 				['calendar', 'showTasks', 'yes', 'defaultShowTasks'],
-				['calendar', 'installed_version', null, '1.0.0']
+				['calendar', 'installed_version', '', '1.0.0']
 			]);
 		$this->request->expects(self::once())
 			->method('getServerProtocol')

diff --git a/tests/php/unit/Controller/ViewControllerTest.php b/tests/php/unit/Controller/ViewControllerTest.php
@@ -107,7 +107,7 @@ public function testIndex(): void {
 				['calendar', 'showTasks', 'yes', 'defaultShowTasks'],
 				['calendar', 'hideEventExport', 'no', 'yes'],
 				['calendar', 'forceEventAlarmType', '', ''],
-				['calendar', 'installed_version', null, '1.0.0'],
+				['calendar', 'installed_version', '', '1.0.0'],
 			]);
 		$this->config
 			->method('getUserValue')
@@ -212,7 +212,7 @@ public function testIndexViewFix(string $savedView, string $expectedView): void
 				['calendar', 'slotDuration', '00:30:00', 'defaultSlotDuration'],
 				['calendar', 'defaultReminder', 'none', 'defaultDefaultReminder'],
 				['calendar', 'showTasks', 'yes', 'defaultShowTasks'],
-				['calendar', 'installed_version', null, '1.0.0'],
+				['calendar', 'installed_version', '', '1.0.0'],
 			]);
 		$this->config
 			->method('getUserValue')