Skip to content

fix(hooks): support Closure as dynamic filter reply - #272

Open
faisalahammad wants to merge 1 commit into
10up:trunkfrom
faisalahammad:fix/235-closure-dynamic-reply-for-filters
Open

fix(hooks): support Closure as dynamic filter reply#272
faisalahammad wants to merge 1 commit into
10up:trunkfrom
faisalahammad:fix/235-closure-dynamic-reply-for-filters

Conversation

@faisalahammad

Copy link
Copy Markdown

Summary

Fixes #235. A Closure passed to WP_Mock::expectFilter(, ) or to onFilter()->with(...)->reply() is now invoked at runtime with the actual filter arguments and its return value becomes the filter reply. This mirrors the WP_Mock::userFunction(..., ['return' => ) pattern that already exists for mocked functions.

Changes

php/WP_Mock/Filter.php

Why: The trie-walk in Filter::apply() had no path to a closure reply when the runtime argument offsets did not match the stored bucket key, and Filter_Responder::send() returned any non-InvokedFilterValue value verbatim, leaking the closure object out of apply_filters().

  • Filter_Responder::send(): new instanceof \Closure branch ahead of the existing InvokedFilterValue check. The closure is invoked with the runtime args.
  • Filter::apply(): new fall-through to a '__CLOSURE__' bucket when the trie walk misses. The argsnull branch now forwards `` to send() (was passing no args) so a closure stored in either bucket receives the runtime filter values.

php/WP_Mock.php

Why: The existing expectFilter() signature accepts variadic args that get forwarded to onFilter()->with(...). With a closure as a variadic arg, the existing InvokedFilterValue shim path was a no-op for closures, so a separate short-circuit was needed.

  • expectFilter(): detects a \Closure in the variadic args and routes through onFilter()->with(\Closure::class)->reply(). The Intercept mock setup is skipped in this path (assertions still work via onFilter's existing EventManager::called call).

tests/Unit/WP_MockTest.php

Why: Three PHPUnit tests exercise the new behavior end-to-end:

  • testExpectFilterWithClosureRepliesDynamically: confirms expectFilter('foo', $closure) returns the closure's value, and that the closure is invoked per call.
  • testOnFilterWithClosureReplyIsInvokedWithRuntimeArgs: confirms the lower-level onFilter(...)->with(\Closure::class)->reply($closure) works and receives the runtime filter args.
  • testOnFilterWithClosureMatcherAndClosureReply: confirms the '__CLOSURE__' fall-through path is reachable.

features/hooks.feature and features/bootstrap/HooksContext.php

Why: BDD coverage for the new behavior via a dedicated step I expect filter :filter to reply dynamically with a :operation closure.

features/bootstrap/FunctionsContext.php

Why: Unrelated to the feature, but blocked the Behat suite from running. A pre-existing parse error (missing semicolon) and a step-collision in the iExcpectWithoutR typo method prevented the suite from running at all. Without this fix, the Behat suite returns a fatal error and CI never executes the feature scenarios.

phpstan-baseline.neon

Why: Two new call_user_func_array(array(..., 'send'), $args) call sites in Filter::apply() match an existing baseline pattern; updated count from 1 to 4.

docs/usage/mocking-wp-action-and-filter-hooks.md

Added a ## Dynamic return values with a Closure section with examples for both the high-level and low-level APIs.

CHANGELOG.md

Added an Unreleased entry under ### Added for the new feature and ### Removed for the typo method.

Testing

composer install
vendor/bin/phpunit --testsuite Unit          # 163 tests, 1 skipped, 0 failures
vendor/bin/phpunit --testsuite Integration   # 29 tests, 0 failures
vendor/bin/behat                             # 41 passed, 1 pre-existing failure
vendor/bin/phpstan analyse --memory-limit=1G # 0 errors
vendor/bin/phpcs                            # clean

The pre-existing Behat failure (expectFilterNotAdded fails when filter added) is unrelated to this fix and exists on trunk.

Reproduction

Before the fix, the following test fails because the closure is returned as a Closure object from apply_filters() instead of being invoked:

WP_Mock::expectFilter('my_filter', function ($value) {
    return strtoupper($value);
});

apply_filters('my_filter', 'hello'); // returns 'hello', expected 'HELLO'

After the fix, the same call returns 'HELLO'.

- WP_Mock::expectFilter('foo', $closure) now invokes the closure at
  runtime with the actual filter arguments and uses its return value
  as the filter reply. Mirrors the userFunction closure-return pattern
  requested in 10up#235.
- The lower-level onFilter('foo')->with(...)->reply($closure) also
  works: Filter_Responder::send() gains a Closure branch that invokes
  the stored closure with the runtime args, ahead of the existing
  InvokedFilterValue and raw-value branches.
- Filter::apply() falls through to a __CLOSURE__ bucket when the trie
  walk misses, and forwards runtime args in the argsnull branch so a
  closure stored there also receives them.
- Fix pre-existing Behat parse error in features/bootstrap/FunctionsContext.php
  (missing semicolon) and remove a step-collision from the iExcpectWhenIRun
  typo method, which prevented the entire Behat suite from running.

Closes 10up#235
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Pass in Closure for Hooks

1 participant