Page MenuHomePhabricator

Make Phobject detect iteration of non-iterable objects
ClosedPublic

Authored by epriestley on Apr 30 2014, 6:32 PM.
Tags
None
Referenced Files
F13222763: D8915.diff
Sun, May 19, 3:57 AM
F13203295: D8915.diff
Tue, May 14, 11:38 PM
F13189350: D8915.diff
Sat, May 11, 5:53 AM
Unknown Object (File)
Tue, May 7, 9:33 AM
Unknown Object (File)
Fri, May 3, 9:24 AM
Unknown Object (File)
Thu, May 2, 4:01 AM
Unknown Object (File)
Thu, Apr 25, 2:48 AM
Unknown Object (File)
Apr 17 2024, 9:59 AM
Subscribers
Tokens
"Doubloon" token, awarded by btrahan.

Details

Summary

See D8914 and IRC. The root issue was that this code:

foreach ($some_object as $thing) {
  // ...
}

...means "iterate over the public properties of the object" in PHP, and does not raise any warnings or throw an exception.

This has very nearly bitten me several times too; we never want to do this. Instead, throw from objects extending Phobject.

Test Plan
  • Added and ran unit tests.
  • Tried to foreach() an arbitrary object.

Diff Detail

Repository
rPHU libphutil
Branch
phobj
Lint
Lint Passed
Unit
Tests Passed
Build Status
Buildable 148
Build 148: [Placeholder Plan] Wait for 30 Seconds

Event Timeline

epriestley retitled this revision from to Make Phboject detect iteration of non-iterable objects.
epriestley updated this object.
epriestley edited the test plan for this revision. (Show Details)
epriestley added a reviewer: btrahan.
epriestley retitled this revision from Make Phboject detect iteration of non-iterable objects to Make Phobject detect iteration of non-iterable objects.Apr 30 2014, 6:32 PM
btrahan edited edge metadata.
This revision is now accepted and ready to land.Apr 30 2014, 6:34 PM

Thank goodness we have libphutil and friends! =D

epriestley edited edge metadata.
  • Fix comment to read "Iterator" instead of "Iterable".
  • I ran arc unit --everything everywhere (couple of unrelated minor issues in libphutil) to try to catch class hierarchy issues.
  • I grepped for Iterator. We define a few in libphutil, but none of them currently extend Phobject.
epriestley updated this revision to Diff 21152.

Closed by commit rPHU3e7297e073e0 (authored by @epriestley).