Merge pull request #4018 from nextcloud/fix/happy-psalm

Fixing psalm hints

Author: dartcafe

composer.json

@@ -48,7 +48,7 @@
 		"psalm": "psalm --no-diff",
 		"test:unit": "phpunit -c tests/phpunit.xml",
 		"test:integration": "phpunit -c tests/phpunit.xml",
-		"psalm:fix": "psalm --alter --php-version=8.1 --issues=MissingReturnType,InvalidReturnType,InvalidNullableReturnType,MismatchingDocblockParamType,MismatchingDocblockReturnType,MissingParamType,InvalidFalsableReturnType",
+		"psalm:fix": "psalm --alter --issues=MissingReturnType,InvalidReturnType,InvalidNullableReturnType,MismatchingDocblockParamType,MismatchingDocblockReturnType,MissingParamType,InvalidFalsableReturnType",
 		"psalm:info": "psalm --no-diff --show-info=true",
 		"psalm:baseline": "psalm --set-baseline=psalm-baseline.xml",
 		"psalm:baseline:update": "psalm --update-baseline",

lib/Controller/BaseApiV2Controller.php

@@ -5,7 +5,6 @@
  * SPDX-FileCopyrightText: 2024 Nextcloud contributors
  * SPDX-License-Identifier: AGPL-3.0-or-later
  */
-
 namespace OCA\Polls\Controller;
 
 use Closure;
@@ -21,6 +20,7 @@
 
 /**
  * @psalm-api
+ * @psalm-import-type HttpStatusCode from \OCA\Polls\Types
  */
 class BaseApiV2Controller extends OCSController {
 	public function __construct(
@@ -36,6 +36,7 @@ public function __construct(
 	/**
 	 * response
 	 * @param Closure $callback Callback function
+	 * @psalm-param HttpStatusCode $successStatus HTTP status code for success
 	 */
 	#[NoAdminRequired]
 	protected function response(Closure $callback, int $successStatus = Http::STATUS_OK): DataResponse {
@@ -48,7 +49,7 @@ protected function response(Closure $callback, int $successStatus = Http::STATUS
 		} catch (Exception $e) {
 
 			if ($e->getStatus() === Http::STATUS_NOT_MODIFIED) {
-				return new DataResponse(statusCode: $e->getStatus());
+				return new DataResponse(statusCode: Http::STATUS_NOT_MODIFIED);
 			}
 
 			throw new OCSBadRequestException($e->getMessage());

lib/Controller/BaseController.php

@@ -18,6 +18,7 @@
 
 /**
  * @psalm-api
+ * @psalm-import-type HttpStatusCode from \OCA\Polls\Types
  */
 class BaseController extends Controller {
 	public function __construct(
@@ -30,7 +31,7 @@ public function __construct(
 	/**
 	 * response
 	 * @param Closure $callback Callback function
-	 * @param int $successStatus HTTP status code for success
+	 * @psalm-param HttpStatusCode $successStatus HTTP status code for success
 	 */
 	#[NoAdminRequired]
 	protected function response(
@@ -42,10 +43,12 @@ protected function response(
 		} catch (Exception $e) {
 
 			if ($e->getStatus() === Http::STATUS_NOT_MODIFIED) {
-				return new JSONResponse(statusCode: $e->getStatus());
+				return new JSONResponse(statusCode: Http::STATUS_NOT_MODIFIED);
 			}
 
-			return new JSONResponse(['message' => $e->getMessage()], $e->getStatus());
+			/** @var HttpStatusCode $status */
+			$status = $e->getStatus();
+			return new JSONResponse(['message' => $e->getMessage()], $status);
 		}
 	}
 

lib/Controller/SystemController.php

@@ -36,7 +36,8 @@ public function __construct(
 	#[OpenAPI(OpenAPI::SCOPE_IGNORE)]
 	#[FrontpageRoute(verb: 'GET', url: '/search/users/{query}')]
 	public function userSearch(string $query, string $types): JSONResponse {
-		$types = explode(',', $types);
+		$types = array_map('intval', explode(',', $types));
+		// $types = explode(',', $types);
 		return new JSONResponse([
 			'siteusers' => $this->systemService->getSiteUsersAndGroups($query, $types),
 			'types' => $types

lib/Exceptions/DuplicateEntryException.php

@@ -39,22 +39,20 @@ public function getShareCreateAllowed(): bool {
 		// get group exceptions
 		$exceptionGroups = $this->getCoreLimitSharingGroups();
 
-		if ($groupExceptionMode === 'allowGroup') {
-			// exception mode is 'Limit sharing to some groups'
-			// if user is in exception group, allow share creation
-			return $this->userSession->getCurrentUser()->getIsInGroupArray($exceptionGroups);
-		} elseif ($groupExceptionMode === 'denyGroup') {
+		if ($groupExceptionMode === 'denyGroup') {
 			// exception mode is 'Exclude some Groups from sharing'
 			// if user is in exception group, deny share creation
 			return !$this->userSession->getCurrentUser()->getIsInGroupArray($exceptionGroups);
 		}
 
-		return true;
+		// exception mode is 'Limit sharing to some groups'
+		// if user is in exception group, allow share creation
+		return $this->userSession->getCurrentUser()->getIsInGroupArray($exceptionGroups);
 	}
 
 	/**
 	 * Get share group exception mode
-	 * @return string
+	 * @return 'open'|'closed'|'archived'
 	 * @psalm-return 'denyGroup'|'allowGroup'|'off'
 	 * Take value from the core setting 'shareapi_exclude_groups' and translate
 	 * 'yes' => 'denyGroup' ('Exclude some groups from sharing') existing groups are handeled as deny groups

lib/Model/Settings/SystemSettings.php

@@ -100,8 +100,7 @@ public static function extractEmailAddressAndName($eMailString): array {
 		preg_match(self::REGEX_PARSE_MAIL_AND_NAME, $eMailString, $matches);
 
 		// Check if the found element is a valid email address
-		$emailAddress = !empty($matches[1]) ? trim($matches[1]) : null;
-
+		$emailAddress = (isset($matches[1]) && trim($matches[1]) !== '') ? trim($matches[1]) : null;
 		if ($emailAddress !== null && filter_var($emailAddress, FILTER_VALIDATE_EMAIL)) {
 			// Extract the name based on the input string
 			$displayName = trim(str_replace(['<', '>'], '', str_replace($emailAddress, '', $eMailString)));

lib/Service/MailService.php

@@ -20,7 +20,6 @@
 use OCA\Polls\Event\OptionUnconfirmedEvent;
 use OCA\Polls\Event\OptionUpdatedEvent;
 use OCA\Polls\Event\PollOptionReorderedEvent;
-use OCA\Polls\Exceptions\DuplicateEntryException;
 use OCA\Polls\Exceptions\InvalidPollTypeException;
 use OCA\Polls\Model\Sequence;
 use OCA\Polls\Model\SimpleOption;
@@ -168,11 +167,7 @@ public function addBulk(int $pollId, string $bulkText = ''): array {
 
 		foreach ($newOptionsTexts as $pollOptionText) {
 			if ($pollOptionText) {
-				try {
-					$this->add($pollId, new SimpleOption($pollOptionText, 0));
-				} catch (DuplicateEntryException $e) {
-					continue;
-				}
+				$this->add($pollId, new SimpleOption($pollOptionText, 0));
 			}
 		}
 

lib/Service/OptionService.php

@@ -135,20 +135,22 @@ public function takeover(int $pollId, ?UserBase $targetUser = null): Poll {
 	/**
 	 * Transfer ownership of a poll
 	 * @param int|Poll $poll poll or pollId of poll to transfer ownership
-	 * @param null|string|UserBase $targetUser User to transfer polls to. If null the current user will be used
+	 * @param string|UserBase $targetUser User to transfer polls to. If null the current user will be used
 	 */
-	public function transferPoll(int|Poll $poll, null|string|UserBase $targetUser): Poll {
+	public function transferPoll(int|Poll $poll, string|UserBase $targetUser): Poll {
 		if (!($poll instanceof Poll)) {
 			$poll = $this->pollMapper->find($poll);
 		}
 
 		$poll->request(Poll::PERMISSION_POLL_CHANGE_OWNER);
 
 		if (!($targetUser instanceof UserBase)) {
+			$userId = $targetUser;
 			try {
-				$targetUser = $this->userMapper->getUserFromUserBase($targetUser);
+				$targetUser = $this->userMapper->getUserFromUserBase($userId);
 			} catch (UserNotFoundException $e) {
-				throw new InvalidUsernameException('The user id "' . $targetUser . '" for the target user is not valid.');
+				// to keep psalm quiet
+				throw new InvalidUsernameException('The user id "' . $userId . '" for the target user is not valid.');
 			}
 		}
 

lib/Service/PollService.php

@@ -70,6 +70,7 @@ public function getGroups(string $query = ''): array {
 	/**
 	 * Get a combined list of users, groups, circles, contact groups and contacts
 	 *
+	 * @param list<int> $types list of types to search for
 	 * @return (Circle|Email|Group|User|Contact|ContactGroup|mixed)[]
 	 *
 	 * @psalm-return array<array-key, Circle|Email|Group|User|Contact|ContactGroup|mixed>