Allow object type to be given to array_walk and array_walk_recursive.

[niconoe-]

Fixes #6338

[niconoe-]

Sorry that I did not add any tests to ensure the error won't be triggered anymore with objects now. I wasn't capable of finding how/where to do that.

Feel free to enhance this, or giving me tips on how to do it.

Regards,

[orklah]

@weirdan mind triggering CI here?

@niconoe- we don't have any test regarding array_walk yet, it would be great to have some. Could you add a small example with an array and one with an object in ArrayFunctionCallTest::providerValidCodeParse please?

[weirdan]

Not sure if this should be allowed, given that we're already flagging RawObjectIteration: https://psalm.dev/r/9bc231231c, especially since it doesn't respect iterable protocol: https://3v4l.org/6otJZ

[psalm-github-bot]

I found these snippets:

https://psalm.dev/r/9bc231231c

<?php

foreach (new stdClass as $_prop) {}

Psalm output (using commit 3cb28e6):

ERROR: RawObjectIteration - 3:10 - Possibly undesired iteration over regular object stdClass

[niconoe-]

Not sure if this should be allowed, given that we're already flagging RawObjectIteration: https://psalm.dev/r/9bc231231c, especially since it doesn't respect iterable protocol: https://3v4l.org/6otJZ

@weirdan Do you mean passing an object to array_walk should trigger the RawObjectIteration error?
I'm fine with this, I never understood why array_walk could be used with object (internal function is called "array_walk" after all) while array_map, which is functionnaly close, is not.
When I did the proposal for PHPStan, I never questionned if it was a good or bad practice, just that it was a lack of definition in the signature of this internal function. I just wanted Psalm to have the same fix.

In that case, I'm not sure I have the skills to add this error management in Psalm, enhancing the PR. Meanwhile, I think the dictionnary update should be kept, because PHP internal is allowing it: the fact Psalm could consider this as a smell should be defined in the engine, IMHO.

---

@niconoe- we don't have any test regarding array_walk yet, it would be great to have some. Could you add a small example with an array and one with an object in ArrayFunctionCallTest::providerValidCodeParse please?

@orklah Thank you for the location of the providers to test this. Nevertheless, I think we need to status if the array_walk of an object is a smell or not, so I could add the providers in either ArrayFunctionCallTest::providerValidCodeParse or ArrayFunctionCallTest::providerInvalidCodeParse.
In the meantime, I think I can add some tests about valid usages of array_walk with an array 😉 .

[psalm-github-bot]

I found these snippets:

https://psalm.dev/r/9bc231231c

<?php

foreach (new stdClass as $_prop) {}

Psalm output (using commit 65f34d0):

ERROR: RawObjectIteration - 3:10 - Possibly undesired iteration over regular object stdClass

[weirdan]

Do you mean passing an object to array_walk should trigger the RawObjectIteration error?

I mean there should be some issue emitted, and I care less what that specific issue would be. Right now it's InvalidArgument, and I think that's ok, even if not 100% correct. This PR, as it stands now, would make Psalm stop reporting that without introducing a replacement, so this would need to be addressed before we can merge it.

[weirdan]

without introducing a replacement

I'll add it.

[weirdan]

I'll add it.

Done.

[weirdan]

Thanks!