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

[haozi]

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!

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

[SergeyBiryukov]

Related: #54730

[jrf]

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.

[costdev]

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

[SergeyBiryukov]

#56084 was marked as a duplicate.

[SergeyBiryukov]

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

[SergeyBiryukov]

#56485 was marked as a duplicate.

[dingo_d]

#56633 was marked as a duplicate.

…-includes\formatting.php on line 2789

Trac ticket:

[adarshakshatcreedally]

@SergeyBiryukov Kindly check the patch

[jrf]

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

Duplicate of ​https://github.com/WordPress/wordpress-develop/pull/3186, suggest: close.

Duplicate of ​https://github.com/WordPress/wordpress-develop/pull/3186, suggest: close.

[Cybr]

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.

[jrf]

@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.

[ocean90]

#56808 was marked as a duplicate.