Type hook callbacks to match WP_Hook::$callbacks - #226
Conversation
The WordPress stubs now declare WP_Hook::$callbacks as
array<int, array<string, array{function: callable, accepted_args: int}>>,
so handing it the loosely typed array<mixed> the profiler passed around no
longer type checks.
Read the callbacks straight off the WP_Hook instance that stores them and
carry that shape through get_filter_callbacks(), set_filter_callbacks() and
$previous_filter_callbacks with a Hook_Callbacks alias, instead of the
inline @var that papered over the mismatch. WordPress has kept hooks in
WP_Hook instances since 4.7 and WP-CLI requires 4.9 or later, so the
pre-4.7 plain-array fallback in the getter goes away with it.
Co-Authored-By: Claude <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_018HA88i255g4t7hdAUkmW2A
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Team Run ID: 📒 Files selected for processing (1)
Included review availability: Your plan provides up to 2 included reviews per hour; 1 remains after this review. 📝 WalkthroughWalkthrough
ChangesFilter callback typing
Estimated code review effort: 2 (Simple) | ~10 minutes Merge Risk: ⚪ Minimal · up to This PR aligns internal callback handling with the supported WordPress hook representation without changing public behavior or system boundaries. No actionable merge-blocking risk remains beyond normal checks and review. Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
github-actions
Bot
added
bug
command:profile-hook
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
There was a problem hiding this comment.
🟡 Changes recommended
The new is_a( $wp_filter[ $filter ], 'WP_Hook' ) checks can fatally error on PHP 8+ if the value is not an object/string, so they should be guarded (e.g., is_object() check) to avoid runtime TypeErrors.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
This PR updates Profiler’s filter callback handling to align with the newer WordPress stub typing for WP_Hook::$callbacks, introducing a Hook_Callbacks PHPStan alias and propagating that shape through related helpers/state while removing the old pre-WP_Hook getter fallback.
Changes:
- Introduces a
@phpstan-type Hook_Callbacksalias matchingWP_Hook::$callbacksand updates$previous_filter_callbacksto use it. - Refactors
get_filter_callbacks()/set_filter_callbacks()docblocks and behavior to operate directly onWP_Hookinstances. - Removes the getter’s legacy array-based fallback path.
File summaries
| File | Description |
|---|---|
src/Profiler.php |
Adds a PHPStan type alias for hook callbacks and refactors getter/setter helpers to use WP_Hook::$callbacks directly. |
Review details
Suppressed comments (1)
src/Profiler.php:744
- Same
is_a()issue here: on PHP 8+ this can fatally error if$wp_filter[$filter]is not an object/string. Adding anis_object()guard keeps the fallback branch safe.
if ( is_a( $wp_filter[ $filter ], 'WP_Hook' ) ) {
$wp_filter[ $filter ]->callbacks = $callbacks;
} else {
$wp_filter[ $filter ] = $callbacks; // phpcs:ignore
}
- Files reviewed: 1/1 changed files
- Comments generated: 1
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| if ( ! isset( $wp_filter[ $filter ] ) || ! is_a( $wp_filter[ $filter ], 'WP_Hook' ) ) { | ||
| return false; | ||
| } | ||
|
|
||
| if ( is_a( $wp_filter[ $filter ], 'WP_Hook' ) ) { | ||
| $callbacks = $wp_filter[ $filter ]->callbacks; | ||
| } else { | ||
| $callbacks = $wp_filter[ $filter ]; | ||
| } | ||
| if ( is_array( $callbacks ) ) { | ||
| return $callbacks; | ||
| } | ||
| return false; | ||
| return $wp_filter[ $filter ]->callbacks; |
The WordPress stubs now declare WP_Hook::$callbacks as
array<int, array<string, array{function: callable, accepted_args: int}>>,
so handing it the loosely typed array the profiler passed around no
longer type checks.
Read the callbacks straight off the WP_Hook instance that stores them and
carry that shape through get_filter_callbacks(), set_filter_callbacks() and
$previous_filter_callbacks with a Hook_Callbacks alias, instead of the
inline @var that papered over the mismatch. WordPress has kept hooks in
WP_Hook instances since 4.7 and WP-CLI requires 4.9 or later, so the
pre-4.7 plain-array fallback in the getter goes away with it.
Co-Authored-By: Claude noreply@anthropic.com
Claude-Session: https://claude.ai/code/session_018HA88i255g4t7hdAUkmW2A
Summary by CodeRabbit