While porting modules to D7, I've seen enough "Fatal error: Cannot use object of type stdClass as array in includes/somewhere on line 666" for the rest of my live. Imho, this is a *huge* DX issue.

Reason is simple..

Without patch (without display_errors = 1 even a WSOD):

Fatal error: Cannot use object of type stdClass as array in ...includes/theme.inc on line 787

With patch (displayed with the drupal maintenance theme, controlled by drupal settings, not php settings):

Recoverable fatal error: Argument 2 passed to theme() must be an array, object given, called in ...sites/all/modules/user_relationships/user_relationship_blocks/templates/user_relationships-block.tpl.php on line 27 and defined in theme() (line 729 of ...includes/theme.inc).

When the array type is enforced, you don't see a fatal error somewhere deep in the function you're calling, but you see where you call it, which function it is and it's like 10x easier to fix.

The patch converts everything I've found in /includes. /modules is not yet converted. Not that I have not found everything - I've started with theme() (which is the worst one when porting modules to D7) and converted all which had an "= array()" since these were easy to grep for and a few other which I've stumbled over.

Sometimes, arguments can be both an array and a string, so it's possible that I added too much array's, the test bot should be able to tell us that... I need some sleep now.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Berdir’s picture

Status: Active » Needs review
Berdir’s picture

Title: Enforce array arguments to functions (Say good bye to Fatal error: Cannot use object of type stdClass as array errors) » Enforce array arguments (Say good bye to Fatal error: Cannot use object of type stdClass as array errors)

Status: Needs review » Needs work

The last submitted patch, add_array_type.patch, failed testing.

Berdir’s picture

Status: Needs work » Needs review
FileSize
40.5 KB

Reverted the change to entity_load() and related functions for $ids since many functions in core pass NULL or FALSE to them. This "works" because $ids is checked with empty(). Not clean, but enforcing an array there can be considered as an api change.

New try.

Status: Needs review » Needs work

The last submitted patch, add_array_type2.patch, failed testing.

Berdir’s picture

Status: Needs work » Needs review
FileSize
41.38 KB

Wrong watchdog call.. we might want to allow NULL there too, I fixed the call as that's more correct for now...

Status: Needs review » Needs work

The last submitted patch, add_array_type3.patch, failed testing.

Berdir’s picture

Status: Needs work » Needs review
FileSize
41.85 KB

Another watchdog fix.

Status: Needs review » Needs work

The last submitted patch, add_array_type4.patch, failed testing.

Berdir’s picture

Status: Needs work » Needs review
FileSize
41.36 KB

Almost there ;)

Fixed another watchdog() call and removed the array type as it's not technically required for it to be an array (as opposed to other functions like theme()). So wrong watchdog() calls won't produce errors anymore.

I guess we can enforce D8 array arguments for entity loading, watchdog and other things...

BenK’s picture

Okay, I'll use the patch in #10 in combination with our User Relationships D7 testing...

retester2010’s picture

#10: add_array_type5.patch queued for re-testing.

Status: Needs review » Needs work

The last submitted patch, add_array_type5.patch, failed testing.

klausi’s picture

Subscribing, I like the idea of more type safety.

binford2k’s picture

Yes, this is a HUGE issue when porting someone else's module!

RobLoach’s picture

Version: 7.x-dev » 8.x-dev
Issue tags: +Type Hinting

Yes please!!!! Think we should split this patch into a per-module basis, plus core? A 41K patch is quite difficult to review ;-) .

johsw’s picture

I wanna try to help help out here.

Should I make a new issue for each module, or should I just start making per-module patches and post them in this issue?

jbrown’s picture

jbrown’s picture

Status: Needs work » Closed (duplicate)