-
-
Notifications
You must be signed in to change notification settings - Fork 33
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
add new filter to filter to an enum #94
base: 2.31.x
Are you sure you want to change the base?
Changes from 1 commit
fdef40c
84f400f
cd769fb
8a6fd86
7de3e2e
2fca6f1
7182172
fc001a4
18ece90
7d7f97d
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||||||
---|---|---|---|---|---|---|---|---|---|---|
@@ -0,0 +1,113 @@ | ||||||||||
<?php | ||||||||||
|
||||||||||
declare(strict_types=1); | ||||||||||
|
||||||||||
namespace Laminas\Filter; | ||||||||||
|
||||||||||
use BackedEnum; | ||||||||||
use Laminas\Filter\Exception\RuntimeException; | ||||||||||
use Laminas\Stdlib\ArrayUtils; | ||||||||||
use Traversable; | ||||||||||
use UnitEnum; | ||||||||||
|
||||||||||
use function is_array; | ||||||||||
use function is_int; | ||||||||||
use function is_string; | ||||||||||
use function is_subclass_of; | ||||||||||
|
||||||||||
/** | ||||||||||
* @psalm-type Options array{enum: class-string<UnitEnum>} | ||||||||||
* @extends AbstractFilter<Options> | ||||||||||
*/ | ||||||||||
class ToEnum extends AbstractFilter | ||||||||||
gsteel marked this conversation as resolved.
Show resolved
Hide resolved
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think this should be made more generic: /**
* @template TEnum of UnitEnum
*/ Then in the constructor you can: /**
* @param class-string<TEnum> $enum
*/
public function __construct(string $enum)
{
// ...
} This way, /**
* @return TEnum|null
*/
public function filter(mixed $value): ?UnitEnum
{
// ...
} There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I tried it but psalm will not understand my type |
||||||||||
{ | ||||||||||
/** @var Options */ | ||||||||||
protected $options = [ | ||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The psalm errors detected here should be similar to laminas/laminas-validator#168 Perhaps polyfilling with a |
||||||||||
'enum' => null, | ||||||||||
]; | ||||||||||
|
||||||||||
/** | ||||||||||
* @param class-string<UnitEnum>|Traversable|Options $enumOrOptions | ||||||||||
*/ | ||||||||||
public function __construct($enumOrOptions) | ||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I would love for this to only accept There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The constructor arg needs to be
Suggested change
or
Suggested change
(null would also need to be added to the The In order for the filter to be available in the plugin manager, The problem is that at this point, PHPUnit will fail CI on PHP 8.0 (Along with Psalm) I don't see an easy way around this at the moment unless dropping support for PHP 8.0 is an option. Thoughts @Ocramius ? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Correct! 👍🏻 |
||||||||||
{ | ||||||||||
if ($enumOrOptions instanceof Traversable) { | ||||||||||
$enumOrOptions = ArrayUtils::iteratorToArray($enumOrOptions); | ||||||||||
} | ||||||||||
|
||||||||||
if ( | ||||||||||
is_array($enumOrOptions) && | ||||||||||
isset($enumOrOptions['enum']) | ||||||||||
) { | ||||||||||
$this->setOptions($enumOrOptions); | ||||||||||
|
||||||||||
return; | ||||||||||
} | ||||||||||
|
||||||||||
if (is_string($enumOrOptions)) { | ||||||||||
$this->setEnum($enumOrOptions); | ||||||||||
} | ||||||||||
} | ||||||||||
|
||||||||||
/** | ||||||||||
* @param class-string<UnitEnum> $enum | ||||||||||
*/ | ||||||||||
public function setEnum(string $enum): self | ||||||||||
reinfi marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||||||
{ | ||||||||||
if (! is_subclass_of($enum, UnitEnum::class)) { | ||||||||||
throw new Exception\InvalidArgumentException( | ||||||||||
'enum is not of type enum' | ||||||||||
); | ||||||||||
} | ||||||||||
|
||||||||||
$this->options['enum'] = $enum; | ||||||||||
return $this; | ||||||||||
} | ||||||||||
|
||||||||||
/** | ||||||||||
* @return class-string<UnitEnum>|null | ||||||||||
*/ | ||||||||||
public function getEnum(): ?string | ||||||||||
{ | ||||||||||
return $this->options['enum']; | ||||||||||
} | ||||||||||
|
||||||||||
/** | ||||||||||
* Defined by Laminas\Filter\FilterInterface | ||||||||||
* | ||||||||||
* Returns an enum representation of $value or null | ||||||||||
* | ||||||||||
* @param null|array|bool|float|int|string $value | ||||||||||
* @return UnitEnum|BackedEnum|null | ||||||||||
*/ | ||||||||||
reinfi marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||||||
public function filter($value) | ||||||||||
{ | ||||||||||
$enum = $this->getEnum(); | ||||||||||
|
||||||||||
if ($enum === null) { | ||||||||||
throw new RuntimeException( | ||||||||||
'enum class not set' | ||||||||||
); | ||||||||||
} | ||||||||||
|
||||||||||
if (! is_string($value) && ! is_int($value)) { | ||||||||||
return null; | ||||||||||
} | ||||||||||
|
||||||||||
if (is_subclass_of($enum, 'BackedEnum')) { | ||||||||||
gsteel marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||||||
return $enum::tryFrom($value); | ||||||||||
} | ||||||||||
|
||||||||||
if (! is_string($value) || ! is_subclass_of($enum, 'UnitEnum')) { | ||||||||||
gsteel marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||||||
return null; | ||||||||||
} | ||||||||||
|
||||||||||
foreach ($enum::cases() as $enumCase) { | ||||||||||
if ($enumCase->name === $value) { | ||||||||||
return $enumCase; | ||||||||||
} | ||||||||||
} | ||||||||||
reinfi marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||||||
|
||||||||||
return null; | ||||||||||
} | ||||||||||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,11 @@ | ||
<?php | ||
|
||
declare(strict_types=1); | ||
|
||
namespace LaminasTest\Filter\TestAsset; | ||
|
||
enum TestIntBackedEnum: int | ||
{ | ||
case Foo = 1; | ||
case Bar = 2; | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,11 @@ | ||
<?php | ||
|
||
declare(strict_types=1); | ||
|
||
namespace LaminasTest\Filter\TestAsset; | ||
|
||
enum TestStringBackedEnum: string | ||
{ | ||
case Foo = 'foo'; | ||
case Bar = 'bar'; | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,11 @@ | ||
<?php | ||
|
||
declare(strict_types=1); | ||
|
||
namespace LaminasTest\Filter\TestAsset; | ||
|
||
enum TestUnitEnum | ||
{ | ||
case foo; | ||
case bar; | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,94 @@ | ||
<?php | ||
|
||
declare(strict_types=1); | ||
|
||
namespace LaminasTest\Filter; | ||
|
||
use Laminas\Filter\Exception\InvalidArgumentException; | ||
use Laminas\Filter\Exception\RuntimeException; | ||
use Laminas\Filter\ToEnum; | ||
use LaminasTest\Filter\TestAsset\TestIntBackedEnum; | ||
use LaminasTest\Filter\TestAsset\TestStringBackedEnum; | ||
use LaminasTest\Filter\TestAsset\TestUnitEnum; | ||
use PHPUnit\Framework\TestCase; | ||
use UnitEnum; | ||
|
||
/** | ||
* @requires PHP 8.1 | ||
*/ | ||
class ToEnumTest extends TestCase | ||
{ | ||
public function filterableValuesProvider(): array | ||
{ | ||
return [ | ||
'unit enum' => [TestUnitEnum::class, 'foo', TestUnitEnum::foo], | ||
'backed string enum' => [TestStringBackedEnum::class, 'foo', TestStringBackedEnum::Foo], | ||
'backed integer enum' => [TestIntBackedEnum::class, 2, TestIntBackedEnum::Bar], | ||
]; | ||
} | ||
|
||
/** | ||
* @dataProvider filterableValuesProvider | ||
* @param string|int $value | ||
*/ | ||
public function testCanFilterToEnum(string $enumClass, $value, UnitEnum $expectedFilteredValue): void | ||
{ | ||
$filter = new ToEnum($enumClass); | ||
|
||
self::assertSame($expectedFilteredValue, $filter->filter($value)); | ||
} | ||
|
||
/** | ||
* @dataProvider filterableValuesProvider | ||
* @param string|int $value | ||
*/ | ||
public function testCanFilterToEnumWithOptions(string $enumClass, $value, UnitEnum $expectedFilteredValue): void | ||
{ | ||
$filter = new ToEnum(['enum' => $enumClass]); | ||
|
||
self::assertSame($expectedFilteredValue, $filter->filter($value)); | ||
} | ||
|
||
public function unfilterableValuesProvider(): array | ||
{ | ||
return [ | ||
'array' => [TestUnitEnum::class, []], | ||
'float' => [TestUnitEnum::class, 1.1], | ||
'bool' => [TestUnitEnum::class, false], | ||
'unit enum' => [TestUnitEnum::class, 'baz'], | ||
'backed string enum' => [TestStringBackedEnum::class, 'baz'], | ||
'backed integer enum' => [TestIntBackedEnum::class, 3], | ||
]; | ||
} | ||
|
||
/** | ||
* @dataProvider unfilterableValuesProvider | ||
* @param mixed $value | ||
*/ | ||
public function testFiltersToNull(string $enumClass, $value): void | ||
{ | ||
$filter = new ToEnum($enumClass); | ||
|
||
self::assertNull($filter->filter($value)); | ||
} | ||
|
||
public function testThrowsExceptionIfEnumNotSet(): void | ||
{ | ||
$this->expectException(RuntimeException::class); | ||
$this->expectExceptionMessage('enum class not set'); | ||
|
||
$filter = new ToEnum([]); | ||
|
||
$filter->filter('foo'); | ||
} | ||
|
||
public function testThrowsExceptionIfEnumNotOfEnumType(): void | ||
{ | ||
$this->expectException(InvalidArgumentException::class); | ||
$this->expectExceptionMessage('enum is not of type enum'); | ||
|
||
$filter = new ToEnum([]); | ||
|
||
$filter->setEnum('foo'); | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wonder if we should keep extending
AbstractFilter
at all, but that's something @gsteel should help us decide.In this case,
new ToEnum(MyEnum::class)
seems cleaner to me, and doesn't really need the API inherited fromAbstractFilter
, although unsure if divergence from that is better or worse.Adding
AbstractFilter
can also be done afterwards too.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
But that would not work with the factory? Or should I add a factory to handle the case?
Extending AbstractFilter could be easily removed because I do not really need it.