Skip to content

Types: add return type declaration to jsonSerialize methods - #552

Open
SirLouen wants to merge 2 commits into
WordPress:developfrom
SirLouen:types/jsonSerialize-arrays
Open

SirLouen wants to merge 2 commits into
WordPress:developfrom
SirLouen:types/jsonSerialize-arrays

Conversation

@SirLouen

@SirLouen SirLouen commented Dec 21, 2025 •

Copy link
Copy Markdown
Member

What?

There are plenty of deprecation errors to be solved.

Fixes #634

Why?

jsonSerialize return types are missing

@faisalahammad

Copy link
Copy Markdown

Tested this against current develop, since #634 reports the same notices on PHP 8.4 and 8.5.

  • The branch is 11 commits behind develop but merges clean.
  • On PHP 8.5.11 and 8.4.26, loading the 5 classes from develop gives 5 "Return type of ...::jsonSerialize()" deprecation notices. With this PR merged into develop there are none.
  • PHPUnit on PHP 8.4 (wp-env): 62 tests, 84 assertions, 24 skipped, no failures. Same as develop.
  • PHPCS with the current ruleset is clean. On the branch as it is, PHPCS fails because the branch still has the old phpcs.xml.dist (WordPress.CodeAnalysis.EmptyStatement is gone in WPCS 3) plus a few style issues already fixed on develop. A rebase on develop should fix that.
  • PHP 7.4.33: php -l passes on all 5 files and the classes load fine. : array is valid since PHP 7.0.

The red checks here look like a CI infra problem from December, not this change. The jobs exited with code 127 before any step ran. The logs have expired, so I can't confirm more than that. A rebase should re-run CI.

One small note: these methods are not final, so a class outside the plugin that extends them and overrides jsonSerialize() without a return type would now get a fatal error. I found none in this repo, and this is a feature plugin, so it should be fine.

Could you rebase and add "Fixes #634" to the description so the issue closes on merge?

@masteradhoc masteradhoc left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Thank you @SirLouen for the PR! This looks good to me. I've added "Fixes" to the PR description and rebased it. Good to get merged @jonathanbossenger

This branch has not been deployed

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Deprecated messages on PHP 8.5

3 participants