New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Clean up PHPDoc params and return values #40046

Open

summersab wants to merge 1 commit into nextcloud:master from summersab:PHPDoc-cleanup

+28 −28

Contributor

summersab commented Aug 25, 2023

Resolves: # N/A

Summary

This just cleans up some PHPDoc issues. Nothing critical. Let me know if I touched something I shouldn't have.

Checklist

Code is properly formatted
Sign-off message is added to all commits
Tests (unit, integration, api and/or acceptance) are included
Screenshots before/after for front-end changes
Documentation (manuals or wiki) has been updated or is not required
Backports requested where applicable (ex: critical bugfixes)

summersab requested review from ChristophWurst, miaulalala and tcitworld as code owners

August 25, 2023 16:31


          clean up return and param values

2c14407

Signed-off-by: Andrew Summers <[email protected]>

summersab force-pushed the PHPDoc-cleanup branch from f8c8093 to 2c14407 Compare

August 25, 2023 16:32

kesselb reviewed

View reviewed changes

apps/dav/lib/Connector/Sabre/FilesReportPlugin.php

@@ @@ -183,7 +183,7 @@ public function getSupportedReportSet($uri) { @@
               	public function onReport($reportName, $report, $uri) {
               		$reportTargetNode = $this->server->tree->getNodeForPath($uri);
               		if (!$reportTargetNode instanceof Directory || $reportName !== self::REPORT_NAME) {
-              			return;
+              			return false;

Contributor

kesselb Aug 25, 2023

Looks wrong. onReport is an event handler for sabre/event. false = cancel the event handler.

return without argument is null and will not cancel the event handler: https://3v4l.org/Eifl3

I don't know if we want to cancel the event handler here or not.

kesselb reviewed

View reviewed changes

lib/private/App/AppStore/Fetcher/Fetcher.php

@@ @@ -187,7 +187,7 @@ public function get($allowUnstable = false) { @@
               		// Refresh the file content
               		try {
-              			$responseJson = $this->fetch($ETag, $content, $allowUnstable);

Contributor

kesselb Aug 25, 2023

Oh. Please don't change it. I think the signature for fetch is actually wrong, but we should not touch it in this pr.

kesselb reviewed

View reviewed changes

lib/private/DB/Connection.php

@@ @@ -321,7 +321,7 @@ protected function logQueryToFile(string $sql): void { @@
               	 *
               	 * @param string $seqName Name of the sequence object from which the ID should be returned.
               	 *
-              	 * @return string the last inserted ID.
+              	 * @return int the last inserted ID.

Contributor

kesselb Aug 25, 2023

Our Connection class is a wrapper for doctrine: https://github.com/nextcloud/3rdparty/blob/9f53e69990ad79b4e25647c72ffc455918592c24/doctrine/dbal/src/Driver/Connection.php#L54

The return should be int|string|false

kesselb reviewed

View reviewed changes

Contributor

kesselb left a comment

Thanks 👍

kesselb added 3. to review developer experience technical debt labels

kesselb added this to the Nextcloud 28 milestone

github-advanced-security bot found potential problems

View reviewed changes

apps/dav/lib/CalDAV/CalDavBackend.php

@@ @@ -2602,7 +2602,7 @@ @@
               	 *
               	 * @param string $principalUri
               	 * @param string $objectUri
-              	 * @return array
+              	 * @return array|null

Check failure

Code scanning / Psalm

ImplementedReturnTypeMismatch

The inherited return type 'array<array-key, mixed>' for Sabre\CalDAV\Backend\SchedulingSupport::getSchedulingObject is different to the implemented return type for OCA\DAV\CalDAV\CalDavBackend::getschedulingobject 'array<array-key, mixed>|null'

Member

tcitworld Aug 28, 2023

This needs a PR in SabreDAV to fix the PHPDoc there as well.

apps/dav/lib/CalDAV/ResourceBooking/AbstractPrincipalBackend.php

@@ @@ -157,7 +157,7 @@ @@
               	 *
               	 * @param string $prefixPath
               	 *
-              	 * @return array
+              	 * @return array|null

Check failure

Code scanning / Psalm

ImplementedReturnTypeMismatch

The inherited return type 'array<array-key, mixed>' for Sabre\DAVACL\PrincipalBackend\BackendInterface::getPrincipalByPath is different to the implemented return type for OCA\DAV\CalDAV\ResourceBooking\AbstractPrincipalBackend::getprincipalbypath 'array<array-key, mixed>|null'

Member

tcitworld Aug 28, 2023

This needs a PR in SabreDAV to fix the PHPDoc there as well.

apps/dav/lib/CardDAV/CardDavBackend.php

@@ @@ -543,7 +543,7 @@ @@
               	 *
               	 * @param mixed $addressBookId
               	 * @param string $cardUri
-              	 * @return array
+              	 * @return array|false

Check failure

Code scanning / Psalm

ImplementedReturnTypeMismatch

The inherited return type 'array<array-key, mixed>' for Sabre\CardDAV\Backend\BackendInterface::getCard is different to the implemented return type for OCA\DAV\CardDAV\CardDavBackend::getcard 'array<array-key, mixed>|false'

Member

tcitworld Aug 28, 2023

This needs a PR in SabreDAV to fix the PHPDoc there as well.

apps/dav/lib/Connector/Sabre/Principal.php

@@ @@ -159,7 +159,7 @@ @@
               	 * getPrincipalsByPrefix.
               	 *
               	 * @param string $path
-              	 * @return array
+              	 * @return array|null

Check failure

Code scanning / Psalm

ImplementedReturnTypeMismatch

The inherited return type 'array<array-key, mixed>' for Sabre\DAVACL\PrincipalBackend\BackendInterface::getPrincipalByPath is different to the implemented return type for OCA\DAV\Connector\Sabre\Principal::getprincipalbypath 'array<array-key, mixed>|null'

Member

tcitworld Aug 28, 2023

This needs a PR in SabreDAV to fix the PHPDoc there as well.

apps/dav/lib/DAV/GroupPrincipalBackend.php

@@ @@ -101,7 +101,7 @@ @@
               	 * getPrincipalsByPrefix.
               	 *
               	 * @param string $path
-              	 * @return array
+              	 * @return array|null

Check failure

Code scanning / Psalm

ImplementedReturnTypeMismatch

The inherited return type 'array<array-key, mixed>' for Sabre\DAVACL\PrincipalBackend\BackendInterface::getPrincipalByPath is different to the implemented return type for OCA\DAV\DAV\GroupPrincipalBackend::getprincipalbypath 'array<array-key, mixed>|null'

Member

tcitworld Aug 28, 2023

This needs a PR in SabreDAV to fix the PHPDoc there as well.

apps/dav/lib/DAV/SystemPrincipalBackend.php

@@ @@ -65,7 +65,7 @@ @@
               	 * getPrincipalsByPrefix.
               	 *
               	 * @param string $path
-              	 * @return array
+              	 * @return array|null

Check failure

Code scanning / Psalm

ImplementedReturnTypeMismatch

The inherited return type 'array<array-key, mixed>' for Sabre\DAVACL\PrincipalBackend\BackendInterface::getPrincipalByPath is different to the implemented return type for OCA\DAV\DAV\SystemPrincipalBackend::getprincipalbypath 'array<array-key, mixed>|null'

Member

tcitworld Aug 28, 2023

This needs a PR in SabreDAV to fix the PHPDoc there as well.

lib/private/ContactsManager.php

@@ @@ -92,7 +92,7 @@ @@
               	 *
               	 * @param int $id the unique identifier to a contact
               	 * @param string $address_book_key identifier of the address book in which the contact shall be deleted
-              	 * @return bool successful or not
+              	 * @return bool|null successful or not

Check failure

Code scanning / Psalm

ImplementedReturnTypeMismatch

The inherited return type 'bool' for OCP\Contacts\IManager::delete is different to the implemented return type for OC\ContactsManager::delete 'bool|null'

lib/private/ContactsManager.php

@@ @@ -113,7 +113,7 @@ @@
               	 *
               	 * @param array $properties this array if key-value-pairs defines a contact
               	 * @param string $address_book_key identifier of the address book in which the contact shall be created or updated
-              	 * @return array representing the contact just created or updated
+              	 * @return array|null representing the contact just created or updated

Check failure

Code scanning / Psalm

ImplementedReturnTypeMismatch

The inherited return type 'array<array-key, mixed>' for OCP\Contacts\IManager::createOrUpdate is different to the implemented return type for OC\ContactsManager::createorupdate 'array<array-key, mixed>|null'

lib/private/TagManager.php

@@ @@ -63,7 +63,7 @@ @@
               	 * @param boolean $includeShared Whether to include tags for items shared with this user by others.
               	 * @param string $userId user for which to retrieve the tags, defaults to the currently
               	 * logged in user
-              	 * @return \OCP\ITags
+              	 * @return \OCP\ITags|null

Check failure

Code scanning / Psalm

ImplementedReturnTypeMismatch

The inherited return type 'OCP\ITags' for OCP\ITagManager::load is different to the implemented return type for OC\TagManager::load 'OCP\ITags|null'

skjnldsv mentioned this pull request

chore: 28.0.0 beta 1 #41228

Merged

This was referenced Nov 6, 2023

28.0.0 beta 2 #41314

Merged

28.0.0 beta 3 #41379

Merged

This was referenced Nov 14, 2023

28.0.0 beta 4 #41477

Merged

28.0.0 RC1 #41621

Merged

blizzz modified the milestones: Nextcloud 28, Nextcloud 29

This was referenced Mar 12, 2024

29.0.0 beta 2 #44152

Merged

29.0.0 beta 3 #44186

Merged

29.0.0 beta 4 #44275

Merged

This was referenced Mar 20, 2024

29.0.0 beta 5 #44361

Merged

29.0.0 beta 6 #44449

Merged

skjnldsv mentioned this pull request

29.0.0 RC1 #44552

Merged

81 tasks

skjnldsv modified the milestones: Nextcloud 29, Nextcloud 30

skjnldsv removed the 3. to review label

skjnldsv added 2. developing stale labels

This was referenced Jul 30, 2024

30.0.0 beta 2 #46870

Merged

30.0.0 beta 3 #46946

Merged

This was referenced Aug 5, 2024

30.0.0 beta 4 #47028

Merged

30.0.0 beta 5 #47100

Merged

skjnldsv mentioned this pull request

30.0.0 RC1 #47190

Merged

skjnldsv removed this from the Nextcloud 30 milestone

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

kesselb kesselb left review comments

ChristophWurst Awaiting requested review from ChristophWurst

miaulalala Awaiting requested review from miaulalala

tcitworld Awaiting requested review from tcitworld tcitworld is a code owner

At least 2 approving reviews are required to merge this pull request.

Labels

2. developing developer experience stale technical debt