module_invoke_all() can have surprising results thanks to array_merge_recursive()

Background

array_merge_recursive() is a strange function. See e.g. #791860: array_merge_recursive() is never what we want in Drupal: add a drupal_array_merge_recursive() function instead.
For Drupal 8, we already replaced many (or all?) instances of array_merge_recursive() with NestedArray::mergeDeep(). This did include ModuleHandler::invokeAll(). See #1356170: Remove all uses of array_merge_recursive, or document why they are being used instead of NestedArray::mergeDeep().

These changes are too big to backport them to Drupal 7 all at once. Instead, individual parts of this change should be discussed in separate distinct issues for Drupal 7. Each will have its own consequences and tradeoffs, and each should have its own patches and tests.

This one deals with module_invoke_all(), which currently uses array_merge_recursive().

Problem

Consider a site with modules 'aa' and 'bb'. Both of them implement hook_permission(), like so:

function aa_permission() {
  return array(
    'open the door' =>  array(
      'title' => 'Open the door to the garage.',
    ),
  );
}

function bb_permission() {
  return array(
    'open the door' =>  array(
      'title' => 'Open the door to the barn.',
    ),
  );
}

Expected behavior:
The result of module_invoke_all('permission'); for these two modules should be:

array (
  'open the door' => array (
    // Some kind of priority rule to decide which one to pick.
    'title' => 'Open the door to the barn.',
  ),
)

Actual behavior:
The result of module_invoke_all('permission'); for these two modules will be:

array (
  'open the door' => array (
    'title' => array (
      0 => 'Open the door to the garage.',
      1 => 'Open the door to the barn.',
    ),
  ),
)

One example of this bug is documented here, #2852809: Facetapi module now implement hook_i18n_string_info
Bugs of this type can manifest as "function xyz requires a string as first argument, array given", and most users will have no idea why this is happening.

One could argue that developers are not supposed to use the same key twice. But there needs to be a robust behavior and possibly error reporting for this case.

Additional thoughts

module_invoke_all(), like ModuleHandler::invokeAll(), is implemented as a one-size-fits-all function. In many cases, such as hook_menu() or hook_permission(), the recursive merging of return values is not needed, or rather detrimental than useful. And some calls to module_invoke_all() do not need a return value at all.

The recursive merging is needed for cases like hook_token_info(), and maybe some others.

One can ask why all of these cases need to use the same function.

Possible solutions

(At the time of writing this, I am not advocating one specific solution, I am just putting them as options on the table)

drupal_array_merge_deep()

In Drupal 8 ModuleHandler::invokeAll(), the array_merge_recursive() was replaced with NestedArray::mergeDeep().
The equivalent thing in Drupal 7 would be drupal_array_merge_deep().

Possible problems:
This changes the behavior of module_invoke_all(), which might make it a BC break. This needs some discussion or investigation.

Error reporting

Another option could be to not change the behavior at all, but detect if there is a clash and report it to the watchdog.
E.g. compare the result of the old module_invoke_all() with a new version that uses drupal_array_merge_deep(), and log a warning if the results are different.
I do not think this is a good idea at all.

Ignore the problem

This is the easiest "solution". Just say it's the developer's fault who used the same key twice.

Separate dedicated functions

Alternative versions of module_invoke_all() could be provided that treat the return value differently.
This "solution" would require that calling code actually uses these alternative functions. It would be a long time until this would fix any problem.
If we were to follow this idea, it should rather be in a separate issue, with D8 first.
I am still mentioning it here for the sake of completeness.