Skip to content

Commit

Permalink
fix(lint): avoid risky truthy falsy comparison
Browse files Browse the repository at this point in the history
Signed-off-by: Max <[email protected]>
  • Loading branch information
max-nextcloud authored and juliusknorr committed Apr 4, 2024
1 parent 0d67aa0 commit d0f5952
Show file tree
Hide file tree
Showing 10 changed files with 49 additions and 45 deletions.
12 changes: 6 additions & 6 deletions lib/Controller/AttachmentController.php
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand All @@ -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);
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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();

Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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 {
Expand Down
2 changes: 1 addition & 1 deletion lib/Controller/SessionController.php
Original file line number Diff line number Diff line change
Expand Up @@ -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]
Expand Down
11 changes: 6 additions & 5 deletions lib/Controller/UserApiController.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
}

Expand Down
3 changes: 3 additions & 0 deletions lib/Db/StepMapper.php
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down
6 changes: 3 additions & 3 deletions lib/Service/ApiService.php
Original file line number Diff line number Diff line change
Expand Up @@ -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'));

/*
Expand All @@ -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) {
Expand Down Expand Up @@ -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);
}

Expand Down
14 changes: 4 additions & 10 deletions lib/Service/AttachmentService.php
Original file line number Diff line number Diff line change
Expand Up @@ -203,21 +203,15 @@ private function getMediaFilePreviewFile(string $mediaFileName, File $textFile):
}

/**
* @param int $documentId
* @param string|null $userId
* @param Session|null $session
* @param string|null $shareToken
*
* @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 {
if ($shareToken) {
if ($shareToken !== null) {
$textFile = $this->getTextFilePublic($documentId, $shareToken);
} elseif ($userId) {
} elseif ($userId !== null) {
$textFile = $this->getTextFile($documentId, $userId);
} else {
throw new NotPermittedException('Unable to read document');
Expand All @@ -229,15 +223,15 @@ public function getAttachmentList(int $documentId, ?string $userId = null, ?Sess
return [];
}

$shareTokenUrlString = $shareToken
$shareTokenUrlString = $shareToken !== null
? '&shareToken=' . rawurlencode($shareToken)
: '';
$urlParamsBase = $session
? '?documentId=' . $documentId . '&sessionId=' . $session->getId() . '&sessionToken=' . rawurlencode($session->getToken()) . $shareTokenUrlString
: '?documentId=' . $documentId . $shareTokenUrlString;

$attachments = [];
$userFolder = $userId ? $this->rootFolder->getUserFolder($userId) : null;
$userFolder = $userId !== null ? $this->rootFolder->getUserFolder($userId) : null;
foreach ($attachmentDir->getDirectoryListing() as $node) {
if (!($node instanceof File)) {
// Ignore anything but files
Expand Down
12 changes: 6 additions & 6 deletions lib/Service/DocumentService.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down Expand Up @@ -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)) {
Expand Down Expand Up @@ -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);
Expand All @@ -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);
}
});
Expand Down Expand Up @@ -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;
Expand Down
4 changes: 2 additions & 2 deletions lib/Service/EncodingService.php
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ class EncodingService {

public function encodeToUtf8(string $string): ?string {
$encoding = $this->detectEncoding($string);
if (!$encoding) {
if ($encoding === null) {
return null;
}

Expand All @@ -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;
}

Expand Down
8 changes: 4 additions & 4 deletions lib/Service/SessionService.php
Original file line number Diff line number Diff line change
Expand Up @@ -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 ?? '');
}
Expand Down Expand Up @@ -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();
Expand Down
22 changes: 14 additions & 8 deletions tests/psalm-baseline.xml
Original file line number Diff line number Diff line change
@@ -1,35 +1,41 @@
<?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/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>

0 comments on commit d0f5952

Please sign in to comment.