hook_group_permission() / hook_permission collision with field_group module

Dear Brendan,

This is sadly a Drupalism that exists in the current way hooks work.
The problem occurs whenever a module follows the pattern <modulename>_<modulename>, e.g.:

• user_field
• views_node
• taxonomy_views
• ...

For more info, please refer to http://klau.si/sites/default/files/hooks_drupal.pdf, although that PDF's solutions have not been implemented in Drupal 7.

Drupalize.me wrote a nice blog post about this, be sure to read the section "some other tidbits".

Stating the obvious aside, we're open to suggestions on how to resolve this issue. We're not too fond of the idea of having to rename all of our hooks, though. The whole idea of naming our hooks in a similar fashion to Drupal's is to emphasize that these hooks are more or less the same, albeit for groups.

All right, I've given this some thought and came up with the following:

Problem #1: Name collision

There is no hook_permission() for Field and there will probably never be one in D7 either. As a result, there is no need for field_group_permission() as a hook_group_permission() implementation. Given this fact, it is safe to say that installing the Group module will not trigger a PHP name conflict on "field_group_permission".

Problem #2: Wrong hook activation

To further examine the problem at its core, we need to abstract it. The problem with hook_group_permission() is that it may trigger hook_permission() for modules that have <existing module name>_group as their machine name. Inversely, hook_group_permission() will unintentionally be invoked for the existing module.

It is extremely difficult to have a 'per-hook master solution' for this problem, because at its core it is a problem with the way hooks work pre-D8. Luckily, there is a function that allows us to define metadata about a hook's implementation: hook_hook_info().

I therefore suggest writing a patch for hook_hook_info(), adding a 'callback' key to the allowed return values.

/**
 * Implements hook_hook_info().
 */
function field_group_hook_info() {
  $hooks['permission'] = array(
    'callback' => 'field_group_global_permission',
  );

  return $hooks;
}

/**
 * Implements hook_permission().
 *
 * @see field_group_hook_info()
 */
function field_group_global_permission() {
  return array(
    'administer field_group' => array(
      'title' => t('Administer Field Group'),
      'description' => t('Perform administration tasks for Field Group.'),
    ),
  );
}

For the record, I'm willing to write this patch for/with you.
Before I start writing any code, I need to know whether this solution sounds good to you, though.

Edit: On second thought, writing a whole new hook_hook_callback sounds better. Otherwise we use hook_hook_info() to both declare hook exposures and implementations.

I'm personally not a fan of a blacklist because of two reasons:

1. It has to be actively maintained, meaning we have to update the list whenever someone writes a module with a machine name following <existing module>_group
2. It only fixes half the problem, leaving PHP crashes still open. For instance: we intend to write node_group_permission() ourselves. If anyone creates a module with the machine name node_group, we face a problem that even blacklisting won't solve.

I'm going to go ahead and try to solve this the Drupal way first before I resort to less clean approaches. Although I must admit that the chances of a new hook making it into D7 are slim...

See #2016003: Prevent potential naming collisions caused by the hook naming pattern.

Won't fix, as it seems core won't change to avoid this type of collision.