[Validator] Introduce BackedEnumValue constraint

[AurelienPillevesse]

Q	A
Branch?	7.1
Bug fix?	no
New feature?	yes
Deprecations?	no
Issues	-
License	MIT

My use case: I receive data in my DTOs that we only want if this value exists in an Enum and reject the whole thing at validation otherwise.

[derrabus]

Did you read the discussion on #43047?

[AurelienPillevesse]

Didn't see it before, my bad

[AurelienPillevesse]

It's a dedicated constraint so why not but I understand if your point of view is to not accept it and adding the code below (as mentioned in the other discussion) if people want to accept an Enum value with #[MapRequestPayload] for example :

public static function values(): array
{
    return array_column(self::cases(), 'value');
}

Because you are speaking about this topic, can you maybe give me feedback about this PR : symfony/symfony-docs#19590 (if it's added in the documentation, we will probably less try to find a way to achieve it)

[derrabus]

I understand if your point of view is to not accept it

I didn't say that. It's just that there has been a PR on that topic a while ago, and if we revisit the topic, we should take that prior discussion into account.

[derrabus]

I like what we've done here. It's a simple self-contained constraint that should settle the use-cases people described in #43047.

[derrabus]

Suggested change

	* Validates that a value is one of an enum.
	* Validates that a backed enum can be hydrated from a value.

We should add a piece of documentation here, that directs people to Type and Choice constraints, if they want to validate actual enums instead the backing values. The whole discussion in #43047 shows that people mix this up regularly.

[derrabus]

Suggested change

	* BackedEnumValueValidator validates that the value is one of the expected values.
	* BackedEnumValueValidator validates that a backed enum case can be hydrated from a value.

[AurelienPillevesse]

Thanks for your feedbacks during this PR @derrabus

[stof]

This should not reference non-autoloadable enums defined in a different test file as this would break if the other test file has not been loaded

[AurelienPillevesse]

Fixed

[mdeboer]

This looks good, hopefully this gets through this time!

[AurelienPillevesse]

If you want me to do any other change here, please tell me.

Otherwise, please review and approve this PR so maintainers can see it as ready. Thanks!

[mdeboer]

I looked at it and it looks good, just one minor thing about the location of the fixture enums in the tests.

Not going to reject it based on that, nor approve it. Don't know what is recommended right now.

Once that is cleared up I'll approve it if that helps 👍🏻 Great work!

[xabbuh]

Suggested change

	try {
	if ($value instanceof \Stringable) {
	$value = (string) $value;
	}
	if (!\is_string($value)) {
	throw new UnexpectedValueException($value, 'string');
	}
	try {

[ro0NL]

backed enums can be ints, im not sure it's covered in tests

[AurelienPillevesse]

int enum case is covered in tests :

symfony/src/Symfony/Component/Validator/Tests/Constraints/BackedEnumValueValidatorTest.php

Line 141 in b0a1c63

enum MyIntBackedEnum: int

[derrabus]

Yes, but @xabbuh's proposal wouldn't work for integers. I think that's what @ro0NL was getting at. 🙂

[derrabus]

If checking the type would free us from catching the TypeError, I'd say go for it. But given that we still need to catch it, I would not go with @xabbuh's proposal.

[ro0NL]

it depends if we want to expose other types than string|int as a user-error

unexpected types should throw UnexpectedValueException, following the component's design

[derrabus]

So we catch the TypeError and upcast it?

[ro0NL]

yes, we could differentiate between TypeError and ValueError, respectively translating to UnexpectedValueException and a user constraint error in symfony

but then we should use BackedEnum::from() AFAIK, because tryFrom seems really quirky: https://3v4l.org/54qFs

edit: oh tryFrom/from is prone to strict_types=1

[xabbuh]

we should also add tests covering the cases that throw exceptions

[ro0NL]

i think the empty string case should pass through https://3v4l.org/lu4o4

[derrabus]

Empty values are always valid and that's what this check does. This is fine.

[ro0NL]

i think generally so, it's an edge case

[derrabus]

No, your "edge case" simply does not make a difference.

[ro0NL]

allowing empty string, but only when it's a valid enum case, is not possible currently

[ro0NL]

see also Choice constraint

symfony/src/Symfony/Component/Validator/Constraints/ChoiceValidator.php

Lines 40 to 42 in 0f6b3ef

	if (null === $value) {
	return;
	}