fix(lint): avoid risky truthy falsy comparison

Signed-off-by: Max <max@nextcloud.com>
nextcloud · Apr 4, 2024 · 50edcb6 · 50edcb6
1 parent 0d67aa0
commit 50edcb6
Show file tree

Hide file tree

Showing 10 changed files with 53 additions and 38 deletions.
diff --git a/lib/Controller/AttachmentController.php b/lib/Controller/AttachmentController.php
@@ -86,7 +86,7 @@ public function __construct(
 	#[NoAdminRequired]
 	#[PublicPage]
 	#[RequireDocumentSessionOrUserOrShareToken]
-	public function getAttachmentList(?string $shareToken = null): DataResponse {
+	public function getAttachmentList(string $shareToken = ''): DataResponse {
 		$documentId = $this->getDocument()->getId();
 		try {
 			$session = $this->getSession();
@@ -98,7 +98,7 @@ public function getAttachmentList(?string $shareToken = null): DataResponse {
 			$attachments = $this->attachmentService->getAttachmentList($documentId, null, $session, $shareToken);
 		} else {
 			$userId = $this->getUserId();
-			$attachments = $this->attachmentService->getAttachmentList($documentId, $userId, $session, null);
+			$attachments = $this->attachmentService->getAttachmentList($documentId, $userId, $session);
 		}
 
 		return new DataResponse($attachments);
@@ -126,7 +126,7 @@ public function insertAttachmentFile(string $filePath): DataResponse {
 	#[NoAdminRequired]
 	#[PublicPage]
 	#[RequireDocumentSession]
-	public function uploadAttachment(?string $shareToken = null): DataResponse {
+	public function uploadAttachment(string $shareToken = ''): DataResponse {
 		$documentId = $this->getSession()->getDocumentId();
 
 		try {
@@ -193,7 +193,7 @@ private function getUploadedFile(string $key): array {
 	#[PublicPage]
 	#[NoCSRFRequired]
 	#[RequireDocumentSessionOrUserOrShareToken]
-	public function getImageFile(string $imageFileName, ?string $shareToken = null,
+	public function getImageFile(string $imageFileName, string $shareToken = '',
 		int $preferRawImage = 0): DataResponse|DataDownloadResponse {
 		$documentId = $this->getDocument()->getId();
 
@@ -228,7 +228,7 @@ public function getImageFile(string $imageFileName, ?string $shareToken = null,
 	#[PublicPage]
 	#[NoCSRFRequired]
 	#[RequireDocumentSessionOrUserOrShareToken]
-	public function getMediaFile(string $mediaFileName, ?string $shareToken = null): DataResponse|DataDownloadResponse {
+	public function getMediaFile(string $mediaFileName, string $shareToken = ''): DataResponse|DataDownloadResponse {
 		$documentId = $this->getDocument()->getId();
 
 		try {
@@ -259,7 +259,7 @@ public function getMediaFile(string $mediaFileName, ?string $shareToken = null):
 	#[PublicPage]
 	#[NoCSRFRequired]
 	#[RequireDocumentSessionOrUserOrShareToken]
-	public function getMediaFilePreview(string $mediaFileName, ?string $shareToken = null) {
+	public function getMediaFilePreview(string $mediaFileName, string $shareToken = '') {
 		$documentId = $this->getDocument()->getId();
 
 		try {

diff --git a/lib/Controller/SessionController.php b/lib/Controller/SessionController.php
@@ -59,7 +59,7 @@ public function __construct(
 
 	#[NoAdminRequired]
 	public function create(?int $fileId = null, ?string $file = null, ?string $baseVersionEtag = null): DataResponse {
-		return $this->apiService->create($fileId, $file, $baseVersionEtag, null, null);
+		return $this->apiService->create($fileId, $file, $baseVersionEtag);
 	}
 
 	#[NoAdminRequired]

diff --git a/lib/Controller/UserApiController.php b/lib/Controller/UserApiController.php
@@ -39,11 +39,12 @@ public function index(string $filter = '', int $limit = 5): DataResponse {
 		// Add joined users to the autocomplete list
 		foreach ($sessions as $session) {
 			$sessionUserId = $session['userId'];
-			if ($sessionUserId !== null && !isset($users[$sessionUserId])) {
-				$displayName = $this->userManager->getDisplayName($sessionUserId);
-				if ($displayName && stripos($displayName, $filter) !== false || stripos($sessionUserId, $filter) !== false) {
-					$users[$sessionUserId] = $displayName;
-				}
+			if ($sessionUserId === null || isset($users[$sessionUserId])) {
+				continue;
+			}
+			$displayName = $this->userManager->getDisplayName($sessionUserId) ?? '';
+			if (stripos($displayName, $filter) !== false || stripos($sessionUserId, $filter) !== false) {
+				$users[$sessionUserId] = $displayName;
 			}
 		}
 

diff --git a/lib/Db/StepMapper.php b/lib/Db/StepMapper.php
@@ -51,6 +51,9 @@ public function find(int $documentId, int $fromVersion): array {
 		return $this->findEntities($qb);
 	}
 
+	/**
+	 * @psalm-return ?positive-int
+	 */
 	public function getLatestVersion(int $documentId): ?int {
 		/* @var $qb IQueryBuilder */
 		$qb = $this->db->getQueryBuilder();

diff --git a/lib/Service/ApiService.php b/lib/Service/ApiService.php
@@ -73,7 +73,7 @@ public function __construct(IRequest $request,
 
 	public function create(?int $fileId = null, ?string $filePath = null, ?string $baseVersionEtag = null, ?string $token = null, ?string $guestName = null): DataResponse {
 		try {
-			if ($token) {
+			if ($token !== null) {
 				$file = $this->documentService->getFileByShareToken($token, $this->request->getParam('filePath'));
 
 				/*
@@ -87,7 +87,7 @@ public function create(?int $fileId = null, ?string $filePath = null, ?string $b
 				} catch (NotPermittedException $e) {
 					return new DataResponse(['error' => $this->l10n->t('This file cannot be displayed as download is disabled by the share')], 404);
 				}
-			} elseif ($fileId) {
+			} elseif ($fileId !== null) {
 				try {
 					$file = $this->documentService->getFileById($fileId);
 				} catch (NotFoundException|NotPermittedException $e) {
@@ -115,7 +115,7 @@ public function create(?int $fileId = null, ?string $filePath = null, ?string $b
 			$this->sessionService->removeInactiveSessionsWithoutSteps($file->getId());
 			$document = $this->documentService->getDocument($file->getId());
 			$freshSession = $document === null;
-			if ($baseVersionEtag && $baseVersionEtag !== $document?->getBaseVersionEtag()) {
+			if ($baseVersionEtag !== null && $baseVersionEtag !== $document?->getBaseVersionEtag()) {
 				return new DataResponse(['error' => $this->l10n->t('Editing session has expired. Please reload the page.')], Http::STATUS_PRECONDITION_FAILED);
 			}
 

diff --git a/lib/Service/AttachmentService.php b/lib/Service/AttachmentService.php
@@ -204,17 +204,17 @@ private function getMediaFilePreviewFile(string $mediaFileName, File $textFile):
 
 	/**
 	 * @param int          $documentId
-	 * @param string|null  $userId
+	 * @param string       $userId or empty string if there is none.
 	 * @param Session|null $session
-	 * @param string|null  $shareToken
+	 * @param string       $shareToken or empty string if there is none.
 	 *
 	 * @return array
 	 * @throws InvalidPathException
 	 * @throws NoUserException
 	 * @throws NotFoundException
 	 * @throws NotPermittedException
 	 */
-	public function getAttachmentList(int $documentId, ?string $userId = null, ?Session $session = null, ?string $shareToken = null): array {
+	public function getAttachmentList(int $documentId, string $userId = '', ?Session $session = null, string $shareToken = ''): array {
 		if ($shareToken) {
 			$textFile = $this->getTextFilePublic($documentId, $shareToken);
 		} elseif ($userId) {

diff --git a/lib/Service/DocumentService.php b/lib/Service/DocumentService.php
@@ -137,7 +137,7 @@ public function createDocument(File $file): Document {
 			// Do not hard reset if changed from outside since this will throw away possible steps
 			// This way the user can still resolve conflicts in the editor view
 			$stepsVersion = $this->stepMapper->getLatestVersion($document->getId());
-			if ($stepsVersion && ($document->getLastSavedVersion() !== $stepsVersion)) {
+			if ($stepsVersion !== null && ($document->getLastSavedVersion() !== $stepsVersion)) {
 				$this->logger->debug('Unsaved steps, continue collaborative editing');
 				return $document;
 			}
@@ -347,7 +347,7 @@ public function autosave(Document $document, ?File $file, int $version, ?string
 		// Do not save if newer version already saved
 		// Note that $version is the version of the steps the client has fetched.
 		// It may have added steps on top of that - so if the versions match we still save.
-		$stepsVersion = $this->stepMapper->getLatestVersion($documentId)?: 0;
+		$stepsVersion = $this->stepMapper->getLatestVersion($documentId) ?? 0;
 		$savedVersion = $document->getLastSavedVersion();
 		$outdated = $savedVersion > 0 && $savedVersion > $version;
 		if (!$force && ($outdated || $version > (string)$stepsVersion)) {
@@ -378,7 +378,7 @@ public function autosave(Document $document, ?File $file, int $version, ?string
 
 		// Version changed but the content remains the same
 		if ($autoSaveDocument === $file->getContent()) {
-			if ($documentState) {
+			if ($documentState !== null) {
 				$this->writeDocumentState($file->getId(), $documentState);
 			}
 			$document->setLastSavedVersion($stepsVersion);
@@ -397,7 +397,7 @@ public function autosave(Document $document, ?File $file, int $version, ?string
 			), function () use ($file, $autoSaveDocument, $documentState) {
 				$this->saveFromText = true;
 				$file->putContent($autoSaveDocument);
-				if ($documentState) {
+				if ($documentState !== null) {
 					$this->writeDocumentState($file->getId(), $documentState);
 				}
 			});
@@ -546,9 +546,9 @@ public function getFileByShareToken(string $shareToken, ?string $path = null): F
 	}
 
 
-	public function isReadOnly(File $file, string|null $token): bool {
+	public function isReadOnly(File $file, ?string $token): bool {
 		$readOnly = true;
-		if ($token) {
+		if ($token !== null) {
 			try {
 				$this->checkSharePermissions($token, Constants::PERMISSION_UPDATE);
 				$readOnly = false;

diff --git a/lib/Service/EncodingService.php b/lib/Service/EncodingService.php
@@ -38,7 +38,7 @@ class EncodingService {
 
 	public function encodeToUtf8(string $string): ?string {
 		$encoding = $this->detectEncoding($string);
-		if (!$encoding) {
+		if ($encoding === null) {
 			return null;
 		}
 
@@ -48,7 +48,7 @@ public function encodeToUtf8(string $string): ?string {
 
 	public function detectEncoding(string $string): ?string {
 		$bomDetect = $this->detectUtfBom($string);
-		if ($bomDetect) {
+		if ($bomDetect !== null) {
 			return $bomDetect;
 		}
 

diff --git a/lib/Service/SessionService.php b/lib/Service/SessionService.php
@@ -88,10 +88,10 @@ public function __construct(
 	public function initSession(int $documentId, ?string $guestName = null): Session {
 		$session = new Session();
 		$session->setDocumentId($documentId);
-		$userName = $this->userId ? $this->userId : $guestName;
+		$userName = $this->userId ?? $guestName;
 		$session->setUserId($this->userId);
 		$session->setToken($this->secureRandom->generate(64));
-		$session->setColor($this->getColorForGuestName($guestName));
+		$session->setColor($this->getColorForGuestName($guestName ?? ''));
 		if ($this->userId === null) {
 			$session->setGuestName($guestName ?? '');
 		}
@@ -236,8 +236,8 @@ public function updateSessionAwareness(Session $session, string $message): Sessi
 		return $this->sessionMapper->update($session);
 	}
 
-	private function getColorForGuestName(?string $guestName = null): string {
-		$guestName = $this->userId ?? $guestName;
+	private function getColorForGuestName(string $guestName = ''): string {
+		$guestName = $this->userId !== null ? $guestName : '';
 		$uniqueGuestId = !empty($guestName) ? $guestName : $this->secureRandom->generate(12);
 		$color = $this->avatarManager->getGuestAvatar($uniqueGuestId)->avatarBackgroundColor($uniqueGuestId);
 		return $color->name();

diff --git a/tests/psalm-baseline.xml b/tests/psalm-baseline.xml
@@ -1,35 +1,46 @@
 <?xml version="1.0" encoding="UTF-8"?>
-<files psalm-version="5.13.1@086b94371304750d1c673315321a55d15fc59015">
+<files psalm-version="5.23.1@8471a896ccea3526b26d082f4461eeea467f10a4">
+  <file src="lib/Controller/AttachmentController.php">
+    <NullArgument>
+      <code><![CDATA[null]]></code>
+    </NullArgument>
+  </file>
   <file src="lib/Controller/WorkspaceController.php">
     <UndefinedInterfaceMethod>
-      <code>open</code>
+      <code><![CDATA[open]]></code>
     </UndefinedInterfaceMethod>
   </file>
   <file src="lib/DAV/WorkspacePlugin.php">
     <UndefinedClass>
-      <code>ServerPlugin</code>
+      <code><![CDATA[ServerPlugin]]></code>
     </UndefinedClass>
   </file>
   <file src="lib/DirectEditing/TextDirectEditor.php">
     <UndefinedInterfaceMethod>
-      <code>getToken</code>
+      <code><![CDATA[getToken]]></code>
     </UndefinedInterfaceMethod>
   </file>
+  <file src="lib/Service/EncodingService.php">
+    <RiskyTruthyFalsyComparison>
+      <code><![CDATA[mb_detect_encoding($string, $this->getEncodings(), true)]]></code>
+      <code><![CDATA[mb_detect_order()]]></code>
+    </RiskyTruthyFalsyComparison>
+  </file>
   <file src="vendor/nextcloud/ocp/OCP/AppFramework/Controller.php">
     <MissingClosureParamType>
-      <code>$data</code>
+      <code><![CDATA[$data]]></code>
     </MissingClosureParamType>
     <MissingClosureReturnType>
-      <code>function ($data) {</code>
+      <code><![CDATA[function ($data) {]]></code>
     </MissingClosureReturnType>
     <PossiblyInvalidArgument>
       <code><![CDATA[$data->getData()]]></code>
     </PossiblyInvalidArgument>
   </file>
   <file src="vendor/nextcloud/ocp/OCP/AppFramework/OCSController.php">
     <MissingClosureParamType>
-      <code>$data</code>
-      <code>$data</code>
+      <code><![CDATA[$data]]></code>
+      <code><![CDATA[$data]]></code>
     </MissingClosureParamType>
   </file>
 </files>