Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Add support for enum arguments to setValue() #343

Merged

Conversation

greg0ire
Copy link
Member

@greg0ire greg0ire commented Feb 27, 2024

The ORM is relying on that feature.
When trying to use EnumReflectionProperty instead of the corresponding class, the test suite breaks on this line: https://github.com/doctrine/orm/blob/b187bc85881cc85d36b28e7ed9dbe2071d472e30/src/UnitOfWork.php#L2406

EnumReflectionProperty was added #248 so that it could reduce duplication with the ORM, but is not used yet.

@greg0ire greg0ire marked this pull request as ready for review February 27, 2024 18:24
@@ -86,13 +86,25 @@ private function fromEnum($enum)
}

/**
* @param int|string|int[]|string[] $value
* @param int|string|int[]|string[]|BackedEnum|BackedEnum[] $value
*
* @return ($value is int|string ? BackedEnum : BackedEnum[])
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
* @return ($value is int|string ? BackedEnum : BackedEnum[])
* @return ($value is int|string|BackedEnum ? BackedEnum : BackedEnum[])

This should take down one SA baseline entry

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Didn't seem to, but good point anyway.

if (is_array($value)) {
foreach ($value as $v) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was hard to read for me (maybe because I'm in vacations-mode ;)... IMO using reset would be more elegant than this entire loop

Copy link
Member Author

@greg0ire greg0ire Feb 28, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Enjoy your vacation!

The ORM is relying on that feature.
@greg0ire greg0ire force-pushed the add-support-for-set-value-with-enum branch from a102634 to e70bc39 Compare February 28, 2024 22:02
@greg0ire greg0ire added this to the 3.3.0 milestone Feb 29, 2024
@greg0ire greg0ire merged commit 8961eec into doctrine:3.3.x Feb 29, 2024
13 checks passed
@greg0ire greg0ire deleted the add-support-for-set-value-with-enum branch February 29, 2024 22:29
# for free to join this conversation on GitHub. Already have an account? # to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants