Refactor Callback Filter

- Removes runtime mutation of the filter - Drops inheritance Signed-off-by: George Steel <[email protected]>
laminas · Jun 29, 2024 · 6bc59af · 6bc59af
1 parent 0e6a88d
commit 6bc59af
Show file tree

Hide file tree

Showing 6 changed files with 205 additions and 233 deletions.
diff --git a/docs/book/v3/standard-filters.md b/docs/book/v3/standard-filters.md
@@ -304,22 +304,18 @@ $result = $filter->filter(2);
 
 ## Callback
 
-This filter allows you to use own methods in conjunction with `Laminas\Filter`. You
-don't have to create a new filter when you already have a method which does the
-job.
+This filter wraps any callable so that it can be invoked using the filter contract.
 
 ### Supported Options
 
 The following options are supported for `Laminas\Filter\Callback`:
 
 - `callback`: This sets the callback which should be used.
-- `callback_params`: This property sets the options which are used when the
-  callback is processed.
+- `callback_params`: Optional. When provided this should be an array where each element of the array is an additional argument to your callback.
 
 ### Basic Usage
 
-The usage of this filter is quite simple. In this example, we want to create a
-filter which reverses a string:
+The following example uses PHP's built-in function `strrev` as the callback:
 
 ```php
 $filter = new Laminas\Filter\Callback('strrev');
@@ -328,60 +324,46 @@ print $filter->filter('Hello!');
 // returns "!olleH"
 ```
 
-As you can see it's really simple to use a callback to define custom filters. It
-is also possible to use a method, which is defined within a class, by giving an
-array as the callback:
+As previously mentioned, any type of callable can be used as a callback, for example a closure:
 
 ```php
-class MyClass
-{
-    public static function reverse($param);
-}
+$filter = new Laminas\Filter\Callback([
+    'callback' => function (mixed $input): mixed {
+        if (is_string($input)) {
+            return $input . ' there!';
+        }
+
+        return $input;
+    },
+]);
 
-// The filter definition
-$filter = new Laminas\Filter\Callback(array('MyClass', 'reverse'));
-print $filter->filter('Hello!');
+$filter->filter('Hey'); // Returns 'Hey there!'
 ```
 
-As of PHP 5.5 you can use ::class resolution for given callback class:
+### Additional Callback Parameters
+
+If your callback requires additional arguments, these can be passed as a list, or an associative array to the `callback_params` option. The first argument will be the value to be filtered.
 
 ```php
-class MyClass
-{
-    public function __invoke($param);
+class MyClass {
+    public function __invoke(mixed $input, string $a, string $b): string
+    {
+        if (is_string($input)) {
+            return implode(', ', [$input, $a, $b]);
+        }
+
+        return 'Foo';
+    }
 }
 
-// The filter definition
-$filter = new Laminas\Filter\Callback(MyClass::class);
-print $filter->filter('Hello!');
-```
-
-To get the actual set callback use `getCallback()` and to set another callback
-use `setCallback()`.
-
-> ### Possible Exceptions
->
-> You should note that defining a callback method which can not be called will
-> raise an exception.
-
-### Default Parameters within a Callback
-
-It is also possible to define default parameters, which are given to the called
-method as an array when the filter is executed. This array will be concatenated
-with the value which will be filtered.
-
-```php
 $filter = new Laminas\Filter\Callback([
-    'callback' => 'MyMethod',
-    'options'  => ['key' => 'param1', 'key2' => 'param2']
+    'callback' => new MyClass(),
+    'callback_params' => [
+        'a' => 'baz',
+        'b' => 'bat',
+    ],
 ]);
-$filter->filter(['value' => 'Hello']);
-```
-
-Calling the above method definition manually would look like this:
-
-```php
-$value = MyMethod('Hello', 'param1', 'param2');
+$filter->filter('bing'); // returns 'bing,baz,bat'
 ```
 
 ## Compress and Decompress

diff --git a/psalm-baseline.xml b/psalm-baseline.xml
@@ -23,34 +23,6 @@
       <code><![CDATA[is_object($options)]]></code>
     </RedundantConditionGivenDocblockType>
   </file>
-  <file src="src/Callback.php">
-    <MixedFunctionCall>
-      <code><![CDATA[call_user_func_array($this->options['callback'], $params)]]></code>
-    </MixedFunctionCall>
-    <MixedInferredReturnType>
-      <code><![CDATA[array]]></code>
-      <code><![CDATA[callable]]></code>
-    </MixedInferredReturnType>
-    <MixedMethodCall>
-      <code><![CDATA[new $callback()]]></code>
-    </MixedMethodCall>
-    <MixedReturnStatement>
-      <code><![CDATA[$this->options['callback']]]></code>
-      <code><![CDATA[$this->options['callback_params']]]></code>
-    </MixedReturnStatement>
-    <NonInvariantDocblockPropertyType>
-      <code><![CDATA[$options]]></code>
-    </NonInvariantDocblockPropertyType>
-    <PossiblyInvalidArgument>
-      <code><![CDATA[$callbackOrOptions]]></code>
-    </PossiblyInvalidArgument>
-    <PossiblyUnusedMethod>
-      <code><![CDATA[getCallback]]></code>
-    </PossiblyUnusedMethod>
-    <RedundantCastGivenDocblockType>
-      <code><![CDATA[(array) $params]]></code>
-    </RedundantCastGivenDocblockType>
-  </file>
   <file src="src/Compress.php">
     <LessSpecificImplementedReturnType>
       <code><![CDATA[($option is null ? array : mixed)]]></code>
@@ -777,10 +749,12 @@
     </PossiblyUnusedMethod>
   </file>
   <file src="test/CallbackTest.php">
-    <InvalidArgument>
-      <code><![CDATA['param']]></code>
-      <code><![CDATA['param']]></code>
-    </InvalidArgument>
+    <PossiblyUnusedMethod>
+      <code><![CDATA[callbackProvider]]></code>
+      <code><![CDATA[callbackWithArgumentsProvider]]></code>
+      <code><![CDATA[staticMethodTest]]></code>
+      <code><![CDATA[staticMethodWithArgumentsTest]]></code>
+    </PossiblyUnusedMethod>
   </file>
   <file src="test/Compress/Bz2Test.php">
     <DocblockTypeContradiction>
@@ -1063,11 +1037,6 @@
       <code><![CDATA[returnUnfilteredDataProvider]]></code>
     </PossiblyUnusedMethod>
   </file>
-  <file src="test/TestAsset/CallbackClass.php">
-    <PossiblyUnusedMethod>
-      <code><![CDATA[objectCallbackWithParams]]></code>
-    </PossiblyUnusedMethod>
-  </file>
   <file src="test/ToFloatTest.php">
     <PossiblyUnusedMethod>
       <code><![CDATA[filterableValuesProvider]]></code>

diff --git a/src/Callback.php b/src/Callback.php
@@ -4,110 +4,45 @@
 
 namespace Laminas\Filter;
 
-use Traversable;
+use Closure;
 
 use function array_unshift;
-use function call_user_func_array;
-use function class_exists;
 use function is_callable;
-use function is_string;
 
 /**
  * @psalm-type Options = array{
- *     callback?: callable,
- *     callback_params?: array,
- *     ...
+ *     callback: callable(mixed): mixed,
+ *     callback_params?: array<array-key, mixed>,
  * }
- * @extends AbstractFilter<Options>
+ * @implements FilterInterface<mixed>
  */
-final class Callback extends AbstractFilter
+final class Callback implements FilterInterface
 {
-    /** @var array */
-    protected $options = [
-        'callback'        => null,
-        'callback_params' => [],
-    ];
+    /** @var Closure(mixed): mixed */
+    private readonly Closure $callback;
+    private readonly array $arguments;
 
     /**
-     * @param callable|array|string|Traversable $callbackOrOptions
-     * @param array $callbackParams
+     * @param callable(mixed): mixed|Options $options
      */
-    public function __construct($callbackOrOptions = [], $callbackParams = [])
+    public function __construct(array|callable $options)
     {
-        if (is_callable($callbackOrOptions) || is_string($callbackOrOptions)) {
-            $this->setCallback($callbackOrOptions);
-            $this->setCallbackParams($callbackParams);
-        } else {
-            $this->setOptions($callbackOrOptions);
-        }
+        $callback        = is_callable($options) ? $options : $options['callback'];
+        $arguments       = ! is_callable($options) ? $options['callback_params'] ?? [] : [];
+        $this->callback  = $callback(...);
+        $this->arguments = $arguments;
     }
 
-    /**
-     * Sets a new callback for this filter
-     *
-     * @param  callable $callback
-     * @throws Exception\InvalidArgumentException
-     * @return self
-     */
-    public function setCallback($callback)
-    {
-        if (is_string($callback) && class_exists($callback)) {
-            $callback = new $callback();
-        }
-
-        if (! is_callable($callback)) {
-            throw new Exception\InvalidArgumentException(
-                'Invalid parameter for callback: must be callable'
-            );
-        }
-
-        $this->options['callback'] = $callback;
-        return $this;
-    }
-
-    /**
-     * Returns the set callback
-     *
-     * @return callable
-     */
-    public function getCallback()
-    {
-        return $this->options['callback'];
-    }
-
-    /**
-     * Sets parameters for the callback
-     *
-     * @param  array $params
-     * @return self
-     */
-    public function setCallbackParams($params)
+    public function filter(mixed $value): mixed
     {
-        $this->options['callback_params'] = (array) $params;
-        return $this;
-    }
+        $params = $this->arguments;
+        array_unshift($params, $value);
 
-    /**
-     * Get parameters for the callback
-     *
-     * @return array
-     */
-    public function getCallbackParams()
-    {
-        return $this->options['callback_params'];
+        return ($this->callback)(...$params);
     }
 
-    /**
-     * Calls the filter per callback
-     *
-     * @param mixed $value Options for the set callable
-     * @return mixed Result from the filter which was called
-     */
-    public function filter(mixed $value): mixed
+    public function __invoke(mixed $value): mixed
     {
-        $params = (array) $this->options['callback_params'];
-        array_unshift($params, $value);
-
-        return call_user_func_array($this->options['callback'], $params);
+        return $this->filter($value);
     }
 }