Skip to content

Commit

Permalink
Merge pull request #65 from gsteel/filter-chain-types
Browse files Browse the repository at this point in the history 
Improve Filter Chain types and test
  • Loading branch information
@Ocramius
Ocramius authored Sep 21, 2022
2 parents 06322d9 + fd14645 commit fa2ef11
Show file tree
Hide file tree
Showing 3 changed files with 82 additions and 76 deletions.
52 changes: 6 additions & 46 deletions psalm-baseline.xml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1088,38 +1088,19 @@
<code>gettype($callback)</code>
<code>gettype($options)</code>
</DocblockTypeContradiction>
<MixedArgument occurrences="5">
<code>$callback</code>
<code>$key</code>
<code>$name</code>
<code>$priority</code>
<code>$priority</code>
</MixedArgument>
<MixedArrayAccess occurrences="5">
<code>$spec['callback']</code>
<code>$spec['name']</code>
<code>$spec['options']</code>
<code>$spec['priority']</code>
<code>$spec['priority']</code>
</MixedArrayAccess>
<MixedAssignment occurrences="9">
<code>$callback</code>
<code>$key</code>
<code>$name</code>
<code>$options</code>
<code>$priority</code>
<code>$priority</code>
<code>$spec</code>
<code>$spec</code>
<code>$value</code>
</MixedAssignment>
<MixedPropertyTypeCoercion occurrences="1">
<code>new PriorityQueue()</code>
</MixedPropertyTypeCoercion>
<MoreSpecificImplementedParamType occurrences="1">
<code>$options</code>
</MoreSpecificImplementedParamType>
<RedundantConditionGivenDocblockType occurrences="2">
<code>is_object($callback)</code>
<code>is_object($options)</code>
</RedundantConditionGivenDocblockType>
<RedundantFunctionCall occurrences="1">
<code>strtolower</code>
</RedundantFunctionCall>
</file>
<file src="src/FilterPluginManager.php">
<DeprecatedClass occurrences="2">
Expand Down Expand Up @@ -2426,27 +2407,6 @@
<code>returnUnfilteredDataProvider</code>
</MissingReturnType>
</file>
<file src="test/FilterChainTest.php">
<MissingParamType occurrences="1">
<code>$value</code>
</MissingParamType>
<MissingReturnType occurrences="2">
<code>getChainConfig</code>
<code>staticUcaseFilter</code>
</MissingReturnType>
<MixedArgument occurrences="5">
<code>$config</code>
<code>$config</code>
<code>$config</code>
<code>$value</code>
<code>$value</code>
</MixedArgument>
<MixedAssignment occurrences="3">
<code>$config</code>
<code>$config</code>
<code>$config</code>
</MixedAssignment>
</file>
<file src="test/FilterPluginManagerCompatibilityTest.php">
<MissingReturnType occurrences="1">
<code>aliasProvider</code>
Expand Down
17 changes: 14 additions & 3 deletions src/FilterChain.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,17 @@
/**
* @final
* @implements IteratorAggregate<array-key, FilterInterface|callable(mixed): mixed>
* @psalm-type FilterChainConfiguration = array{
* filters?: list<array{
* name: string|class-string<FilterInterface>,
* options?: array<string, mixed>,
* priority?: int,
* }>,
* callbacks?: list<array{
* callback: callable(mixed): mixed,
* priority?: int,
* }>
* }
*/
class FilterChain extends AbstractFilter implements Countable, IteratorAggregate
{
Expand All @@ -45,7 +56,7 @@ class FilterChain extends AbstractFilter implements Countable, IteratorAggregate
/**
* Initialize filter chain
*
* @param null|array|Traversable $options
* @param FilterChainConfiguration|Traversable|null $options
*/
public function __construct($options = null)
{
Expand All @@ -57,8 +68,8 @@ public function __construct($options = null)
}

/**
* @param array|Traversable $options
* @return self
* @param FilterChainConfiguration|Traversable $options
* @return $this
* @throws Exception\InvalidArgumentException
*/
public function setOptions($options)
Expand Down
89 changes: 62 additions & 27 deletions test/FilterChainTest.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,19 +14,21 @@

use function count;
use function function_exists;
use function iterator_to_array;
use function serialize;
use function strtolower;
use function strtoupper;
use function trim;
use function unserialize;

/** @psalm-import-type FilterChainConfiguration from FilterChain */
class FilterChainTest extends TestCase
{
public function testEmptyFilterChainReturnsOriginalValue(): void
{
$chain = new FilterChain();
$value = 'something';
$this->assertSame($value, $chain->filter($value));
self::assertSame($value, $chain->filter($value));
}

public function testFiltersAreExecutedInFifoOrder(): void
Expand All @@ -36,7 +38,7 @@ public function testFiltersAreExecutedInFifoOrder(): void
->attach(new TestAsset\StripUpperCase());
$value = 'AbC';
$valueExpected = 'abc';
$this->assertSame($valueExpected, $chain->filter($value));
self::assertSame($valueExpected, $chain->filter($value));
}

public function testFiltersAreExecutedAccordingToPriority(): void
Expand All @@ -46,21 +48,21 @@ public function testFiltersAreExecutedAccordingToPriority(): void
->attach(new TestAsset\LowerCase(), 100);
$value = 'AbC';
$valueExpected = 'b';
$this->assertSame($valueExpected, $chain->filter($value));
self::assertSame($valueExpected, $chain->filter($value));
}

public function testAllowsConnectingArbitraryCallbacks(): void
{
$chain = new FilterChain();
$chain->attach(static fn($value) => strtolower($value));
$chain->attach(static fn(string $value): string => strtolower($value));
$value = 'AbC';
$this->assertSame('abc', $chain->filter($value));
self::assertSame('abc', $chain->filter($value));
}

public function testAllowsConnectingViaClassShortName(): void
{
if (! function_exists('mb_strtolower')) {
$this->markTestSkipped('mbstring required');
self::markTestSkipped('mbstring required');
}

$chain = new FilterChain();
Expand All @@ -70,7 +72,7 @@ public function testAllowsConnectingViaClassShortName(): void

$value = '<a name="foo"> ABC </a>';
$valueExpected = 'abc';
$this->assertSame($valueExpected, $chain->filter($value));
self::assertSame($valueExpected, $chain->filter($value));
}

public function testAllowsConfiguringFilters(): void
Expand All @@ -80,7 +82,7 @@ public function testAllowsConfiguringFilters(): void
$chain->setOptions($config);
$value = '<a name="foo"> abc </a><img id="bar" />';
$valueExpected = 'ABC <IMG ID="BAR" />';
$this->assertSame($valueExpected, $chain->filter($value));
self::assertSame($valueExpected, $chain->filter($value));
}

public function testAllowsConfiguringFiltersViaConstructor(): void
Expand All @@ -89,7 +91,7 @@ public function testAllowsConfiguringFiltersViaConstructor(): void
$chain = new FilterChain($config);
$value = '<a name="foo"> abc </a>';
$valueExpected = 'ABC';
$this->assertSame($valueExpected, $chain->filter($value));
self::assertSame($valueExpected, $chain->filter($value));
}

public function testConfigurationAllowsTraversableObjects(): void
Expand All @@ -99,7 +101,7 @@ public function testConfigurationAllowsTraversableObjects(): void
$chain = new FilterChain($config);
$value = '<a name="foo"> abc </a>';
$valueExpected = 'ABC';
$this->assertSame($valueExpected, $chain->filter($value));
self::assertSame($valueExpected, $chain->filter($value));
}

public function testCanRetrieveFilterWithUndefinedConstructor(): void
Expand All @@ -110,14 +112,15 @@ public function testCanRetrieveFilterWithUndefinedConstructor(): void
],
]);
$filtered = $chain->filter('127.1');
$this->assertSame(127, $filtered);
self::assertSame(127, $filtered);
}

protected function getChainConfig()
/** @return FilterChainConfiguration */
private function getChainConfig(): array
{
return [
'callbacks' => [
['callback' => self::class . '::staticUcaseFilter'],
['callback' => [self::class, 'staticUcaseFilter']],
[
'priority' => 10000,
'callback' => static fn(string $value): string => trim($value),
Expand All @@ -133,7 +136,7 @@ protected function getChainConfig()
];
}

public static function staticUcaseFilter($value)
public static function staticUcaseFilter(string $value): string
{
return strtoupper($value);
}
Expand All @@ -153,15 +156,15 @@ public function testCanAttachMultipleFiltersOfTheSameTypeAsDiscreteInstances():
'replacement' => 'PARTY',
]);

$this->assertSame(2, count($chain));
self::assertSame(2, count($chain));
$filters = $chain->getFilters();
$compare = null;
foreach ($filters as $filter) {
$this->assertNotSame($compare, $filter);
self::assertNotSame($compare, $filter);
$compare = $filter;
}

$this->assertSame('Tu et PARTY', $chain->filter('Tu et Foo'));
self::assertSame('Tu et PARTY', $chain->filter('Tu et Foo'));
}

public function testClone(): void
Expand All @@ -171,7 +174,7 @@ public function testClone(): void

$chain->attachByName(StripTags::class);

$this->assertCount(0, $clone);
self::assertCount(0, $clone);
}

public function testCanSerializeFilterChain(): void
Expand All @@ -182,11 +185,11 @@ public function testCanSerializeFilterChain(): void
$serialized = serialize($chain);

$unserialized = unserialize($serialized);
$this->assertInstanceOf(FilterChain::class, $unserialized);
$this->assertSame(2, count($unserialized));
self::assertInstanceOf(FilterChain::class, $unserialized);
self::assertSame(2, count($unserialized));
$value = 'AbC';
$valueExpected = 'abc';
$this->assertSame($valueExpected, $unserialized->filter($value));
self::assertSame($valueExpected, $unserialized->filter($value));
}

public function testMergingTwoFilterChainsKeepFiltersPriority(): void
Expand All @@ -197,27 +200,59 @@ public function testMergingTwoFilterChainsKeepFiltersPriority(): void
$chain = new FilterChain();
$chain->attach(new TestAsset\StripUpperCase())
->attach(new TestAsset\LowerCase(), 1001);
$this->assertSame($valueExpected, $chain->filter($value));
self::assertSame($valueExpected, $chain->filter($value));

$chain = new FilterChain();
$chain->attach(new TestAsset\LowerCase(), 1001)
->attach(new TestAsset\StripUpperCase());
$this->assertSame($valueExpected, $chain->filter($value));
self::assertSame($valueExpected, $chain->filter($value));

$chain = new FilterChain();
$chain->attach(new TestAsset\LowerCase(), 1001);
$chainToMerge = new FilterChain();
$chainToMerge->attach(new TestAsset\StripUpperCase());
$chain->merge($chainToMerge);
$this->assertSame(2, $chain->count());
$this->assertSame($valueExpected, $chain->filter($value));
self::assertSame(2, $chain->count());
self::assertSame($valueExpected, $chain->filter($value));

$chain = new FilterChain();
$chain->attach(new TestAsset\StripUpperCase());
$chainToMerge = new FilterChain();
$chainToMerge->attach(new TestAsset\LowerCase(), 1001);
$chain->merge($chainToMerge);
$this->assertSame(2, $chain->count());
$this->assertSame($valueExpected, $chain->filter($value));
self::assertSame(2, $chain->count());
self::assertSame($valueExpected, $chain->filter($value));
}

public function testThatIteratingOverAFilterChainDirectlyYieldsExpectedFilters(): void
{
$filter1 = new StringToLower();
$filter2 = new StripTags();

$chain = new FilterChain();
$chain->attach($filter1, 10);
$chain->attach($filter2, 20);

$filters = iterator_to_array($chain);
self::assertEquals([
0 => $filter1,
1 => $filter2,
], $filters);
}

public function testThatIteratingOverGetFiltersYieldsExpectedFilters(): void
{
$filter1 = new StringToLower();
$filter2 = new StripTags();

$chain = new FilterChain();
$chain->attach($filter1, 10);
$chain->attach($filter2, 20);

$filters = iterator_to_array($chain->getFilters());
self::assertEquals([
0 => $filter1,
1 => $filter2,
], $filters);
}
}

0 comments on commit fa2ef11

Please sign in to comment.