Make WordPress Core

Opened 2 years ago

Closed 20 months ago

Last modified 20 months ago

#54997 closed defect (bug) (fixed)

untrailingslashit() throws a deprecation notice for null on PHP 8.1

Reported by: haozi's profile haozi Owned by:
Milestone: 6.1 Priority: normal
Severity: normal Version: 6.0
Component: Formatting Keywords: has-patch php81 close
Focuses: Cc:

Description

In PHP8.1,WordPress will throw rtrim(): Passing null to parameter #1 ($string) of type string is deprecated

To fix it, I try to add (string) in /wp-includes/formatting.php on line 2761 like:

        return rtrim( (string) $string, '/\\' );

It works well!

Change History (20)

This ticket was mentioned in PR #2249 on WordPress/wordpress-develop by DevHaoZi.


2 years ago
#1

In PHP8.1,WordPress will throw rtrim(): Passing null to parameter #1 ($string) of type string is deprecated.

Trac ticket: https://core.trac.wordpress.org/ticket/54997

#2 @costdev
2 years ago

  • Focuses administration removed
  • Keywords php81 added
  • Milestone changed from Awaiting Review to 6.0

#4 @jrf
2 years ago

Deprecations are not errors. The functions in the formatting.php file all suffer from similar issues and we should implement a proper, consistent solution for all, which is why these issues have not been addressed yet as a discussion is needed on the solution direction.

The only time it would be possible to get this deprecation notice, is if the function is being called with a null as the $string parameter, which is doing it wrong. More than anything, the incorrect function call, should be fixed. Not this function.

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


2 years ago

#6 @costdev
2 years ago

  • Milestone changed from 6.0 to 6.1

As the solution to deprecation notices is still needed, I'm moving this ticket to the 6.1 milestone.

#8 @SergeyBiryukov
2 years ago

#56084 was marked as a duplicate.

#9 @SergeyBiryukov
2 years ago

  • Component changed from General to Formatting

Moving to the Formatting component, as the deprecation notice comes from the untrailingslashit() function.

#10 @SergeyBiryukov
21 months ago

#56485 was marked as a duplicate.

#11 @SergeyBiryukov
21 months ago

  • Summary changed from PHP8.1 throw 'deprecated' in WordPress 6.0-alpha-52650 to untrailingslashit() throws a deprecation notice for null on PHP 8.1

This ticket was mentioned in Slack in #core-test by martin.krcho. View the logs.


20 months ago

#13 @dingo_d
20 months ago

#56633 was marked as a duplicate.

This ticket was mentioned in PR #3355 on WordPress/wordpress-develop by creedally-dev.


20 months ago
#14

…-includes\formatting.php on line 2789

Trac ticket:

#15 @adarshakshatcreedally
20 months ago

@SergeyBiryukov Kindly check the patch

#16 @jrf
20 months ago

Without a backtrace it is impossible to determine where this is coming from.

@adarshakshatcreedally I appreciate your willingness to help with this, but your patch is not necessarily related to this issue and there is already a more comprehensive patch available for the issue you are trying to fix: https://github.com/WordPress/wordpress-develop/pull/3186

#19 @Cybr
20 months ago

  • Keywords close added

I dumped a backtrace, and the problem lies in WP_Scripts::print_translations() calling load_script_textdomain(), where the third parameter given is NULL. This is further detailed by @jrf in https://github.com/WordPress/wordpress-develop/pull/3186.

There is argued that the call to unstrailingslashit() from load_script_textdomain() needs fixing and must test for is_string( $path ).

(1) However, for load_script_textdomain(), I reckon we should change $path from default NULL to '', as the PHPdoc for the function also requires, and only expect a string henceforth.

The same goes for function unstrailingslashit(): guide via PHPDoc that it requires a string (which it already does), do nothing further but let PHP send out warnings via rtrim(). Again, expect a string from developers.

(2) Finally, fix the actual offending caller WP_Scripts::print_translations() for passing NULL to $path.

That said, I just downloaded a copy of trunk and saw this issue was fixed as described in this comment ((1) and (2)) via pull https://github.com/WordPress/wordpress-develop/pull/2801. I think it's already fixed perfectly, thus marking for "close".

I highlighted "expect" above: If we'd add forceful type checks for every function call and property before requiring PHP 7.4 or later, we might as well develop a new server language. Even if we'd require PHP 7.4 or later, I still recommend staying clear from strictly typing because that is notoriously slow: https://3v4l.org/j7FPnW. I recommend keeping PHPDoc leading instead of imposing type declarations.

It isn't difficult to fix bugs, but it is inconceivable and detrimental to anticipate every mistake.

Last edited 20 months ago by Cybr (previous) (diff)

#20 @jrf
20 months ago

  • Resolution set to fixed
  • Status changed from new to closed

@Cybr Thanks for confirming my suspicion that this is the same issue as addressed in GH PR 2801 and GH PR 3186, for which commits were made yesterday combining the two patches to the most appropriate fix [54349] + [54351].

Closing as fixed.

#21 @ocean90
20 months ago

#56808 was marked as a duplicate.

Note: See TracTickets for help on using tickets.