Replace usage of strpos with str_contains

[spacedmonkey]

Replace usage of false === strpos or false !== strpos with str_contains. It makes the code more readable and use a PHP native function ( which may have a performance benefit ).

Follow on from #58012.

[spacedmonkey]

Pinging @SergeyBiryukov

[dingo_d]

str_starts_with is PHP 8+ syntax, so you cannot just replace strpos checks with it until the minimum PHP version is raised to PHP 8 if I'm not mistaken...

[SergeyBiryukov]

Replying to dingo_d:

str_starts_with is PHP 8+ syntax, so you cannot just replace strpos checks with it until the minimum PHP version is raised to PHP 8 if I'm not mistaken...

WordPress core includes a polyfill for str_contains() as of [52039] / #49652, as well as str_starts_with() and str_ends_with() as of [52040] / #54377, so the change should be safe to make, see the discussion in #58012.

This ticket is about str_contains() though, updating the description to avoid confusion.

[dingo_d]

Thanks for the clarification @SergeyBiryukov. Wasn't aware of the polyfills 👍🏻

[costdev]

+1 on these tickets @spacedmonkey!

Patchers: Be careful of loose checks like if ( strpos() ) and if ( ! strpos() ). These should be evaluated for both str_contains() and str_starts_with() (or strpos() > 0) to maintain backward compatibility and prevent possible security regressions.

Replace usage of strpos with str_contains.

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

[Soean]

I created a pull request for str_contains, I will create new tickets for str_starts_with and str_ends_with.

[spacedmonkey]

Replying to Soean:

I created a pull request for str_contains, I will create new tickets for str_starts_with and str_ends_with.

There is already a ticket for str_starts_with. #58012

@Soean Good start. I found some more places we could use this function.

​https://github.com/WordPress/wordpress-develop/blob/6d7430cfe1f8853b75787e03effe8f16f9242ad8/src/wp-admin/includes/theme.php#L1165

​https://github.com/WordPress/wordpress-develop/blob/6d7430cfe1f8853b75787e03effe8f16f9242ad8/src/wp-admin/includes/theme.php#L1167

​https://github.com/WordPress/wordpress-develop/blob/f3e65b1e441146d19f650971e4c932b358a7b10e/src/wp-admin/includes/ajax-actions.php#L3825

​https://github.com/WordPress/wordpress-develop/blob/f3e65b1e441146d19f650971e4c932b358a7b10e/src/wp-admin/includes/file.php#L2334

​https://github.com/WordPress/wordpress-develop/blob/6c47ed1dc697a81755d3892e41cede7b9a318d5b/src/wp-admin/includes/post.php#L1110

​https://github.com/WordPress/wordpress-develop/blob/6c47ed1dc697a81755d3892e41cede7b9a318d5b/src/wp-admin/includes/post.php#L1119

​https://github.com/WordPress/wordpress-develop/blob/f3e65b1e441146d19f650971e4c932b358a7b10e/src/wp-admin/includes/ajax-actions.php#L3362

​https://github.com/WordPress/wordpress-develop/blob/f3e65b1e441146d19f650971e4c932b358a7b10e/src/wp-includes/class-wp-query.php#L4414

​https://github.com/WordPress/wordpress-develop/blob/f3e65b1e441146d19f650971e4c932b358a7b10e/src/wp-includes/class-wp-query.php#L4414

​https://github.com/WordPress/wordpress-develop/blob/f3e65b1e441146d19f650971e4c932b358a7b10e/src/wp-includes/class-wp-query.php#L4524

​https://github.com/WordPress/wordpress-develop/blob/e4419d727ca99efff002ed7dbb70500e5093ed53/src/wp-includes/rss.php#L115

​https://github.com/WordPress/wordpress-develop/blob/a028958dd6f01427fcf2fda5dd74c3a3a543b44a/src/wp-includes/blocks/navigation-submenu.php#L266

​https://github.com/WordPress/wordpress-develop/blob/e4419d727ca99efff002ed7dbb70500e5093ed53/src/wp-includes/rss.php#L115

​https://github.com/WordPress/wordpress-develop/blob/a028958dd6f01427fcf2fda5dd74c3a3a543b44a/src/wp-includes/blocks/navigation-submenu.php#L266

​https://github.com/WordPress/wordpress-develop/blob/f3e65b1e441146d19f650971e4c932b358a7b10e/src/wp-includes/link-template.php#L4430

​https://github.com/WordPress/wordpress-develop/blob/f3e65b1e441146d19f650971e4c932b358a7b10e/src/wp-includes/media.php#L1300

​https://github.com/WordPress/wordpress-develop/blob/dcd1ba9330c23d3745c154eb11069ac6288c0665/src/wp-includes/option.php#L1263

​https://github.com/WordPress/wordpress-develop/blob/f3e65b1e441146d19f650971e4c932b358a7b10e/src/wp-includes/pluggable.php#L1236

​https://github.com/WordPress/wordpress-develop/blob/f3e65b1e441146d19f650971e4c932b358a7b10e/src/wp-includes/post.php#L6106

​https://github.com/WordPress/wordpress-develop/blob/e4419d727ca99efff002ed7dbb70500e5093ed53/src/wp-includes/rss.php#L115

​https://github.com/WordPress/wordpress-develop/blob/6c47ed1dc697a81755d3892e41cede7b9a318d5b/src/wp-includes/user.php#L3061

​https://github.com/WordPress/wordpress-develop/blob/f3e65b1e441146d19f650971e4c932b358a7b10e/src/wp-includes/rest-api/class-wp-rest-request.php#L309

@spacedmonkey Thanks, I added the post.php line.

In the first step I only changed the strpos( ) === false and strpos( ) !== false, as described in the ticket . The others we have to look at very closely, because if a sting starts with a substing strpos returns 0 which is false and str_contains returns true.

{{{php
$string = '/wp-content'
strpos( $string, '/' ) 0 -> false
str_contains( $string, '/') -> true
}}}

[spacedmonkey]

@Soean Can you rebase / merge trunk on PR. Core changes have added merge conflicts.

[azaozz]

I have "mixed feelings" about this. Imho replacing false === strpos( ... ) and false !== strpos( ... ) with str_contains() is premature.

The problem is that only ~20% of the current WP sites are on PHP 8+. So 80% will have to use the polyfill. Then the question is: how much slower is the polyfill compared to the native PHP 7.4 code? I did a quick comparison and it seems about 300% slower:

2.350807ms
false !== strpos(), 100000 times

7.260107ms
str_contains(), 100000 times

So it seems this should be postponed until (at least) more than half of the WP installs are on PHP 8+ as it reduces performance.

[spacedmonkey]

I run benchmarks against this change. I see a net posative.

500 runs of the benchmark - trunk ( 0746cc7324b4b025d1ad3f2afeeb2aadd7d1d19f ) - Theme tt1.

	Trunk - PHP 7.4	PR - PHP 7.4	Trunk - PHP 8.0	PR - PHP 8.0
Response Time (median)	79.64	79.6	79.4	79.31
wp-before-template (median)	32.88	32.93	32.39	32.24
wp-template (median)	40.59	40.42	40.09	40.59
wp-total (median)	73.72	73.37	72.57	72.77

There is not a notable improvement performance. As a code quality improvement, this is +1 from me. There are lots of parts of core that use str_contains. For consistency we should use it everywhere.

[spacedmonkey]

This change looks good to me. I think we are good to commit.

[azaozz]

Replying to spacedmonkey:

I run benchmarks against this change. I see a net posative.

Hmmmm, that's really strange? Why would the benchmark you're using miss the 300% slow-down of the PHP < 8 native code vs. the polyfill, and the PHP 8+ native code? Seems something's not right?

[spacedmonkey]

Maybe the performance lead, @flixos90 could take a look into this.

To be clear, if they’re a slight performance loss, I think this change is worth for the code quality improvement.

Worth noting, the performance team is currently working on docs on how we run our benchmarks. That way we could run the same benchmarks and we could see similar results.