Skip to content

docs(http): update return type for getBody#53679

Merged
susnux merged 2 commits intomasterfrom
debt/noid/wrong-return-type-iresponse
Jun 30, 2025
Merged

docs(http): update return type for getBody#53679
susnux merged 2 commits intomasterfrom
debt/noid/wrong-return-type-iresponse

Conversation

@kesselb
Copy link
Copy Markdown
Contributor

@kesselb kesselb commented Jun 25, 2025

Summary

The code style update in pull request #53625 appears to trigger a Psalm warning about Response returning null, which makes it non-compliant with the interface.

Checklist

@kesselb kesselb force-pushed the debt/noid/wrong-return-type-iresponse branch from 2781784 to 636af10 Compare June 25, 2025 13:34
@kesselb kesselb self-assigned this Jun 25, 2025
@kesselb kesselb added 3. to review Waiting for reviews technical debt 🧱 🤔🚀 labels Jun 25, 2025
@kesselb kesselb added this to the Nextcloud 32 milestone Jun 25, 2025
@kesselb kesselb marked this pull request as ready for review June 25, 2025 13:35
@kesselb kesselb requested a review from a team as a code owner June 25, 2025 13:35
@kesselb kesselb requested review from leftybournes and removed request for a team June 25, 2025 13:35
@kesselb kesselb marked this pull request as draft June 25, 2025 13:43
@kesselb
Copy link
Copy Markdown
Contributor Author

kesselb commented Jun 25, 2025

Changing the status back to draft, as there are some other changes needed.

image

@ChristophWurst ChristophWurst added 2. developing Work in progress and removed 3. to review Waiting for reviews labels Jun 25, 2025
@kesselb kesselb force-pushed the debt/noid/wrong-return-type-iresponse branch 2 times, most recently from a0aef55 to 33feb34 Compare June 27, 2025 08:57
@codecov
Copy link
Copy Markdown

codecov bot commented Jun 27, 2025

❌ Unsupported file format

Upload processing failed due to unsupported file format. Please review the parser error message:
Error deserializing json

Caused by:
expected value at line 1 column 1

For more help, visit our troubleshooting guide.

@kesselb kesselb marked this pull request as ready for review June 27, 2025 09:07
@kesselb kesselb added 3. to review Waiting for reviews and removed 2. developing Work in progress labels Jun 27, 2025
return $response->getBody();
$content = $response->getBody();

if (is_resource($content)) {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I’m always scared by is_resource calls because of the long ongoing migration from resources to objects in PHP.
See https://php.watch/articles/resource-object
File handles are not planned to move to object yet but that may happen someday.

What about:

				if (is_string($content) || $content === null) {
					return false;
				}
				return $content;

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure

kesselb added 2 commits June 30, 2025 11:50
Signed-off-by: Daniel Kesselberg <mail@danielkesselberg.de>
Signed-off-by: Daniel Kesselberg <mail@danielkesselberg.de>
@kesselb kesselb force-pushed the debt/noid/wrong-return-type-iresponse branch from 33feb34 to de54bdb Compare June 30, 2025 09:50
@susnux susnux merged commit 59ae739 into master Jun 30, 2025
222 of 226 checks passed
@susnux susnux deleted the debt/noid/wrong-return-type-iresponse branch June 30, 2025 11:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

3. to review Waiting for reviews technical debt 🧱 🤔🚀

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants