Hash::remove() doesn't work if a child element is an object

[mehov]

Description

I fetch something from the database, pass it through Hash::combine() and I end up with an array of entities.

APP/Controller/ExampleController.php (line 123)
[
(int) 1 => object(MyPlugin\Model\Entity\User) id:0 { },
(int) 2 => object(MyPlugin\Model\Entity\User) id:11 { },
(int) 3 => object(MyPlugin\Model\Entity\User) id:18 { },
(int) 4 => object(MyPlugin\Model\Entity\User) id:25 { },
]

If I call Hash::remove() on it, nothing gets removed.

$result = Hash::remove(
    $users->toArray(), '{n}.email'
);

I looked through Hash::remove() source and I think this is due to the is_array($v) check here:

cakephp/src/Utility/Hash.php

Line 416 in cc93110

if ($match && is_array($v)) {

When I do debug(is_array($v)), I get false, and debug(\gettype($v)) outputs object.

So because the above is false, it continues to elseif where $nextPath === '' also evaluates to false since $nextPath is in this case 'email':

cakephp/src/Utility/Hash.php

Line 431 in cc93110

} elseif ($match && $nextPath === '') {

As a result, nothing happens.

Is this the intended behaviour?

The only way out of this, from the top of my head, is to loop through the input and force-convert every object to an array, but that negates the whole point of using the Hash shorthands, as I can remove what I need to remove in a loop in the first place, and as a bonus, avoid the (likely negligible) cost of parsing the string'{n}.email' at all.

CakePHP Version

5.0.2

PHP Version

8.3.1

[markstory]

I think expanding the feature set of Hash to handle objects that use a property based protocol is a good idea. The intention for Hash is to be a higher level query/map/reduce toolkit. I think operating on objects fits within those goals. By property based protocol, I'm thinking of a case like

} elseif (is_object($v) && isset($v->$nextPath)) {
   unset($v->$nextPath);
}

I think that would cover your scenario and also cover working with object results from json_encode.

[ADmad]

I think expanding the feature set of Hash to handle objects that use a property based protocol is a good idea

That would create inconsistency with other methods like get() and extract() which only support ArrayAccess. So IMO the only change needed here is to include instanceof ArrayAccess along with the is_array() check.

[markstory]

That would create inconsistency with other methods like get() and extract() which only support ArrayAccess.

I agree. Expanding to support object properties would need to be done across all of Hash if we were going to expand one of the methods.