TYPO3 Core

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:

switch ($propertyType) {
   case 'integer':
   ...
   case is_subclass_of($propertyType, \DateTimeInterface::class):
       $propertyValue = $this->mapDateTime(
           ...
       );
       break;
   ...
}

This error has been introduced in 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 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 cases, and originally even handled the DateTime case (until changed by commit aabe5f717). So I propose to put the DateTime case back to its original position.