WordPress.org

Make WordPress Core

Opened 21 months ago

Last modified 4 weeks ago

#49728 assigned defect (bug)

[PHP 8] Prepare for the internal functions throwing `TypeError` or `ValueError` exceptions on unexpected types/values

Reported by: ayeshrajans Owned by: hellofromTonya
Milestone: 6.0 Priority: normal
Severity: minor Version:
Component: Build/Test Tools Keywords: needs-patch needs-unit-tests needs-dev-note needs-codex php8
Focuses: Cc:

Description

Description

PHP 8.0 is still in the making, but we know that certain changes are coming up to make things more straightforward and in favor of exceptions instead of rather silent PHP warnings.

One of these changes is that from PHP 8+, internal functions will throw an exception if the function call arguments are of a type that is not expected.

The simplest example would be strlen([]), which would throw TypeError: strlen() expects parameter 1 to be string, array given exception in PHP 8. You can read more at the RFC (https://wiki.php.net/rfc/consistent_type_errors) and a detailed Change Record (https://php.watch/versions/8.0/internal-function-exceptions).

WordPress has a PHPUnit annotation with the name @expectedIncorrectUsage that are used to track these kinds of errors. One example is at Tests_DB::test_prepare_sprintf_invalid_args, which uses the @expectedIncorrectUsage annotation to track PHP warnings, but instead, PHP 8's new TypeError triggers an exception (https://travis-ci.com/github/Ayesh/wordpress-develop/jobs/308369270#L2827).

Proposal

I propose to extend the expectedIncorrectUsage annotation to check the PHP version, and make PHPUnit expect a TypeError or a ValueError. Ideally, we should be expecting exactly a TypeError or a ValueError (sprintf too few arguments for example), which we can pass either from the annotation in the caller, or a check on the parent/sibling classes.

Change History (15)

#1 @desrosj
18 months ago

  • Milestone changed from Awaiting Review to 5.6

Let's aim to get this in 5.6, though it is dependent on PHP8 entering the beta/RC stages in time.

#2 @desrosj
18 months ago

  • Keywords php8 added

Also, feel free to use the php8 keyword for anything related. It makes it easier to get the big picture of all those tickets.

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


15 months ago

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


15 months ago

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


13 months ago

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


13 months ago

#7 @desrosj
13 months ago

  • Milestone changed from 5.6 to 5.7

Looks like we were not able to circle back to this one during 5.6. RC1 is in less than 24 hours, and even though this is a test related ticket (which can still be committed during RC), there is no patch and a bit more discussion needed.

Punting to 5.7 for now, but if we're able to get this ready before 5.6 is released, we can move it back!

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


10 months ago

#10 @lukecarbis
10 months ago

  • Milestone changed from 5.7 to Future Release

This ticket was mentioned in Slack in #core by hareesh-pillai. View the logs.


4 months ago

#12 @SergeyBiryukov
4 months ago

  • Milestone changed from Future Release to 5.9

#13 @hellofromTonya
4 months ago

  • Owner set to hellofromTonya
  • Status changed from new to assigned

#14 @hellofromTonya
4 weeks ago

  • Milestone changed from 5.9 to Future Release

Moving this to the next cycle to be part of ongoing work in the tests and further PHP 8+ compatibility. As the next cycle is not yet available to select, moving to Future Release.

As test patches can be committed at any time into trunk, once there's a patch ready, please pull into the "current" milestone cycle.

#15 @hellofromTonya
4 weeks ago

  • Milestone changed from Future Release to 6.0

Hey 6.0 is now available. Moving this ticket into the cycle.

Note: See TracTickets for help on using tickets.