Ticket #2784 (closed defect: fixed)

Opened 2 years ago

Last modified 4 months ago

Make all foreach() loops cast to array

Reported by: markjaquith Assigned to: jacobsantos
Priority: normal Milestone: 2.7
Component: Optimization Version: 2.1
Severity: normal Keywords: has-patch needs-testing dev-feedback
Cc:

Description

Every so often, we get bitten by a random foreach() error due to the input not being an array.

The elegant solution is this: 

foreach ( (array) $maybe_array as $foo ) :

The attached patch is just the result of a giant regular expressions find/replace: 

search: foreach( *)\(( *)\$
replace: foreach$1( (array) $

this is more proof of concept than anything else. I'm not getting any errors after applying it. I'd love to banish foreach errors once and for all.

Attachments

cast_to_array.diff (153.1 kB) - added by markjaquith on 06/04/06 09:52:20.
foreach_refresh.diff (149.8 kB) - added by markjaquith on 10/03/06 22:18:32.
Patch refresh for /trunk/
arraytypecast.patch (43.0 kB) - added by darkdragon on 01/06/08 07:41:03.
Corrected version based off of r6563 for adding array type casting to foreach loops in wp-includes folder
2784.diff (39.4 kB) - added by jacobsantos on 07/29/08 03:13:28.
type casting for foreach in wp-includes.
2784.r8483.diff (40.4 kB) - added by jacobsantos on 07/29/08 03:20:15.
type casting for foreach in wp-includes based off of r8483
2784.r8483.patch (39.5 kB) - added by jacobsantos on 07/29/08 03:21:34.
type casting for foreach in wp-includes based off of r8483

Change History

06/04/06 09:52:20 changed by markjaquith

  • attachment cast_to_array.diff added.

06/04/06 13:46:51 changed by davidhouse

The _real_ solution is for PHP to change their crappy foreach semantics :) But yeah, nice.

10/03/06 13:47:28 changed by Nazgul

  • keywords set to has-patch.

10/03/06 22:18:32 changed by markjaquith

  • attachment foreach_refresh.diff added.

Patch refresh for /trunk/

10/03/06 22:19:22 changed by markjaquith

Patch refreshed for the bug hunt

10/04/06 07:09:35 changed by markjaquith

  • owner changed from anonymous to markjaquith.
  • status changed from new to assigned.
  • component changed from Administration to Optimization.
  • milestone set to 2.1.

Because there is a good chance this will rot many existing patches, let's wait on this until right before we release a 2.1 beta zip.

11/30/06 01:16:43 changed by matt

  • milestone changed from 2.1 to 2.2.

04/12/07 18:29:39 changed by foolswisdom

  • keywords changed from has-patch to needs-patch.
  • milestone changed from 2.2 to 2.3.

Likely needs refresh

09/06/07 22:19:16 changed by Nazgul

  • milestone changed from 2.3 to 2.4 (next).

A candidate for doing very early in the 2.4 cycle.

09/13/07 03:58:45 changed by foolswisdom

  • keywords changed from needs-patch to needs-patch early.

01/06/08 05:00:11 changed by darkdragon

I don't think bug rot will be an issue for this, since the phpdoc has done that already. Will submit updated patch.

01/06/08 07:31:42 changed by darkdragon

Patch contains items which would corrupt trunk, please do not commit. Will fix issues and resubmit.

01/06/08 07:41:03 changed by darkdragon

  • attachment arraytypecast.patch added.

Corrected version based off of r6563 for adding array type casting to foreach loops in wp-includes folder

(follow-up: ↓ 12 ) 01/06/08 07:43:30 changed by darkdragon

  • keywords changed from needs-patch early to has-patch early.

Updated patch fixes four spaces to tabs issue, however patch does not display in Trac, so I'm worried it won't patch for committing.

(in reply to: ↑ 11 ) 01/06/08 08:05:34 changed by DD32

Replying to darkdragon:

however patch does not display in Trac, so I'm worried it won't patch for committing.

I've found that you need to save as a .diff instead of a .patch. Simply renaming a .patch to .diff doesnt work either.

D

01/12/08 04:09:36 changed by darkdragon

Is this ticket being completely ignored or is there hope that one of the patches will be applied?

01/12/08 04:10:10 changed by darkdragon

If there is something that is missing then let me know and I'll add it.

01/27/08 04:55:10 changed by darkdragon

What is up with this? Is this ticket defunct? Close it as won't fix if nothing is going to be done.

07/15/08 21:00:50 changed by santosj

  • owner changed from markjaquith to jacobsantos.
  • status changed from assigned to new.

07/15/08 21:01:29 changed by santosj

  • milestone changed from 2.9 to 2.7.

Time to update patch. Hopefully, if I'm going to spend another hour on this it will get through faster, than the last time.

07/28/08 22:03:20 changed by santosj

Patch made, will upload in 1 hour.

07/29/08 03:13:28 changed by jacobsantos

  • attachment 2784.diff added.

type casting for foreach in wp-includes.

07/29/08 03:13:53 changed by jacobsantos

It is discomforting when the diff does not display.

07/29/08 03:14:02 changed by jacobsantos

  • keywords changed from has-patch early to has-patch needs-testing.

07/29/08 03:17:59 changed by DD32

It is discomforting when the diff does not display.

Try removing the extra blank line at the end of the file, I've found that causes it sometimes.. It is anoying though, I wish i could work out what causes it sometimes.

07/29/08 03:20:15 changed by jacobsantos

  • attachment 2784.r8483.diff added.

type casting for foreach in wp-includes based off of r8483

07/29/08 03:21:34 changed by jacobsantos

  • attachment 2784.r8483.patch added.

type casting for foreach in wp-includes based off of r8483

07/29/08 03:21:52 changed by jacobsantos

Well, hopefully one of those works.

07/29/08 03:22:38 changed by jacobsantos

The only problem I suppose is that this doesn't check wp-admin, which will need to be for this ticket to be closed.

08/04/08 17:35:27 changed by santosj

  • keywords changed from has-patch needs-testing to has-patch needs-testing dev-feedback.

I would rather this not go another 3 months without being looked at.

08/06/08 16:28:15 changed by santosj

Suggest closing as wontfix if not going to commit.

08/06/08 20:31:55 changed by markjaquith

  • status changed from new to closed.
  • resolution set to fixed.

(In [8572]) Cast to array when using foreach(). Props santosj (and thanks for your perseverance!). fixes #2784