\Drupal\Core\Extension\ModuleHandlerInterface::invoke() and \Drupal\Core\Extension\ModuleHandlerInterface::invokeAll() document that their second arguments must be arrays, but there is no type hinting. Lack of this caused #2170989: Migrate provides invalid arguments when invoking hooks to remain unnoticed, for instance.

CommentFileSizeAuthor
#6 drupal_2170991_5.patch3.03 KBXano
#1 drupal_2170991_1.patch2.05 KBXano
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Xano’s picture

Xano
he/they
Primary language English
Location Southampton
CreditAttribution: Xano commented
Assigned: Xano » Unassigned
Status: Active » Needs review
FileSize
drupal_2170991_1.patch2.05 KB
Xano’s picture

Xano
he/they
Primary language English
Location Southampton
CreditAttribution: Xano commented

For the record: this depends on #2170989: Migrate provides invalid arguments when invoking hooks being fixed first.

Xano’s picture

Xano
he/they
Primary language English
Location Southampton
CreditAttribution: Xano commented
Related issues: +#2170989: Migrate provides invalid arguments when invoking hooks

Status: Needs review » Needs work

The last submitted patch, 1: drupal_2170991_1.patch, failed testing.

The last submitted patch, 1: drupal_2170991_1.patch, failed testing.

Xano’s picture

Xano
he/they
Primary language English
Location Southampton
CreditAttribution: Xano commented
Status: Needs work » Needs review
FileSize
drupal_2170991_5.patch3.03 KB

Fixed some of the failures.

Xano’s picture

Xano
he/they
Primary language English
Location Southampton
CreditAttribution: Xano commented
Title: Type hint array arguments in ModuleHandlerInterface » Migrate provides invalid arguments when invoking hooks/type hint array arguments in ModuleHandlerInterface
chx’s picture

chx CreditAttribution: chx commented
Status: Needs review » Reviewed & tested by the community

Good catch.

webchick’s picture

webchick
she/they
Primary language English
Location Vancouver 🇨🇦
CreditAttribution: webchick commented
Status: Reviewed & tested by the community » Fixed

Since this is just changing type-hinting and fixing a typo, I don't think we need tests for this.

Committed and pushed to 8.x. Thanks!

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.