Make WordPress Core

Opened 7 months ago

Last modified 7 weeks ago

#60190 new enhancement

Drop PHP 4 support for arrays with 1 object element

Reported by: kkmuffme's profile kkmuffme Owned by:
Milestone: Future Release Priority: normal
Severity: normal Version:
Component: Plugins Keywords: has-patch has-unit-tests
Focuses: Cc:

Description (last modified by sabernhardt)

do_action( 'foo', array( (object) null ) );

function foo_cb( array $arg ) {}
add_action( 'foo', 'foo_cb' );

This will fail with a fatal error in PHP 7+, since do_action (as the only one, this isn't an issue with apply_filters or do_action_ref_array) will convert arrays with only 1 object into that object.
The reason for that is some PHP 4 legacy behavior, see #48312

It's time to drop this PHP 4 support, as this prevent using native types in callback functions in cases where the argument is an array of objects.

Change History (11)

This ticket was mentioned in PR #5839 on WordPress/wordpress-develop by @kkmuffme.


7 months ago
#1

  • Keywords has-patch has-unit-tests added

Drop PHP 4 support for do_action with 1 array<object> argument where the array contains only 1 element, in favor of better support for PHP 7+ native types

#2 @sabernhardt
7 months ago

  • Description modified (diff)

This ticket was mentioned in Slack in #core by kkmuffme. View the logs.


6 months ago

#4 @swissspidy
6 months ago

This will fail with a fatal error in PHP 7+

Can you elaborate on that? The existing test_action_args_with_php4_syntax test seems to work just fine without triggering errors.

#5 @kkmuffme
6 months ago

As outlined in OP it will lead to a fatal if any callback uses PHP native types for array; you'll get:

Fatal error: Uncaught TypeError: ... must be of type array, stdClass given

Now you could say: just use a union type array|object, but that is:
a) only available since PHP 8+
b) unnecessarily weakening the PHP type system to keep PHP 4 compat that has been deprecated since a decade at this point.

#6 @swissspidy
5 months ago

  • Keywords needs-unit-tests added; has-unit-tests removed
  • Milestone changed from Awaiting Review to Future Release
  • Type changed from defect (bug) to enhancement
  • Version trunk deleted

As outlined in OP it will lead to a fatal if any callback uses PHP native types for array; you'll get:

A test to demonstrate this would be great, just for completeness sake

Right now this patch removes a unit test, but the test actually still passes with the proposed patch, so why remove it?

@kkmuffme commented on PR #5839:


4 months ago
#7

Yes, I added a test now (which would result in a fatal error without this PR on standard WP 6.5)

#8 @kkmuffme
4 months ago

  • Keywords has-unit-tests added; needs-unit-tests removed

Right now this patch removes a unit test, but the test actually still passes with the proposed patch, so why remove it?

Tested it, not true. The test fails. Removed it again.

A test to demonstrate this would be great, just for completeness sake

Done. This test would result in a fatal error without the changes of this PR (tested on WP 6.5)
(had some confusion initially, since I wasn't familiar how the args are tested/returned in WP phpunit tests)

This ticket was mentioned in Slack in #core by kkmuffme. View the logs.


4 months ago

This ticket was mentioned in Slack in #core by kkmuffme. View the logs.


2 months ago

This ticket was mentioned in Slack in #core by mikachan. View the logs.


7 weeks ago

Note: See TracTickets for help on using tickets.