-
Notifications
You must be signed in to change notification settings - Fork 7.9k
Treat \stdClass as iterable #3388
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Foreach handles all objects without issue, will all objects be iterable too? Huge 👎 from me on this one. |
No, all objects will not be iterable too. I wouldn't say that including some properties and not others, acting differently based on whether interfaces are implemented or not, is "without issue" |
It's not some, it's all visible to current scope, IIRC just like Regardless, stdClass doesn't implement Traversable, so it can't be iterable. |
Treating |
That's not how stdClass works. stdClass is a class, it passes To quote php.net documentation:
Yes. This breaks: is_array($iterable) ? $iterable : iterator_to_array($iterable) Because you are breaking the basic invariant of |
I see your point on the I've taken your comments on board and will include them in the RFC, thanks |
@duncan3dc The contract of |
Not correct. It depends on the very basic assumption that iterable is either array or Traversable. Also with |
@nikic I decided against that because then classes that extend \stdClass will be automatically iterable now, but it sounds like that's the lesser BC issue if people are depending on the current implementation of iterable |
@duncan3dc I'd argue that if stdClass is iterable, it would be very weird if classes extending stdClass weren't. That would be like a class "unimplementing" an interface during inheritance. |
@nikic I guess I'm alone in not seeing |
Very likely. |
9ff3886
to
efe1149
Compare
The
stdClass
is analogous in many ways to an array,foreach
handles it without issue, so it is unexpected that theiterable
type rejects this structure. This PR adds support for itThis replaces #3382, I'll be drafting an RFC once the list is a little quieter