Refactor StringTrim Filter

- Remove inheritance and implement `FilterInterface` directly - Remove getters and setters - Clean up and improve tests - Update docs and migration guide Signed-off-by: George Steel <[email protected]>
laminas · Sep 3, 2024 · 34cef83 · 34cef83
1 parent 7754a99
commit 34cef83
Show file tree

Hide file tree

Showing 5 changed files with 93 additions and 173 deletions.
diff --git a/docs/book/v3/migration/v2-to-v3.md b/docs/book/v3/migration/v2-to-v3.md
@@ -46,6 +46,17 @@ In addition to this, the default filter aliases have changed:
 
 The impact of the removal of these aliases will not affect you if you use a FQCN to retrieve filters from the plugin manager. If you experience `ServiceNotFoundException` errors, audit your usage of filters and the strings you use to retrieve them from the plugin manager and replace any outdated values with either the FQCN of the filter or a known, configured alias.
 
+### Changes to Individual Filters
+
+#### `StringTrim`
+
+The following methods have been removed:
+
+- `setCharList`
+- `getCharList`
+
+The constructor now only accepts an associative array of [documented options](../standard-filters.md#stringtrim).
+
 ## Removed Filters
 
 The following filters were deprecated in the 2.0.x series of releases and have now been removed:

diff --git a/docs/book/v3/standard-filters.md b/docs/book/v3/standard-filters.md
@@ -1336,17 +1336,12 @@ characters have been removed.
 ### Specifying alternate Characters
 
 ```php
-$filter = new Laminas\Filter\StringTrim(':');
-// or new Laminas\Filter\StringTrim(array('charlist' => ':'));
+$filter = new Laminas\Filter\StringTrim(['charlist' => ':']);
 
 print $filter->filter(' This is (my) content:');
 ```
 
-The above example returns `This is (my) content`. Notice that the whitespace
-characters and colon are removed. You can also provide a `Traversable` or an
-array with a `charlist` key. To set the desired character list after
-instantiation, use the `setCharList()` method. `getCharList()` returns the
-current character list.
+The above example returns ` This is (my) content`. Notice that only the colon is removed.
 
 ## StripNewlines
 

diff --git a/psalm-baseline.xml b/psalm-baseline.xml
@@ -560,12 +560,6 @@
       <code><![CDATA[is_string($suffix)]]></code>
     </DocblockTypeContradiction>
   </file>
-  <file src="src/StringTrim.php">
-    <PossiblyUndefinedArrayOffset>
-      <code><![CDATA[$this->options['charlist']]]></code>
-      <code><![CDATA[$this->options['charlist']]]></code>
-    </PossiblyUndefinedArrayOffset>
-  </file>
   <file src="src/StripTags.php">
     <MixedArgument>
       <code><![CDATA[$options['allowAttribs']]]></code>
@@ -935,7 +929,7 @@
   </file>
   <file src="test/StringTrimTest.php">
     <PossiblyUnusedMethod>
-      <code><![CDATA[getNonStringValues]]></code>
+      <code><![CDATA[defaultBehaviourDataProvider]]></code>
     </PossiblyUnusedMethod>
   </file>
   <file src="test/StripNewlinesTest.php">

diff --git a/src/StringTrim.php b/src/StringTrim.php
@@ -4,110 +4,58 @@
 
 namespace Laminas\Filter;
 
-use Traversable;
-
-use function is_array;
 use function is_string;
 use function preg_replace;
-use function strlen;
 
 /**
  * @psalm-type Options = array{
  *     charlist?: string|null,
  * }
- * @extends AbstractFilter<Options>
+ * @implements FilterInterface<string>
  */
-final class StringTrim extends AbstractFilter
+final class StringTrim implements FilterInterface
 {
-    /** @var Options */
-    protected $options = [
-        'charlist' => null,
-    ];
-
-    /**
-     * Sets filter options
-     *
-     * @param  string|Options|iterable|null $charlistOrOptions
-     */
-    public function __construct($charlistOrOptions = null)
-    {
-        if ($charlistOrOptions !== null) {
-            if (! is_array($charlistOrOptions) && ! $charlistOrOptions instanceof Traversable) {
-                $this->setCharList($charlistOrOptions);
-            } else {
-                $this->setOptions($charlistOrOptions);
-            }
-        }
-    }
-
-    /**
-     * Sets the charList option
-     *
-     * @param  string $charList
-     * @return self Provides a fluent interface
-     */
-    public function setCharList($charList)
-    {
-        if (! strlen($charList)) {
-            $charList = null;
-        }
+    private readonly string $charlist;
 
-        $this->options['charlist'] = $charList;
-
-        return $this;
-    }
-
-    /**
-     * Returns the charList option
-     *
-     * @return string|null
-     */
-    public function getCharList()
+    /** @param Options $options */
+    public function __construct(array $options = [])
     {
-        return $this->options['charlist'];
+        $list           = $options['charlist'] ?? '\\\\s';
+        $this->charlist = $list === '' ? '\\\\s' : $list;
     }
 
     /**
-     * Defined by Laminas\Filter\FilterInterface
-     *
      * Returns the string $value with characters stripped from the beginning and end
      *
-     * @psalm-return ($value is string ? string : mixed)
+     * @inheritDoc
      */
     public function filter(mixed $value): mixed
     {
         if (! is_string($value)) {
             return $value;
         }
-        $value = (string) $value;
 
-        $charlist = $this->options['charlist'];
-
-        if ($charlist === null) {
-            return $this->unicodeTrim($value);
-        }
-
-        return $this->unicodeTrim($value, $charlist);
+        return $this->unicodeTrim($value);
     }
 
     /**
      * Unicode aware trim method
-     * Fixes a PHP problem
-     *
-     * @param string $value
-     * @param string $charlist
-     * @return string
      */
-    protected function unicodeTrim($value, $charlist = '\\\\s')
+    private function unicodeTrim(string $value): string
     {
         $chars = preg_replace(
             ['/[\^\-\]\\\]/S', '/\\\{4}/S', '/\//'],
             ['\\\\\\0', '\\', '\/'],
-            $charlist
+            $this->charlist,
         );
 
         $pattern = '/^[' . $chars . ']+|[' . $chars . ']+$/usSD';
 
         return preg_replace($pattern, '', $value);
     }
+
+    public function __invoke(mixed $value): mixed
+    {
+        return $this->filter($value);
+    }
 }
diff --git a/test/StringTrimTest.php b/test/StringTrimTest.php
@@ -8,129 +8,101 @@
 use PHPUnit\Framework\Attributes\DataProvider;
 use PHPUnit\Framework\Attributes\Group;
 use PHPUnit\Framework\TestCase;
-use stdClass;
 
-use function mb_convert_encoding;
+use function mb_chr;
+use function str_repeat;
 
 class StringTrimTest extends TestCase
 {
-    private StringTrim $filter;
-
-    public function setUp(): void
-    {
-        $this->filter = new StringTrim();
-    }
-
-    /**
-     * Ensures that the filter follows expected behavior
-     */
-    public function testBasic(): void
+    /** @return array<string, array{0: mixed, 1: mixed}> */
+    public static function defaultBehaviourDataProvider(): array
     {
-        $valuesExpected = [
-            'string' => 'string',
-            ' str '  => 'str',
-            "\ns\t"  => 's',
+        return [
+            'Ascii, no whitespace'    => ['string', 'string'],
+            'Empty String'            => ['', ''],
+            'Only Ascii whitespace'   => ["   \n\t   ", ''],
+            'Only Unicode whitespace' => [str_repeat(mb_chr(0x2029), 10), ''],
+            'Narrow Spaces'           => [mb_chr(0x202F) . 'Foo' . mb_chr(0x202F), 'Foo'],
+            'Em Spaces'               => [mb_chr(0x2003) . 'Foo' . mb_chr(0x2003), 'Foo'],
+            'Thin Spaces'             => [mb_chr(0x2009) . 'Foo' . mb_chr(0x2009), 'Foo'],
+            'ZF-7183'                 => ['Зенд', 'Зенд'],
+            'ZF-170'                  => ['Расчет', 'Расчет'],
+            'ZF-7902'                 => ['/', '/'],
+            'ZF-10891'                => ['   Зенд   ', 'Зенд'],
+            // Non-String Input
+            'Null'    => [null, null],
+            'Integer' => [123, 123],
+            'Float'   => [1.23, 1.23],
+            'Array'   => [['Foo'], ['Foo']],
+            'Boolean' => [true, true],
         ];
-        foreach ($valuesExpected as $input => $output) {
-            self::assertSame($output, $this->filter->filter($input));
-        }
     }
 
-    /**
-     * Ensures that the filter follows expected behavior
-     */
-    public function testUtf8(): void
+    #[DataProvider('defaultBehaviourDataProvider')]
+    public function testDefaultBehaviour(mixed $input, mixed $expect): void
     {
-        $value = mb_convert_encoding("\xa0a\xa0", 'UTF-8', 'ISO-8859-1');
-        self::assertSame('a', $this->filter->filter($value));
+        $filter = new StringTrim();
+        self::assertSame(
+            $expect,
+            $filter->filter($input),
+        );
     }
 
-    /**
-     * Ensures that getCharList() returns expected default value
-     */
-    public function testGetCharList(): void
+    public function testAsciiCharListOption(): void
     {
-        self::assertSame(null, $this->filter->getCharList());
-    }
+        $filter = new StringTrim([
+            'charlist' => '@&*',
+        ]);
 
-    /**
-     * Ensures that setCharList() follows expected behavior
-     */
-    public function testSetCharList(): void
-    {
-        $this->filter->setCharList('&');
-        self::assertSame('&', $this->filter->getCharList());
+        self::assertSame('Foo', $filter->filter('**&&@@Foo@@&&**'));
+        self::assertSame('Foo', $filter->filter('Foo'));
+        self::assertSame('F&o&o', $filter->filter('F&o&o'));
     }
 
-    /**
-     * Ensures expected behavior under custom character list
-     */
-    public function testCharList(): void
+    public function testUnicodeCharListOption(): void
     {
-        $this->filter->setCharList('&');
-        self::assertSame('a&b', $this->filter->__invoke('&&a&b&&'));
-    }
+        $filter = new StringTrim([
+            'charlist' => '👍',
+        ]);
 
-    #[Group('Laminas-7183')]
-    public function testLaminas7183(): void
-    {
-        self::assertSame('Зенд', $this->filter->filter('Зенд'));
-    }
-
-    #[Group('Laminas-170')]
-    public function testLaminas170(): void
-    {
-        self::assertSame('Расчет', $this->filter->filter('Расчет'));
-    }
-
-    #[Group('Laminas-7902')]
-    public function testLaminas7902(): void
-    {
-        self::assertSame('/', $this->filter->filter('/'));
+        self::assertSame('Foo', $filter->filter('Foo👍👍'));
+        self::assertSame('Foo', $filter->filter('👍Foo👍'));
+        self::assertSame('Fo👍o', $filter->filter('Fo👍o👍'));
     }
 
     #[Group('Laminas-10891')]
     public function testLaminas10891(): void
     {
-        self::assertSame('Зенд', $this->filter->filter('   Зенд   '));
-        self::assertSame('Зенд', $this->filter->filter('Зенд   '));
-        self::assertSame('Зенд', $this->filter->filter('   Зенд'));
+        $filter = new StringTrim([
+            'charlist' => " \t\n\r\x0B・。",
+        ]);
 
-        $trimCharList = " \t\n\r\x0B・。";
-        $filter       = new StringTrim($trimCharList);
         self::assertSame('Зенд', $filter->filter('。  Зенд  。'));
     }
 
-    /** @return list<array{0: mixed}> */
-    public static function getNonStringValues(): array
-    {
-        return [
-            [1],
-            [1.0],
-            [true],
-            [false],
-            [null],
-            [[1, 2, 3]],
-            [new stdClass()],
-        ];
-    }
-
-    #[DataProvider('getNonStringValues')]
-    public function testShouldNotFilterNonStringValues(mixed $value): void
-    {
-        self::assertSame($value, $this->filter->filter($value));
-    }
-
     /**
      * Ensures expected behavior with '0' as character list
      */
     #[Group('6261')]
     public function testEmptyCharList(): void
     {
-        $this->filter->setCharList('0');
-        self::assertSame('a0b', $this->filter->filter('00a0b00'));
+        $filter = new StringTrim([
+            'charlist' => '0',
+        ]);
+
+        self::assertSame('a0b', $filter->filter('00a0b00'));
+
+        $filter = new StringTrim([
+            'charlist' => '',
+        ]);
+
+        self::assertSame('str', $filter->filter(' str '));
+    }
+
+    public function testConfiguredCharListCanIncludeMetaChar(): void
+    {
+        $filter = new StringTrim(['charlist' => '!\\\s']);
 
-        $this->filter->setCharList('');
-        self::assertSame('str', $this->filter->filter(' str '));
+        self::assertSame('Foo', $filter->filter('  ! Foo !  '));
     }
 }