Fixed bug getter reflection logic not being used to find property #4
Conversation
|
Nice find! |
|
Awesome now it also works for immutable models without setters 👍 |
src/Generator.php
Outdated
|
|
||
| if (null === $writable || false === $writable) { | ||
| if ((null === $writable || false === $writable) && null === $readable || false === $readable) { |
There was a problem hiding this comment.
Can you add parenthesis on the right expression for clarity?
|
Can you also add a non regression test? Before merging, I need to check why the tests are red. It will not be before September unfortunately (I’m in vacation). |
I will look into added some test cases. It seems like the build is due to some PHP version mismatches. Travis CI:
Edit: I've updated the PHP versions, but there are more errors in the (already existing) test cases. |
| @@ -5,7 +5,7 @@ cache: | |||
| - "$HOME/.composer/cache" | |||
|
|
|||
| php: | |||
| - 7 | |||
| - 7.2 | |||
There was a problem hiding this comment.
When using the php-7.2 test in Travis CI build, the PHPUnit will use the latest stable version.
The PHPUnit namespace changes into PHPUnit\Framework\TestCase.
It should be fixed and the Travis CI build will be successful.
The getter reflection logic was not used to fetch properties, meaning the script would only work if the classes had setters, even though that's by all means not always the case for simple models. This pull request fixes that and now uses the getter reflection logic.