#3672 closed defect (bug) (fixed)
bbp_get_reply_position_raw() triggers PHP 8.3 deprecation: increment on bool
| Reported by: |
|
Owned by: |
|
|---|---|---|---|
| Milestone: | 2.7 | Priority: | normal |
| Severity: | normal | Version: | 2.1 |
| Component: | Component - Replies | Keywords: | has-patch php8 php83 deprecation |
| Cc: |
Description
This ticket was AI generated
On PHP 8.3+, bbp_get_reply_position_raw() emits:
PHP Warning: Increment on type bool has no effect, this will change in the next major version of PHP in .../bbpress/includes/replies/functions.php on line 2374
(In trunk the warning shifts up one line because the operator is now pre-increment, but the bug is identical.)
The deprecation is exposing a latent silent-wrong-answer bug, not just a syntactic issue — see below.
Affected code
<?php $topic_replies = array_reverse( $topic_replies ); $reply_position = array_search( $reply_id, $topic_replies, true ); // Bump the position to compensate for the lead topic post ++$reply_position;
array_search() returns false when the reply isn't in the children
list returned by bbp_get_all_child_ids(). That list is built by
(see includes/common/functions.php:2067):
SELECT ID FROM wp_posts WHERE post_parent = %d AND post_status NOT IN ('draft', 'future') AND post_type = %s ORDER BY ID DESC
So array_search returns false whenever any of these are true:
- The reply has
post_statusofdraftorfuture— both excluded by the SQL. - The reply's
post_parentdoesn't match the$topic_idpassed in (callers pass an explicit$topic_idthat's stale, or the reply was moved). - A plugin filters the reply out via the
bbp_get_all_child_idsfilter (moderation plugins hiding pending replies, etc.). - Object-cache staleness for the
bbpress_postscache group. $reply_idresolves to0or to a reply outside this topic.
In all of those, $reply_position is false, ++false is a deprecated no-op (fatal in PHP 9), and the function returns (int) false === 0 — silently claiming the reply is in lead-post position.
Steps to reproduce
- PHP 8.3+.
- Create a topic; create a reply; set the reply's
post_statustodraft(or use a plugin that filtersbbp_get_all_child_idsto remove the reply). - Call
bbp_get_reply_position_raw( $reply_id, $topic_id )(or hit any page that does — reply permalinks, pagination, anchor links). - Observe deprecation in the error log.
Proposed patch
--- a/src/includes/replies/functions.php +++ b/src/includes/replies/functions.php @@ $topic_replies = bbp_get_all_child_ids( $topic_id, bbp_get_reply_post_type() ); if ( ! empty( $topic_replies ) ) { // Reverse replies array and search for current reply position $topic_replies = array_reverse( $topic_replies ); $reply_position = array_search( $reply_id, $topic_replies, true ); - // Bump the position to compensate for the lead topic post - ++$reply_position; + // Bump the position to compensate for the lead topic post + if ( false !== $reply_position ) { + ++$reply_position; + } else { + $reply_position = 0; + } }
Preserves the current return-0 behaviour when the reply can't be located (no behavioural change for downstream consumers), while removing the deprecated bool-increment.
Environment
PHP 8.4.21, bbPress 2.7.0-alpha-2.
Change History (4)
This ticket was mentioned in PR #42 on bbpress/bbPress by @dd32.
5 days ago
#1
@johnjamesjacoby commented on PR #42:
4 days ago
#2
Fixed in trunk for 2.7 via https://bbpress.trac.wordpress.org/changeset/7408
Fixes the PHP 8.1+
Implicit conversion from float to int loses precisiondeprecation emitted every time the Akismet history metabox renders for a topic or reply:## Root cause
BBP_Akismet::update_post_history()stores a fractionalmicrotime()float in each_bbp_akismet_historyevent:// This used to be akismet_microtime() but it was removed in 3.0 $mtime = explode( ' ', microtime() ); $message_time = $mtime[1] + $mtime[0]; $event = array( 'time' => $message_time, // float, e.g. 1779210785.780218 ... );The two readers —
gmdate()andbbp_time_since()— both expectintand operate on whole seconds, so the sub-second precision is recorded, persisted to post meta forever, then thrown away on every read. PHP 8.1+ deprecates the implicit lossy float-to-int conversion at the read site.The inlined
explode(' ', microtime())+ add pattern is a relic from the pre-PHP-5.0 string-form ofmicrotime()(the inline comment says "This used to beakismet_microtime()but it was removed in 3.0"). It was inlined into bbPress verbatim when Akismet 3.0 dropped the helper, but the rationale for sub-second precision didn't carry over to bbPress.## Fix
Two changes that need to ship together:
time()(int) so new history rows don't have unused sub-second precision.(int)on the two consumers (gmdate()andbbp_time_since()), so the deprecation also goes away for the millions of legacy float rows already in_bbp_akismet_historypost meta.Read-site-only would still create new float rows; write-site-only would still warn forever on existing data.
## Related
## Test environment
PHP 8.4.21, bbPress 2.7.0-alpha-2 / current trunk.