Merge pull request #1747 from nextcloud/techdebt/noid/bump-psalm

Bump psalm to level 3
nextcloud · Dec 11, 2023 · 74f1b07 · 74f1b07
2 parents 6cb1651 + 91f911c
commit 74f1b07
Show file tree

Hide file tree

Showing 14 changed files with 50 additions and 91 deletions.
diff --git a/composer.json b/composer.json
@@ -37,8 +37,10 @@
 		"lint": "find . -name \\*.php -not -path './vendor/*' -not -path './build/*' -print0 | xargs -0 -n1 php -l",
 		"cs:check": "php-cs-fixer fix --dry-run --diff",
 		"cs:fix": "php-cs-fixer fix",
+		"openapi": "generate-spec",
 		"psalm": "psalm --threads=1",
-		"psalm:update-baseline": "psalm --threads=1 --update-baseline --set-baseline=tests/psalm-baseline.xml",
+		"psalm:dev": "psalm --no-cache --threads=$(nproc)",
+		"psalm:update-baseline": "psalm --threads=1 --update-baseline",
 		"psalm:clear": "psalm --clear-cache && psalm --clear-global-cache",
 		"psalm:fix": "psalm --alter --issues=InvalidReturnType,InvalidNullableReturnType,MissingParamType,InvalidFalsableReturnType",
 		"test:unit": "vendor/bin/phpunit --color -c tests/Unit/phpunit.xml",

diff --git a/lib/AppInfo/Application.php b/lib/AppInfo/Application.php
@@ -25,7 +25,6 @@
 
 namespace OCA\Notifications\AppInfo;
 
-use OC\Authentication\Token\IProvider;
 use OCA\Notifications\App;
 use OCA\Notifications\Capabilities;
 use OCA\Notifications\Listener\BeforeTemplateRenderedListener;
@@ -38,7 +37,6 @@
 use OCP\AppFramework\Bootstrap\IBootstrap;
 use OCP\AppFramework\Bootstrap\IRegistrationContext;
 use OCP\AppFramework\Http\Events\BeforeTemplateRenderedEvent;
-use OCP\AppFramework\IAppContainer;
 use OCP\Notification\IManager;
 use OCP\User\Events\PostLoginEvent;
 use OCP\User\Events\UserCreatedEvent;
@@ -54,10 +52,6 @@ public function __construct() {
 	public function register(IRegistrationContext $context): void {
 		$context->registerCapability(Capabilities::class);
 
-		$context->registerService(IProvider::class, function (IAppContainer $c) {
-			return $c->getServer()->get(IProvider::class);
-		});
-
 		$context->registerSetupCheck(SetupWarningOnRateLimitReached::class);
 
 		$context->registerNotifierService(AdminNotifications::class);

diff --git a/lib/Controller/PushController.php b/lib/Controller/PushController.php
@@ -26,14 +26,14 @@
 
 namespace OCA\Notifications\Controller;
 
-use OC\Authentication\Exceptions\InvalidTokenException;
 use OC\Authentication\Token\IProvider;
-use OC\Authentication\Token\IToken;
 use OC\Security\IdentityProof\Manager;
 use OCA\Notifications\ResponseDefinitions;
 use OCP\AppFramework\Http;
 use OCP\AppFramework\Http\DataResponse;
 use OCP\AppFramework\OCSController;
+use OCP\Authentication\Exceptions\InvalidTokenException;
+use OCP\Authentication\Token\IToken;
 use OCP\DB\QueryBuilder\IQueryBuilder;
 use OCP\IDBConnection;
 use OCP\IRequest;
@@ -208,7 +208,7 @@ protected function savePushToken(IUser $user, IToken $token, string $deviceIdent
 			->from('notifications_pushhash')
 			->where($query->expr()->eq('uid', $query->createNamedParameter($user->getUID())))
 			->andWhere($query->expr()->eq('token', $query->createNamedParameter($token->getId())));
-		$result = $query->execute();
+		$result = $query->executeQuery();
 		$row = $result->fetch();
 		$result->closeCursor();
 

diff --git a/lib/Handler.php b/lib/Handler.php
@@ -249,7 +249,7 @@ public function confirmIdsForUser(string $user, array $ids): array {
 	 * @param int $startAfterId
 	 * @param string $userId
 	 * @param int $limit
-	 * @return array [notification_id => INotification]
+	 * @return array<int, INotification> [notification_id => INotification]
 	 */
 	public function getAfterId(int $startAfterId, string $userId, int $limit = 25): array {
 		$sql = $this->connection->getQueryBuilder();

diff --git a/lib/Listener/BeforeTemplateRenderedListener.php b/lib/Listener/BeforeTemplateRenderedListener.php
@@ -67,14 +67,15 @@ public function handle(Event $event): void {
 			return;
 		}
 
-		if (!$this->userSession->getUser() instanceof IUser) {
+		$user = $this->userSession->getUser();
+		if (!$user instanceof IUser) {
 			return;
 		}
 
 		$this->initialState->provideInitialState(
 			'sound_notification',
 			$this->config->getUserValue(
-				$this->userSession->getUser()->getUID(),
+				$user->getUID(),
 				Application::APP_ID,
 				'sound_notification',
 				'yes'
@@ -84,7 +85,7 @@ public function handle(Event $event): void {
 		$this->initialState->provideInitialState(
 			'sound_talk',
 			$this->config->getUserValue(
-				$this->userSession->getUser()->getUID(),
+				$user->getUID(),
 				Application::APP_ID,
 				'sound_talk',
 				'yes'

diff --git a/lib/MailNotifications.php b/lib/MailNotifications.php
@@ -131,6 +131,7 @@ public function sendEmails(int $batchSize, int $sendTime): void {
 
 		// Batch-read settings
 		$fallbackTimeZone = date_default_timezone_get();
+		/** @psalm-var array<string, string> $userTimezones */
 		$userTimezones = $this->config->getUserValueForUsers('core', 'timezone', $userIds);
 		$userEnabled = $this->config->getUserValueForUsers('core', 'enabled', $userIds);
 
@@ -160,7 +161,7 @@ public function sendEmails(int $batchSize, int $sendTime): void {
 			$languageCode = $userLanguages[$settings->getUserId()] ?? $fallbackLang;
 			$timezone = $userTimezones[$settings->getUserId()] ?? $fallbackTimeZone;
 
-			/** @var INotification[] $notifications */
+			/** @var array<int, INotification> $notifications */
 			$notifications = $this->handler->getAfterId($settings->getLastSendId(), $settings->getUserId());
 			if (!empty($notifications)) {
 				$oldestNotification = end($notifications);
@@ -183,10 +184,10 @@ public function sendEmails(int $batchSize, int $sendTime): void {
 	}
 
 	/**
-	 * send an email to the user containing given list of notifications
+	 * Send an email to the user containing given list of notifications
 	 *
 	 * @param Settings $settings
-	 * @param INotification[] $notifications
+	 * @param non-empty-array<int, INotification> $notifications
 	 * @param string $language
 	 * @param string $timezone
 	 */
@@ -280,7 +281,13 @@ protected function prepareEmailMessage(string $uid, array $notifications, string
 
 		foreach ($notifications as $notification) {
 			try {
-				$relativeDateTime = $this->dateFormatter->formatDateTimeRelativeDay($notification->getDateTime(), 'long', 'short', new \DateTimeZone($timezone), $l10n);
+				$relativeDateTime = $this->dateFormatter->formatDateTimeRelativeDay(
+					$notification->getDateTime(),
+					'long',
+					'short',
+					new \DateTimeZone($timezone ?: 'UTC'),
+					$l10n
+				);
 				$template->addBodyListItem($this->getHTMLContents($notification), $relativeDateTime, $notification->getIcon(), $notification->getParsedSubject());
 
 				// Buttons probably were not intended for this, but it works ok enough for showing the idea.

diff --git a/lib/Migration/Version2010Date20210218082811.php b/lib/Migration/Version2010Date20210218082811.php
@@ -128,7 +128,7 @@ public function postSchemaChange(IOutput $output, Closure $schemaClosure, array
 		$query = $this->connection->getQueryBuilder();
 		$query->select('*')
 			->from('notifications_pushtokens');
-		$result = $query->execute();
+		$result = $query->executeQuery();
 
 		while ($row = $result->fetch()) {
 			$insert
@@ -142,7 +142,7 @@ public function postSchemaChange(IOutput $output, Closure $schemaClosure, array
 				->setParameter('apptype', $row['apptype'])
 			;
 
-			$insert->execute();
+			$insert->executeStatement();
 		}
 		$result->closeCursor();
 	}

diff --git a/lib/Migration/Version2011Date20210930134607.php b/lib/Migration/Version2011Date20210930134607.php
@@ -27,8 +27,8 @@
 namespace OCA\Notifications\Migration;
 
 use Closure;
-use Doctrine\DBAL\Types\Types;
 use OCP\DB\ISchemaWrapper;
+use OCP\DB\Types;
 use OCP\Migration\IOutput;
 use OCP\Migration\SimpleMigrationStep;
 

diff --git a/lib/Push.php b/lib/Push.php
@@ -280,7 +280,9 @@ public function pushToDevice(int $id, INotification $notification, ?OutputInterf
 
 		if (isset($this->userStatuses[$notification->getUser()])) {
 			$userStatus = $this->userStatuses[$notification->getUser()];
-			if ($userStatus->getStatus() === IUserStatus::DND && empty($this->allowedDNDPushList[$notification->getApp()])) {
+			if ($userStatus instanceof IUserStatus
+				&& $userStatus->getStatus() === IUserStatus::DND
+				&& empty($this->allowedDNDPushList[$notification->getApp()])) {
 				$this->printInfo('<error>User status is set to DND - no push notifications will be sent</error>');
 				return;
 			}
@@ -498,14 +500,22 @@ protected function sendNotificationsToProxies(): void {
 
 				$response = $client->post($proxyServer . '/notifications', $requestData);
 				$status = $response->getStatusCode();
-				$body = $response->getBody();
-				$bodyData = json_decode($body, true);
+				$body = (string) $response->getBody();
+				try {
+					$bodyData = json_decode($body, true);
+				} catch (\JsonException $e) {
+					$bodyData = null;
+				}
 			} catch (ClientException $e) {
 				// Server responded with 4xx (400 Bad Request mostlikely)
 				$response = $e->getResponse();
 				$status = $response->getStatusCode();
 				$body = $response->getBody()->getContents();
-				$bodyData = json_decode($body, true);
+				try {
+					$bodyData = json_decode($body, true);
+				} catch (\JsonException $e) {
+					$bodyData = null;
+				}
 			} catch (ServerException $e) {
 				// Server responded with 5xx
 				$response = $e->getResponse();

diff --git a/psalm.xml b/psalm.xml
@@ -1,13 +1,14 @@
 <?xml version="1.0"?>
 <psalm
 	errorBaseline="tests/psalm-baseline.xml"
-	errorLevel="5"
+	errorLevel="3"
 	findUnusedBaselineEntry="true"
 	findUnusedCode="false"
 	resolveFromConfigFile="true"
+	phpVersion="8.0"
 	xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
 	xmlns="https://getpsalm.org/schema/config"
-	xsi:schemaLocation="https://getpsalm.org/schema/config vendor-bin/psalm/vendor/vimeo/psalm/config.xsd"
+	xsi:schemaLocation="https://getpsalm.org/schema/config vendor/vimeo/psalm/config.xsd"
 >
 	<projectFiles>
 		<directory name="lib" />
@@ -29,7 +30,6 @@
 		<UndefinedClass>
 			<errorLevel type="suppress">
 				<referencedClass name="OC" />
-				<referencedClass name="Doctrine\DBAL\Types\Types" />
 			</errorLevel>
 		</UndefinedClass>
 		<UndefinedDocblockClass>

diff --git a/tests/Integration/composer.json b/tests/Integration/composer.json
@@ -8,7 +8,6 @@
 	"require-dev": {
 		"behat/behat": "^3.12",
 		"guzzlehttp/guzzle": "^7.5",
-		"jarnaiz/behat-junit-formatter": "^1.3",
 		"phpunit/phpunit": "^9.6"
 	}
 }
diff --git a/tests/Integration/composer.lock b/tests/Integration/composer.lock
diff --git a/tests/Integration/config/behat.yml b/tests/Integration/config/behat.yml
@@ -1,4 +1,10 @@
 default:
+  formatters:
+    junit:
+      output_path: '%paths.base%/../output'
+    pretty:
+      output_styles:
+        comment: [ 'bright-blue' ]
   autoload:
     '': '%paths.base%/../features/bootstrap'
   suites:
@@ -7,8 +13,3 @@ default:
         - '%paths.base%/../features'
       contexts:
         - FeatureContext
-
-  extensions:
-      jarnaiz\JUnitFormatter\JUnitFormatterExtension:
-          filename: report.xml
-          outputDir: '%paths.base%/../output/'
diff --git a/tests/psalm-baseline.xml b/tests/psalm-baseline.xml
@@ -1,20 +1,8 @@
 <?xml version="1.0" encoding="UTF-8"?>
-<files psalm-version="5.14.1@b9d355e0829c397b9b3b47d0c0ed042a8a70284d">
-  <file src="lib/AppInfo/Application.php">
-    <UndefinedClass>
-      <code>IProvider</code>
-      <code>IProvider</code>
-    </UndefinedClass>
-  </file>
+<files psalm-version="5.16.0@2897ba636551a8cb61601cc26f6ccfbba6c36591">
   <file src="lib/Controller/PushController.php">
     <UndefinedClass>
       <code>IProvider</code>
-      <code>IToken</code>
-      <code>IToken</code>
-      <code>IToken</code>
-      <code>IToken</code>
-      <code>InvalidTokenException</code>
-      <code>InvalidTokenException</code>
       <code>Manager</code>
     </UndefinedClass>
     <UndefinedDocblockClass>