Bug #90992
Updated by Volker Diels-Grabsch over 4 years ago
While developing an extension I noticed a misleading error message in the DataMapper. Hunting it down showed a clear programming error in @DataMapper::thawProperties()@. That method contains a big @switch@ over a string, where one case is a boolean instead of a string: <pre> switch ($propertyType) { case 'integer': ... case is_subclass_of($propertyType, \DateTimeInterface::class): $propertyValue = $this->mapDateTime( ... ); break; ... } </pre> This error has been introduced in commit commit:aabe5f717. Surprisingly, this code is not as broken as it seems. It does work, but only for subtle reasons: It works because @switch@ uses @==@ instead of @===@ for comparison, and because @$propertyType == true@ holds for almost all strings @$propertyType@. Exceptions, of course, are the values @''@, @'0'@ and @null@. However, for those falsy values, the call to @is_subclass_of(...)@ returns @false@ as well, so the case comparison yields @true@ and the @mapDateTime(...)@ code is actually executed! Among others, this means that if the property type could not be identified, we get an error message regarding a failed DateTime conversion, instead of the proper error message generated by the @mapObjectToClassProperty(...)@ call in the @default@ case. To complicate things, this bug has already been noticed in #89857, but has been "fixed" in commit:7ca1f49c9 by catching the case @$propertyType === null@ immediately before the @switch@. The cases of other falsy values such as @''@ and @'0'@ can still trigger the bug. I propose to actually fix this programming error instead of adding more workarounds: The @default@ case already contains @if@ statements for the more complex non-trivial cases, and originally even handled the DateTime case (until changed by commit commit:aabe5f717). So I propose to put the DateTime case back to its original position.