Repository navigation
Fix #139: Add EnumException and throw it in Hydrator::create() when an enum is passed, which also prevents HydratorTypeCaster from hydrating enums from array input - #140
Conversation
Reviewer's GuideThe PR prevents array input from reaching nested enum hydration by explicitly treating reflected enums as non-instantiable, and adds focused factory and type-caster regression coverage for direct and union enum types. Sequence diagram for preventing enum hydration from array inputsequenceDiagram
participant HydratorTypeCaster
participant ReflectionObjectFactory
participant EnumType
HydratorTypeCaster->>ReflectionObjectFactory: create(ReflectionClass, constructorArguments)
ReflectionObjectFactory->>EnumType: isEnum()
EnumType-->>ReflectionObjectFactory: true
ReflectionObjectFactory-->>HydratorTypeCaster: throw NonInstantiableException
File-Level Changes
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #140 +/- ##
===========================================
Coverage 100.00% 100.00%
- Complexity 388 390 +2
===========================================
Files 44 45 +1
Lines 913 921 +8
===========================================
+ Hits 913 921 +8 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
737b094 to
e88da3b
Compare
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've reviewed your changes and they look great!
Linked issues
#139 HydratorTypeCaster attempts to instantiate enum from array · 1 of 1 met
| # | Requirement | Status | Notes |
|---|---|---|---|
| 1 | Return Result::fail() for enum types instead of attempting nested hydration. |
✅ Met | castInternal() checks whether the class implements UnitEnum and fails before calling $hydrator->create(). This also applies when an enum is a member of a union type. |
Sourcery assessment
Approved.
e88da3b to
cb4141c
Compare
There was a problem hiding this comment.
Hey - I've found 2 issues
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location path="src/ObjectFactory/ReflectionObjectFactory.php" line_range="31" />
<code_context>
*/
public function create(ReflectionClass $reflectionClass, array $constructorArguments): object
{
+ if ($reflectionClass->isEnum()) {
+ throw new NonInstantiableException(
+ sprintf(
</code_context>
<issue_to_address>
**Enum arrays abort hydration**
When the hydrator uses a custom object factory that does not reject enums with `NonInstantiableException` and receives an array for an enum type, `HydratorTypeCaster` delegates enum creation through `Hydrator::create` to the configured factory, so `ContainerObjectFactory` bypasses `ReflectionObjectFactory`'s guard and injector exceptions escape the caster's `NonInstantiableException` catch; array hydration aborts instead of returning a failed cast.
Reject enum types in `HydratorTypeCaster` before delegating to the configured object factory.
</issue_to_address>
### Comment 2
<location path="src/ObjectFactory/ReflectionObjectFactory.php" line_range="31-38" />
<code_context>
*/
public function create(ReflectionClass $reflectionClass, array $constructorArguments): object
{
+ if ($reflectionClass->isEnum()) {
+ throw new NonInstantiableException(
+ sprintf(
+ '"%s" is not instantiable because it is an enum.',
+ $reflectionClass->getName(),
+ ),
+ );
+ }
+
if ($reflectionClass->isAbstract()) {
</code_context>
<issue_to_address>
**Rejected enum leaves resolver side effects**
When an enum in a union has a data-attribute resolver with external side effects and a later union arm can be hydrated, the enum’s data-attribute resolver runs before the factory rejects instantiation; the type caster catches that rejection and can continue to a later union arm, so the caller sees a successful cast while the resolver’s external state change remains.
Reject enum types before calling `Hydrator::create` in the type caster, while keeping the factory check as a general safeguard.
</issue_to_address>Linked issues
#139 HydratorTypeCaster attempts to instantiate enum from array · 1 of 1 met
| # | Requirement | Status | Notes |
|---|---|---|---|
| 1 | For array input, do not hydrate an enum as a nested object; return Result::fail() for the enum type. |
✅ Met | ReflectionObjectFactory rejects enum instantiation with NonInstantiableException, which HydratorTypeCaster catches and converts to Result::fail(); tests cover named and union enum types. |
Sourcery assessment
Approval pending. 2 findings to address first.
Blocking findings: src/ObjectFactory/ReflectionObjectFactory.php:31, src/ObjectFactory/ReflectionObjectFactory.php:38
cb4141c to
298d53f
Compare
Sourcery withdrew this approval because the latest commits introduced blocking findings.
samdark
left a comment
samdark
left a comment
There was a problem hiding this comment.
The default reflection-factory case is fixed, but the paths described in the existing container-factory thread and enum-resolver thread remain incomplete. With ContainerObjectFactory(new Injector()), an array for an enum still throws InvalidArgumentException and prevents an enum-first union from reaching a valid object alternative. With the default factory, an enum’s data resolver still runs before rejection and can abort that fallback.
Please reject enum types in HydratorTypeCaster before calling Hydrator::create(), while retaining the factory safeguard. Both remaining paths already existed on master; they are incomplete coverage of #139 rather than new regressions.
|
I moved the enum check to |
samdark
changed the title
EnumException and throw it in Hydrator::create() when an enum is passed, which also prevents HydratorTypeCaster from hydrating enums from array input
|
👍 |
Summary
HydratorTypeCasterfrom attempting nested hydration for enum types when input is an array.Fixes #139
Summary by Sourcery
Prevent invalid enum hydration by rejecting enum instantiation explicitly and handling enum types safely during type casting.
Bug Fixes:
Enhancements:
Tests: