[GH-36] Bugfix: TypedNoDefaultReflectionProperty::setValue NULL when null allowed #37
Conversation
|
Scrutinizer and Travis-ci/push are false positives here. |
|
To fix the issue on Scrutinizer and Travis, the solution would be to set the |
| @@ -31,7 +31,7 @@ public function getValue($object = null) | |||
| */ | |||
| public function setValue($object, $value = null) | |||
| { | |||
| if ($value === null) { | |||
| if ($value === null && ! $this->getType()->allowsNull()) { | |||
There was a problem hiding this comment.
In case of a non-typed property, ReflectionProperty::getType() will return null.
There was a problem hiding this comment.
but its by definition a typed property here.
There was a problem hiding this comment.
Okay, in that case this shouldn't be an issue if the class, that has to be used for a property, is determined elsewhere and will always result in TypedNoDefaultReflectionProperty for PHP 7.4 when typed properties are in use.
There was a problem hiding this comment.
@SenseException i am not sure anymore, so i added a hasType() check inbetween.
There was a problem hiding this comment.
Then I'd like to see a test where no property type is given.
|
Shouldn't 1.2.x be the target? I can see that master is aliased to 1.2.x, weird… |
|
@greg0ire we don't have version branches here, because usually nothing much would have to happen here anyways. |
Yes we do: they are needed to work with the release bot. |
|
@alcaeus hm right, weird i remember working on master last week, maybe I misremembered or somebody fixed my mess without telling and slapping me :-D |
|
|
Should we set |
|
BTW, I extracted your Composer related commit in another PR that makes it easier to work with Composer: #38 |
No, as new features should still go to master. Setting a version branch as default only makes sense while we're preparing a new major release and want all new features to go into the last minor of the current major release number. |
1.2 introduced a regression
Fixes #36