Correct implicit and explicit property inheritance #151
Conversation
baspeeters
force-pushed
the
bugfix/property-inheritance
branch
from
5df6b19 to
0aec771
Compare
src/TypesGenerator.php
Outdated
|
|
||
| // Third pass | ||
| foreach ($classes as &$class) { | ||
| foreach ($classes as $classKey => &$class) { |
There was a problem hiding this comment.
what is classKey used for? I think we can remove this unused variable.
There was a problem hiding this comment.
@soyuka You're right, it's unused. I've removed it.
baspeeters
force-pushed
the
bugfix/property-inheritance
branch
4 times, most recently
from
8706c1a to
cbda773
Compare
|
Had pushed some changes because coverage tests for php 7.2 were failing, but that appears to be unrelated. |
Setting up a config where a child class inherits its parent's properties through class inheritance did not work. The logic in TypesGenerator handled implicit inheritance based on whether the parent's properties key was not set or not an array, but did not account for empty arrays. Also added tests for these implicit and explicit cases.
baspeeters
force-pushed
the
bugfix/property-inheritance
branch
from
cbda773 to
050d2c1
Compare
|
wdyt @teohhanhui ? LGTM, though it should target 2.x branch as a bug fix? |
| if (!isset($parentConfig['properties']) || !is_array($parentConfig['properties'])) { | ||
| if (!isset($parentConfig['properties']) || | ||
| !is_array($parentConfig['properties']) || | ||
| 0 === count($parentConfig['properties']) |
There was a problem hiding this comment.
It's weird: is_array([]) returns true. What the type of the value in this case?
There was a problem hiding this comment.
Should be '' === $parentConfig['properties'] then right?
There was a problem hiding this comment.
It's weird:
is_array([])returns true. What the type of the value in this case?
is_array([]) should return true, as [] is indeed an array, even if it's empty.
Should be
'' === $parentConfig['properties']then right?
This is already caught by !is_array($parentConfig['properties']).
|
Thanks @baspeeters |
To maintainers @dunglas, @sroze or @theofidry
Setting up a config where a child class inherits its parent's properties
through class inheritance did not work.
The logic in TypesGenerator handled implicit inheritance based on
whether the parent's properties key was not set or not an array, but did
not account for empty arrays.
Also added tests for these implicit and explicit cases.